From e72316ac4a00a3257275b60450280c8b565a10e6 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Tue, 27 Aug 2024 14:42:52 +0300 Subject: [PATCH 01/12] commit base e2e test --- commit/observation.go | 2 + commit/plugin_e2e_test.go | 188 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+) create mode 100644 commit/plugin_e2e_test.go diff --git a/commit/observation.go b/commit/observation.go index ba0d9c2ca..8eb92729f 100644 --- a/commit/observation.go +++ b/commit/observation.go @@ -106,6 +106,8 @@ func (o ObserverImpl) ObserveOffRampNextSeqNums(ctx context.Context) []plugintyp o.lggr.Warnw("call to KnownSourceChainsSlice failed", "err", err) return nil } + + sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) offRampNextSeqNums, err := o.ccipReader.NextSeqNum(ctx, sourceChains) if err != nil { o.lggr.Warnw("call to NextSeqNum failed", "err", err) diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go new file mode 100644 index 000000000..f44761052 --- /dev/null +++ b/commit/plugin_e2e_test.go @@ -0,0 +1,188 @@ +package commit + +import ( + "context" + "sort" + "testing" + + mapset "github.com/deckarep/golang-set/v2" + "github.com/smartcontractkit/chainlink-common/pkg/logger" + "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3" + "github.com/smartcontractkit/libocr/commontypes" + "github.com/smartcontractkit/libocr/offchainreporting2plus/ocr3types" + ocr2types "github.com/smartcontractkit/libocr/offchainreporting2plus/types" + libocrtypes "github.com/smartcontractkit/libocr/ragep2p/types" + "github.com/stretchr/testify/assert" + + "github.com/smartcontractkit/chainlink-ccip/chainconfig" + "github.com/smartcontractkit/chainlink-ccip/internal/libs/testhelpers" + "github.com/smartcontractkit/chainlink-ccip/internal/mocks" + "github.com/smartcontractkit/chainlink-ccip/internal/reader" + reader_mock "github.com/smartcontractkit/chainlink-ccip/mocks/internal_/reader" + "github.com/smartcontractkit/chainlink-ccip/pluginconfig" +) + +func TestPlugin_E2E(t *testing.T) { + destChain := ccipocr3.ChainSelector(123) + sourceChain1 := ccipocr3.ChainSelector(1) + sourceChain2 := ccipocr3.ChainSelector(2) + + ctx := context.Background() + lggr := logger.Test(t) + + nodeIDs := []commontypes.OracleID{1, 2, 3} + peerIDs := []libocrtypes.PeerID{{1}, {2}, {3}} + + oracleIDToP2pID := make(map[commontypes.OracleID]libocrtypes.PeerID) + for i := range nodeIDs { + oracleIDToP2pID[nodeIDs[i]] = peerIDs[i] + } + + chainConfig := map[ccipocr3.ChainSelector]reader.ChainConfig{ + destChain: { + FChain: 1, + SupportedNodes: mapset.NewSet(peerIDs...), + Config: chainconfig.ChainConfig{FinalityDepth: 1}, + }, + sourceChain1: { + FChain: 1, + SupportedNodes: mapset.NewSet(peerIDs...), + Config: chainconfig.ChainConfig{FinalityDepth: 1}, + }, + sourceChain2: { + FChain: 1, + SupportedNodes: mapset.NewSet(peerIDs...), + Config: chainconfig.ChainConfig{FinalityDepth: 1}, + }, + } + + nextSeqNum := map[ccipocr3.ChainSelector]uint64{ + sourceChain1: 10, + sourceChain2: 20, + } + + cfg := pluginconfig.CommitPluginConfig{ + DestChain: destChain, + NewMsgScanBatchSize: 0, + MaxReportTransmissionCheckAttempts: 0, + SyncTimeout: 0, + SyncFrequency: 0, + OffchainConfig: pluginconfig.CommitOffchainConfig{}, + } + + reportingCfg := ocr3types.ReportingPluginConfig{ + ConfigDigest: ocr2types.ConfigDigest{}, + OracleID: 0, + N: 0, + F: 0, + OnchainConfig: nil, + OffchainConfig: nil, + EstimatedRoundInterval: 0, + MaxDurationQuery: 0, + MaxDurationObservation: 0, + MaxDurationShouldAcceptAttestedReport: 0, + MaxDurationShouldTransmitAcceptedReport: 0, + } + + n0 := setupNode(ctx, t, lggr, nodeIDs[0], reportingCfg, oracleIDToP2pID, cfg, chainConfig, nextSeqNum) + n1 := setupNode(ctx, t, lggr, nodeIDs[1], reportingCfg, oracleIDToP2pID, cfg, chainConfig, nextSeqNum) + n2 := setupNode(ctx, t, lggr, nodeIDs[2], reportingCfg, oracleIDToP2pID, cfg, chainConfig, nextSeqNum) + + nodes := []ocr3types.ReportingPlugin[[]byte]{ + n0.node, + n1.node, + n2.node, + } + + initialOutcome := ocr3types.Outcome{} + + runner := testhelpers.NewOCR3Runner(nodes, nodeIDs, initialOutcome) + + res, err := runner.RunRound(ctx) + assert.NoError(t, err) + t.Logf("%#v", res) +} + +type nodeSetup struct { + node *Plugin + ccipReader *reader_mock.MockCCIP + priceReader *mocks.TokenPricesReader + reportCodec *mocks.CommitPluginJSONReportCodec + msgHasher *mocks.MessageHasher +} + +func setupNode( + ctx context.Context, + t *testing.T, + lggr logger.Logger, + nodeID commontypes.OracleID, + reportingCfg ocr3types.ReportingPluginConfig, + oracleIDToP2pID map[commontypes.OracleID]libocrtypes.PeerID, + pluginCfg pluginconfig.CommitPluginConfig, + chainCfg map[ccipocr3.ChainSelector]reader.ChainConfig, + nextSeqNum map[ccipocr3.ChainSelector]uint64, +) nodeSetup { + ccipReader := reader_mock.NewMockCCIP(t) + tokenPricesReader := mocks.NewTokenPricesReader() + reportCodec := mocks.NewCommitPluginJSONReportCodec() + msgHasher := mocks.NewMessageHasher() + homeChainReader := reader_mock.NewMockHomeChain(t) + + fChain := map[ccipocr3.ChainSelector]int{} + supportedChainsForPeer := make(map[libocrtypes.PeerID]mapset.Set[ccipocr3.ChainSelector]) + for chainSel, cfg := range chainCfg { + fChain[chainSel] = cfg.FChain + + for _, peerID := range cfg.SupportedNodes.ToSlice() { + if _, ok := supportedChainsForPeer[peerID]; !ok { + supportedChainsForPeer[peerID] = mapset.NewSet[ccipocr3.ChainSelector]() + } + supportedChainsForPeer[peerID].Add(chainSel) + } + } + + homeChainReader.On("GetFChain").Return(fChain, nil) + + for peerID, supportedChains := range supportedChainsForPeer { + homeChainReader.On("GetSupportedChainsForPeer", peerID).Return(supportedChains, nil).Maybe() + } + + knownCCIPChains := mapset.NewSet[ccipocr3.ChainSelector]() + + for chainSel, cfg := range chainCfg { + homeChainReader.On("GetChainConfig", chainSel).Return(cfg, nil).Maybe() + knownCCIPChains.Add(chainSel) + } + homeChainReader.On("GetKnownCCIPChains").Return(knownCCIPChains, nil) + + sourceChains := make([]ccipocr3.ChainSelector, 0) + seqNums := make([]ccipocr3.SeqNum, 0) + for chainSel, ns := range nextSeqNum { + sourceChains = append(sourceChains, chainSel) + seqNums = append(seqNums, ccipocr3.SeqNum(ns)) + } + sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) + ccipReader.On("NextSeqNum", ctx, sourceChains).Return(seqNums, nil) + + p := NewPlugin( + ctx, + nodeID, + oracleIDToP2pID, + pluginCfg, + ccipReader, + tokenPricesReader, + reportCodec, + msgHasher, + lggr, + homeChainReader, + reportingCfg, + ) + + return nodeSetup{ + node: p, + ccipReader: ccipReader, + priceReader: tokenPricesReader, + reportCodec: reportCodec, + msgHasher: msgHasher, + } +} From d1f023bacac8a7753bd787195c45b50dedc98783 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Tue, 27 Aug 2024 15:26:54 +0300 Subject: [PATCH 02/12] don't use sort.slice in plugin logic --- commit/observation.go | 1 - commit/plugin_e2e_test.go | 10 ++++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/commit/observation.go b/commit/observation.go index 8eb92729f..c6db74b0a 100644 --- a/commit/observation.go +++ b/commit/observation.go @@ -107,7 +107,6 @@ func (o ObserverImpl) ObserveOffRampNextSeqNums(ctx context.Context) []plugintyp return nil } - sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) offRampNextSeqNums, err := o.ccipReader.NextSeqNum(ctx, sourceChains) if err != nil { o.lggr.Warnw("call to NextSeqNum failed", "err", err) diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index f44761052..0fb02a069 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -13,6 +13,7 @@ import ( ocr2types "github.com/smartcontractkit/libocr/offchainreporting2plus/types" libocrtypes "github.com/smartcontractkit/libocr/ragep2p/types" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/smartcontractkit/chainlink-ccip/chainconfig" "github.com/smartcontractkit/chainlink-ccip/internal/libs/testhelpers" @@ -161,8 +162,13 @@ func setupNode( sourceChains = append(sourceChains, chainSel) seqNums = append(seqNums, ccipocr3.SeqNum(ns)) } - sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) - ccipReader.On("NextSeqNum", ctx, sourceChains).Return(seqNums, nil) + + ccipReader.On("NextSeqNum", ctx, mock.Anything).Run(func(args mock.Arguments) { + providedChains := args[1].([]ccipocr3.ChainSelector) + sort.Slice(providedChains, func(i, j int) bool { return providedChains[i] < providedChains[j] }) + sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) + assert.Equal(t, sourceChains, providedChains) + }).Return(seqNums, nil) p := NewPlugin( ctx, From b39837199cb83fde1f28151fd25722f2ac0fcbc5 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Tue, 27 Aug 2024 15:29:12 +0300 Subject: [PATCH 03/12] fix mock import --- commit/plugin_e2e_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index 0fb02a069..24e024bbe 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -107,7 +107,7 @@ func TestPlugin_E2E(t *testing.T) { type nodeSetup struct { node *Plugin ccipReader *reader_mock.MockCCIP - priceReader *mocks.TokenPricesReader + priceReader *reader_mock.MockTokenPrices reportCodec *mocks.CommitPluginJSONReportCodec msgHasher *mocks.MessageHasher } @@ -124,7 +124,7 @@ func setupNode( nextSeqNum map[ccipocr3.ChainSelector]uint64, ) nodeSetup { ccipReader := reader_mock.NewMockCCIP(t) - tokenPricesReader := mocks.NewTokenPricesReader() + tokenPricesReader := reader_mock.NewMockTokenPrices(t) reportCodec := mocks.NewCommitPluginJSONReportCodec() msgHasher := mocks.NewMessageHasher() homeChainReader := reader_mock.NewMockHomeChain(t) From 149109cd4c992f0b1f3466a3479bf3f2d7d450a2 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Tue, 27 Aug 2024 17:23:12 +0300 Subject: [PATCH 04/12] table driven tests --- commit/observation.go | 1 + commit/outcome.go | 3 + commit/plugin_e2e_test.go | 190 ++++++++++++++++++++++++-------------- 3 files changed, 127 insertions(+), 67 deletions(-) diff --git a/commit/observation.go b/commit/observation.go index c6db74b0a..8eb92729f 100644 --- a/commit/observation.go +++ b/commit/observation.go @@ -107,6 +107,7 @@ func (o ObserverImpl) ObserveOffRampNextSeqNums(ctx context.Context) []plugintyp return nil } + sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) offRampNextSeqNums, err := o.ccipReader.NextSeqNum(ctx, sourceChains) if err != nil { o.lggr.Warnw("call to NextSeqNum failed", "err", err) diff --git a/commit/outcome.go b/commit/outcome.go index 675721545..0f78a06c6 100644 --- a/commit/outcome.go +++ b/commit/outcome.go @@ -87,6 +87,9 @@ func ReportRangesOutcome( } } + // deterministic outcome + sort.Slice(rangesToReport, func(i, j int) bool { return rangesToReport[i].ChainSel < rangesToReport[j].ChainSel }) + outcome := Outcome{ OutcomeType: ReportIntervalsSelected, RangesSelectedForReport: rangesToReport, diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index 24e024bbe..1a37068f7 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -2,18 +2,20 @@ package commit import ( "context" + "crypto/sha256" + "fmt" "sort" "testing" + "time" mapset "github.com/deckarep/golang-set/v2" "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3" "github.com/smartcontractkit/libocr/commontypes" "github.com/smartcontractkit/libocr/offchainreporting2plus/ocr3types" - ocr2types "github.com/smartcontractkit/libocr/offchainreporting2plus/types" libocrtypes "github.com/smartcontractkit/libocr/ragep2p/types" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "github.com/smartcontractkit/chainlink-ccip/chainconfig" "github.com/smartcontractkit/chainlink-ccip/internal/libs/testhelpers" @@ -21,87 +23,115 @@ import ( "github.com/smartcontractkit/chainlink-ccip/internal/reader" reader_mock "github.com/smartcontractkit/chainlink-ccip/mocks/internal_/reader" "github.com/smartcontractkit/chainlink-ccip/pluginconfig" + "github.com/smartcontractkit/chainlink-ccip/plugintypes" ) func TestPlugin_E2E(t *testing.T) { - destChain := ccipocr3.ChainSelector(123) - sourceChain1 := ccipocr3.ChainSelector(1) - sourceChain2 := ccipocr3.ChainSelector(2) + destChain := ccipocr3.ChainSelector(1) + sourceChain1 := ccipocr3.ChainSelector(2) + sourceChain2 := ccipocr3.ChainSelector(3) + require.Len(t, mapset.NewSet(destChain, sourceChain1, sourceChain2).ToSlice(), 3) ctx := context.Background() lggr := logger.Test(t) - nodeIDs := []commontypes.OracleID{1, 2, 3} + oracleIDs := []commontypes.OracleID{1, 2, 3} peerIDs := []libocrtypes.PeerID{{1}, {2}, {3}} + require.Equal(t, len(oracleIDs), len(peerIDs)) - oracleIDToP2pID := make(map[commontypes.OracleID]libocrtypes.PeerID) - for i := range nodeIDs { - oracleIDToP2pID[nodeIDs[i]] = peerIDs[i] + oracleIDToPeerID := make(map[commontypes.OracleID]libocrtypes.PeerID) + for i := range oracleIDs { + oracleIDToPeerID[oracleIDs[i]] = peerIDs[i] } - chainConfig := map[ccipocr3.ChainSelector]reader.ChainConfig{ - destChain: { - FChain: 1, - SupportedNodes: mapset.NewSet(peerIDs...), - Config: chainconfig.ChainConfig{FinalityDepth: 1}, - }, - sourceChain1: { - FChain: 1, - SupportedNodes: mapset.NewSet(peerIDs...), - Config: chainconfig.ChainConfig{FinalityDepth: 1}, - }, - sourceChain2: { - FChain: 1, - SupportedNodes: mapset.NewSet(peerIDs...), - Config: chainconfig.ChainConfig{FinalityDepth: 1}, - }, + peerIDsMap := mapset.NewSet(peerIDs...) + homeChainConfig := map[ccipocr3.ChainSelector]reader.ChainConfig{ + destChain: {FChain: 1, SupportedNodes: peerIDsMap, Config: chainconfig.ChainConfig{FinalityDepth: 1}}, + sourceChain1: {FChain: 1, SupportedNodes: peerIDsMap, Config: chainconfig.ChainConfig{FinalityDepth: 1}}, + sourceChain2: {FChain: 1, SupportedNodes: peerIDsMap, Config: chainconfig.ChainConfig{FinalityDepth: 1}}, } - nextSeqNum := map[ccipocr3.ChainSelector]uint64{ + offRampNextSeqNum := map[ccipocr3.ChainSelector]ccipocr3.SeqNum{ sourceChain1: 10, sourceChain2: 20, } + onRampLastSeqNum := map[ccipocr3.ChainSelector]ccipocr3.SeqNum{ + sourceChain1: 10, // one new msg -> 10 + sourceChain2: 19, // no new msg, still on 19 + } + cfg := pluginconfig.CommitPluginConfig{ DestChain: destChain, - NewMsgScanBatchSize: 0, + NewMsgScanBatchSize: 100, MaxReportTransmissionCheckAttempts: 0, - SyncTimeout: 0, - SyncFrequency: 0, - OffchainConfig: pluginconfig.CommitOffchainConfig{}, + SyncTimeout: 10 * time.Second, + SyncFrequency: time.Hour, } - reportingCfg := ocr3types.ReportingPluginConfig{ - ConfigDigest: ocr2types.ConfigDigest{}, - OracleID: 0, - N: 0, - F: 0, - OnchainConfig: nil, - OffchainConfig: nil, - EstimatedRoundInterval: 0, - MaxDurationQuery: 0, - MaxDurationObservation: 0, - MaxDurationShouldAcceptAttestedReport: 0, - MaxDurationShouldTransmitAcceptedReport: 0, + nodes := make([]ocr3types.ReportingPlugin[[]byte], len(oracleIDs)) + reportingCfg := ocr3types.ReportingPluginConfig{F: 1} + for i := range oracleIDs { + n := setupNode(ctx, t, lggr, oracleIDs[i], reportingCfg, oracleIDToPeerID, + cfg, homeChainConfig, offRampNextSeqNum, onRampLastSeqNum) + nodes[i] = n.node } - n0 := setupNode(ctx, t, lggr, nodeIDs[0], reportingCfg, oracleIDToP2pID, cfg, chainConfig, nextSeqNum) - n1 := setupNode(ctx, t, lggr, nodeIDs[1], reportingCfg, oracleIDToP2pID, cfg, chainConfig, nextSeqNum) - n2 := setupNode(ctx, t, lggr, nodeIDs[2], reportingCfg, oracleIDToP2pID, cfg, chainConfig, nextSeqNum) - - nodes := []ocr3types.ReportingPlugin[[]byte]{ - n0.node, - n1.node, - n2.node, + testCases := []struct { + name string + prevOutcome Outcome + expOutcome Outcome + }{ + { + name: "empty previous outcome, should select ranges for report", + prevOutcome: Outcome{}, + expOutcome: Outcome{ + OutcomeType: ReportIntervalsSelected, + RangesSelectedForReport: []plugintypes.ChainRange{ + {ChainSel: sourceChain1, SeqNumRange: ccipocr3.SeqNumRange{10, 10}}, + {ChainSel: sourceChain2, SeqNumRange: ccipocr3.SeqNumRange{20, 20}}, + }, + }, + }, + { + name: "selected ranges for report in previous outcome", + prevOutcome: Outcome{ + OutcomeType: ReportIntervalsSelected, + RangesSelectedForReport: []plugintypes.ChainRange{ + {ChainSel: sourceChain1, SeqNumRange: ccipocr3.SeqNumRange{10, 10}}, + {ChainSel: sourceChain2, SeqNumRange: ccipocr3.SeqNumRange{20, 20}}, + }, + }, + expOutcome: Outcome{ + OutcomeType: ReportGenerated, + RootsToReport: []ccipocr3.MerkleRootChain{ + { + ChainSel: sourceChain1, + SeqNumsRange: ccipocr3.SeqNumRange{0xa, 0xa}, + MerkleRoot: ccipocr3.Bytes32{0x4a, 0x44, 0xdc, 0x15, 0x36, 0x42, 0x4, 0xa8, 0xf, 0xe8, 0xe, + 0x90, 0x39, 0x45, 0x5c, 0xc1, 0x60, 0x82, 0x81, 0x82, 0xf, 0xe2, 0xb2, 0x4f, 0x1e, 0x52, + 0x33, 0xad, 0xe6, 0xaf, 0x1d, 0xd5}, + }, + }, + TokenPrices: make([]ccipocr3.TokenPrice, 0), + GasPrices: make([]ccipocr3.GasPriceChain, 0), + }, + }, } - initialOutcome := ocr3types.Outcome{} - - runner := testhelpers.NewOCR3Runner(nodes, nodeIDs, initialOutcome) - - res, err := runner.RunRound(ctx) - assert.NoError(t, err) - t.Logf("%#v", res) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + encodedPrevOutcome, err := tc.prevOutcome.Encode() + assert.NoError(t, err) + runner := testhelpers.NewOCR3Runner(nodes, oracleIDs, encodedPrevOutcome) + res, err := runner.RunRound(ctx) + assert.NoError(t, err) + + decodedOutcome, err := DecodeOutcome(res.Outcome) + assert.NoError(t, err) + assert.Equal(t, tc.expOutcome, decodedOutcome) + }) + } } type nodeSetup struct { @@ -121,7 +151,8 @@ func setupNode( oracleIDToP2pID map[commontypes.OracleID]libocrtypes.PeerID, pluginCfg pluginconfig.CommitPluginConfig, chainCfg map[ccipocr3.ChainSelector]reader.ChainConfig, - nextSeqNum map[ccipocr3.ChainSelector]uint64, + nextSeqNum map[ccipocr3.ChainSelector]ccipocr3.SeqNum, + onRampLastSeqNum map[ccipocr3.ChainSelector]ccipocr3.SeqNum, ) nodeSetup { ccipReader := reader_mock.NewMockCCIP(t) tokenPricesReader := reader_mock.NewMockTokenPrices(t) @@ -154,21 +185,46 @@ func setupNode( homeChainReader.On("GetChainConfig", chainSel).Return(cfg, nil).Maybe() knownCCIPChains.Add(chainSel) } - homeChainReader.On("GetKnownCCIPChains").Return(knownCCIPChains, nil) + homeChainReader.On("GetKnownCCIPChains").Return(knownCCIPChains, nil).Maybe() sourceChains := make([]ccipocr3.ChainSelector, 0) seqNums := make([]ccipocr3.SeqNum, 0) - for chainSel, ns := range nextSeqNum { + chainsWithNewMsgs := make([]ccipocr3.ChainSelector, 0) + for chainSel, offRampNextSeqNum := range nextSeqNum { sourceChains = append(sourceChains, chainSel) - seqNums = append(seqNums, ccipocr3.SeqNum(ns)) + seqNums = append(seqNums, offRampNextSeqNum) + + newMsgs := make([]ccipocr3.Message, 0) + numNewMsgs := (onRampLastSeqNum[chainSel] - offRampNextSeqNum) + 1 + for i := uint64(0); i < uint64(numNewMsgs); i++ { + messageID := sha256.Sum256([]byte(fmt.Sprintf("%d", uint64(offRampNextSeqNum)+i))) + newMsgs = append(newMsgs, ccipocr3.Message{ + Header: ccipocr3.RampMessageHeader{ + MessageID: messageID, + SequenceNumber: offRampNextSeqNum + ccipocr3.SeqNum(i), + }, + }) + } + + ccipReader.On("MsgsBetweenSeqNums", ctx, chainSel, + ccipocr3.NewSeqNumRange(offRampNextSeqNum, offRampNextSeqNum)). + Return(newMsgs, nil).Maybe() + + if len(newMsgs) > 0 { + chainsWithNewMsgs = append(chainsWithNewMsgs, chainSel) + } } - ccipReader.On("NextSeqNum", ctx, mock.Anything).Run(func(args mock.Arguments) { - providedChains := args[1].([]ccipocr3.ChainSelector) - sort.Slice(providedChains, func(i, j int) bool { return providedChains[i] < providedChains[j] }) - sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) - assert.Equal(t, sourceChains, providedChains) - }).Return(seqNums, nil) + sort.Slice(chainsWithNewMsgs, func(i, j int) bool { return chainsWithNewMsgs[i] < chainsWithNewMsgs[j] }) + seqNumsOfChainsWithNewMsgs := make([]ccipocr3.SeqNum, 0) + for _, chainSel := range chainsWithNewMsgs { + seqNumsOfChainsWithNewMsgs = append(seqNumsOfChainsWithNewMsgs, nextSeqNum[chainSel]) + } + if len(chainsWithNewMsgs) > 0 { + ccipReader.On("NextSeqNum", ctx, chainsWithNewMsgs).Return(seqNumsOfChainsWithNewMsgs, nil).Maybe() + } + sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) + ccipReader.On("NextSeqNum", ctx, sourceChains).Return(seqNums, nil).Maybe() p := NewPlugin( ctx, From bd2927f276e9034b46c3fbce1e2d99c381698b46 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Tue, 27 Aug 2024 17:38:25 +0300 Subject: [PATCH 05/12] test fixes --- commit/plugin_e2e_test.go | 49 +++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index 1a37068f7..8d6ab3b5e 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -11,6 +11,7 @@ import ( mapset "github.com/deckarep/golang-set/v2" "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3" + "github.com/smartcontractkit/chainlink-common/pkg/utils/tests" "github.com/smartcontractkit/libocr/commontypes" "github.com/smartcontractkit/libocr/offchainreporting2plus/ocr3types" libocrtypes "github.com/smartcontractkit/libocr/ragep2p/types" @@ -26,13 +27,17 @@ import ( "github.com/smartcontractkit/chainlink-ccip/plugintypes" ) -func TestPlugin_E2E(t *testing.T) { - destChain := ccipocr3.ChainSelector(1) - sourceChain1 := ccipocr3.ChainSelector(2) - sourceChain2 := ccipocr3.ChainSelector(3) +const ( + destChain = ccipocr3.ChainSelector(1) + sourceChain1 = ccipocr3.ChainSelector(2) + sourceChain2 = ccipocr3.ChainSelector(3) +) + +func TestPlugin_E2E_AllNodesAgree(t *testing.T) { require.Len(t, mapset.NewSet(destChain, sourceChain1, sourceChain2).ToSlice(), 3) + require.Less(t, sourceChain1, sourceChain2, "test below expected source chain1 to be LT chain2") - ctx := context.Background() + ctx := tests.Context(t) lggr := logger.Test(t) oracleIDs := []commontypes.OracleID{1, 2, 3} @@ -173,29 +178,35 @@ func setupNode( } } - homeChainReader.On("GetFChain").Return(fChain, nil) + homeChainReader.EXPECT().GetFChain().Return(fChain, nil) for peerID, supportedChains := range supportedChainsForPeer { - homeChainReader.On("GetSupportedChainsForPeer", peerID).Return(supportedChains, nil).Maybe() + homeChainReader.EXPECT().GetSupportedChainsForPeer(peerID).Return(supportedChains, nil).Maybe() } knownCCIPChains := mapset.NewSet[ccipocr3.ChainSelector]() for chainSel, cfg := range chainCfg { - homeChainReader.On("GetChainConfig", chainSel).Return(cfg, nil).Maybe() + homeChainReader.EXPECT().GetChainConfig(chainSel).Return(cfg, nil).Maybe() knownCCIPChains.Add(chainSel) } - homeChainReader.On("GetKnownCCIPChains").Return(knownCCIPChains, nil).Maybe() + homeChainReader.EXPECT().GetKnownCCIPChains().Return(knownCCIPChains, nil).Maybe() sourceChains := make([]ccipocr3.ChainSelector, 0) - seqNums := make([]ccipocr3.SeqNum, 0) - chainsWithNewMsgs := make([]ccipocr3.ChainSelector, 0) - for chainSel, offRampNextSeqNum := range nextSeqNum { + for chainSel := range nextSeqNum { sourceChains = append(sourceChains, chainSel) - seqNums = append(seqNums, offRampNextSeqNum) + } + sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) + + offRampNextSeqNums := make([]ccipocr3.SeqNum, 0) + chainsWithNewMsgs := make([]ccipocr3.ChainSelector, 0) + for _, sourceChain := range sourceChains { + offRampNextSeqNum, ok := nextSeqNum[sourceChain] + assert.True(t, ok) + offRampNextSeqNums = append(offRampNextSeqNums, offRampNextSeqNum) newMsgs := make([]ccipocr3.Message, 0) - numNewMsgs := (onRampLastSeqNum[chainSel] - offRampNextSeqNum) + 1 + numNewMsgs := (onRampLastSeqNum[sourceChain] - offRampNextSeqNum) + 1 for i := uint64(0); i < uint64(numNewMsgs); i++ { messageID := sha256.Sum256([]byte(fmt.Sprintf("%d", uint64(offRampNextSeqNum)+i))) newMsgs = append(newMsgs, ccipocr3.Message{ @@ -206,25 +217,23 @@ func setupNode( }) } - ccipReader.On("MsgsBetweenSeqNums", ctx, chainSel, + ccipReader.EXPECT().MsgsBetweenSeqNums(ctx, sourceChain, ccipocr3.NewSeqNumRange(offRampNextSeqNum, offRampNextSeqNum)). Return(newMsgs, nil).Maybe() if len(newMsgs) > 0 { - chainsWithNewMsgs = append(chainsWithNewMsgs, chainSel) + chainsWithNewMsgs = append(chainsWithNewMsgs, sourceChain) } } - sort.Slice(chainsWithNewMsgs, func(i, j int) bool { return chainsWithNewMsgs[i] < chainsWithNewMsgs[j] }) seqNumsOfChainsWithNewMsgs := make([]ccipocr3.SeqNum, 0) for _, chainSel := range chainsWithNewMsgs { seqNumsOfChainsWithNewMsgs = append(seqNumsOfChainsWithNewMsgs, nextSeqNum[chainSel]) } if len(chainsWithNewMsgs) > 0 { - ccipReader.On("NextSeqNum", ctx, chainsWithNewMsgs).Return(seqNumsOfChainsWithNewMsgs, nil).Maybe() + ccipReader.EXPECT().NextSeqNum(ctx, chainsWithNewMsgs).Return(seqNumsOfChainsWithNewMsgs, nil).Maybe() } - sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) - ccipReader.On("NextSeqNum", ctx, sourceChains).Return(seqNums, nil).Maybe() + ccipReader.EXPECT().NextSeqNum(ctx, sourceChains).Return(offRampNextSeqNums, nil).Maybe() p := NewPlugin( ctx, From aa882698b90c56c9ebfb561f1143a9c84999ce7c Mon Sep 17 00:00:00 2001 From: dimkouv Date: Wed, 28 Aug 2024 11:12:07 +0300 Subject: [PATCH 06/12] add expected transmitted reports checks --- commit/plugin_e2e_test.go | 43 +++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index 8d6ab3b5e..b4eb0c1d3 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -75,17 +75,22 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { } nodes := make([]ocr3types.ReportingPlugin[[]byte], len(oracleIDs)) + var reportCodec ccipocr3.CommitPluginCodec reportingCfg := ocr3types.ReportingPluginConfig{F: 1} for i := range oracleIDs { n := setupNode(ctx, t, lggr, oracleIDs[i], reportingCfg, oracleIDToPeerID, cfg, homeChainConfig, offRampNextSeqNum, onRampLastSeqNum) nodes[i] = n.node + if i == 0 { + reportCodec = n.reportCodec + } } testCases := []struct { - name string - prevOutcome Outcome - expOutcome Outcome + name string + prevOutcome Outcome + expOutcome Outcome + expTransmittedReports []ccipocr3.CommitPluginReport }{ { name: "empty previous outcome, should select ranges for report", @@ -113,14 +118,27 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { { ChainSel: sourceChain1, SeqNumsRange: ccipocr3.SeqNumRange{0xa, 0xa}, - MerkleRoot: ccipocr3.Bytes32{0x4a, 0x44, 0xdc, 0x15, 0x36, 0x42, 0x4, 0xa8, 0xf, 0xe8, 0xe, - 0x90, 0x39, 0x45, 0x5c, 0xc1, 0x60, 0x82, 0x81, 0x82, 0xf, 0xe2, 0xb2, 0x4f, 0x1e, 0x52, - 0x33, 0xad, 0xe6, 0xaf, 0x1d, 0xd5}, + MerkleRoot: merkleRoot1, }, }, TokenPrices: make([]ccipocr3.TokenPrice, 0), GasPrices: make([]ccipocr3.GasPriceChain, 0), }, + expTransmittedReports: []ccipocr3.CommitPluginReport{ + { + MerkleRoots: []ccipocr3.MerkleRootChain{ + { + ChainSel: sourceChain1, + SeqNumsRange: ccipocr3.NewSeqNumRange(0xa, 0xa), + MerkleRoot: merkleRoot1, + }, + }, + PriceUpdates: ccipocr3.PriceUpdates{ + TokenPriceUpdates: []ccipocr3.TokenPrice{}, + GasPriceUpdates: []ccipocr3.GasPriceChain{}, + }, + }, + }, }, } @@ -135,6 +153,13 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { decodedOutcome, err := DecodeOutcome(res.Outcome) assert.NoError(t, err) assert.Equal(t, tc.expOutcome, decodedOutcome) + + assert.Len(t, res.Transmitted, len(tc.expTransmittedReports)) + for i := range res.Transmitted { + decoded, err := reportCodec.Decode(ctx, res.Transmitted[i].Report) + assert.NoError(t, err) + assert.Equal(t, tc.expTransmittedReports[i], decoded) + } }) } } @@ -257,3 +282,9 @@ func setupNode( msgHasher: msgHasher, } } + +// merkleRoot1 is the markle root that the test generates, the merkle root generation logic is not supposed to be +// tested in this context, so we just assume it's correct. +var merkleRoot1 = ccipocr3.Bytes32{0x4a, 0x44, 0xdc, 0x15, 0x36, 0x42, 0x4, 0xa8, 0xf, 0xe8, 0xe, + 0x90, 0x39, 0x45, 0x5c, 0xc1, 0x60, 0x82, 0x81, 0x82, 0xf, 0xe2, 0xb2, 0x4f, 0x1e, 0x52, + 0x33, 0xad, 0xe6, 0xaf, 0x1d, 0xd5} From 958cac6126d7dde08d460b71d02c288487bd9452 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Wed, 28 Aug 2024 11:21:53 +0300 Subject: [PATCH 07/12] test inflight and max inflight check attempts --- commit/plugin_e2e_test.go | 73 ++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 28 deletions(-) diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index b4eb0c1d3..44c52ce9a 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -69,7 +69,7 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { cfg := pluginconfig.CommitPluginConfig{ DestChain: destChain, NewMsgScanBatchSize: 100, - MaxReportTransmissionCheckAttempts: 0, + MaxReportTransmissionCheckAttempts: 2, SyncTimeout: 10 * time.Second, SyncFrequency: time.Hour, } @@ -86,6 +86,30 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { } } + outcomeIntervalsSelected := Outcome{ + OutcomeType: ReportIntervalsSelected, + RangesSelectedForReport: []plugintypes.ChainRange{ + {ChainSel: sourceChain1, SeqNumRange: ccipocr3.SeqNumRange{10, 10}}, + {ChainSel: sourceChain2, SeqNumRange: ccipocr3.SeqNumRange{20, 20}}, + }, + } + + outcomeReportGenerated := Outcome{ + OutcomeType: ReportGenerated, + RootsToReport: []ccipocr3.MerkleRootChain{ + { + ChainSel: sourceChain1, + SeqNumsRange: ccipocr3.SeqNumRange{0xa, 0xa}, + MerkleRoot: merkleRoot1, + }, + }, + TokenPrices: make([]ccipocr3.TokenPrice, 0), + GasPrices: make([]ccipocr3.GasPriceChain, 0), + } + + outcomeReportGeneratedOneInflightCheck := outcomeReportGenerated + outcomeReportGeneratedOneInflightCheck.ReportTransmissionCheckAttempts = 1 + testCases := []struct { name string prevOutcome Outcome @@ -95,35 +119,12 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { { name: "empty previous outcome, should select ranges for report", prevOutcome: Outcome{}, - expOutcome: Outcome{ - OutcomeType: ReportIntervalsSelected, - RangesSelectedForReport: []plugintypes.ChainRange{ - {ChainSel: sourceChain1, SeqNumRange: ccipocr3.SeqNumRange{10, 10}}, - {ChainSel: sourceChain2, SeqNumRange: ccipocr3.SeqNumRange{20, 20}}, - }, - }, + expOutcome: outcomeIntervalsSelected, }, { - name: "selected ranges for report in previous outcome", - prevOutcome: Outcome{ - OutcomeType: ReportIntervalsSelected, - RangesSelectedForReport: []plugintypes.ChainRange{ - {ChainSel: sourceChain1, SeqNumRange: ccipocr3.SeqNumRange{10, 10}}, - {ChainSel: sourceChain2, SeqNumRange: ccipocr3.SeqNumRange{20, 20}}, - }, - }, - expOutcome: Outcome{ - OutcomeType: ReportGenerated, - RootsToReport: []ccipocr3.MerkleRootChain{ - { - ChainSel: sourceChain1, - SeqNumsRange: ccipocr3.SeqNumRange{0xa, 0xa}, - MerkleRoot: merkleRoot1, - }, - }, - TokenPrices: make([]ccipocr3.TokenPrice, 0), - GasPrices: make([]ccipocr3.GasPriceChain, 0), - }, + name: "selected ranges for report in previous outcome", + prevOutcome: outcomeIntervalsSelected, + expOutcome: outcomeReportGenerated, expTransmittedReports: []ccipocr3.CommitPluginReport{ { MerkleRoots: []ccipocr3.MerkleRootChain{ @@ -140,6 +141,22 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { }, }, }, + { + name: "report generated in previous outcome, still inflight", + prevOutcome: outcomeReportGenerated, + expOutcome: Outcome{ + OutcomeType: ReportInFlight, + ReportTransmissionCheckAttempts: 1, + }, + }, + { + name: "report generated in previous outcome, still inflight, reached all inflight check attempts", + prevOutcome: outcomeReportGeneratedOneInflightCheck, + expOutcome: Outcome{ + OutcomeType: ReportTransmissionFailed, + ReportTransmissionCheckAttempts: 0, + }, + }, } for _, tc := range testCases { From 0debae2a9d6fd5167c568a5f6370d3b01b950ac6 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Wed, 28 Aug 2024 11:37:19 +0300 Subject: [PATCH 08/12] deterministic build report --- commit/outcome.go | 2 ++ commit/outcome_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++ commit/types.go | 13 +++++++++++-- 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 commit/outcome_test.go diff --git a/commit/outcome.go b/commit/outcome.go index 0f78a06c6..f1aaa6443 100644 --- a/commit/outcome.go +++ b/commit/outcome.go @@ -111,6 +111,8 @@ func buildReport( outcomeType = ReportEmpty } + sort.Slice(roots, func(i, j int) bool { return roots[i].ChainSel < roots[j].ChainSel }) + outcome := Outcome{ OutcomeType: outcomeType, RootsToReport: roots, diff --git a/commit/outcome_test.go b/commit/outcome_test.go new file mode 100644 index 000000000..1455c058e --- /dev/null +++ b/commit/outcome_test.go @@ -0,0 +1,44 @@ +package commit + +import ( + "testing" + + cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3" + "github.com/smartcontractkit/libocr/offchainreporting2plus/types" + "github.com/stretchr/testify/require" +) + +func Test_buildReport(t *testing.T) { + t.Run("determinism check", func(t *testing.T) { + const rounds = 50 + + obs := ConsensusObservation{ + MerkleRoots: map[cciptypes.ChainSelector]cciptypes.MerkleRootChain{ + cciptypes.ChainSelector(1): { + ChainSel: 1, + SeqNumsRange: cciptypes.NewSeqNumRange(10, 20), + MerkleRoot: cciptypes.Bytes32{1}, + }, + cciptypes.ChainSelector(2): { + ChainSel: 2, + SeqNumsRange: cciptypes.NewSeqNumRange(20, 30), + MerkleRoot: cciptypes.Bytes32{2}, + }, + }, + GasPrices: map[cciptypes.ChainSelector]cciptypes.BigInt{ + cciptypes.ChainSelector(1): cciptypes.NewBigIntFromInt64(1000), + cciptypes.ChainSelector(2): cciptypes.NewBigIntFromInt64(2000), + }, + TokenPrices: map[types.Account]cciptypes.BigInt{ + types.Account("1"): cciptypes.NewBigIntFromInt64(1000), + types.Account("2"): cciptypes.NewBigIntFromInt64(2000), + }, + } + + for i := 0; i < rounds; i++ { + report1 := buildReport(Query{}, obs) + report2 := buildReport(Query{}, obs) + require.Equal(t, report1, report2) + } + }) +} diff --git a/commit/types.go b/commit/types.go index b5a1a62a1..4dacc09d8 100644 --- a/commit/types.go +++ b/commit/types.go @@ -6,6 +6,7 @@ import ( "sort" "github.com/smartcontractkit/libocr/offchainreporting2plus/types" + "golang.org/x/exp/maps" "github.com/smartcontractkit/chainlink-ccip/plugintypes" @@ -159,8 +160,12 @@ type ConsensusObservation struct { // GasPricesArray returns a list of gas prices func (co ConsensusObservation) GasPricesArray() []cciptypes.GasPriceChain { + chains := maps.Keys(co.GasPrices) + sort.Slice(chains, func(i, j int) bool { return chains[i] < chains[j] }) + gasPrices := make([]cciptypes.GasPriceChain, 0, len(co.GasPrices)) - for chain, gasPrice := range co.GasPrices { + for _, chain := range chains { + gasPrice := co.GasPrices[chain] gasPrices = append(gasPrices, cciptypes.NewGasPriceChain(gasPrice.Int, chain)) } @@ -169,8 +174,12 @@ func (co ConsensusObservation) GasPricesArray() []cciptypes.GasPriceChain { // TokenPricesArray returns a list of token prices func (co ConsensusObservation) TokenPricesArray() []cciptypes.TokenPrice { + tokenIDs := maps.Keys(co.TokenPrices) + sort.Slice(tokenIDs, func(i, j int) bool { return tokenIDs[i] < tokenIDs[j] }) + tokenPrices := make([]cciptypes.TokenPrice, 0, len(co.TokenPrices)) - for tokenID, tokenPrice := range co.TokenPrices { + for _, tokenID := range tokenIDs { + tokenPrice := co.TokenPrices[tokenID] tokenPrices = append(tokenPrices, cciptypes.NewTokenPrice(tokenID, tokenPrice.Int)) } From aeb89522628177d03b06a58a406978ec8c6c7046 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Wed, 28 Aug 2024 13:00:57 +0300 Subject: [PATCH 09/12] makram review pass --- commit/types.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/commit/types.go b/commit/types.go index 4dacc09d8..22a65e780 100644 --- a/commit/types.go +++ b/commit/types.go @@ -6,7 +6,6 @@ import ( "sort" "github.com/smartcontractkit/libocr/offchainreporting2plus/types" - "golang.org/x/exp/maps" "github.com/smartcontractkit/chainlink-ccip/plugintypes" @@ -160,28 +159,22 @@ type ConsensusObservation struct { // GasPricesArray returns a list of gas prices func (co ConsensusObservation) GasPricesArray() []cciptypes.GasPriceChain { - chains := maps.Keys(co.GasPrices) - sort.Slice(chains, func(i, j int) bool { return chains[i] < chains[j] }) - gasPrices := make([]cciptypes.GasPriceChain, 0, len(co.GasPrices)) - for _, chain := range chains { - gasPrice := co.GasPrices[chain] + for chain, gasPrice := range co.GasPrices { gasPrices = append(gasPrices, cciptypes.NewGasPriceChain(gasPrice.Int, chain)) } + sort.Slice(gasPrices, func(i, j int) bool { return gasPrices[i].ChainSel < gasPrices[j].ChainSel }) return gasPrices } // TokenPricesArray returns a list of token prices func (co ConsensusObservation) TokenPricesArray() []cciptypes.TokenPrice { - tokenIDs := maps.Keys(co.TokenPrices) - sort.Slice(tokenIDs, func(i, j int) bool { return tokenIDs[i] < tokenIDs[j] }) - tokenPrices := make([]cciptypes.TokenPrice, 0, len(co.TokenPrices)) - for _, tokenID := range tokenIDs { - tokenPrice := co.TokenPrices[tokenID] + for tokenID, tokenPrice := range co.TokenPrices { tokenPrices = append(tokenPrices, cciptypes.NewTokenPrice(tokenID, tokenPrice.Int)) } + sort.Slice(tokenPrices, func(i, j int) bool { return tokenPrices[i].TokenID < tokenPrices[j].TokenID }) return tokenPrices } From 6c2e6e5feec1c9df1d823131835f3089eb84c95d Mon Sep 17 00:00:00 2001 From: dimkouv Date: Wed, 28 Aug 2024 13:52:34 +0300 Subject: [PATCH 10/12] fix issue on WaitingForReportTransmission state --- commit/outcome.go | 22 +++++++++++++++++----- commit/outcome_test.go | 4 ++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/commit/outcome.go b/commit/outcome.go index f1aaa6443..3fcce9826 100644 --- a/commit/outcome.go +++ b/commit/outcome.go @@ -37,7 +37,7 @@ func (p *Plugin) Outcome( outcome = ReportRangesOutcome(commitQuery, consensusObservation) case BuildingReport: - outcome = buildReport(commitQuery, consensusObservation) + outcome = buildReport(commitQuery, consensusObservation, previousOutcome) case WaitingForReportTransmission: outcome = checkForReportTransmission( @@ -67,6 +67,7 @@ func ReportRangesOutcome( observedOnRampMaxSeqNumsMap := consensusObservation.OnRampMaxSeqNums observedOffRampNextSeqNumsMap := consensusObservation.OffRampNextSeqNums + offRampNextSeqNums := make([]plugintypes.SeqNumChain, 0) for chainSel, offRampNextSeqNum := range observedOffRampNextSeqNumsMap { onRampMaxSeqNum, exists := observedOnRampMaxSeqNumsMap[chainSel] @@ -85,14 +86,23 @@ func ReportRangesOutcome( } rangesToReport = append(rangesToReport, chainRange) } + + offRampNextSeqNums = append(offRampNextSeqNums, plugintypes.SeqNumChain{ + ChainSel: chainSel, + SeqNum: offRampNextSeqNum, + }) } // deterministic outcome sort.Slice(rangesToReport, func(i, j int) bool { return rangesToReport[i].ChainSel < rangesToReport[j].ChainSel }) + sort.Slice(offRampNextSeqNums, func(i, j int) bool { + return offRampNextSeqNums[i].ChainSel < offRampNextSeqNums[j].ChainSel + }) outcome := Outcome{ OutcomeType: ReportIntervalsSelected, RangesSelectedForReport: rangesToReport, + OffRampNextSeqNums: offRampNextSeqNums, } return outcome @@ -103,6 +113,7 @@ func ReportRangesOutcome( func buildReport( _ Query, consensusObservation ConsensusObservation, + prevOutcome Outcome, ) Outcome { roots := maps.Values(consensusObservation.MerkleRoots) @@ -114,10 +125,11 @@ func buildReport( sort.Slice(roots, func(i, j int) bool { return roots[i].ChainSel < roots[j].ChainSel }) outcome := Outcome{ - OutcomeType: outcomeType, - RootsToReport: roots, - GasPrices: consensusObservation.GasPricesArray(), - TokenPrices: consensusObservation.TokenPricesArray(), + OutcomeType: outcomeType, + RootsToReport: roots, + GasPrices: consensusObservation.GasPricesArray(), + TokenPrices: consensusObservation.TokenPricesArray(), + OffRampNextSeqNums: prevOutcome.OffRampNextSeqNums, } return outcome diff --git a/commit/outcome_test.go b/commit/outcome_test.go index 1455c058e..350b26f2e 100644 --- a/commit/outcome_test.go +++ b/commit/outcome_test.go @@ -36,8 +36,8 @@ func Test_buildReport(t *testing.T) { } for i := 0; i < rounds; i++ { - report1 := buildReport(Query{}, obs) - report2 := buildReport(Query{}, obs) + report1 := buildReport(Query{}, obs, Outcome{}) + report2 := buildReport(Query{}, obs, Outcome{}) require.Equal(t, report1, report2) } }) From f9b94b1bdf61630eb3b7de4be367836bbf6e29e4 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Wed, 28 Aug 2024 13:53:11 +0300 Subject: [PATCH 11/12] add more tests for WaitingForReportTransmission state --- commit/plugin_e2e_test.go | 62 +++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index 44c52ce9a..1168df9be 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -75,16 +75,8 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { } nodes := make([]ocr3types.ReportingPlugin[[]byte], len(oracleIDs)) - var reportCodec ccipocr3.CommitPluginCodec + reportingCfg := ocr3types.ReportingPluginConfig{F: 1} - for i := range oracleIDs { - n := setupNode(ctx, t, lggr, oracleIDs[i], reportingCfg, oracleIDToPeerID, - cfg, homeChainConfig, offRampNextSeqNum, onRampLastSeqNum) - nodes[i] = n.node - if i == 0 { - reportCodec = n.reportCodec - } - } outcomeIntervalsSelected := Outcome{ OutcomeType: ReportIntervalsSelected, @@ -92,6 +84,10 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { {ChainSel: sourceChain1, SeqNumRange: ccipocr3.SeqNumRange{10, 10}}, {ChainSel: sourceChain2, SeqNumRange: ccipocr3.SeqNumRange{20, 20}}, }, + OffRampNextSeqNums: []plugintypes.SeqNumChain{ + {ChainSel: sourceChain1, SeqNum: 10}, + {ChainSel: sourceChain2, SeqNum: 20}, + }, } outcomeReportGenerated := Outcome{ @@ -103,6 +99,10 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { MerkleRoot: merkleRoot1, }, }, + OffRampNextSeqNums: []plugintypes.SeqNumChain{ + {ChainSel: sourceChain1, SeqNum: 10}, + {ChainSel: sourceChain2, SeqNum: 20}, + }, TokenPrices: make([]ccipocr3.TokenPrice, 0), GasPrices: make([]ccipocr3.GasPriceChain, 0), } @@ -115,6 +115,9 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { prevOutcome Outcome expOutcome Outcome expTransmittedReports []ccipocr3.CommitPluginReport + + offRampNextSeqNumDefaultOverrideKeys []ccipocr3.ChainSelector + offRampNextSeqNumDefaultOverrideValues []ccipocr3.SeqNum }{ { name: "empty previous outcome, should select ranges for report", @@ -147,6 +150,10 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { expOutcome: Outcome{ OutcomeType: ReportInFlight, ReportTransmissionCheckAttempts: 1, + OffRampNextSeqNums: []plugintypes.SeqNumChain{ + {ChainSel: sourceChain1, SeqNum: 10}, + {ChainSel: sourceChain2, SeqNum: 20}, + }, }, }, { @@ -157,10 +164,39 @@ func TestPlugin_E2E_AllNodesAgree(t *testing.T) { ReportTransmissionCheckAttempts: 0, }, }, + { + name: "report generated in previous outcome, transmitted with success", + prevOutcome: outcomeReportGenerated, + offRampNextSeqNumDefaultOverrideKeys: []ccipocr3.ChainSelector{sourceChain1, sourceChain2}, + offRampNextSeqNumDefaultOverrideValues: []ccipocr3.SeqNum{11, 20}, + expOutcome: Outcome{ + OutcomeType: ReportTransmitted, + ReportTransmissionCheckAttempts: 0, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + var reportCodec ccipocr3.CommitPluginCodec + for i := range oracleIDs { + n := setupNode(ctx, t, lggr, oracleIDs[i], reportingCfg, oracleIDToPeerID, + cfg, homeChainConfig, offRampNextSeqNum, onRampLastSeqNum) + nodes[i] = n.node + if i == 0 { + reportCodec = n.reportCodec + } + + if len(tc.offRampNextSeqNumDefaultOverrideKeys) > 0 { + assert.Equal(t, len(tc.offRampNextSeqNumDefaultOverrideKeys), len(tc.offRampNextSeqNumDefaultOverrideValues)) + n.ccipReader.EXPECT().NextSeqNum(ctx, tc.offRampNextSeqNumDefaultOverrideKeys).Unset() + n.ccipReader.EXPECT(). + NextSeqNum(ctx, tc.offRampNextSeqNumDefaultOverrideKeys). + Return(tc.offRampNextSeqNumDefaultOverrideValues, nil). + Maybe() + } + } + encodedPrevOutcome, err := tc.prevOutcome.Encode() assert.NoError(t, err) runner := testhelpers.NewOCR3Runner(nodes, oracleIDs, encodedPrevOutcome) @@ -198,7 +234,7 @@ func setupNode( oracleIDToP2pID map[commontypes.OracleID]libocrtypes.PeerID, pluginCfg pluginconfig.CommitPluginConfig, chainCfg map[ccipocr3.ChainSelector]reader.ChainConfig, - nextSeqNum map[ccipocr3.ChainSelector]ccipocr3.SeqNum, + offRampNextSeqNum map[ccipocr3.ChainSelector]ccipocr3.SeqNum, onRampLastSeqNum map[ccipocr3.ChainSelector]ccipocr3.SeqNum, ) nodeSetup { ccipReader := reader_mock.NewMockCCIP(t) @@ -235,7 +271,7 @@ func setupNode( homeChainReader.EXPECT().GetKnownCCIPChains().Return(knownCCIPChains, nil).Maybe() sourceChains := make([]ccipocr3.ChainSelector, 0) - for chainSel := range nextSeqNum { + for chainSel := range offRampNextSeqNum { sourceChains = append(sourceChains, chainSel) } sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) @@ -243,7 +279,7 @@ func setupNode( offRampNextSeqNums := make([]ccipocr3.SeqNum, 0) chainsWithNewMsgs := make([]ccipocr3.ChainSelector, 0) for _, sourceChain := range sourceChains { - offRampNextSeqNum, ok := nextSeqNum[sourceChain] + offRampNextSeqNum, ok := offRampNextSeqNum[sourceChain] assert.True(t, ok) offRampNextSeqNums = append(offRampNextSeqNums, offRampNextSeqNum) @@ -270,7 +306,7 @@ func setupNode( seqNumsOfChainsWithNewMsgs := make([]ccipocr3.SeqNum, 0) for _, chainSel := range chainsWithNewMsgs { - seqNumsOfChainsWithNewMsgs = append(seqNumsOfChainsWithNewMsgs, nextSeqNum[chainSel]) + seqNumsOfChainsWithNewMsgs = append(seqNumsOfChainsWithNewMsgs, offRampNextSeqNum[chainSel]) } if len(chainsWithNewMsgs) > 0 { ccipReader.EXPECT().NextSeqNum(ctx, chainsWithNewMsgs).Return(seqNumsOfChainsWithNewMsgs, nil).Maybe() From 25320d0163d752b12513326a881baca18b2807d1 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Thu, 29 Aug 2024 10:49:13 +0300 Subject: [PATCH 12/12] remove test setup validations --- commit/plugin_e2e_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index 1168df9be..fc88383cd 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -34,9 +34,6 @@ const ( ) func TestPlugin_E2E_AllNodesAgree(t *testing.T) { - require.Len(t, mapset.NewSet(destChain, sourceChain1, sourceChain2).ToSlice(), 3) - require.Less(t, sourceChain1, sourceChain2, "test below expected source chain1 to be LT chain2") - ctx := tests.Context(t) lggr := logger.Test(t)