Add remote token address check#482
Conversation
|
👋 JohnChangUK, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
| remoteTokenAddressMatches sourceTokenAddress configuredRemoteTokenAddress = | ||
| sourceTokenAddress == configuredRemoteTokenAddress | ||
| || (byteCount sourceTokenAddress == 32 | ||
| && byteCount configuredRemoteTokenAddress == 20 |
There was a problem hiding this comment.
how is this handled on EVM? is there this exceptional clause for EVM addresses?
There was a problem hiding this comment.
- EVM doesn't have this check as
sourceTokenAddressfield doesn't exist there. The token pool address has the token bound do it as a field during creation. - EVM uses
abi.encodeto encode the address into the message. This pads the address to 32 bytes. https://github.com/smartcontractkit/chainlink-ccip/blob/main/chains/evm/contracts/onRamp/OnRamp.sol#L777
There was a problem hiding this comment.
EVM doesn't have this check as
sourceTokenAddressfield doesn't exist there. The token pool address has the token bound do it as a field during creation.
it's right here? https://github.com/smartcontractkit/chainlink-ccip/blob/22e2d05695cd4fea667a9037d47cffb33acb12db/chains/evm/contracts/libraries/MessageV1Codec.sol#L165
from the comments at that line:
// Address of destination token, NOT abi encoded but raw bytes matching destination chains address byte length.
which means that this should always match the source chain's address length? and we should be decoding it correctly here?
There was a problem hiding this comment.
// Address of destination token, NOT abi encoded but raw bytes matching destination chains address byte length.
This comment above is for destTokenAddress though, not on sourceTokenAddress.
bytes sourceTokenAddress; // Address of source token, abi encoded for EVM chains.
| -- - In the incoming directions, the pools will validate that sourcePoolAddress === the stored remote pool address | ||
| -- - In the outgoing direction (to EVM), the pools will add this stored address to the message in destTokenAddress | ||
| -- - In the outgoing direction, EVM expects the incoming destTokenAddress the be unpadded 20 bytes, so we must strip the padding before sending the outgoing message | ||
| validateDestChainAddress : BytesHex -> Int -> BytesHex |
There was a problem hiding this comment.
| ensure | ||
| isValidHex receiver && byteCount receiver == destAddressBytesLength | ||
| && (tokenReceiver == "" || (isValidHex tokenReceiver && byteCount tokenReceiver == destAddressBytesLength)) | ||
| && (case tokenSendData of | ||
| None -> True | ||
| Some tsd -> isValidHex tsd.destTokenAddress && byteCount tsd.destTokenAddress == destAddressBytesLength) |
There was a problem hiding this comment.
Adding an ensure clause here, the actual validation is supposed to happen via validateDestChainAddress
| let tsd = TokenSendData with | ||
| poolInstanceId, poolOwner, instrumentId, amount | ||
| destTokenAddress, extraData | ||
| destTokenAddress = validateDestChainAddress destTokenAddress destAddressBytesLength |
There was a problem hiding this comment.
This has to happen here instead of in the OnRamp, since the OnRamp cannot modify the value
3 participants
No description provided.