From 7236361119339369127a488a7c238745e4c44099 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Thu, 21 Dec 2023 14:32:31 -0600 Subject: [PATCH] core/services/relay/evm: start RequestRoundTracker; report full health (#11643) * core/services/relay/evm: start RequestRoundTracker; report full health * Tests round requests and implicit changes separately * Add test to CI * Fixes other OCR2 checks --------- Co-authored-by: Adam Hamrick --- .github/workflows/integration-tests.yml | 6 ++ core/services/relay/evm/evm.go | 25 +++-- core/services/relay/evm/median.go | 22 ++++- integration-tests/actions/ocr2_helpers.go | 20 ++++ .../smoke/forwarders_ocr2_test.go | 4 +- integration-tests/smoke/ocr2_test.go | 92 ++++++++++++++++++- 6 files changed, 144 insertions(+), 25 deletions(-) diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 0fa9370b60d..08e24fc40a9 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -340,6 +340,12 @@ jobs: run: -run TestOCRv2Basic file: ocr2 pyroscope_env: ci-smoke-ocr2-evm-simulated + - name: ocr2 + nodes: 1 + os: ubuntu-latest + run: -run TestOCRv2Request + file: ocr2 + pyroscope_env: ci-smoke-ocr2-evm-simulated - name: ocr2 nodes: 1 os: ubuntu-latest diff --git a/core/services/relay/evm/evm.go b/core/services/relay/evm/evm.go index 08e25dba545..37e6d64452e 100644 --- a/core/services/relay/evm/evm.go +++ b/core/services/relay/evm/evm.go @@ -12,7 +12,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/jmoiron/sqlx" pkgerrors "github.com/pkg/errors" - "go.uber.org/multierr" "golang.org/x/exp/maps" "github.com/smartcontractkit/libocr/gethwrappers2/ocr2aggregator" @@ -483,6 +482,7 @@ func (r *Relayer) NewMedianProvider(rargs commontypes.RelayArgs, pargs commontyp } medianProvider := medianProvider{ + lggr: lggr.Named("MedianProvider"), configWatcher: configWatcher, reportCodec: reportCodec, contractTransmitter: contractTransmitter, @@ -513,6 +513,7 @@ func (r *Relayer) NewAutomationProvider(rargs commontypes.RelayArgs, pargs commo var _ commontypes.MedianProvider = (*medianProvider)(nil) type medianProvider struct { + lggr logger.Logger configWatcher *configWatcher contractTransmitter ContractTransmitter reportCodec median.ReportCodec @@ -521,26 +522,22 @@ type medianProvider struct { ms services.MultiStart } -func (p *medianProvider) Name() string { - return "EVM.MedianProvider" -} +func (p *medianProvider) Name() string { return p.lggr.Name() } func (p *medianProvider) Start(ctx context.Context) error { - return p.ms.Start(ctx, p.configWatcher, p.contractTransmitter) + return p.ms.Start(ctx, p.configWatcher, p.contractTransmitter, p.medianContract) } -func (p *medianProvider) Close() error { - return p.ms.Close() -} +func (p *medianProvider) Close() error { return p.ms.Close() } -func (p *medianProvider) Ready() error { - return multierr.Combine(p.configWatcher.Ready(), p.contractTransmitter.Ready()) -} +func (p *medianProvider) Ready() error { return nil } func (p *medianProvider) HealthReport() map[string]error { - report := p.configWatcher.HealthReport() - services.CopyHealth(report, p.contractTransmitter.HealthReport()) - return report + hp := map[string]error{p.Name(): p.Ready()} + services.CopyHealth(hp, p.configWatcher.HealthReport()) + services.CopyHealth(hp, p.contractTransmitter.HealthReport()) + services.CopyHealth(hp, p.medianContract.HealthReport()) + return hp } func (p *medianProvider) ContractTransmitter() ocrtypes.ContractTransmitter { diff --git a/core/services/relay/evm/median.go b/core/services/relay/evm/median.go index db521a97208..e3200d8e867 100644 --- a/core/services/relay/evm/median.go +++ b/core/services/relay/evm/median.go @@ -14,6 +14,7 @@ import ( "github.com/smartcontractkit/libocr/offchainreporting2plus/types" ocrtypes "github.com/smartcontractkit/libocr/offchainreporting2plus/types" + "github.com/smartcontractkit/chainlink-common/pkg/services" "github.com/smartcontractkit/chainlink/v2/core/chains/legacyevm" offchain_aggregator_wrapper "github.com/smartcontractkit/chainlink/v2/core/internal/gethwrappers2/generated/offchainaggregator" "github.com/smartcontractkit/chainlink/v2/core/logger" @@ -22,12 +23,15 @@ import ( var _ median.MedianContract = &medianContract{} type medianContract struct { + services.StateMachine + lggr logger.Logger configTracker types.ContractConfigTracker contractCaller *ocr2aggregator.OCR2AggregatorCaller requestRoundTracker *RequestRoundTracker } func newMedianContract(configTracker types.ContractConfigTracker, contractAddress common.Address, chain legacyevm.Chain, specID int32, db *sqlx.DB, lggr logger.Logger) (*medianContract, error) { + lggr = lggr.Named("MedianContract") contract, err := offchain_aggregator_wrapper.NewOffchainAggregator(contractAddress, chain.Client()) if err != nil { return nil, errors.Wrap(err, "could not instantiate NewOffchainAggregator") @@ -44,6 +48,7 @@ func newMedianContract(configTracker types.ContractConfigTracker, contractAddres } return &medianContract{ + lggr: lggr, configTracker: configTracker, contractCaller: contractCaller, requestRoundTracker: NewRequestRoundTracker( @@ -60,13 +65,22 @@ func newMedianContract(configTracker types.ContractConfigTracker, contractAddres ), }, nil } - -func (oc *medianContract) Start() error { - return oc.requestRoundTracker.Start() +func (oc *medianContract) Start(context.Context) error { + return oc.StartOnce("MedianContract", func() error { + return oc.requestRoundTracker.Start() + }) } func (oc *medianContract) Close() error { - return oc.requestRoundTracker.Close() + return oc.StopOnce("MedianContract", func() error { + return oc.requestRoundTracker.Close() + }) +} + +func (oc *medianContract) Name() string { return oc.lggr.Name() } + +func (oc *medianContract) HealthReport() map[string]error { + return map[string]error{oc.Name(): oc.Ready()} } func (oc *medianContract) LatestTransmissionDetails(ctx context.Context) (ocrtypes.ConfigDigest, uint32, uint8, *big.Int, time.Time, error) { diff --git a/integration-tests/actions/ocr2_helpers.go b/integration-tests/actions/ocr2_helpers.go index c4bc30c7c5e..37db348815b 100644 --- a/integration-tests/actions/ocr2_helpers.go +++ b/integration-tests/actions/ocr2_helpers.go @@ -391,6 +391,26 @@ func StartNewOCR2Round( return nil } +// WatchNewOCR2Round is the same as StartNewOCR2Round but does NOT explicitly request a new round +// as that can cause odd behavior in tandem with changing adapter values in OCR2 +func WatchNewOCR2Round( + roundNumber int64, + ocrInstances []contracts.OffchainAggregatorV2, + client blockchain.EVMClient, + timeout time.Duration, + logger zerolog.Logger, +) error { + for i := 0; i < len(ocrInstances); i++ { + ocrRound := contracts.NewOffchainAggregatorV2RoundConfirmer(ocrInstances[i], big.NewInt(roundNumber), timeout, logger) + client.AddHeaderEventSubscription(ocrInstances[i].Address(), ocrRound) + err := client.WaitForEvents() + if err != nil { + return fmt.Errorf("failed to wait for event subscriptions of OCR instance %d: %w", i+1, err) + } + } + return nil +} + // SetOCR2AdapterResponse sets a single adapter response that correlates with an ocr contract and a chainlink node // used for OCR2 tests func SetOCR2AdapterResponse( diff --git a/integration-tests/smoke/forwarders_ocr2_test.go b/integration-tests/smoke/forwarders_ocr2_test.go index c2e2b12e51d..71d35508175 100644 --- a/integration-tests/smoke/forwarders_ocr2_test.go +++ b/integration-tests/smoke/forwarders_ocr2_test.go @@ -90,7 +90,7 @@ func TestForwarderOCR2Basic(t *testing.T) { err = actions.ConfigureOCRv2AggregatorContracts(env.EVMClient, ocrv2Config, ocrInstances) require.NoError(t, err, "Error configuring OCRv2 aggregator contracts") - err = actions.StartNewOCR2Round(1, ocrInstances, env.EVMClient, time.Minute*10, l) + err = actions.WatchNewOCR2Round(1, ocrInstances, env.EVMClient, time.Minute*10, l) require.NoError(t, err) answer, err := ocrInstances[0].GetLatestAnswer(testcontext.Get(t)) @@ -101,7 +101,7 @@ func TestForwarderOCR2Basic(t *testing.T) { ocrRoundVal := (5 + i) % 10 err = env.MockAdapter.SetAdapterBasedIntValuePath("ocr2", []string{http.MethodGet, http.MethodPost}, ocrRoundVal) require.NoError(t, err) - err = actions.StartNewOCR2Round(int64(i), ocrInstances, env.EVMClient, time.Minute*10, l) + err = actions.WatchNewOCR2Round(int64(i), ocrInstances, env.EVMClient, time.Minute*10, l) require.NoError(t, err) answer, err = ocrInstances[0].GetLatestAnswer(testcontext.Get(t)) diff --git a/integration-tests/smoke/ocr2_test.go b/integration-tests/smoke/ocr2_test.go index a1a37c3b18e..376c56e9d64 100644 --- a/integration-tests/smoke/ocr2_test.go +++ b/integration-tests/smoke/ocr2_test.go @@ -11,6 +11,7 @@ import ( "github.com/smartcontractkit/chainlink-testing-framework/logging" "github.com/smartcontractkit/chainlink-testing-framework/utils/testcontext" + "github.com/smartcontractkit/chainlink/integration-tests/actions" "github.com/smartcontractkit/chainlink/integration-tests/contracts" "github.com/smartcontractkit/chainlink/integration-tests/docker/test_env" @@ -75,7 +76,7 @@ func TestOCRv2Basic(t *testing.T) { err = actions.ConfigureOCRv2AggregatorContracts(env.EVMClient, ocrv2Config, aggregatorContracts) require.NoError(t, err, "Error configuring OCRv2 aggregator contracts") - err = actions.StartNewOCR2Round(1, aggregatorContracts, env.EVMClient, time.Minute*5, l) + err = actions.WatchNewOCR2Round(1, aggregatorContracts, env.EVMClient, time.Minute*5, l) require.NoError(t, err, "Error starting new OCR2 round") roundData, err := aggregatorContracts[0].GetRound(testcontext.Get(t), big.NewInt(1)) require.NoError(t, err, "Getting latest answer from OCR contract shouldn't fail") @@ -86,7 +87,7 @@ func TestOCRv2Basic(t *testing.T) { err = env.MockAdapter.SetAdapterBasedIntValuePath("ocr2", []string{http.MethodGet, http.MethodPost}, 10) require.NoError(t, err) - err = actions.StartNewOCR2Round(2, aggregatorContracts, env.EVMClient, time.Minute*5, l) + err = actions.WatchNewOCR2Round(2, aggregatorContracts, env.EVMClient, time.Minute*5, l) require.NoError(t, err) roundData, err = aggregatorContracts[0].GetRound(testcontext.Get(t), big.NewInt(2)) @@ -97,6 +98,87 @@ func TestOCRv2Basic(t *testing.T) { ) } +// Tests that just calling requestNewRound() will properly induce more rounds +func TestOCRv2Request(t *testing.T) { + t.Parallel() + l := logging.GetTestLogger(t) + + network, err := actions.EthereumNetworkConfigFromEnvOrDefault(l) + require.NoError(t, err, "Error building ethereum network config") + + env, err := test_env.NewCLTestEnvBuilder(). + WithTestInstance(t). + WithPrivateEthereumNetwork(network). + WithMockAdapter(). + WithCLNodeConfig(node.NewConfig(node.NewBaseConfig(), + node.WithOCR2(), + node.WithP2Pv2(), + node.WithTracing(), + )). + WithCLNodes(6). + WithFunding(big.NewFloat(.1)). + WithStandardCleanup(). + WithLogStream(). + Build() + require.NoError(t, err) + + env.ParallelTransactions(true) + + nodeClients := env.ClCluster.NodeAPIs() + bootstrapNode, workerNodes := nodeClients[0], nodeClients[1:] + + linkToken, err := env.ContractDeployer.DeployLinkTokenContract() + require.NoError(t, err, "Deploying Link Token Contract shouldn't fail") + + err = actions.FundChainlinkNodesLocal(workerNodes, env.EVMClient, big.NewFloat(.05)) + require.NoError(t, err, "Error funding Chainlink nodes") + + // Gather transmitters + var transmitters []string + for _, node := range workerNodes { + addr, err := node.PrimaryEthAddress() + if err != nil { + require.NoError(t, fmt.Errorf("error getting node's primary ETH address: %w", err)) + } + transmitters = append(transmitters, addr) + } + + ocrOffchainOptions := contracts.DefaultOffChainAggregatorOptions() + aggregatorContracts, err := actions.DeployOCRv2Contracts(1, linkToken, env.ContractDeployer, transmitters, env.EVMClient, ocrOffchainOptions) + require.NoError(t, err, "Error deploying OCRv2 aggregator contracts") + + err = actions.CreateOCRv2JobsLocal(aggregatorContracts, bootstrapNode, workerNodes, env.MockAdapter, "ocr2", 5, env.EVMClient.GetChainID().Uint64(), false) + require.NoError(t, err, "Error creating OCRv2 jobs") + + ocrv2Config, err := actions.BuildMedianOCR2ConfigLocal(workerNodes, ocrOffchainOptions) + require.NoError(t, err, "Error building OCRv2 config") + + err = actions.ConfigureOCRv2AggregatorContracts(env.EVMClient, ocrv2Config, aggregatorContracts) + require.NoError(t, err, "Error configuring OCRv2 aggregator contracts") + + err = actions.WatchNewOCR2Round(1, aggregatorContracts, env.EVMClient, time.Minute*5, l) + require.NoError(t, err, "Error starting new OCR2 round") + roundData, err := aggregatorContracts[0].GetRound(testcontext.Get(t), big.NewInt(1)) + require.NoError(t, err, "Getting latest answer from OCR contract shouldn't fail") + require.Equal(t, int64(5), roundData.Answer.Int64(), + "Expected latest answer from OCR contract to be 5 but got %d", + roundData.Answer.Int64(), + ) + + // Keep the mockserver value the same and continually request new rounds + for round := 2; round <= 4; round++ { + err = actions.StartNewOCR2Round(int64(round), aggregatorContracts, env.EVMClient, time.Minute*5, l) + require.NoError(t, err, "Error starting new OCR2 round") + roundData, err := aggregatorContracts[0].GetRound(testcontext.Get(t), big.NewInt(int64(round))) + require.NoError(t, err, "Getting latest answer from OCR contract shouldn't fail") + require.Equal(t, int64(5), roundData.Answer.Int64(), + "Expected round %d answer from OCR contract to be 5 but got %d", + round, + roundData.Answer.Int64(), + ) + } +} + func TestOCRv2JobReplacement(t *testing.T) { l := logging.GetTestLogger(t) @@ -150,7 +232,7 @@ func TestOCRv2JobReplacement(t *testing.T) { err = actions.ConfigureOCRv2AggregatorContracts(env.EVMClient, ocrv2Config, aggregatorContracts) require.NoError(t, err, "Error configuring OCRv2 aggregator contracts") - err = actions.StartNewOCR2Round(1, aggregatorContracts, env.EVMClient, time.Minute*5, l) + err = actions.WatchNewOCR2Round(1, aggregatorContracts, env.EVMClient, time.Minute*5, l) require.NoError(t, err, "Error starting new OCR2 round") roundData, err := aggregatorContracts[0].GetRound(testcontext.Get(t), big.NewInt(1)) require.NoError(t, err, "Getting latest answer from OCR contract shouldn't fail") @@ -161,7 +243,7 @@ func TestOCRv2JobReplacement(t *testing.T) { err = env.MockAdapter.SetAdapterBasedIntValuePath("ocr2", []string{http.MethodGet, http.MethodPost}, 10) require.NoError(t, err) - err = actions.StartNewOCR2Round(2, aggregatorContracts, env.EVMClient, time.Minute*5, l) + err = actions.WatchNewOCR2Round(2, aggregatorContracts, env.EVMClient, time.Minute*5, l) require.NoError(t, err) roundData, err = aggregatorContracts[0].GetRound(testcontext.Get(t), big.NewInt(2)) @@ -180,7 +262,7 @@ func TestOCRv2JobReplacement(t *testing.T) { err = actions.CreateOCRv2JobsLocal(aggregatorContracts, bootstrapNode, workerNodes, env.MockAdapter, "ocr2", 15, env.EVMClient.GetChainID().Uint64(), false) require.NoError(t, err, "Error creating OCRv2 jobs") - err = actions.StartNewOCR2Round(3, aggregatorContracts, env.EVMClient, time.Minute*3, l) + err = actions.WatchNewOCR2Round(3, aggregatorContracts, env.EVMClient, time.Minute*3, l) require.NoError(t, err, "Error starting new OCR2 round") roundData, err = aggregatorContracts[0].GetRound(testcontext.Get(t), big.NewInt(3)) require.NoError(t, err, "Getting latest answer from OCR contract shouldn't fail")