ccip - EVM Implementation of RMNCrypto interface #14416
Conversation
…chainlink into dk/ccip-impl-rmncrypto
dimkouv
requested review from
vyzaldysanchez
and removed request for
a team
| return &EVMRMNCrypto{} | ||
| } | ||
|
|
||
| type evmRMNRemoteReport struct { |
There was a problem hiding this comment.
Maybe add a TODO also to use the types from the gethwrappers when they're available?
| return fmt.Errorf("no lane updates provided") | ||
| } | ||
|
|
||
| rmnVersionHash := crypto.Keccak256Hash([]byte(report.ReportVersion)) |
There was a problem hiding this comment.
I wonder if this version hash should be pulled out into a var (similar to how we have the LEAF_DOMAIN_SEPARATOR for the message hasher) and have this struct versioned (i.e EVMRMNCryptoV1_6 for V1.6)
There was a problem hiding this comment.
Hmm could be but then the interface/common-types might also not be compatible with next versions.
I recommend waiting for next version before defining a generic report type to use across different versions.
|
Failing CI check |
| // abiEncodeMethodInputs encodes the inputs for a method call. | ||
| // example abi: `[{ "name" : "method", "type": "function", "inputs": [{"name": "a", "type": "uint256"}]}]` | ||
| func abiEncodeMethodInputs(abiDef abi.ABI, inputs ...interface{}) ([]byte, error) { | ||
| packed, err := abiDef.Pack("method", inputs...) |
There was a problem hiding this comment.
Hmm I thought the name matters here, at least for the contract ABI's, or is that not the case in this particular scenario?
There was a problem hiding this comment.
I see this only used in one place and the method is indeed called method, so thats why this worse. Might be worth checking this with a test, but I feel like this will fail if there is no method called method in the ABI
There was a problem hiding this comment.
or is that not the case in this particular scenario?
It's not the case,this is similar to https://github.com/smartcontractkit/chainlink/blob/e37f7e2d1a1859a0853ad6ecfbe3c339c02d2248/core/chains/evm/utils/ethabi.go#L34-L33 but more performant, since it doesn't parse the abi on each call. That's why i didn't re-use what we have.
| EncodingUtilsABI abi.ABI | ||
| AddressEncodeABI abi.ABI |
| report cciptypes.RMNReport, | ||
| signerAddresses []cciptypes.Bytes, | ||
| ) error { | ||
| const v = 27 // used in ecrecover |
There was a problem hiding this comment.
Can pull this out to a const block outside this object
There was a problem hiding this comment.
also added a more proper comment
|
EVM Implementation of
RMNCryptoccip related interface.