Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core/services/relay/evm: start RequestRoundTracker; report full health #11643

Merged
merged 4 commits into from
Dec 21, 2023
Merged

core/services/relay/evm: start RequestRoundTracker; report full health #11643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a0a5a66
core/services/relay/evm: start RequestRoundTracker; report full health
jmank88 Dec 20, 2023
b1f1928
Tests round requests and implicit changes separately
kalverra Dec 21, 2023
0ec4cb9
Add test to CI
kalverra Dec 21, 2023
17ae2a1
Fixes other OCR2 checks
kalverra Dec 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 11 additions & 14 deletions core/services/relay/evm/evm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
22 changes: 18 additions & 4 deletions core/services/relay/evm/median.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
Expand All @@ -44,6 +48,7 @@ func newMedianContract(configTracker types.ContractConfigTracker, contractAddres
}

return &medianContract{
lggr: lggr,
configTracker: configTracker,
contractCaller: contractCaller,
requestRoundTracker: NewRequestRoundTracker(
Expand All @@ -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) {
Expand Down
20 changes: 20 additions & 0 deletions integration-tests/actions/ocr2_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/smoke/forwarders_ocr2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down
92 changes: 87 additions & 5 deletions integration-tests/smoke/ocr2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand All @@ -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))
Expand All @@ -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(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we should also assert that roundData.RoundId is the correct round, right?

As a side note, I just noticed there is a roundData.AnsweredInRound field--probably moot now, but I wonder if that could have helped determine whether we were getting an answer from the current round or the previous round.

Copy link
Collaborator

@kalverra kalverra Dec 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're kind of implicitly doing that already. We only stop waiting when round number round appears, then we grab data from specifically that round.

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

Expand Down Expand Up @@ -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")
Expand All @@ -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))
Expand All @@ -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")
Expand Down
Loading