support address codec implementation #16485
Conversation
huangzhen1997
requested review from
ilija42,
winder,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara,
0xAustinWang and
0xnogo
To provide a service of address encoding/decoding scheme for different source chain |
It's for the chain reader, the oracle creator provide this codec and will be used by ccip chain reader to replace this evm specific util function (internal/libs/typeconv/address.go). Check the PR Blaž was trying to do, this's going to replace it |
Flakeguard SummaryRan new or updated tests between View Flaky Detector Details | Compare Changes Found Flaky Tests ❌4 Results
ArtifactsFor detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json. |
| @@ -307,6 +304,12 @@ func (i *pluginOracleCreator) createFactoryAndTransmitter( | |||
| return nil, nil, fmt.Errorf("unsupported chain %v", chainFamily) | |||
| } | |||
| messageHasher := plugin.MessageHasher(i.lggr.Named(chainFamily).Named("MessageHasherV1")) | |||
| addressCodec := ccipcommon.NewAddressCodec( | |||
There was a problem hiding this comment.
Could you instead create this addressCodec in NewPluginOracleCreator(), store it as a member of the pluginOracleCreator struct, and then here in this function, pass the same instance to the commit/exec plugin factories.
We basically only need 1 instance. No need to create separate one per-chain or plugn
| if len(addr) != common.AddressLength { | ||
| return "", fmt.Errorf("invalid EVM address length, expected %v, got %d", common.AddressLength, len(addr)) | ||
| } | ||
| return hexutil.Encode(addr), nil |
There was a problem hiding this comment.
Instead of this, use this version:
common.BytesToAddress(addr).Hex()
The reason is because the latter is EIP55-compliant.
More context: https://medium.com/@zakhard/eip-55-explained-solving-the-address-checksum-problem-01ac2bb0efc4
There was a problem hiding this comment.
Good catch. Now that this is inside chainlink we can use the standard evm libraries.
| @@ -307,6 +304,12 @@ func (i *pluginOracleCreator) createFactoryAndTransmitter( | |||
| return nil, nil, fmt.Errorf("unsupported chain %v", chainFamily) | |||
| } | |||
| messageHasher := plugin.MessageHasher(i.lggr.Named(chainFamily).Named("MessageHasherV1")) | |||
| addressCodec := ccipcommon.NewAddressCodec( | |||
| ccipcommon.NewAddressCodecParams( | |||
There was a problem hiding this comment.
Why do you need to send these?
Why can't the NewAddressCodecParams() just create locally itself?
There was a problem hiding this comment.
Yes it's not ideal, but a workaround for the import circle when I import ccipevm or ccipsolana from common
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
| @@ -286,6 +277,7 @@ func (i *pluginOracleCreator) createFactoryAndTransmitter( | |||
| destFromAccounts []string, | |||
| publicConfig ocr3confighelper.PublicConfig, | |||
| destChainFamily string, | |||
There was a problem hiding this comment.
nit: Now that you are passing offrampAddrStr, I think the previous param destChainFamily is not needed?
|
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
No description provided.