From 0c91e44ba1ba15758f96249ba39a47aa45729544 Mon Sep 17 00:00:00 2001 From: nogo <110664798+0xnogo@users.noreply.github.com> Date: Fri, 27 Sep 2024 15:15:44 +0400 Subject: [PATCH] Adding support for RMNRemote discovery (#155) * adding support for RMNRemote discovery * lint * Refactor contract discovery processor tests and add RMNRemote support * Refactor contract discovery processor tests and add RMNRemote support * rename to rmn * adding the getallchainconfigs * Refactor method name in consts.go * Refactor observation processing logic in ContractDiscoveryProcessor --- .../discovery/discoverytypes/types.go | 1 + internal/plugincommon/discovery/processor.go | 29 +++++++++++++- .../plugincommon/discovery/processor_test.go | 39 ++++++++++++++++--- pkg/consts/consts.go | 4 ++ pkg/reader/ccip.go | 7 +++- pkg/reader/ccip_test.go | 10 +++++ 6 files changed, 80 insertions(+), 10 deletions(-) diff --git a/internal/plugincommon/discovery/discoverytypes/types.go b/internal/plugincommon/discovery/discoverytypes/types.go index a7d578e56..6b714a186 100644 --- a/internal/plugincommon/discovery/discoverytypes/types.go +++ b/internal/plugincommon/discovery/discoverytypes/types.go @@ -16,6 +16,7 @@ type Observation struct { FChain map[ccipocr3.ChainSelector]int OnRamp map[ccipocr3.ChainSelector][]byte DestNonceManager []byte + RMNRemote []byte // TODO: some sort of request flag to avoid including this every time. // Request bool diff --git a/internal/plugincommon/discovery/processor.go b/internal/plugincommon/discovery/processor.go index 24ea83573..67d5bd17b 100644 --- a/internal/plugincommon/discovery/processor.go +++ b/internal/plugincommon/discovery/processor.go @@ -5,11 +5,12 @@ import ( "errors" "fmt" - "github.com/smartcontractkit/chainlink-common/pkg/logger" - cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3" "github.com/smartcontractkit/libocr/commontypes" ragep2ptypes "github.com/smartcontractkit/libocr/ragep2p/types" + "github.com/smartcontractkit/chainlink-common/pkg/logger" + cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3" + "github.com/smartcontractkit/chainlink-ccip/internal/plugincommon" "github.com/smartcontractkit/chainlink-ccip/internal/plugincommon/consensus" dt "github.com/smartcontractkit/chainlink-ccip/internal/plugincommon/discovery/discoverytypes" @@ -83,6 +84,7 @@ func (cdp *ContractDiscoveryProcessor) Observation( FChain: fChain, OnRamp: contracts[consts.ContractNameOnRamp], DestNonceManager: contracts[consts.ContractNameNonceManager][cdp.dest], + RMNRemote: contracts[consts.ContractNameRMNRemote][cdp.dest], }, nil } @@ -133,6 +135,7 @@ func (cdp *ContractDiscoveryProcessor) Outcome( // collect onramp and nonce manager addresses onrampAddrs := make(map[cciptypes.ChainSelector][][]byte) var nonceManagerAddrs [][]byte + var rmnRemoteAddrs [][]byte for _, ao := range aos { for chain, addr := range ao.Observation.OnRamp { // we don't want invalid observations to "poison" the consensus. @@ -151,6 +154,14 @@ func (cdp *ContractDiscoveryProcessor) Outcome( nonceManagerAddrs, ao.Observation.DestNonceManager, ) + if len(ao.Observation.RMNRemote) == 0 { + cdp.lggr.Warnf("skipping empty RMNRemote address in observation from Oracle %d", ao.OracleID) + continue + } + rmnRemoteAddrs = append( + rmnRemoteAddrs, + ao.Observation.RMNRemote, + ) } contracts := make(reader.ContractAddresses) @@ -189,6 +200,20 @@ func (cdp *ContractDiscoveryProcessor) Outcome( ) contracts[consts.ContractNameNonceManager] = nonceManagerConsensus + // RMNRemote address consensus + rmnRemoteConsensus := consensus.GetConsensusMap( + cdp.lggr, + "rmnRemote", + map[cciptypes.ChainSelector][][]byte{cdp.dest: rmnRemoteAddrs}, + fDestChainThresh, + ) + cdp.lggr.Infow("Determined consensus RMNRemote", + "rmnRemoteConsensus", rmnRemoteConsensus, + "rmnRemoteAddrs", rmnRemoteAddrs, + "fDestChainThresh", fDestChainThresh, + ) + contracts[consts.ContractNameRMNRemote] = rmnRemoteConsensus + // call Sync to bind contracts. if err := (*cdp.reader).Sync(context.Background(), contracts); err != nil { return dt.Outcome{}, fmt.Errorf("unable to sync contracts: %w", err) diff --git a/internal/plugincommon/discovery/processor_test.go b/internal/plugincommon/discovery/processor_test.go index de565b7f6..10a20af7b 100644 --- a/internal/plugincommon/discovery/processor_test.go +++ b/internal/plugincommon/discovery/processor_test.go @@ -6,14 +6,15 @@ import ( "testing" mapset "github.com/deckarep/golang-set/v2" - "github.com/smartcontractkit/chainlink-common/pkg/logger" - cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3" - "github.com/smartcontractkit/chainlink-common/pkg/utils/tests" "github.com/smartcontractkit/libocr/commontypes" ragep2ptypes "github.com/smartcontractkit/libocr/ragep2p/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/smartcontractkit/chainlink-common/pkg/logger" + cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3" + "github.com/smartcontractkit/chainlink-common/pkg/utils/tests" + "github.com/smartcontractkit/chainlink-ccip/internal/plugincommon" "github.com/smartcontractkit/chainlink-ccip/internal/plugincommon/discovery/discoverytypes" mock_home_chain "github.com/smartcontractkit/chainlink-ccip/mocks/internal_/reader" @@ -40,11 +41,15 @@ func TestContractDiscoveryProcessor_Observation_SupportsDest_HappyPath(t *testin expectedOnRamp := map[cciptypes.ChainSelector][]byte{ source: []byte("onRamp"), } + expectedRMNRemote := []byte("rmnRemote") expectedContracts := reader.ContractAddresses{ consts.ContractNameNonceManager: map[cciptypes.ChainSelector][]byte{ dest: expectedNonceManager, }, consts.ContractNameOnRamp: expectedOnRamp, + consts.ContractNameRMNRemote: map[cciptypes.ChainSelector][]byte{ + dest: expectedRMNRemote, + }, } mockReader. EXPECT(). @@ -68,6 +73,7 @@ func TestContractDiscoveryProcessor_Observation_SupportsDest_HappyPath(t *testin assert.Equal(t, expectedFChain, observation.FChain) assert.Equal(t, expectedOnRamp, observation.OnRamp) assert.Equal(t, expectedNonceManager, observation.DestNonceManager) + assert.Equal(t, expectedRMNRemote, observation.RMNRemote) } func TestContractDiscoveryProcessor_Observation_ErrorGettingFChain(t *testing.T) { @@ -97,6 +103,7 @@ func TestContractDiscoveryProcessor_Observation_ErrorGettingFChain(t *testing.T) assert.Error(t, err) assert.Empty(t, observation.DestNonceManager) assert.Empty(t, observation.OnRamp) + assert.Empty(t, observation.RMNRemote) assert.Empty(t, observation.FChain) } @@ -136,6 +143,7 @@ func TestContractDiscoveryProcessor_Observation_DontSupportDest_StillObserveFCha assert.Equal(t, expectedFChain, observation.FChain) assert.Empty(t, observation.DestNonceManager) assert.Empty(t, observation.OnRamp) + assert.Empty(t, observation.RMNRemote) } func TestContractDiscoveryProcessor_Observation_ErrorDiscoveringContracts(t *testing.T) { @@ -175,6 +183,7 @@ func TestContractDiscoveryProcessor_Observation_ErrorDiscoveringContracts(t *tes assert.Empty(t, observation.FChain) assert.Empty(t, observation.DestNonceManager) assert.Empty(t, observation.OnRamp) + assert.Empty(t, observation.RMNRemote) } func TestContractDiscoveryProcessor_Outcome_HappyPath(t *testing.T) { @@ -199,11 +208,15 @@ func TestContractDiscoveryProcessor_Outcome_HappyPath(t *testing.T) { expectedOnRamp := map[cciptypes.ChainSelector][]byte{ dest: []byte("onRamp"), } + expectedRMNRemote := []byte("rmnRemote") expectedContracts := reader.ContractAddresses{ consts.ContractNameNonceManager: map[cciptypes.ChainSelector][]byte{ dest: expectedNonceManager, }, consts.ContractNameOnRamp: expectedOnRamp, + consts.ContractNameRMNRemote: map[cciptypes.ChainSelector][]byte{ + dest: expectedRMNRemote, + }, } mockReader. EXPECT(). @@ -224,6 +237,7 @@ func TestContractDiscoveryProcessor_Outcome_HappyPath(t *testing.T) { FChain: expectedFChain, OnRamp: expectedOnRamp, DestNonceManager: expectedNonceManager, + RMNRemote: expectedRMNRemote, } // here we have: // 2*fRoleDON + 1 observations of fChain @@ -261,11 +275,15 @@ func TestContractDiscovery_Outcome_HappyPath_FRoleDONAndFDestChainAreDifferent(t expectedOnRamp := map[cciptypes.ChainSelector][]byte{ dest: []byte("onRamp"), } + expectedRMNRemote := []byte("rmnRemote") expectedContracts := reader.ContractAddresses{ consts.ContractNameNonceManager: map[cciptypes.ChainSelector][]byte{ dest: expectedNonceManager, }, consts.ContractNameOnRamp: expectedOnRamp, + consts.ContractNameRMNRemote: map[cciptypes.ChainSelector][]byte{ + dest: expectedRMNRemote, + }, } mockReader. EXPECT(). @@ -289,10 +307,11 @@ func TestContractDiscovery_Outcome_HappyPath_FRoleDONAndFDestChainAreDifferent(t FChain: expectedFChain, OnRamp: expectedOnRamp, DestNonceManager: expectedNonceManager, + RMNRemote: expectedRMNRemote, } // here we have: // 2*fRoleDON + 1 == 7 observations of fChain - // 2*fDest + 1 == 5 observations of the onramps and the dest nonce manager + // 2*fDest + 1 == 5 observations of the onramps, the dest nonce manager, and the RMNRemote aos := []plugincommon.AttributedObservation[discoverytypes.Observation]{ {Observation: destObs}, {Observation: destObs}, @@ -327,6 +346,7 @@ func TestContractDiscoveryProcessor_Outcome_NotEnoughObservations(t *testing.T) source2: fSource2, } observedNonceManager := []byte("nonceManager") + observedRMNRemote := []byte("rmnRemote") observedOnRamp := map[cciptypes.ChainSelector][]byte{ source1: []byte("onRamp"), source2: []byte("onRamp"), @@ -335,6 +355,7 @@ func TestContractDiscoveryProcessor_Outcome_NotEnoughObservations(t *testing.T) expectedContracts := reader.ContractAddresses{ consts.ContractNameNonceManager: map[cciptypes.ChainSelector][]byte{}, consts.ContractNameOnRamp: map[cciptypes.ChainSelector][]byte{}, + consts.ContractNameRMNRemote: map[cciptypes.ChainSelector][]byte{}, } mockReader. EXPECT(). @@ -358,10 +379,11 @@ func TestContractDiscoveryProcessor_Outcome_NotEnoughObservations(t *testing.T) FChain: expectedFChain, OnRamp: observedOnRamp, DestNonceManager: observedNonceManager, + RMNRemote: observedRMNRemote, } // here we have: // 2*fRoleDON + 1 == 7 observations of fChain - // 2*fDest == 4 observations of the onramps and the dest nonce manager (not enough). + // 2*fDest == 4 observations of the onramps, the dest nonce manager and the RMNRemote (not enough) aos := []plugincommon.AttributedObservation[discoverytypes.Observation]{ {Observation: destObs}, {Observation: destObs}, @@ -399,11 +421,15 @@ func TestContractDiscoveryProcessor_Outcome_ErrorSyncingContracts(t *testing.T) expectedOnRamp := map[cciptypes.ChainSelector][]byte{ dest: []byte("onRamp"), } + expectedRmnRemote := []byte("rmnRemote") expectedContracts := reader.ContractAddresses{ consts.ContractNameNonceManager: map[cciptypes.ChainSelector][]byte{ dest: expectedNonceManager, }, consts.ContractNameOnRamp: expectedOnRamp, + consts.ContractNameRMNRemote: map[cciptypes.ChainSelector][]byte{ + dest: expectedRmnRemote, + }, } syncErr := errors.New("sync error") mockReader. @@ -425,10 +451,11 @@ func TestContractDiscoveryProcessor_Outcome_ErrorSyncingContracts(t *testing.T) FChain: expectedFChain, OnRamp: expectedOnRamp, DestNonceManager: expectedNonceManager, + RMNRemote: expectedRmnRemote, } // here we have: // 2*fRoleDON + 1 observations of fChain - // 2*fDest + 1 observations of the onramps and the dest nonce manager + // 2*fDest + 1 observations of the onramps, the dest nonce manager and the RMNRemote aos := []plugincommon.AttributedObservation[discoverytypes.Observation]{ {Observation: obs}, {Observation: obs}, diff --git a/pkg/consts/consts.go b/pkg/consts/consts.go index 3845bcf80..9ab9a3952 100644 --- a/pkg/consts/consts.go +++ b/pkg/consts/consts.go @@ -85,6 +85,10 @@ const ( // RMNHome.sol methods // Used by the rmn home reader. MethodNameGetAllConfigs = "GetAllConfigs" + + // RMNRemote.sol methods + // Used by the rmn remote reader. + MethodNameGetVersionedConfig = "GetVersionedConfig" ) // Event Names diff --git a/pkg/reader/ccip.go b/pkg/reader/ccip.go index 6bd869de5..9ee478a41 100644 --- a/pkg/reader/ccip.go +++ b/pkg/reader/ccip.go @@ -566,7 +566,7 @@ func (r *ccipChainReader) DiscoverContracts( return nil, fmt.Errorf("unable to get SourceChainsConfig: %w", err) } - // NonceManager is in the offramp static config. + // NonceManager and RMNRemote are in the offramp static config staticConfig, err := r.getOfframpStaticConfig(ctx, r.destChain) if err != nil { return nil, fmt.Errorf("unable to lookup nonce manager: %w", err) @@ -585,6 +585,9 @@ func (r *ccipChainReader) DiscoverContracts( consts.ContractNameNonceManager: { destChain: staticConfig.NonceManager, }, + consts.ContractNameRMNRemote: { + destChain: staticConfig.Rmn, + }, } return resp, nil } @@ -742,7 +745,7 @@ func (r *ccipChainReader) getOfframpStaticConfig( //nolint:lll // It's a URL. type offrampStaticChainConfig struct { ChainSelector cciptypes.ChainSelector `json:"chainSelector"` - RmnProxy []byte `json:"rmnProxy"` + Rmn []byte `json:"rmn"` TokenAdminRegistry []byte `json:"tokenAdminRegistry"` NonceManager []byte `json:"nonceManager"` } diff --git a/pkg/reader/ccip_test.go b/pkg/reader/ccip_test.go index 1c68e718e..96484d3c4 100644 --- a/pkg/reader/ccip_test.go +++ b/pkg/reader/ccip_test.go @@ -380,6 +380,7 @@ func TestCCIPChainReader_DiscoverContracts_HappyPath(t *testing.T) { s1Onramp := []byte{0x1} s2Onramp := []byte{0x2} destNonceMgr := []byte{0x3} + rmnRemote := []byte{0x4} expectedContractAddresses := ContractAddresses{ consts.ContractNameOnRamp: { sourceChain1: s1Onramp, @@ -388,6 +389,9 @@ func TestCCIPChainReader_DiscoverContracts_HappyPath(t *testing.T) { consts.ContractNameNonceManager: { destChain: destNonceMgr, }, + consts.ContractNameRMNRemote: { + destChain: rmnRemote, + }, } destExtended := reader_mocks.NewMockExtended(t) @@ -428,6 +432,7 @@ func TestCCIPChainReader_DiscoverContracts_HappyPath(t *testing.T) { ).Return(nil).Run(withReturnValueOverridden(func(returnVal interface{}) { v := returnVal.(*offrampStaticChainConfig) v.NonceManager = destNonceMgr + v.Rmn = rmnRemote })) // create the reader @@ -454,6 +459,7 @@ func TestCCIPChainReader_DiscoverContracts_HappyPath_OnlySupportDest(t *testing. ctx := tests.Context(t) destChain := cciptypes.ChainSelector(1) destNonceMgr := []byte{0x3} + destRMNRemote := []byte{0x4} expectedContractAddresses := ContractAddresses{ // since the source chains are not supported, we should not have any onramp addresses // after discovery. @@ -463,6 +469,9 @@ func TestCCIPChainReader_DiscoverContracts_HappyPath_OnlySupportDest(t *testing. consts.ContractNameNonceManager: { destChain: destNonceMgr, }, + consts.ContractNameRMNRemote: { + destChain: destRMNRemote, + }, } destExtended := reader_mocks.NewMockExtended(t) @@ -477,6 +486,7 @@ func TestCCIPChainReader_DiscoverContracts_HappyPath_OnlySupportDest(t *testing. ).Return(nil).Run(withReturnValueOverridden(func(returnVal interface{}) { v := returnVal.(*offrampStaticChainConfig) v.NonceManager = destNonceMgr + v.Rmn = destRMNRemote })) // create the reader