CipherLabs
/
hyperlane-monorepo
mirror of https://github.com/hyperlane-xyz/hyperlane-monorepo
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: eliminate branching from `TokenRouter` (#3878)

Browse Source 
### Description

Fixes bug in `TokenRouter` which dispatches messages to token recipient
rather than address in `Router` table when hook overrides are provided.

### Drive-by changes

Simplifies inheritance tree of `TokenRouter` > `GasRouter` > `Router` >
`MailboxClient`

### Backward compatibility

Yes

### Testing

Unit Tests
pull/3980/head
Yorke Rhodes 5 months ago committed by GitHub
parent 27580329eb
commit 6620fe6367
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 160 additions and 183 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 6
      .changeset/mean-impalas-leave.md
  2. 38
      solidity/contracts/client/GasRouter.sol
  3. 81
      solidity/contracts/client/MailboxClient.sol
  4. 65
      solidity/contracts/client/Router.sol
  5. 2
      solidity/contracts/test/TestGasRouter.sol
  6. 16
      solidity/contracts/token/HypNative.sol
  7. 12
      solidity/contracts/token/extensions/HypNativeScaled.sol
  8. 6
      solidity/contracts/token/libs/FastTokenRouter.sol
  9. 71
      solidity/contracts/token/libs/TokenRouter.sol
  10. 7
      solidity/test/InterchainAccountRouter.t.sol
  11. 33
      solidity/test/token/HypERC20.t.sol
  12. 6
      typescript/helloworld/contracts/HelloWorld.sol

6
.changeset/mean-impalas-leave.md
Unescape View File

@ -0,0 +1,6 @@
---
"@hyperlane-xyz/core": patch
"@hyperlane-xyz/helloworld": patch
---
fix: `TokenRouter.transferRemote` with hook overrides

38
solidity/contracts/client/GasRouter.sol
Unescape View File

@ -43,13 +43,13 @@ abstract contract GasRouter is Router {
*/
function quoteGasPayment(
uint32 _destinationDomain
) external view returns (uint256 _gasPayment) {
return _quoteDispatch(_destinationDomain, "");
) external view returns (uint256) {
return _GasRouter_quoteDispatch(_destinationDomain, "", address(hook));
}
function _metadata(
function _GasRouter_hookMetadata(
uint32 _destination
) internal view virtual override returns (bytes memory) {
) internal view returns (bytes memory) {
return
StandardHookMetadata.overrideGasLimit(destinationGas[_destination]);
}
@ -57,4 +57,34 @@ abstract contract GasRouter is Router {
function _setDestinationGas(uint32 domain, uint256 gas) internal {
destinationGas[domain] = gas;
}
function _GasRouter_dispatch(
uint32 _destination,
uint256 _value,
bytes memory _messageBody,
address _hook
) internal returns (bytes32) {
return
_Router_dispatch(
_destination,
_value,
_messageBody,
_GasRouter_hookMetadata(_destination),
_hook
);
}
function _GasRouter_quoteDispatch(
uint32 _destination,
bytes memory _messageBody,
address _hook
) internal view returns (uint256) {
return
_Router_quoteDispatch(
_destination,
_messageBody,
_GasRouter_hookMetadata(_destination),
_hook
);
}
}

81
solidity/contracts/client/MailboxClient.sol
Unescape View File

@ -95,85 +95,4 @@ abstract contract MailboxClient is OwnableUpgradeable {
function _isDelivered(bytes32 id) internal view returns (bool) {
return mailbox.delivered(id);
}
function _metadata(
uint32 /*_destinationDomain*/
) internal view virtual returns (bytes memory) {
return "";
}
function _dispatch(
uint32 _destinationDomain,
bytes32 _recipient,
bytes memory _messageBody
) internal virtual returns (bytes32) {
return
_dispatch(_destinationDomain, _recipient, msg.value, _messageBody);
}
function _dispatch(
uint32 _destinationDomain,
bytes32 _recipient,
uint256 _value,
bytes memory _messageBody
) internal virtual returns (bytes32) {
return
mailbox.dispatch{value: _value}(
_destinationDomain,
_recipient,
_messageBody,
_metadata(_destinationDomain),
hook
);
}
function _dispatch(
uint32 _destinationDomain,
bytes32 _recipient,
uint256 _value,
bytes memory _messageBody,
bytes memory _hookMetadata,
IPostDispatchHook _hook
) internal virtual returns (bytes32) {
return
mailbox.dispatch{value: _value}(
_destinationDomain,
_recipient,
_messageBody,
_hookMetadata,
_hook
);
}
function _quoteDispatch(
uint32 _destinationDomain,
bytes32 _recipient,
bytes memory _messageBody
) internal view virtual returns (uint256) {
return
mailbox.quoteDispatch(
_destinationDomain,
_recipient,
_messageBody,
_metadata(_destinationDomain),
hook
);
}
function _quoteDispatch(
uint32 _destinationDomain,
bytes32 _recipient,
bytes memory _messageBody,
bytes calldata _hookMetadata,
IPostDispatchHook _hook
) internal view virtual returns (uint256) {
return
mailbox.quoteDispatch(
_destinationDomain,
_recipient,
_messageBody,
_hookMetadata,
_hook
);
}
}

65
solidity/contracts/client/Router.sol
Unescape View File

@ -167,28 +167,73 @@ abstract contract Router is MailboxClient, IMessageRecipient {
);
}
function _dispatch(
function _Router_dispatch(
uint32 _destinationDomain,
bytes memory _messageBody
) internal virtual returns (bytes32) {
return _dispatch(_destinationDomain, msg.value, _messageBody);
uint256 _value,
bytes memory _messageBody,
bytes memory _hookMetadata,
address _hook
) internal returns (bytes32) {
bytes32 _router = _mustHaveRemoteRouter(_destinationDomain);
return
mailbox.dispatch{value: _value}(
_destinationDomain,
_router,
_messageBody,
_hookMetadata,
IPostDispatchHook(_hook)
);
}
/**
* DEPRECATED: Use `_Router_dispatch` instead
* @dev For backward compatibility with v2 client contracts
*/
function _dispatch(
uint32 _destinationDomain,
uint256 _value,
bytes memory _messageBody
) internal virtual returns (bytes32) {
) internal returns (bytes32) {
return
_Router_dispatch(
_destinationDomain,
msg.value,
_messageBody,
"",
address(hook)
);
}
function _Router_quoteDispatch(
uint32 _destinationDomain,
bytes memory _messageBody,
bytes memory _hookMetadata,
address _hook
) internal view returns (uint256) {
bytes32 _router = _mustHaveRemoteRouter(_destinationDomain);
return
super._dispatch(_destinationDomain, _router, _value, _messageBody);
mailbox.quoteDispatch(
_destinationDomain,
_router,
_messageBody,
_hookMetadata,
IPostDispatchHook(_hook)
);
}
/**
* DEPRECATED: Use `_Router_quoteDispatch` instead
* @dev For backward compatibility with v2 client contracts
*/
function _quoteDispatch(
uint32 _destinationDomain,
bytes memory _messageBody
) internal view virtual returns (uint256) {
bytes32 _router = _mustHaveRemoteRouter(_destinationDomain);
return super._quoteDispatch(_destinationDomain, _router, _messageBody);
) internal view returns (uint256) {
return
_Router_quoteDispatch(
_destinationDomain,
_messageBody,
"",
address(hook)
);
}
}

2
solidity/contracts/test/TestGasRouter.sol
Unescape View File

@ -7,7 +7,7 @@ contract TestGasRouter is GasRouter {
constructor(address _mailbox) GasRouter(_mailbox) {}
function dispatch(uint32 _destination, bytes memory _msg) external payable {
_dispatch(_destination, _msg);
_GasRouter_dispatch(_destination, msg.value, _msg, address(hook));
}
function _handle(uint32, bytes32, bytes calldata) internal pure override {}

16
solidity/contracts/token/HypNative.sol
Unescape View File

@ -36,24 +36,16 @@ contract HypNative is TokenRouter {
/**
* @inheritdoc TokenRouter
* @dev uses (`msg.value` - `_amount`) as interchain gas payment and `msg.sender` as refund address.
* @dev uses (`msg.value` - `_amount`) as hook payment and `msg.sender` as refund address.
*/
function transferRemote(
uint32 _destination,
bytes32 _recipient,
uint256 _amount
) public payable virtual override returns (bytes32 messageId) {
) external payable virtual override returns (bytes32 messageId) {
require(msg.value >= _amount, "Native: amount exceeds msg.value");
uint256 gasPayment = msg.value - _amount;
return
_transferRemote(
_destination,
_recipient,
_amount,
gasPayment,
bytes(""),
address(0)
);
uint256 _hookPayment = msg.value - _amount;
return _transferRemote(_destination, _recipient, _amount, _hookPayment);
}
function balanceOf(

12
solidity/contracts/token/extensions/HypNativeScaled.sol
Unescape View File

@ -25,18 +25,16 @@ contract HypNativeScaled is HypNative {
uint32 _destination,
bytes32 _recipient,
uint256 _amount
) public payable override returns (bytes32 messageId) {
) external payable override returns (bytes32 messageId) {
require(msg.value >= _amount, "Native: amount exceeds msg.value");
uint256 gasPayment = msg.value - _amount;
uint256 scaledAmount = _amount / scale;
uint256 _hookPayment = msg.value - _amount;
uint256 _scaledAmount = _amount / scale;
return
_transferRemote(
_destination,
_recipient,
scaledAmount,
gasPayment,
bytes(""),
address(0)
_scaledAmount,
_hookPayment
);
}

6
solidity/contracts/token/libs/FastTokenRouter.sol
Unescape View File

@ -109,9 +109,11 @@ abstract contract FastTokenRouter is TokenRouter {
_fastTransferId
);
messageId = _dispatch(
messageId = _GasRouter_dispatch(
_destination,
TokenMessage.format(_recipient, _amountOrId, metadata)
msg.value,
TokenMessage.format(_recipient, _amountOrId, metadata),
address(hook)
);
emit SentTransferRemote(_destination, _recipient, _amountOrId);
}

71
solidity/contracts/token/libs/TokenRouter.sol
Unescape View File

@ -57,14 +57,7 @@ abstract contract TokenRouter is GasRouter {
uint256 _amountOrId
) external payable virtual returns (bytes32 messageId) {
return
_transferRemote(
_destination,
_recipient,
_amountOrId,
msg.value,
bytes(""),
address(0)
);
_transferRemote(_destination, _recipient, _amountOrId, msg.value);
}
/**
@ -97,45 +90,45 @@ abstract contract TokenRouter is GasRouter {
);
}
/**
* @notice Transfers `_amountOrId` token to `_recipient` on `_destination` domain.
* @dev Delegates transfer logic to `_transferFromSender` implementation.
* @dev The metadata is the token metadata, and is DIFFERENT than the hook metadata.
* @dev Emits `SentTransferRemote` event on the origin chain.
* @param _destination The identifier of the destination chain.
* @param _recipient The address of the recipient on the destination chain.
* @param _amountOrId The amount or identifier of tokens to be sent to the remote recipient.
* @param _gasPayment The amount of native token to pay for interchain gas.
* @param _hookMetadata The metadata passed into the hook
* @param _hook The post dispatch hook to be called by the Mailbox
* @return messageId The identifier of the dispatched message.
*/
function _transferRemote(
uint32 _destination,
bytes32 _recipient,
uint256 _amountOrId,
uint256 _gasPayment,
bytes memory _hookMetadata,
address _hook
uint256 _value
) internal returns (bytes32 messageId) {
bytes memory metadata = _transferFromSender(_amountOrId);
if (address(_hook) == address(0)) {
messageId = _dispatch(
_destination,
_gasPayment,
TokenMessage.format(_recipient, _amountOrId, metadata)
);
} else {
messageId = _dispatch(
return
_transferRemote(
_destination,
_recipient,
_gasPayment,
TokenMessage.format(_recipient, _amountOrId, metadata),
_hookMetadata,
IPostDispatchHook(_hook)
_amountOrId,
_value,
_GasRouter_hookMetadata(_destination),
address(hook)
);
}
}
function _transferRemote(
uint32 _destination,
bytes32 _recipient,
uint256 _amountOrId,
uint256 _value,
bytes memory _hookMetadata,
address _hook
) internal virtual returns (bytes32 messageId) {
bytes memory _tokenMetadata = _transferFromSender(_amountOrId);
bytes memory _tokenMessage = TokenMessage.format(
_recipient,
_amountOrId,
_tokenMetadata
);
messageId = _Router_dispatch(
_destination,
_value,
_tokenMessage,
_hookMetadata,
_hook
);
emit SentTransferRemote(_destination, _recipient, _amountOrId);
}

7
solidity/test/InterchainAccountRouter.t.sol
Unescape View File

@ -479,6 +479,7 @@ contract InterchainAccountRouterTest is Test {
uint64 payment,
bytes32 data
) public {
CallLib.Call[] memory calls = getCalls(data);
vm.assume(payment < gasLimit * igp.gasPrice());
// arrange
bytes memory metadata = StandardHookMetadata.formatMetadata(
@ -495,11 +496,7 @@ contract InterchainAccountRouterTest is Test {
// act
vm.expectRevert("IGP: insufficient interchain gas payment");
originRouter.callRemote{value: payment}(
destination,
getCalls(data),
metadata
);
originRouter.callRemote{value: payment}(destination, calls, metadata);
}
function testFuzz_callRemoteWithOverrides_default(bytes32 data) public {

33
solidity/test/token/HypERC20.t.sol
Unescape View File

@ -23,6 +23,7 @@ import {XERC20LockboxTest, XERC20Test, FiatTokenTest, ERC20Test} from "../../con
import {TestPostDispatchHook} from "../../contracts/test/TestPostDispatchHook.sol";
import {TestInterchainGasPaymaster} from "../../contracts/test/TestInterchainGasPaymaster.sol";
import {GasRouter} from "../../contracts/client/GasRouter.sol";
import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol";
import {HypERC20} from "../../contracts/token/HypERC20.sol";
import {HypERC20Collateral} from "../../contracts/token/HypERC20Collateral.sol";
@ -198,38 +199,38 @@ abstract contract HypTokenTest is Test {
function _performRemoteTransferWithHook(
uint256 _msgValue,
uint256 _amount
uint256 _amount,
address _hook,
bytes memory _hookMetadata
) internal returns (bytes32 messageId) {
vm.prank(ALICE);
messageId = localToken.transferRemote{value: _msgValue}(
DESTINATION,
BOB.addressToBytes32(),
_amount,
bytes(""),
address(noopHook)
_hookMetadata,
address(_hook)
);
_processTransfers(BOB, _amount);
assertEq(remoteToken.balanceOf(BOB), _amount);
}
function testTransfer_withHookSpecified() public {
function testTransfer_withHookSpecified(
uint256 fee,
bytes calldata metadata
) public {
TestPostDispatchHook hook = new TestPostDispatchHook();
hook.setFee(fee);
vm.prank(ALICE);
primaryToken.approve(address(localToken), TRANSFER_AMT);
bytes32 messageId = _performRemoteTransferWithHook(
REQUIRED_VALUE,
TRANSFER_AMT
TRANSFER_AMT,
address(hook),
metadata
);
assertTrue(noopHook.messageDispatched(messageId));
/// @dev Using this test would be ideal, but vm.expectCall with nested functions more than 1 level deep is broken
/// In other words, the call graph of Route.transferRemote() -> Mailbox.dispatch() -> Hook.postDispatch() does not work with expectCall
// vm.expectCall(
// address(noopHook),
// abi.encodeCall(
// IPostDispatchHook.postDispatch,
// (bytes(""), outboundMessage)
// )
// );
/// @dev Also, using expectedCall with Mailbox.dispatch() won't work either because overloaded function selection is broken, see https://github.com/ethereum/solidity/issues/13815
assertTrue(hook.messageDispatched(messageId));
}
function testBenchmark_overheadGasUsage() public virtual {

6
typescript/helloworld/contracts/HelloWorld.sol
Unescape View File

@ -83,12 +83,6 @@ contract HelloWorld is Router {
}
// ============ Internal functions ============
function _metadata(
uint32 /*_destinationDomain*/
) internal view override returns (bytes memory) {
return StandardHookMetadata.overrideGasLimit(HANDLE_GAS_AMOUNT);
}
/**
* @notice Handles a message from a remote router.
* @dev Only called for messages sent from a remote router, as enforced by Router.sol.

Write Preview
Loading…
Cancel
Save