From 67e180e2ab16e322c8f225f74f5e503ef8261cd5 Mon Sep 17 00:00:00 2001 From: "Abdelrahman Soliman (Boda)" <2677789+asoliman92@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:44:47 +0200 Subject: [PATCH 1/3] Don't observe destChain fee (#392) * Don't observe destChain fee --- commit/chainfee/observation.go | 3 +- commit/chainfee/observation_test.go | 50 ++++++++++++++++++++------ commit/plugin_e2e_test.go | 43 ++++++++++------------ internal/plugincommon/chain_support.go | 2 +- 4 files changed, 60 insertions(+), 38 deletions(-) diff --git a/commit/chainfee/observation.go b/commit/chainfee/observation.go index f6716078d..a0e0df29e 100644 --- a/commit/chainfee/observation.go +++ b/commit/chainfee/observation.go @@ -31,8 +31,9 @@ func (p *processor) Observation( return Observation{}, err } + supportedChains.Remove(p.destChain) if supportedChains.Cardinality() == 0 { - p.lggr.Info("no supported chains, nothing to observe") + p.lggr.Info("no supported chains other than dest chain to observe") return Observation{}, nil } diff --git a/commit/chainfee/observation_test.go b/commit/chainfee/observation_test.go index 60c520d6a..8fa3bf4c8 100644 --- a/commit/chainfee/observation_test.go +++ b/commit/chainfee/observation_test.go @@ -3,9 +3,12 @@ package chainfee import ( "math/big" "math/rand" + "sort" "testing" "time" + "golang.org/x/exp/maps" + mapset "github.com/deckarep/golang-set/v2" "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/types" @@ -32,11 +35,15 @@ func Test_processor_Observation(t *testing.T) { fChain map[ccipocr3.ChainSelector]int expectedChainFeePriceUpdates map[ccipocr3.ChainSelector]Update - expErr bool + dstChain ccipocr3.ChainSelector + + expErr bool + emptyObs bool }{ { - name: "two chains", - supportedChains: []ccipocr3.ChainSelector{1}, + name: "two chains excluding dest", + supportedChains: []ccipocr3.ChainSelector{1, 2, 3}, + dstChain: 3, chainFeeComponents: map[ccipocr3.ChainSelector]types.ChainFeeComponents{ 1: { ExecutionFee: big.NewInt(10), @@ -86,9 +93,16 @@ func Test_processor_Observation(t *testing.T) { fChain: map[ccipocr3.ChainSelector]int{ 1: 1, 2: 2, + 3: 1, }, expErr: false, }, + { + name: "only dest chain", + supportedChains: []ccipocr3.ChainSelector{1}, + dstChain: 1, + emptyObs: true, + }, } for _, tc := range testCases { @@ -103,25 +117,32 @@ func Test_processor_Observation(t *testing.T) { p := &processor{ lggr: lggr, chainSupport: cs, + destChain: tc.dstChain, ccipReader: ccipReader, oracleID: oracleID, homeChain: homeChain, metricsReporter: NoopMetrics{}, } + supportedSet := mapset.NewSet(tc.supportedChains...) + cs.EXPECT().DestChain().Return(tc.dstChain).Maybe() cs.EXPECT().SupportedChains(oracleID). - Return(mapset.NewSet(tc.supportedChains...), nil) + Return(supportedSet, nil).Maybe() + + supportedSet.Remove(tc.dstChain) + slicesWithoutDst := supportedSet.ToSlice() + sort.Slice(slicesWithoutDst, func(i, j int) bool { return slicesWithoutDst[i] < slicesWithoutDst[j] }) - ccipReader.EXPECT().GetChainsFeeComponents(ctx, tc.supportedChains). - Return(tc.chainFeeComponents) + ccipReader.EXPECT().GetChainsFeeComponents(ctx, slicesWithoutDst). + Return(tc.chainFeeComponents).Maybe() - ccipReader.EXPECT().GetWrappedNativeTokenPriceUSD(ctx, tc.supportedChains). - Return(tc.nativeTokenPrices) + ccipReader.EXPECT().GetWrappedNativeTokenPriceUSD(ctx, slicesWithoutDst). + Return(tc.nativeTokenPrices).Maybe() - ccipReader.EXPECT().GetChainFeePriceUpdate(ctx, tc.supportedChains). - Return(tc.existingChainFeePriceUpdates) + ccipReader.EXPECT().GetChainFeePriceUpdate(ctx, slicesWithoutDst). + Return(tc.existingChainFeePriceUpdates).Maybe() - homeChain.EXPECT().GetFChain().Return(tc.fChain, nil) + homeChain.EXPECT().GetFChain().Return(tc.fChain, nil).Maybe() tStart := time.Now() obs, err := p.Observation(ctx, Outcome{}, Query{}) @@ -130,13 +151,20 @@ func Test_processor_Observation(t *testing.T) { require.Error(t, err) return } + if tc.emptyObs { + require.Empty(t, obs) + return + } require.NoError(t, err) require.GreaterOrEqual(t, obs.TimestampNow.UnixNano(), tStart.UnixNano()) require.LessOrEqual(t, obs.TimestampNow.UnixNano(), tEnd.UnixNano()) require.Equal(t, tc.chainFeeComponents, obs.FeeComponents) + require.ElementsMatch(t, slicesWithoutDst, maps.Keys(obs.FeeComponents)) require.Equal(t, tc.nativeTokenPrices, obs.NativeTokenPrices) + require.ElementsMatch(t, slicesWithoutDst, maps.Keys(obs.NativeTokenPrices)) require.Equal(t, tc.expectedChainFeePriceUpdates, obs.ChainFeeUpdates) + require.ElementsMatch(t, slicesWithoutDst, maps.Keys(obs.ChainFeeUpdates)) require.Equal(t, tc.fChain, obs.FChain) }) } diff --git a/commit/plugin_e2e_test.go b/commit/plugin_e2e_test.go index 1afd9449e..eb53b1ce9 100644 --- a/commit/plugin_e2e_test.go +++ b/commit/plugin_e2e_test.go @@ -438,11 +438,11 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { nodes := make([]ocr3types.ReportingPlugin[[]byte], len(oracleIDs)) newFeeComponents, newNativePrice, packedGasPrice := newRandomFees() - expectedChainFeeOutcome := chainfee.Outcome{ + expectedChain1FeeOutcome := chainfee.Outcome{ GasPrices: []ccipocr3.GasPriceChain{ { GasPrice: packedGasPrice, - ChainSel: destChain, + ChainSel: sourceChain1, }, }, } @@ -464,10 +464,6 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { MerkleRootOutcome: merkleOutcome, ChainFeeOutcome: chainfee.Outcome{ GasPrices: []ccipocr3.GasPriceChain{ - { - GasPrice: packedGasPrice, - ChainSel: destChain, - }, { GasPrice: packedGasPrice, ChainSel: sourceChain1, @@ -485,7 +481,6 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { GetChainsFeeComponents(params.ctx, mock.Anything). Return( map[ccipocr3.ChainSelector]types.ChainFeeComponents{ - destChain: newFeeComponents, sourceChain1: newFeeComponents, sourceChain2: newFeeComponents, }) @@ -493,7 +488,6 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { m.EXPECT(). GetWrappedNativeTokenPriceUSD(params.ctx, mock.Anything). Return(map[ccipocr3.ChainSelector]ccipocr3.BigInt{ - destChain: newNativePrice, sourceChain1: newNativePrice, sourceChain2: newNativePrice, }) @@ -504,7 +498,7 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { prevOutcome: committypes.Outcome{}, expOutcome: committypes.Outcome{ MerkleRootOutcome: merkleOutcome, - ChainFeeOutcome: expectedChainFeeOutcome, + ChainFeeOutcome: expectedChain1FeeOutcome, }, expTransmittedReportLen: 1, mockCCIPReader: func(m *readerpkg_mock.MockCCIPReader) { @@ -512,13 +506,12 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { GetChainsFeeComponents(params.ctx, mock.Anything). Return( map[ccipocr3.ChainSelector]types.ChainFeeComponents{ - destChain: newFeeComponents, + sourceChain1: newFeeComponents, }) m.EXPECT(). GetWrappedNativeTokenPriceUSD(params.ctx, mock.Anything). Return(map[ccipocr3.ChainSelector]ccipocr3.BigInt{ - destChain: newNativePrice, sourceChain1: newNativePrice, sourceChain2: newNativePrice, }) @@ -528,11 +521,11 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { name: "fee components should not be updated within deviation", prevOutcome: committypes.Outcome{ MerkleRootOutcome: merkleOutcome, - ChainFeeOutcome: expectedChainFeeOutcome, + ChainFeeOutcome: expectedChain1FeeOutcome, }, expOutcome: committypes.Outcome{ MerkleRootOutcome: noReportMerkleOutcome(params.rmnReportCfg), - ChainFeeOutcome: expectedChainFeeOutcome, + ChainFeeOutcome: expectedChain1FeeOutcome, }, expTransmittedReportLen: 1, mockCCIPReader: func(m *readerpkg_mock.MockCCIPReader) { @@ -540,13 +533,13 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { GetChainsFeeComponents(params.ctx, mock.Anything). Return( map[ccipocr3.ChainSelector]types.ChainFeeComponents{ - destChain: newFeeComponents, + sourceChain1: newFeeComponents, }) m.EXPECT(). GetWrappedNativeTokenPriceUSD(params.ctx, mock.Anything). Return(map[ccipocr3.ChainSelector]ccipocr3.BigInt{ - destChain: newNativePrice, + sourceChain1: newNativePrice, }) }, }, @@ -554,7 +547,7 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { name: "fresh fees (timestamped) should not be updated, even outside of deviation", prevOutcome: committypes.Outcome{ MerkleRootOutcome: merkleOutcome, - ChainFeeOutcome: expectedChainFeeOutcome, + ChainFeeOutcome: expectedChain1FeeOutcome, }, expOutcome: committypes.Outcome{ MerkleRootOutcome: noReportMerkleOutcome(params.rmnReportCfg), @@ -562,7 +555,7 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { GasPrices: []ccipocr3.GasPriceChain{ { GasPrice: newPackedGasPrice2, - ChainSel: destChain, + ChainSel: sourceChain1, }, }, }, @@ -573,13 +566,13 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { GetChainsFeeComponents(params.ctx, mock.Anything). Return( map[ccipocr3.ChainSelector]types.ChainFeeComponents{ - destChain: newFeeComponents2, + sourceChain1: newFeeComponents2, }) m.EXPECT(). GetWrappedNativeTokenPriceUSD(params.ctx, mock.Anything). Return(map[ccipocr3.ChainSelector]ccipocr3.BigInt{ - destChain: newNativePrice2, + sourceChain1: newNativePrice2, }) }, }, @@ -587,7 +580,7 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { name: "stale fees should be updated", prevOutcome: committypes.Outcome{ MerkleRootOutcome: merkleOutcome, - ChainFeeOutcome: expectedChainFeeOutcome, + ChainFeeOutcome: expectedChain1FeeOutcome, }, expOutcome: committypes.Outcome{ MerkleRootOutcome: noReportMerkleOutcome(params.rmnReportCfg), @@ -595,7 +588,7 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { GasPrices: []ccipocr3.GasPriceChain{ { GasPrice: newPackedGasPrice2, - ChainSel: destChain, + ChainSel: sourceChain1, }, }, }, @@ -606,12 +599,12 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { GetChainsFeeComponents(params.ctx, mock.Anything). Return( map[ccipocr3.ChainSelector]types.ChainFeeComponents{ - destChain: newFeeComponents2, + sourceChain1: newFeeComponents2, }) m.EXPECT(). GetWrappedNativeTokenPriceUSD(params.ctx, mock.Anything). Return(map[ccipocr3.ChainSelector]ccipocr3.BigInt{ - destChain: newNativePrice2, + sourceChain1: newNativePrice2, }) m.EXPECT().GetChainFeePriceUpdate(params.ctx, mock.Anything).Unset() @@ -621,9 +614,9 @@ func TestPlugin_E2E_AllNodesAgree_ChainFee(t *testing.T) { m.EXPECT(). GetChainFeePriceUpdate(params.ctx, mock.Anything). Return(map[ccipocr3.ChainSelector]plugintypes.TimestampedBig{ - destChain: { + sourceChain1: { Timestamp: t, - Value: expectedChainFeeOutcome.GasPrices[0].GasPrice, + Value: expectedChain1FeeOutcome.GasPrices[0].GasPrice, }, }) }, diff --git a/internal/plugincommon/chain_support.go b/internal/plugincommon/chain_support.go index 61df07ce0..8f6ebe092 100644 --- a/internal/plugincommon/chain_support.go +++ b/internal/plugincommon/chain_support.go @@ -87,7 +87,7 @@ func (c ccipChainSupport) SupportedChains(oracleID commontypes.OracleID) (mapset return mapset.NewSet[cciptypes.ChainSelector](), fmt.Errorf("error getting supported chains: %w", err) } - return supportedChains, nil + return supportedChains.Clone(), nil } // SupportsDestChain returns true if the given oracle supports the dest chain, returns false otherwise From f9157a76ad3d8245816a59d78c021471aebeb8e3 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Thu, 19 Dec 2024 11:23:26 -0500 Subject: [PATCH 2/3] Don't panic when decoding empty slice. (#383) --- pkg/types/ccipocr3/plugin_execute_types.go | 4 ++++ pkg/types/ccipocr3/plugin_execute_types_test.go | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/pkg/types/ccipocr3/plugin_execute_types.go b/pkg/types/ccipocr3/plugin_execute_types.go index 9ea1aefc6..8b4b5ad3c 100644 --- a/pkg/types/ccipocr3/plugin_execute_types.go +++ b/pkg/types/ccipocr3/plugin_execute_types.go @@ -33,6 +33,10 @@ func (eri ExecuteReportInfo) Encode() ([]byte, error) { // DecodeExecuteReportInfo is a version aware decode function for the execute // report info bytes. func DecodeExecuteReportInfo(data []byte) (ExecuteReportInfo, error) { + if len(data) == 0 { + return ExecuteReportInfo{}, nil + } + switch data[0] { case 1: var result ExecuteReportInfo diff --git a/pkg/types/ccipocr3/plugin_execute_types_test.go b/pkg/types/ccipocr3/plugin_execute_types_test.go index 974fd3b35..dffb438a9 100644 --- a/pkg/types/ccipocr3/plugin_execute_types_test.go +++ b/pkg/types/ccipocr3/plugin_execute_types_test.go @@ -28,6 +28,13 @@ func TestDecodeExecuteReportInfo(t *testing.T) { _, err := DecodeExecuteReportInfo(data) require.ErrorContains(t, err, "object") // not super helpful... } + + // empty + { + ri, err := DecodeExecuteReportInfo(nil) + require.NoError(t, err) + require.Equal(t, ExecuteReportInfo{}, ri) + } } func TestExecuteReportInfo_EncodeDecode(t *testing.T) { From 8c8e37e7a8180a2ab20648cc0c13e23f852feefa Mon Sep 17 00:00:00 2001 From: Will Winder Date: Thu, 19 Dec 2024 11:52:38 -0500 Subject: [PATCH 3/3] Finality violation detection. (#387) --- commit/factory.go | 7 +- execute/factory.go | 7 +- .../contractreader/contract_reader_facade.go | 47 ++++ mocks/pkg/contractreader/extended.go | 265 ++++++++++++++++++ pkg/contractreader/contractreader_facade.go | 9 +- pkg/contractreader/extended.go | 124 +++++++- pkg/contractreader/extended_test.go | 11 + pkg/contractreader/extended_unit_test.go | 6 +- pkg/contractreader/observed.go | 5 + pkg/reader/ccip_test.go | 3 + 10 files changed, 472 insertions(+), 12 deletions(-) diff --git a/commit/factory.go b/commit/factory.go index f7555b37d..07be68134 100644 --- a/commit/factory.go +++ b/commit/factory.go @@ -155,14 +155,17 @@ func (p *PluginFactory) NewReportingPlugin(ctx context.Context, config ocr3types oracleIDToP2PID[commontypes.OracleID(oracleID)] = node.P2pID } - // map types to the facade. + // Map contract readers to ContractReaderFacade: + // - Extended reader adds finality violation and contract binding management. + // - Observed reader adds metric reporting. readers := make(map[cciptypes.ChainSelector]contractreader.ContractReaderFacade, len(p.contractReaders)) for chain, cr := range p.contractReaders { chainID, err1 := sel.GetChainIDFromSelector(uint64(chain)) if err1 != nil { return nil, ocr3types.ReportingPluginInfo{}, fmt.Errorf("failed to get chain id from selector: %w", err1) } - readers[chain] = contractreader.NewObserverReader(cr, lggr, chainID) + readers[chain] = contractreader.NewExtendedContractReader( + contractreader.NewObserverReader(cr, lggr, chainID)) } // Bind the RMNHome contract diff --git a/execute/factory.go b/execute/factory.go index 99eeae754..82d3f97b4 100644 --- a/execute/factory.go +++ b/execute/factory.go @@ -149,14 +149,17 @@ func (p PluginFactory) NewReportingPlugin( oracleIDToP2PID[commontypes.OracleID(oracleID)] = node.P2pID } - // map types to the facade. + // Map contract readers to ContractReaderFacade: + // - Extended reader adds finality violation and contract binding management. + // - Observed reader adds metric reporting. readers := make(map[cciptypes.ChainSelector]contractreader.ContractReaderFacade) for chain, cr := range p.contractReaders { chainID, err1 := sel.GetChainIDFromSelector(uint64(chain)) if err1 != nil { return nil, ocr3types.ReportingPluginInfo{}, fmt.Errorf("failed to get chain id from selector: %w", err1) } - readers[chain] = contractreader.NewObserverReader(cr, lggr, chainID) + readers[chain] = contractreader.NewExtendedContractReader( + contractreader.NewObserverReader(cr, lggr, chainID)) } ccipReader := readerpkg.NewCCIPChainReader( diff --git a/mocks/pkg/contractreader/contract_reader_facade.go b/mocks/pkg/contractreader/contract_reader_facade.go index d5bc97a2b..fa561b7ed 100644 --- a/mocks/pkg/contractreader/contract_reader_facade.go +++ b/mocks/pkg/contractreader/contract_reader_facade.go @@ -183,6 +183,53 @@ func (_c *MockContractReaderFacade_GetLatestValue_Call) RunAndReturn(run func(co return _c } +// HealthReport provides a mock function with given fields: +func (_m *MockContractReaderFacade) HealthReport() map[string]error { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for HealthReport") + } + + var r0 map[string]error + if rf, ok := ret.Get(0).(func() map[string]error); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(map[string]error) + } + } + + return r0 +} + +// MockContractReaderFacade_HealthReport_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'HealthReport' +type MockContractReaderFacade_HealthReport_Call struct { + *mock.Call +} + +// HealthReport is a helper method to define mock.On call +func (_e *MockContractReaderFacade_Expecter) HealthReport() *MockContractReaderFacade_HealthReport_Call { + return &MockContractReaderFacade_HealthReport_Call{Call: _e.mock.On("HealthReport")} +} + +func (_c *MockContractReaderFacade_HealthReport_Call) Run(run func()) *MockContractReaderFacade_HealthReport_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockContractReaderFacade_HealthReport_Call) Return(_a0 map[string]error) *MockContractReaderFacade_HealthReport_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockContractReaderFacade_HealthReport_Call) RunAndReturn(run func() map[string]error) *MockContractReaderFacade_HealthReport_Call { + _c.Call.Return(run) + return _c +} + // QueryKey provides a mock function with given fields: ctx, contract, filter, limitAndSort, sequenceDataType func (_m *MockContractReaderFacade) QueryKey(ctx context.Context, contract types.BoundContract, filter query.KeyFilter, limitAndSort query.LimitAndSort, sequenceDataType interface{}) ([]types.Sequence, error) { ret := _m.Called(ctx, contract, filter, limitAndSort, sequenceDataType) diff --git a/mocks/pkg/contractreader/extended.go b/mocks/pkg/contractreader/extended.go index f562c8cc9..2b748854c 100644 --- a/mocks/pkg/contractreader/extended.go +++ b/mocks/pkg/contractreader/extended.go @@ -28,6 +28,65 @@ func (_m *MockExtended) EXPECT() *MockExtended_Expecter { return &MockExtended_Expecter{mock: &_m.Mock} } +// BatchGetLatestValues provides a mock function with given fields: ctx, request +func (_m *MockExtended) BatchGetLatestValues(ctx context.Context, request types.BatchGetLatestValuesRequest) (types.BatchGetLatestValuesResult, error) { + ret := _m.Called(ctx, request) + + if len(ret) == 0 { + panic("no return value specified for BatchGetLatestValues") + } + + var r0 types.BatchGetLatestValuesResult + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, types.BatchGetLatestValuesRequest) (types.BatchGetLatestValuesResult, error)); ok { + return rf(ctx, request) + } + if rf, ok := ret.Get(0).(func(context.Context, types.BatchGetLatestValuesRequest) types.BatchGetLatestValuesResult); ok { + r0 = rf(ctx, request) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.BatchGetLatestValuesResult) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, types.BatchGetLatestValuesRequest) error); ok { + r1 = rf(ctx, request) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockExtended_BatchGetLatestValues_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'BatchGetLatestValues' +type MockExtended_BatchGetLatestValues_Call struct { + *mock.Call +} + +// BatchGetLatestValues is a helper method to define mock.On call +// - ctx context.Context +// - request types.BatchGetLatestValuesRequest +func (_e *MockExtended_Expecter) BatchGetLatestValues(ctx interface{}, request interface{}) *MockExtended_BatchGetLatestValues_Call { + return &MockExtended_BatchGetLatestValues_Call{Call: _e.mock.On("BatchGetLatestValues", ctx, request)} +} + +func (_c *MockExtended_BatchGetLatestValues_Call) Run(run func(ctx context.Context, request types.BatchGetLatestValuesRequest)) *MockExtended_BatchGetLatestValues_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(types.BatchGetLatestValuesRequest)) + }) + return _c +} + +func (_c *MockExtended_BatchGetLatestValues_Call) Return(_a0 types.BatchGetLatestValuesResult, _a1 error) *MockExtended_BatchGetLatestValues_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockExtended_BatchGetLatestValues_Call) RunAndReturn(run func(context.Context, types.BatchGetLatestValuesRequest) (types.BatchGetLatestValuesResult, error)) *MockExtended_BatchGetLatestValues_Call { + _c.Call.Return(run) + return _c +} + // Bind provides a mock function with given fields: ctx, bindings func (_m *MockExtended) Bind(ctx context.Context, bindings []types.BoundContract) error { ret := _m.Called(ctx, bindings) @@ -295,6 +354,212 @@ func (_c *MockExtended_GetBindings_Call) RunAndReturn(run func(string) []contrac return _c } +// GetLatestValue provides a mock function with given fields: ctx, readIdentifier, confidenceLevel, params, returnVal +func (_m *MockExtended) GetLatestValue(ctx context.Context, readIdentifier string, confidenceLevel primitives.ConfidenceLevel, params interface{}, returnVal interface{}) error { + ret := _m.Called(ctx, readIdentifier, confidenceLevel, params, returnVal) + + if len(ret) == 0 { + panic("no return value specified for GetLatestValue") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, primitives.ConfidenceLevel, interface{}, interface{}) error); ok { + r0 = rf(ctx, readIdentifier, confidenceLevel, params, returnVal) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockExtended_GetLatestValue_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetLatestValue' +type MockExtended_GetLatestValue_Call struct { + *mock.Call +} + +// GetLatestValue is a helper method to define mock.On call +// - ctx context.Context +// - readIdentifier string +// - confidenceLevel primitives.ConfidenceLevel +// - params interface{} +// - returnVal interface{} +func (_e *MockExtended_Expecter) GetLatestValue(ctx interface{}, readIdentifier interface{}, confidenceLevel interface{}, params interface{}, returnVal interface{}) *MockExtended_GetLatestValue_Call { + return &MockExtended_GetLatestValue_Call{Call: _e.mock.On("GetLatestValue", ctx, readIdentifier, confidenceLevel, params, returnVal)} +} + +func (_c *MockExtended_GetLatestValue_Call) Run(run func(ctx context.Context, readIdentifier string, confidenceLevel primitives.ConfidenceLevel, params interface{}, returnVal interface{})) *MockExtended_GetLatestValue_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(primitives.ConfidenceLevel), args[3].(interface{}), args[4].(interface{})) + }) + return _c +} + +func (_c *MockExtended_GetLatestValue_Call) Return(_a0 error) *MockExtended_GetLatestValue_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockExtended_GetLatestValue_Call) RunAndReturn(run func(context.Context, string, primitives.ConfidenceLevel, interface{}, interface{}) error) *MockExtended_GetLatestValue_Call { + _c.Call.Return(run) + return _c +} + +// HealthReport provides a mock function with given fields: +func (_m *MockExtended) HealthReport() map[string]error { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for HealthReport") + } + + var r0 map[string]error + if rf, ok := ret.Get(0).(func() map[string]error); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(map[string]error) + } + } + + return r0 +} + +// MockExtended_HealthReport_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'HealthReport' +type MockExtended_HealthReport_Call struct { + *mock.Call +} + +// HealthReport is a helper method to define mock.On call +func (_e *MockExtended_Expecter) HealthReport() *MockExtended_HealthReport_Call { + return &MockExtended_HealthReport_Call{Call: _e.mock.On("HealthReport")} +} + +func (_c *MockExtended_HealthReport_Call) Run(run func()) *MockExtended_HealthReport_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockExtended_HealthReport_Call) Return(_a0 map[string]error) *MockExtended_HealthReport_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockExtended_HealthReport_Call) RunAndReturn(run func() map[string]error) *MockExtended_HealthReport_Call { + _c.Call.Return(run) + return _c +} + +// QueryKey provides a mock function with given fields: ctx, contract, filter, limitAndSort, sequenceDataType +func (_m *MockExtended) QueryKey(ctx context.Context, contract types.BoundContract, filter query.KeyFilter, limitAndSort query.LimitAndSort, sequenceDataType interface{}) ([]types.Sequence, error) { + ret := _m.Called(ctx, contract, filter, limitAndSort, sequenceDataType) + + if len(ret) == 0 { + panic("no return value specified for QueryKey") + } + + var r0 []types.Sequence + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, interface{}) ([]types.Sequence, error)); ok { + return rf(ctx, contract, filter, limitAndSort, sequenceDataType) + } + if rf, ok := ret.Get(0).(func(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, interface{}) []types.Sequence); ok { + r0 = rf(ctx, contract, filter, limitAndSort, sequenceDataType) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]types.Sequence) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, interface{}) error); ok { + r1 = rf(ctx, contract, filter, limitAndSort, sequenceDataType) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockExtended_QueryKey_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'QueryKey' +type MockExtended_QueryKey_Call struct { + *mock.Call +} + +// QueryKey is a helper method to define mock.On call +// - ctx context.Context +// - contract types.BoundContract +// - filter query.KeyFilter +// - limitAndSort query.LimitAndSort +// - sequenceDataType interface{} +func (_e *MockExtended_Expecter) QueryKey(ctx interface{}, contract interface{}, filter interface{}, limitAndSort interface{}, sequenceDataType interface{}) *MockExtended_QueryKey_Call { + return &MockExtended_QueryKey_Call{Call: _e.mock.On("QueryKey", ctx, contract, filter, limitAndSort, sequenceDataType)} +} + +func (_c *MockExtended_QueryKey_Call) Run(run func(ctx context.Context, contract types.BoundContract, filter query.KeyFilter, limitAndSort query.LimitAndSort, sequenceDataType interface{})) *MockExtended_QueryKey_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(types.BoundContract), args[2].(query.KeyFilter), args[3].(query.LimitAndSort), args[4].(interface{})) + }) + return _c +} + +func (_c *MockExtended_QueryKey_Call) Return(_a0 []types.Sequence, _a1 error) *MockExtended_QueryKey_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockExtended_QueryKey_Call) RunAndReturn(run func(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, interface{}) ([]types.Sequence, error)) *MockExtended_QueryKey_Call { + _c.Call.Return(run) + return _c +} + +// Unbind provides a mock function with given fields: ctx, bindings +func (_m *MockExtended) Unbind(ctx context.Context, bindings []types.BoundContract) error { + ret := _m.Called(ctx, bindings) + + if len(ret) == 0 { + panic("no return value specified for Unbind") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, []types.BoundContract) error); ok { + r0 = rf(ctx, bindings) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockExtended_Unbind_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Unbind' +type MockExtended_Unbind_Call struct { + *mock.Call +} + +// Unbind is a helper method to define mock.On call +// - ctx context.Context +// - bindings []types.BoundContract +func (_e *MockExtended_Expecter) Unbind(ctx interface{}, bindings interface{}) *MockExtended_Unbind_Call { + return &MockExtended_Unbind_Call{Call: _e.mock.On("Unbind", ctx, bindings)} +} + +func (_c *MockExtended_Unbind_Call) Run(run func(ctx context.Context, bindings []types.BoundContract)) *MockExtended_Unbind_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].([]types.BoundContract)) + }) + return _c +} + +func (_c *MockExtended_Unbind_Call) Return(_a0 error) *MockExtended_Unbind_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockExtended_Unbind_Call) RunAndReturn(run func(context.Context, []types.BoundContract) error) *MockExtended_Unbind_Call { + _c.Call.Return(run) + return _c +} + // NewMockExtended creates a new instance of MockExtended. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewMockExtended(t interface { diff --git a/pkg/contractreader/contractreader_facade.go b/pkg/contractreader/contractreader_facade.go index b2aea2a71..9ea7c93fb 100644 --- a/pkg/contractreader/contractreader_facade.go +++ b/pkg/contractreader/contractreader_facade.go @@ -9,6 +9,7 @@ import ( ) // ContractReaderFacade wraps the public functions of ContractReader in chainlink-common so that we can mock it. +// See types.ContractReader in chainlink-common/pkg/types/contract_reader.go for details. // //nolint:lll // don't read this interface. type ContractReaderFacade interface { @@ -17,5 +18,11 @@ type ContractReaderFacade interface { Bind(ctx context.Context, bindings []types.BoundContract) error Unbind(ctx context.Context, bindings []types.BoundContract) error QueryKey(ctx context.Context, contract types.BoundContract, filter query.KeyFilter, limitAndSort query.LimitAndSort, sequenceDataType any) ([]types.Sequence, error) - //mustEmbedUnimplementedContractReaderServer() + + // HealthReport returns a full health report of the callee including its dependencies. + // Keys are based on Name(), with nil values when healthy or errors otherwise. + // Use CopyHealth to collect reports from sub-services. + // This should run very fast, so avoid doing computation and instead prefer reporting pre-calculated state. + // On finality violation report must contain at least one ErrFinalityViolation. + HealthReport() map[string]error } diff --git a/pkg/contractreader/extended.go b/pkg/contractreader/extended.go index dfd8a9845..ccdd0540d 100644 --- a/pkg/contractreader/extended.go +++ b/pkg/contractreader/extended.go @@ -7,7 +7,9 @@ import ( "sync" "time" + "github.com/smartcontractkit/chainlink-common/pkg/services" "github.com/smartcontractkit/chainlink-common/pkg/types" + clcommontypes "github.com/smartcontractkit/chainlink-common/pkg/types" "github.com/smartcontractkit/chainlink-common/pkg/types/query" "github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives" @@ -15,16 +17,31 @@ import ( ) var ( - ErrTooManyBindings = errors.New("contract binding not found") - ErrNoBindings = errors.New("no bindings found") + ErrFinalityViolated = errors.New("finality violated") + ErrTooManyBindings = errors.New("too many bindings") + ErrNoBindings = errors.New("no bindings found") ) // Extended version of a ContractReader. type Extended interface { + // Unbind is included for compatibility with ContractReader + Unbind(ctx context.Context, bindings []types.BoundContract) error + // HealthReport is included for compatibility with ContractReader + HealthReport() map[string]error + Bind(ctx context.Context, bindings []types.BoundContract) error GetBindings(contractName string) []ExtendedBoundContract + // QueryKey is from the base contract reader interface. + QueryKey( + ctx context.Context, + contract types.BoundContract, + filter query.KeyFilter, + limitAndSort query.LimitAndSort, + sequenceDataType any, + ) ([]types.Sequence, error) + // ExtendedQueryKey performs automatic binding from contractName to the first bound contract. // An error is generated if there are more than one bound contract for the contractName. ExtendedQueryKey( @@ -35,6 +52,14 @@ type Extended interface { sequenceDataType any, ) ([]types.Sequence, error) + // GetLatestValue is from the base contract reader interface. + GetLatestValue( + ctx context.Context, + readIdentifier string, + confidenceLevel primitives.ConfidenceLevel, + params, returnVal any, + ) error + // ExtendedGetLatestValue performs automatic binding from contractName to the first bound contract, and // constructs a read identifier for a given method name. An error is generated if there are more than one // bound contract for the contractName. @@ -45,6 +70,12 @@ type Extended interface { params, returnVal any, ) error + // BatchGetLatestValues is from the base contract reader interface. + BatchGetLatestValues( + ctx context.Context, + request types.BatchGetLatestValuesRequest, + ) (types.BatchGetLatestValuesResult, error) + // ExtendedBatchGetLatestValues performs automatic binding from contractNames to bound contracts, and // contructs a BatchGetLatestValuesRequest with the resolved bindings. ExtendedBatchGetLatestValues( @@ -62,14 +93,18 @@ type ExtendedBoundContract struct { // extendedContractReader is an extended version of the contract reader. type extendedContractReader struct { - ContractReaderFacade + reader ContractReaderFacade contractBindingsByName map[string][]ExtendedBoundContract mu *sync.RWMutex } func NewExtendedContractReader(baseContractReader ContractReaderFacade) Extended { + // avoid double wrapping + if ecr, ok := baseContractReader.(Extended); ok { + return ecr + } return &extendedContractReader{ - ContractReaderFacade: baseContractReader, + reader: baseContractReader, contractBindingsByName: make(map[string][]ExtendedBoundContract), mu: &sync.RWMutex{}, } @@ -89,6 +124,29 @@ func (e *extendedContractReader) getOneBinding(contractName string) (ExtendedBou } } +func (e *extendedContractReader) QueryKey( + ctx context.Context, + contract types.BoundContract, + filter query.KeyFilter, + limitAndSort query.LimitAndSort, + sequenceDataType any, +) ([]types.Sequence, error) { + result, err := e.reader.QueryKey( + ctx, + contract, + filter, + limitAndSort, + sequenceDataType, + ) + + // reads may update the reader health, so check for violations after every read. + if e.hasFinalityViolation() { + return nil, ErrFinalityViolated + } + + return result, err +} + func (e *extendedContractReader) ExtendedQueryKey( ctx context.Context, contractName string, @@ -110,6 +168,28 @@ func (e *extendedContractReader) ExtendedQueryKey( ) } +func (e *extendedContractReader) GetLatestValue( + ctx context.Context, + readIdentifier string, + confidenceLevel primitives.ConfidenceLevel, + params, returnVal any, +) error { + err := e.reader.GetLatestValue( + ctx, + readIdentifier, + confidenceLevel, + params, + returnVal, + ) + + // reads may update the reader health, so check for violations after every read. + if e.hasFinalityViolation() { + return ErrFinalityViolated + } + + return err +} + func (e *extendedContractReader) ExtendedGetLatestValue( ctx context.Context, contractName, methodName string, @@ -131,6 +211,20 @@ func (e *extendedContractReader) ExtendedGetLatestValue( ) } +func (e *extendedContractReader) BatchGetLatestValues( + ctx context.Context, + request types.BatchGetLatestValuesRequest, +) (types.BatchGetLatestValuesResult, error) { + result, err := e.reader.BatchGetLatestValues(ctx, request) + + // reads may update the reader health, so check for violations after every read. + if e.hasFinalityViolation() { + return nil, ErrFinalityViolated + } + + return result, err +} + func (e *extendedContractReader) ExtendedBatchGetLatestValues( ctx context.Context, request ExtendedBatchGetLatestValuesRequest, @@ -150,7 +244,7 @@ func (e *extendedContractReader) ExtendedBatchGetLatestValues( } // Call the underlying BatchGetLatestValues with the converted request - return e.ContractReaderFacade.BatchGetLatestValues(ctx, convertedRequest) + return e.BatchGetLatestValues(ctx, convertedRequest) } func (e *extendedContractReader) Bind(ctx context.Context, allBindings []types.BoundContract) error { @@ -159,7 +253,7 @@ func (e *extendedContractReader) Bind(ctx context.Context, allBindings []types.B return nil } - err := e.ContractReaderFacade.Bind(ctx, validBindings) + err := e.reader.Bind(ctx, validBindings) if err != nil { return fmt.Errorf("failed to call ContractReader.Bind: %w", err) } @@ -201,5 +295,23 @@ func (e *extendedContractReader) bindingExists(b types.BoundContract) bool { return false } +// hasFinalityViolation checks the reader's HealthReport for a finality violated error. +// The report is based on the current known state, it does not proactively check for new errors. +// The state is typically updated as the LogPoller reads events from an rpc. +func (e *extendedContractReader) hasFinalityViolation() bool { + report := e.reader.HealthReport() + return services.ContainsError( + report, + clcommontypes.ErrFinalityViolated) +} + +func (e *extendedContractReader) Unbind(ctx context.Context, bindings []types.BoundContract) error { + return e.reader.Unbind(ctx, bindings) +} + +func (e *extendedContractReader) HealthReport() map[string]error { + return e.reader.HealthReport() +} + // Interface compliance check var _ Extended = (*extendedContractReader)(nil) diff --git a/pkg/contractreader/extended_test.go b/pkg/contractreader/extended_test.go index e218bf162..2b716efa8 100644 --- a/pkg/contractreader/extended_test.go +++ b/pkg/contractreader/extended_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/smartcontractkit/chainlink-common/pkg/types" @@ -47,3 +48,13 @@ func TestExtendedContractReader(t *testing.T) { assert.Equal(t, "0x123", bindings[0].Binding.Address) assert.Equal(t, "0x124", bindings[1].Binding.Address) } + +func TestDoubleWrap(t *testing.T) { + var cr contractreader.ContractReaderFacade + + wrapped := contractreader.NewExtendedContractReader(cr) + require.NotEqual(t, &cr, &wrapped) + + doubleWrapped := contractreader.NewExtendedContractReader(cr) + require.Equal(t, wrapped, doubleWrapped) +} diff --git a/pkg/contractreader/extended_unit_test.go b/pkg/contractreader/extended_unit_test.go index abef0ea15..a30014260 100644 --- a/pkg/contractreader/extended_unit_test.go +++ b/pkg/contractreader/extended_unit_test.go @@ -180,7 +180,7 @@ func TestExtendedBatchGetLatestValues(t *testing.T) { // Create extended reader with mock extendedReader := &extendedContractReader{ - ContractReaderFacade: mockReader, + reader: mockReader, contractBindingsByName: tt.bindings, mu: &sync.RWMutex{}, } @@ -211,3 +211,7 @@ func (m *mockContractReader) BatchGetLatestValues( ) (types.BatchGetLatestValuesResult, error) { return m.BatchGetLatestValuesResponse, nil } + +func (m *mockContractReader) HealthReport() map[string]error { + return nil +} diff --git a/pkg/contractreader/observed.go b/pkg/contractreader/observed.go index 699f5e1da..b64a5b426 100644 --- a/pkg/contractreader/observed.go +++ b/pkg/contractreader/observed.go @@ -90,6 +90,11 @@ func NewObserverReader( } } +func (o *Observed) HealthReport() map[string]error { + // Health report doesn't seem to be an IO operation, so no need to observe. + return o.ContractReaderFacade.HealthReport() +} + func (o *Observed) GetLatestValue( ctx context.Context, readIdentifier string, diff --git a/pkg/reader/ccip_test.go b/pkg/reader/ccip_test.go index 45271a8a2..09d4ec198 100644 --- a/pkg/reader/ccip_test.go +++ b/pkg/reader/ccip_test.go @@ -44,6 +44,7 @@ func TestCCIPChainReader_getSourceChainsConfig(t *testing.T) { destCR := reader_mocks.NewMockContractReaderFacade(t) destCR.EXPECT().Bind(mock.Anything, mock.Anything).Return(nil) + destCR.EXPECT().HealthReport().Return(nil) destCR.EXPECT().GetLatestValue( mock.Anything, mock.Anything, @@ -837,6 +838,7 @@ func withReturnValueOverridden(mapper func(returnVal interface{})) func(ctx cont func TestCCIPChainReader_getDestFeeQuoterStaticConfig(t *testing.T) { destCR := reader_mocks.NewMockContractReaderFacade(t) destCR.EXPECT().Bind(mock.Anything, mock.Anything).Return(nil) + destCR.EXPECT().HealthReport().Return(nil) destCR.EXPECT().GetLatestValue( mock.Anything, mock.Anything, @@ -882,6 +884,7 @@ func TestCCIPChainReader_getFeeQuoterTokenPriceUSD(t *testing.T) { tokenAddr := []byte{0x3, 0x4} destCR := reader_mocks.NewMockContractReaderFacade(t) destCR.EXPECT().Bind(mock.Anything, mock.Anything).Return(nil) + destCR.EXPECT().HealthReport().Return(nil) destCR.EXPECT().GetLatestValue( mock.Anything, mock.Anything,