Wormhole adaptor #94
Wormhole adaptor #94
Conversation
There was a problem hiding this comment.
Let's do the same as #202 and flatten everything down to one contract.
| payload, | ||
| new bytes[](0), | ||
| toUniversalAddress(msg.sender), | ||
| targetChain, |
There was a problem hiding this comment.
This should be the source chain.
Any reasonable way to do that in this mock? I think it should at least not be the target chain.
| * @dev A contract that combines the functionality of both the source and destination gateway | ||
| * adapters for the Wormhole Network. Allowing to either send or receive messages across chains. | ||
| * | ||
| * Note: only EVM chains are currently supported |
There was a problem hiding this comment.
Is this because of address encoding?
There was a problem hiding this comment.
Ah I see we're using sendPayloadToEvm.
| } | ||
|
|
||
| /// @dev Returns whether a binary interoperable chain id is supported. | ||
| function supportedChain(bytes memory chain) public view virtual returns (bool) { |
There was a problem hiding this comment.
It's clear or intuitive that chain has to be a full interop address. I'd consider renaming the argument at least, possibly the function, and making this explicit in the documentation including a @param.
| /// @dev Returns the Wormhole chain id that correspond to a given EVM chain id. | ||
| function getWormholeChain(uint256 chainId) public view virtual returns (uint16) { | ||
| uint24 wormholeId = _chainIdToWormhole[chainId]; | ||
| require(wormholeId & MASK == MASK, UnsupportedChainId(chainId)); |
There was a problem hiding this comment.
Is this check needed? Why? Please add a comment.
Makes me ask why the mapping holds uint24 chain ids since uint16 seems to be the only thing that's supported.
There was a problem hiding this comment.
Ok I understand now that this is necessary for | MASK because a wormhole id may be 0. I would add this in a comment on the mapping and/or the MASK variable.
| } | ||
|
|
||
| /// @dev Relay a message that was initiated by {sendMessage}. | ||
| function requestRelay(bytes32 sendId, uint256 gasLimit, address /*refundRecipient*/) external payable { |
There was a problem hiding this comment.
Why is refundRecipient ignored? There's a version of sendPayloadToEvm that takes a refund address.
| uint256 value = msg.value; | ||
| uint256 gasLimit = 0; | ||
|
|
||
| for (uint256 i = 0; i < attributes.length; ++i) { |
There was a problem hiding this comment.
This supports duplicate requestRelay attributes which we probably shouldn't...
|
|
||
| for (uint256 i = 0; i < attributes.length; ++i) { | ||
| (relay, value, gasLimit, ) = ERC7786Attributes.tryDecodeRequestRelayCalldata(attributes[i]); | ||
| require(relay, UnsupportedAttribute(attributes[i].length < 0x04 ? bytes4(0) : bytes4(attributes[i]))); |
There was a problem hiding this comment.
There should probably be a utility function to get the selector from a bytes calldata attribute.
| if (relay) { | ||
| // TODO: Do we care about the returned "sequence"? | ||
| bytes memory encoded = abi.encode( | ||
| sendId, |
There was a problem hiding this comment.
right, we need another way to deduplicate ...
Co-authored-by: Francisco Giordano <[email protected]>
Replaces #13