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 <adam.hamrick@smartcontract.com>

.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

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 {

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) {

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(

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))

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")