diff --git a/pkg/chainaccessor/default_accessor.go b/pkg/chainaccessor/default_accessor.go index 8a0818854..690433c88 100644 --- a/pkg/chainaccessor/default_accessor.go +++ b/pkg/chainaccessor/default_accessor.go @@ -76,9 +76,36 @@ func (l *DefaultAccessor) GetChainFeeComponents(ctx context.Context) (cciptypes. return *fc, nil } -func (l *DefaultAccessor) Sync(ctx context.Context, contracts cciptypes.ContractAddresses) error { - // TODO(NONEVM-1865): implement - panic("implement me") +func (l *DefaultAccessor) Sync( + ctx context.Context, + contractName string, + contractAddress cciptypes.UnknownAddress, +) error { + lggr := logutil.WithContextValues(ctx, l.lggr) + addressStr, err := l.addrCodec.AddressBytesToString(contractAddress, l.chainSelector) + if err != nil { + return fmt.Errorf("unable to convert address bytes to string: %w, address: %v", err, contractAddress) + } + + contract := types.BoundContract{ + Address: addressStr, + Name: contractName, + } + + lggr.Debugw("Binding contract", + "chainSelector", l.chainSelector, + "contractName", contractName, + "address", addressStr, + ) + // Bind the contract address to the reader. + // If the same address exists -> no-op + // If the address is changed -> updates the address, overwrites the existing one + // If the contract not bound -> binds to the new address + if err := l.contractReader.Bind(ctx, []types.BoundContract{contract}); err != nil { + return fmt.Errorf("unable to bind %s %s for chain %d: %w", contractName, addressStr, l.chainSelector, err) + } + + return nil } func (l *DefaultAccessor) MsgsBetweenSeqNums( diff --git a/pkg/reader/ccip.go b/pkg/reader/ccip.go index 9c80c7612..7d2cdf229 100644 --- a/pkg/reader/ccip.go +++ b/pkg/reader/ccip.go @@ -527,8 +527,6 @@ func (r *ccipChainReader) buildSigners(signers []signer) []cciptypes.RemoteSigne } func (r *ccipChainReader) GetRMNRemoteConfig(ctx context.Context) (cciptypes.RemoteConfig, error) { - lggr := logutil.WithContextValues(ctx, r.lggr) - if err := validateReaderExistence(r.contractReaders, r.destChain); err != nil { return cciptypes.RemoteConfig{}, fmt.Errorf("validate dest=%d extended reader existence: %w", r.destChain, err) } @@ -549,7 +547,7 @@ func (r *ccipChainReader) GetRMNRemoteConfig(ctx context.Context) (cciptypes.Rem return cciptypes.RemoteConfig{}, fmt.Errorf("get RMNRemote proxy contract address: %w", err) } - rmnRemoteAddress, err := r.getRMNRemoteAddress(ctx, lggr, r.destChain, proxyContractAddress) + rmnRemoteAddress, err := r.getRMNRemoteAddress(ctx, r.destChain, proxyContractAddress) if err != nil { return cciptypes.RemoteConfig{}, fmt.Errorf("get RMNRemote address: %w", err) } @@ -794,21 +792,14 @@ func (r *ccipChainReader) Sync(ctx context.Context, contracts ContractAddresses) return nil } - // the extended reader will do nothing if the contract is already bound. - _, err := bindReaderContract( - ctx, - lggr, - r.contractReaders, - chainSelector, - boundContract.name, - boundContract.address, - r.addrCodec) - if err != nil { - if errors.Is(err, ErrContractReaderNotFound) { - // don't support this chain, nothing to do. - return nil - } + chainAccessor, err := getChainAccessor(r.accessors, chainSelector) + if err != nil && errors.Is(err, ErrChainAccessorNotFound) { + // don't support this chain, nothing to do. + return nil + } + err = chainAccessor.Sync(ctx, boundContract.name, boundContract.address) + if err != nil { return fmt.Errorf("bind reader contract %s on chain %s: %w", boundContract.name, chainSelector, err) } @@ -1129,12 +1120,15 @@ type versionedConfig struct { //nolint:lll func (r *ccipChainReader) getRMNRemoteAddress( ctx context.Context, - lggr logger.Logger, chain cciptypes.ChainSelector, rmnRemoteProxyAddress []byte) ([]byte, error) { - _, err := bindReaderContract(ctx, lggr, r.contractReaders, chain, consts.ContractNameRMNProxy, rmnRemoteProxyAddress, r.addrCodec) + chainAccessor, err := getChainAccessor(r.accessors, chain) + if err != nil { + return nil, fmt.Errorf("unable to getChainAccessor: %w", err) + } + err = chainAccessor.Sync(ctx, consts.ContractNameRMNProxy, rmnRemoteProxyAddress) if err != nil { - return nil, fmt.Errorf("bind RMN proxy contract: %w", err) + return nil, fmt.Errorf("sync RMN proxy contract: %w", err) } // Get the address from cache instead of making a contract call diff --git a/pkg/reader/ccip_interface.go b/pkg/reader/ccip_interface.go index f88333b9b..f36da6ca1 100644 --- a/pkg/reader/ccip_interface.go +++ b/pkg/reader/ccip_interface.go @@ -16,6 +16,7 @@ import ( ) var ( + ErrChainAccessorNotFound = errors.New("chain accessor not found") ErrContractReaderNotFound = errors.New("contract reader not found") ErrContractWriterNotFound = errors.New("contract writer not found") ) diff --git a/pkg/reader/ccip_test.go b/pkg/reader/ccip_test.go index b247cd848..9b5a5356a 100644 --- a/pkg/reader/ccip_test.go +++ b/pkg/reader/ccip_test.go @@ -151,11 +151,20 @@ func TestCCIPChainReader_Sync_HappyPath_BindsContractsSuccessfully(t *testing.T) s1Onramp := []byte{0x1} s2Onramp := []byte{0x2} destNonceMgr := []byte{0x3} + offRamp := []byte{0x4} mockAddrCodec := internal.NewMockAddressCodecHex(t) destNonceMgrAddrStr, err := mockAddrCodec.AddressBytesToString(destNonceMgr, destChain) require.NoError(t, err) + offRampAddrStr, err := mockAddrCodec.AddressBytesToString(offRamp, destChain) + require.NoError(t, err) destExtended := reader_mocks.NewMockExtended(t) + destExtended.EXPECT().Bind(mock.Anything, []types.BoundContract{ + { + Name: consts.ContractNameOffRamp, + Address: offRampAddrStr, + }, + }).Return(nil) destExtended.EXPECT().Bind(mock.Anything, []types.BoundContract{ { Name: consts.ContractNameNonceManager, @@ -187,17 +196,26 @@ func TestCCIPChainReader_Sync_HappyPath_BindsContractsSuccessfully(t *testing.T) defer source1Extended.AssertExpectations(t) defer source2Extended.AssertExpectations(t) - ccipReader := &ccipChainReader{ - contractReaders: map[cciptypes.ChainSelector]contractreader.Extended{ + cw := writer_mocks.NewMockContractWriter(t) + contractWriters := make(map[cciptypes.ChainSelector]types.ContractWriter) + contractWriters[destChain] = cw + contractWriters[sourceChain1] = cw + contractWriters[sourceChain2] = cw + + ccipReader, err := newCCIPChainReaderInternal( + ctx, + logger.Test(t), + map[cciptypes.ChainSelector]contractreader.ContractReaderFacade{ destChain: destExtended, sourceChain1: source1Extended, sourceChain2: source2Extended, }, - donAddressBook: addressbook.NewBook(), - destChain: destChain, - lggr: logger.Test(t), - addrCodec: mockAddrCodec, - } + contractWriters, + destChain, + offRamp, + mockAddrCodec, + ) + require.NoError(t, err) contracts := ContractAddresses{ consts.ContractNameOnRamp: { @@ -211,6 +229,8 @@ func TestCCIPChainReader_Sync_HappyPath_BindsContractsSuccessfully(t *testing.T) err = ccipReader.Sync(ctx, contracts) require.NoError(t, err) + err = ccipReader.Close() + require.NoError(t, err) } func TestCCIPChainReader_Sync_HappyPath_SkipsEmptyAddress(t *testing.T) { @@ -219,6 +239,7 @@ func TestCCIPChainReader_Sync_HappyPath_SkipsEmptyAddress(t *testing.T) { sourceChain1 := cciptypes.ChainSelector(2) sourceChain2 := cciptypes.ChainSelector(3) s1Onramp := []byte{0x1} + offRamp := []byte{0x4} // empty address, should get skipped s2Onramp := []byte{} @@ -228,6 +249,14 @@ func TestCCIPChainReader_Sync_HappyPath_SkipsEmptyAddress(t *testing.T) { mockAddrCodec := internal.NewMockAddressCodecHex(t) destNonceMgrAddrStr, err := mockAddrCodec.AddressBytesToString(destNonceMgr, destChain) require.NoError(t, err) + offRampAddrStr, err := mockAddrCodec.AddressBytesToString(offRamp, destChain) + require.NoError(t, err) + destExtended.EXPECT().Bind(mock.Anything, []types.BoundContract{ + { + Name: consts.ContractNameOffRamp, + Address: offRampAddrStr, + }, + }).Return(nil) destExtended.EXPECT().Bind(mock.Anything, []types.BoundContract{ { Name: consts.ContractNameNonceManager, @@ -252,17 +281,26 @@ func TestCCIPChainReader_Sync_HappyPath_SkipsEmptyAddress(t *testing.T) { defer source1Extended.AssertExpectations(t) defer source2Extended.AssertExpectations(t) - ccipReader := &ccipChainReader{ - contractReaders: map[cciptypes.ChainSelector]contractreader.Extended{ + cw := writer_mocks.NewMockContractWriter(t) + contractWriters := make(map[cciptypes.ChainSelector]types.ContractWriter) + contractWriters[destChain] = cw + contractWriters[sourceChain1] = cw + contractWriters[sourceChain2] = cw + + ccipReader, err := newCCIPChainReaderInternal( + ctx, + logger.Test(t), + map[cciptypes.ChainSelector]contractreader.ContractReaderFacade{ destChain: destExtended, sourceChain1: source1Extended, sourceChain2: source2Extended, }, - donAddressBook: addressbook.NewBook(), - destChain: destChain, - lggr: logger.Test(t), - addrCodec: mockAddrCodec, - } + contractWriters, + destChain, + offRamp, + mockAddrCodec, + ) + require.NoError(t, err) contracts := ContractAddresses{ consts.ContractNameOnRamp: { @@ -276,6 +314,8 @@ func TestCCIPChainReader_Sync_HappyPath_SkipsEmptyAddress(t *testing.T) { err = ccipReader.Sync(ctx, contracts) require.NoError(t, err) + err = ccipReader.Close() + require.NoError(t, err) } func TestCCIPChainReader_Sync_HappyPath_DontSupportAllChains(t *testing.T) { @@ -286,11 +326,20 @@ func TestCCIPChainReader_Sync_HappyPath_DontSupportAllChains(t *testing.T) { s1Onramp := []byte{0x1} s2Onramp := []byte{0x2} destNonceMgr := []byte{0x3} + offRamp := []byte{0x4} destExtended := reader_mocks.NewMockExtended(t) mockAddrCodec := internal.NewMockAddressCodecHex(t) destNonceMgrAddrStr, err := mockAddrCodec.AddressBytesToString(destNonceMgr, destChain) require.NoError(t, err) + offRampAddrStr, err := mockAddrCodec.AddressBytesToString(offRamp, destChain) + require.NoError(t, err) + destExtended.EXPECT().Bind(mock.Anything, []types.BoundContract{ + { + Name: consts.ContractNameOffRamp, + Address: offRampAddrStr, + }, + }).Return(nil) destExtended.EXPECT().Bind(mock.Anything, []types.BoundContract{ { Name: consts.ContractNameNonceManager, @@ -312,16 +361,24 @@ func TestCCIPChainReader_Sync_HappyPath_DontSupportAllChains(t *testing.T) { defer destExtended.AssertExpectations(t) defer source2Extended.AssertExpectations(t) - ccipReader := &ccipChainReader{ - contractReaders: map[cciptypes.ChainSelector]contractreader.Extended{ + cw := writer_mocks.NewMockContractWriter(t) + contractWriters := make(map[cciptypes.ChainSelector]types.ContractWriter) + contractWriters[destChain] = cw + contractWriters[sourceChain2] = cw + + ccipReader, err := newCCIPChainReaderInternal( + ctx, + logger.Test(t), + map[cciptypes.ChainSelector]contractreader.ContractReaderFacade{ destChain: destExtended, sourceChain2: source2Extended, }, - donAddressBook: addressbook.NewBook(), - destChain: destChain, - lggr: logger.Test(t), - addrCodec: mockAddrCodec, - } + contractWriters, + destChain, + offRamp, + mockAddrCodec, + ) + require.NoError(t, err) contracts := ContractAddresses{ consts.ContractNameOnRamp: { @@ -335,6 +392,8 @@ func TestCCIPChainReader_Sync_HappyPath_DontSupportAllChains(t *testing.T) { err = ccipReader.Sync(ctx, contracts) require.NoError(t, err) + err = ccipReader.Close() + require.NoError(t, err) } func TestCCIPChainReader_Sync_BindError(t *testing.T) { @@ -345,11 +404,20 @@ func TestCCIPChainReader_Sync_BindError(t *testing.T) { s1Onramp := []byte{0x1} s2Onramp := []byte{0x2} destNonceMgr := []byte{0x3} + offRamp := []byte{0x4} mockAddrCodec := internal.NewMockAddressCodecHex(t) destNonceMgrAddrStr, err := mockAddrCodec.AddressBytesToString(destNonceMgr, destChain) require.NoError(t, err) + offRampAddrStr, err := mockAddrCodec.AddressBytesToString(offRamp, destChain) + require.NoError(t, err) destExtended := reader_mocks.NewMockExtended(t) + destExtended.EXPECT().Bind(mock.Anything, []types.BoundContract{ + { + Name: consts.ContractNameOffRamp, + Address: offRampAddrStr, + }, + }).Return(nil) destExtended.EXPECT().Bind(mock.Anything, []types.BoundContract{ { Name: consts.ContractNameNonceManager, @@ -382,17 +450,26 @@ func TestCCIPChainReader_Sync_BindError(t *testing.T) { defer source1Extended.AssertExpectations(t) defer source2Extended.AssertExpectations(t) - ccipReader := &ccipChainReader{ - contractReaders: map[cciptypes.ChainSelector]contractreader.Extended{ + cw := writer_mocks.NewMockContractWriter(t) + contractWriters := make(map[cciptypes.ChainSelector]types.ContractWriter) + contractWriters[destChain] = cw + contractWriters[sourceChain1] = cw + contractWriters[sourceChain2] = cw + + ccipReader, err := newCCIPChainReaderInternal( + ctx, + logger.Test(t), + map[cciptypes.ChainSelector]contractreader.ContractReaderFacade{ destChain: destExtended, sourceChain1: source1Extended, sourceChain2: source2Extended, }, - donAddressBook: addressbook.NewBook(), - destChain: destChain, - lggr: logger.Test(t), - addrCodec: mockAddrCodec, - } + contractWriters, + destChain, + offRamp, + mockAddrCodec, + ) + require.NoError(t, err) contracts := ContractAddresses{ consts.ContractNameOnRamp: { @@ -407,6 +484,8 @@ func TestCCIPChainReader_Sync_BindError(t *testing.T) { err = ccipReader.Sync(ctx, contracts) require.Error(t, err) require.ErrorIs(t, err, expectedErr) + err = ccipReader.Close() + require.NoError(t, err) } // The round1 version returns NoBindingFound errors for onramp contracts to simulate diff --git a/pkg/reader/helpers.go b/pkg/reader/helpers.go index 434bf2feb..e179f880c 100644 --- a/pkg/reader/helpers.go +++ b/pkg/reader/helpers.go @@ -1,13 +1,9 @@ package reader import ( - "context" "fmt" "time" - "github.com/smartcontractkit/chainlink-common/pkg/logger" - "github.com/smartcontractkit/chainlink-common/pkg/types" - "github.com/smartcontractkit/chainlink-ccip/pkg/contractreader" cciptypes "github.com/smartcontractkit/chainlink-ccip/pkg/types/ccipocr3" ) @@ -23,51 +19,6 @@ const ( HomeChainPollingInterval = 15 * time.Second ) -// bindReaderContract is a generic helper for binding contracts to readers, the addresses input is the same object -// returned by DiscoverContracts. -// -// No error is returned if contractName is not found in the contracts. This allows calling the function before all -// contracts are discovered. -func bindReaderContract[T contractreader.ContractReaderFacade]( - ctx context.Context, - lggr logger.Logger, - readers map[cciptypes.ChainSelector]T, - chainSel cciptypes.ChainSelector, - contractName string, - address []byte, - codec cciptypes.AddressCodec, -) (types.BoundContract, error) { - if err := validateReaderExistence(readers, chainSel); err != nil { - return types.BoundContract{}, fmt.Errorf("validate reader existence: %w", err) - } - - addressStr, err := codec.AddressBytesToString(address, chainSel) - if err != nil { - return types.BoundContract{}, fmt.Errorf("unable to convert address bytes to string: %w, address: %v", err, address) - } - - contract := types.BoundContract{ - Address: addressStr, - Name: contractName, - } - - lggr.Debugw("Binding contract", - "chainSel", chainSel, - "contractName", contractName, - "address", addressStr, - ) - // Bind the contract address to the reader. - // If the same address exists -> no-op - // If the address is changed -> updates the address, overwrites the existing one - // If the contract not bound -> binds to the new address - if err := readers[chainSel].Bind(ctx, []types.BoundContract{contract}); err != nil { - return types.BoundContract{}, - fmt.Errorf("unable to bind %s %s for chain %d: %w", contractName, addressStr, chainSel, err) - } - - return contract, nil -} - func validateReaderExistence[T contractreader.ContractReaderFacade]( readers map[cciptypes.ChainSelector]T, chains ...cciptypes.ChainSelector, @@ -88,5 +39,5 @@ func getChainAccessor( if accessor, exists := accessors[chainSelector]; exists { return accessor, nil } - return nil, fmt.Errorf("chain accessor not found for chain %d", chainSelector) + return nil, fmt.Errorf("chain %d: %w", chainSelector, ErrChainAccessorNotFound) } diff --git a/pkg/reader/usdc_reader.go b/pkg/reader/usdc_reader.go index 00a5b9a38..0f801803d 100644 --- a/pkg/reader/usdc_reader.go +++ b/pkg/reader/usdc_reader.go @@ -129,6 +129,48 @@ func NewUSDCMessageReader( }, nil } +// Deprecated +// TODO(NONEVM-1865): Remove once the chainAccessor is passed down here from the factory. Then use accessor.Sync(). +func bindReaderContract[T contractreader.ContractReaderFacade]( + ctx context.Context, + lggr logger.Logger, + readers map[cciptypes.ChainSelector]T, + chainSel cciptypes.ChainSelector, + contractName string, + address []byte, + codec cciptypes.AddressCodec, +) (types.BoundContract, error) { + if err := validateReaderExistence(readers, chainSel); err != nil { + return types.BoundContract{}, fmt.Errorf("validate reader existence: %w", err) + } + + addressStr, err := codec.AddressBytesToString(address, chainSel) + if err != nil { + return types.BoundContract{}, fmt.Errorf("unable to convert address bytes to string: %w, address: %v", err, address) + } + + contract := types.BoundContract{ + Address: addressStr, + Name: contractName, + } + + lggr.Debugw("Binding contract", + "chainSel", chainSel, + "contractName", contractName, + "address", addressStr, + ) + // Bind the contract address to the reader. + // If the same address exists -> no-op + // If the address is changed -> updates the address, overwrites the existing one + // If the contract not bound -> binds to the new address + if err := readers[chainSel].Bind(ctx, []types.BoundContract{contract}); err != nil { + return types.BoundContract{}, + fmt.Errorf("unable to bind %s %s for chain %d: %w", contractName, addressStr, chainSel, err) + } + + return contract, nil +} + // compositeUSDCMessageReader is a USDCMessageReader that can handle different chain families. type compositeUSDCMessageReader struct { lggr logger.Logger diff --git a/pkg/types/ccipocr3/chain_accessor.go b/pkg/types/ccipocr3/chain_accessor.go index 943451f1f..2d1243c95 100644 --- a/pkg/types/ccipocr3/chain_accessor.go +++ b/pkg/types/ccipocr3/chain_accessor.go @@ -71,8 +71,8 @@ type AllAccessors interface { GetChainFeeComponents(ctx context.Context) (ChainFeeComponents, error) // Sync can be used to perform frequent syncing operations inside the reader implementation. - // Returns a bool indicating whether something was updated. - Sync(ctx context.Context, contracts ContractAddresses) error + // Returns an error if the sync operation failed. + Sync(ctx context.Context, contractName string, contractAddress UnknownAddress) error } // DestinationAccessor contains all functions typically associated by the destination chain.