From 1a584ad8a4fc0d588404de053cf503657ba3059a Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 21 Nov 2023 18:11:43 -0500 Subject: [PATCH 1/4] Improve standard hook metadata API --- solidity/contracts/client/GasRouter.sol | 9 +-- solidity/contracts/client/Router.sol | 1 - solidity/contracts/hooks/PausableHook.sol | 3 - .../aggregation/StaticAggregationHook.sol | 3 - .../hooks/libs/StandardHookMetadata.sol | 56 ++++++++++++------- .../hooks/routing/DomainRoutingHook.sol | 1 - solidity/contracts/test/TestSendReceiver.sol | 14 +---- solidity/test/hooks/StaticProtocolFee.t.sol | 7 +-- .../test/igps/InterchainGasPaymaster.t.sol | 14 ++--- solidity/test/isms/OPStackIsm.t.sol | 10 +--- 10 files changed, 47 insertions(+), 71 deletions(-) diff --git a/solidity/contracts/client/GasRouter.sol b/solidity/contracts/client/GasRouter.sol index b013e373d8..6f4b248e55 100644 --- a/solidity/contracts/client/GasRouter.sol +++ b/solidity/contracts/client/GasRouter.sol @@ -47,18 +47,11 @@ abstract contract GasRouter is Router { return _quoteDispatch(_destinationDomain, ""); } - function _refundAddress(uint32) internal view virtual returns (address) { - return msg.sender; - } - function _metadata( uint32 _destination ) internal view virtual override returns (bytes memory) { return - StandardHookMetadata.formatMetadata( - destinationGas[_destination], - _refundAddress(_destination) - ); + StandardHookMetadata.overrideGasLimit(destinationGas[_destination]); } function _setDestinationGas(uint32 domain, uint256 gas) internal { diff --git a/solidity/contracts/client/Router.sol b/solidity/contracts/client/Router.sol index f7853659a0..ef14912f90 100644 --- a/solidity/contracts/client/Router.sol +++ b/solidity/contracts/client/Router.sol @@ -7,7 +7,6 @@ import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; import {IInterchainSecurityModule} from "../interfaces/IInterchainSecurityModule.sol"; import {MailboxClient} from "./MailboxClient.sol"; import {EnumerableMapExtended} from "../libs/EnumerableMapExtended.sol"; -import {StandardHookMetadata} from "../hooks/libs/StandardHookMetadata.sol"; // ============ External Imports ============ import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; diff --git a/solidity/contracts/hooks/PausableHook.sol b/solidity/contracts/hooks/PausableHook.sol index 03f9aa8cc4..aeffc7630a 100644 --- a/solidity/contracts/hooks/PausableHook.sol +++ b/solidity/contracts/hooks/PausableHook.sol @@ -13,7 +13,6 @@ pragma solidity >=0.8.0; @@@@@@@@@ @@@@@@@@@ @@@@@@@@@ @@@@@@@@*/ -import {StandardHookMetadata} from "../hooks/libs/StandardHookMetadata.sol"; import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; import {AbstractPostDispatchHook} from "./libs/AbstractPostDispatchHook.sol"; @@ -21,8 +20,6 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import {Pausable} from "@openzeppelin/contracts/security/Pausable.sol"; contract PausableHook is AbstractPostDispatchHook, Ownable, Pausable { - using StandardHookMetadata for bytes; - // ============ External functions ============ function pause() external onlyOwner { diff --git a/solidity/contracts/hooks/aggregation/StaticAggregationHook.sol b/solidity/contracts/hooks/aggregation/StaticAggregationHook.sol index 05290c611e..5146ca2c02 100644 --- a/solidity/contracts/hooks/aggregation/StaticAggregationHook.sol +++ b/solidity/contracts/hooks/aggregation/StaticAggregationHook.sol @@ -13,14 +13,11 @@ pragma solidity >=0.8.0; @@@@@@@@@ @@@@@@@@@ @@@@@@@@@ @@@@@@@@*/ -import {StandardHookMetadata} from "../libs/StandardHookMetadata.sol"; import {AbstractPostDispatchHook} from "../libs/AbstractPostDispatchHook.sol"; import {IPostDispatchHook} from "../../interfaces/hooks/IPostDispatchHook.sol"; import {MetaProxy} from "../../libs/MetaProxy.sol"; contract StaticAggregationHook is AbstractPostDispatchHook { - using StandardHookMetadata for bytes; - // ============ External functions ============ /// @inheritdoc IPostDispatchHook diff --git a/solidity/contracts/hooks/libs/StandardHookMetadata.sol b/solidity/contracts/hooks/libs/StandardHookMetadata.sol index e225c39574..be2b889a54 100644 --- a/solidity/contracts/hooks/libs/StandardHookMetadata.sol +++ b/solidity/contracts/hooks/libs/StandardHookMetadata.sol @@ -16,13 +16,20 @@ pragma solidity >=0.8.0; /** * Format of metadata: * - * [0:1] variant - * [2:33] msg.value - * [34:65] Gas limit for message (IGP) - * [66:85] Refund address for message (IGP) + * [0:2] variant + * [2:34] msg.value + * [34:66] Gas limit for message (IGP) + * [66:86] Refund address for message (IGP) * [86:] Custom metadata */ library StandardHookMetadata { + struct Metadata { + uint16 variant; + uint256 msgValue; + uint256 gasLimit; + address refundAddress; + } + uint8 private constant VARIANT_OFFSET = 0; uint8 private constant MSG_VALUE_OFFSET = 2; uint8 private constant GAS_LIMIT_OFFSET = 34; @@ -33,7 +40,7 @@ library StandardHookMetadata { /** * @notice Returns the variant of the metadata. - * @param _metadata ABI encoded global hook metadata. + * @param _metadata ABI encoded standard hook metadata. * @return variant of the metadata as uint8. */ function variant(bytes calldata _metadata) internal pure returns (uint16) { @@ -43,7 +50,7 @@ library StandardHookMetadata { /** * @notice Returns the specified value for the message. - * @param _metadata ABI encoded global hook metadata. + * @param _metadata ABI encoded standard hook metadata. * @param _default Default fallback value. * @return Value for the message as uint256. */ @@ -58,7 +65,7 @@ library StandardHookMetadata { /** * @notice Returns the specified gas limit for the message. - * @param _metadata ABI encoded global hook metadata. + * @param _metadata ABI encoded standard hook metadata. * @param _default Default fallback gas limit. * @return Gas limit for the message as uint256. */ @@ -73,7 +80,7 @@ library StandardHookMetadata { /** * @notice Returns the specified refund address for the message. - * @param _metadata ABI encoded global hook metadata. + * @param _metadata ABI encoded standard hook metadata. * @param _default Default fallback refund address. * @return Refund address for the message as address. */ @@ -92,7 +99,7 @@ library StandardHookMetadata { /** * @notice Returns the specified refund address for the message. - * @param _metadata ABI encoded global hook metadata. + * @param _metadata ABI encoded standard hook metadata. * @return Refund address for the message as address. */ function getCustomMetadata( @@ -103,12 +110,12 @@ library StandardHookMetadata { } /** - * @notice Formats the specified gas limit and refund address into global hook metadata. + * @notice Formats the specified gas limit and refund address into standard hook metadata. * @param _msgValue msg.value for the message. * @param _gasLimit Gas limit for the message. * @param _refundAddress Refund address for the message. - * @param _customMetadata Additional metadata to include in the global hook metadata. - * @return ABI encoded global hook metadata. + * @param _customMetadata Additional metadata to include in the standard hook metadata. + * @return ABI encoded standard hook metadata. */ function formatMetadata( uint256 _msgValue, @@ -127,26 +134,35 @@ library StandardHookMetadata { } /** - * @notice Formats the specified gas limit and refund address into global hook metadata. + * @notice Formats the specified gas limit and refund address into standard hook metadata. * @param _msgValue msg.value for the message. - * @return ABI encoded global hook metadata. + * @return ABI encoded standard hook metadata. */ - function formatMetadata( + function overrideMsgValue( uint256 _msgValue ) internal view returns (bytes memory) { return formatMetadata(_msgValue, uint256(0), msg.sender, ""); } /** - * @notice Formats the specified gas limit and refund address into global hook metadata. + * @notice Formats the specified gas limit and refund address into standard hook metadata. * @param _gasLimit Gas limit for the message. + * @return ABI encoded standard hook metadata. + */ + function overrideGasLimit( + uint256 _gasLimit + ) internal view returns (bytes memory) { + return formatMetadata(uint256(0), _gasLimit, msg.sender, ""); + } + + /** + * @notice Formats the specified refund address into standard hook metadata. * @param _refundAddress Refund address for the message. - * @return ABI encoded global hook metadata. + * @return ABI encoded standard hook metadata. */ - function formatMetadata( - uint256 _gasLimit, + function overrideRefundAddress( address _refundAddress ) internal pure returns (bytes memory) { - return formatMetadata(uint256(0), _gasLimit, _refundAddress, ""); + return formatMetadata(uint256(0), uint256(0), _refundAddress, ""); } } diff --git a/solidity/contracts/hooks/routing/DomainRoutingHook.sol b/solidity/contracts/hooks/routing/DomainRoutingHook.sol index 4b893d8dba..377f17fa7f 100644 --- a/solidity/contracts/hooks/routing/DomainRoutingHook.sol +++ b/solidity/contracts/hooks/routing/DomainRoutingHook.sol @@ -14,7 +14,6 @@ pragma solidity >=0.8.0; @@@@@@@@@ @@@@@@@@*/ // ============ Internal Imports ============ -import {StandardHookMetadata} from "../libs/StandardHookMetadata.sol"; import {AbstractPostDispatchHook} from "../libs/AbstractPostDispatchHook.sol"; import {MailboxClient} from "../../client/MailboxClient.sol"; import {Message} from "../../libs/Message.sol"; diff --git a/solidity/contracts/test/TestSendReceiver.sol b/solidity/contracts/test/TestSendReceiver.sol index d3dbcde3f8..ef5be37a1c 100644 --- a/solidity/contracts/test/TestSendReceiver.sol +++ b/solidity/contracts/test/TestSendReceiver.sol @@ -9,7 +9,6 @@ import {IMailbox} from "../interfaces/IMailbox.sol"; import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; import {IInterchainSecurityModule, ISpecifiesInterchainSecurityModule} from "../interfaces/IInterchainSecurityModule.sol"; -import {StandardHookMetadata} from "../hooks/libs/StandardHookMetadata.sol"; import {MailboxClient} from "../client/MailboxClient.sol"; contract TestSendReceiver is IMessageRecipient { @@ -24,16 +23,11 @@ contract TestSendReceiver is IMessageRecipient { uint32 _destinationDomain, bytes calldata _messageBody ) external payable { - bytes memory hookMetadata = StandardHookMetadata.formatMetadata( - HANDLE_GAS_AMOUNT, - msg.sender - ); // TODO: handle topping up? _mailbox.dispatch{value: msg.value}( _destinationDomain, address(this).addressToBytes32(), - _messageBody, - hookMetadata + _messageBody ); } @@ -43,16 +37,12 @@ contract TestSendReceiver is IMessageRecipient { bytes calldata _messageBody, IPostDispatchHook hook ) external payable { - bytes memory hookMetadata = StandardHookMetadata.formatMetadata( - HANDLE_GAS_AMOUNT, - msg.sender - ); // TODO: handle topping up? _mailbox.dispatch{value: msg.value}( _destinationDomain, address(this).addressToBytes32(), _messageBody, - hookMetadata, + bytes(""), hook ); } diff --git a/solidity/test/hooks/StaticProtocolFee.t.sol b/solidity/test/hooks/StaticProtocolFee.t.sol index 7d9456de6f..a110c8304f 100644 --- a/solidity/test/hooks/StaticProtocolFee.t.sol +++ b/solidity/test/hooks/StaticProtocolFee.t.sol @@ -122,12 +122,7 @@ contract StaticProtocolFeeTest is Test { uint256 feeRequired, uint256 feeSent ) public { - bytes memory metadata = StandardHookMetadata.formatMetadata( - 0, - 0, - bob, - "" - ); + bytes memory metadata = StandardHookMetadata.overrideRefundAddress(bob); feeRequired = bound(feeRequired, 1, fees.MAX_PROTOCOL_FEE()); feeSent = bound(feeSent, feeRequired, 10 * feeRequired); diff --git a/solidity/test/igps/InterchainGasPaymaster.t.sol b/solidity/test/igps/InterchainGasPaymaster.t.sol index dcdf7b4a60..de5337ef68 100644 --- a/solidity/test/igps/InterchainGasPaymaster.t.sol +++ b/solidity/test/igps/InterchainGasPaymaster.t.sol @@ -410,11 +410,8 @@ contract InterchainGasPaymasterTest is Test { TEST_GAS_PRICE ); - bytes memory metadata = StandardHookMetadata.formatMetadata( - 0, - uint256(testGasLimit), // gas limit - testRefundAddress, // refund address, - bytes("") + bytes memory metadata = StandardHookMetadata.overrideGasLimit( + uint256(testGasLimit) ); // 150 * (300_000 + 123_000) = 45_000_000 assertEq(igp.quoteDispatch(metadata, testEncodedMessage), 63_450_000); @@ -524,11 +521,8 @@ contract InterchainGasPaymasterTest is Test { igp.destinationGasLimit(testDestinationDomain, testGasLimit) ); - bytes memory metadata = StandardHookMetadata.formatMetadata( - 0, - uint256(testGasLimit), // gas limit - testRefundAddress, // refund address - bytes("") + bytes memory metadata = StandardHookMetadata.overrideGasLimit( + uint256(testGasLimit) ); bytes memory message = _encodeTestMessage(); igp.postDispatch{value: _quote}(metadata, message); diff --git a/solidity/test/isms/OPStackIsm.t.sol b/solidity/test/isms/OPStackIsm.t.sol index 3040166f60..d07ce02ca9 100644 --- a/solidity/test/isms/OPStackIsm.t.sol +++ b/solidity/test/isms/OPStackIsm.t.sol @@ -51,7 +51,7 @@ contract OPStackIsmTest is Test { bytes internal testMessage = abi.encodePacked("Hello from the other chain!"); bytes internal testMetadata = - StandardHookMetadata.formatMetadata(0, 0, address(this), ""); + StandardHookMetadata.overrideRefundAddress(address(this)); bytes internal encodedMessage; bytes32 internal messageId; @@ -195,12 +195,8 @@ contract OPStackIsmTest is Test { vm.selectFork(mainnetFork); vm.deal(address(this), uint256(2 ** 255 + 1)); - bytes memory excessValueMetadata = StandardHookMetadata.formatMetadata( - uint256(2 ** 255 + 1), - DEFAULT_GAS_LIMIT, - address(this), - "" - ); + bytes memory excessValueMetadata = StandardHookMetadata + .overrideMsgValue(uint256(2 ** 255 + 1)); l1Mailbox.updateLatestDispatchedId(messageId); vm.expectRevert("OPStackHook: msgValue must be less than 2 ** 255"); From 62211b92e3c8336b0c6a1ea65ec248524106a707 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 21 Nov 2023 18:12:22 -0500 Subject: [PATCH 2/4] Add requiredHook to IMailbox --- solidity/contracts/interfaces/IMailbox.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/solidity/contracts/interfaces/IMailbox.sol b/solidity/contracts/interfaces/IMailbox.sol index b0b8fa418e..60d2e9a2b6 100644 --- a/solidity/contracts/interfaces/IMailbox.sol +++ b/solidity/contracts/interfaces/IMailbox.sol @@ -52,6 +52,8 @@ interface IMailbox { function defaultHook() external view returns (IPostDispatchHook); + function requiredHook() external view returns (IPostDispatchHook); + function latestDispatchedId() external view returns (bytes32); function dispatch( From ed0391cd34cdd571ca745f06c162a8f45bb78297 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 21 Nov 2023 18:57:27 -0500 Subject: [PATCH 3/4] Fix helloworld metadata usage --- typescript/helloworld/contracts/HelloWorld.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/typescript/helloworld/contracts/HelloWorld.sol b/typescript/helloworld/contracts/HelloWorld.sol index 4888c49ecd..b3bdb16fdc 100644 --- a/typescript/helloworld/contracts/HelloWorld.sol +++ b/typescript/helloworld/contracts/HelloWorld.sol @@ -86,8 +86,7 @@ contract HelloWorld is Router { function _metadata( uint32 /*_destinationDomain*/ ) internal view override returns (bytes memory) { - return - StandardHookMetadata.formatMetadata(HANDLE_GAS_AMOUNT, msg.sender); + return StandardHookMetadata.overrideGasLimit(HANDLE_GAS_AMOUNT); } /** From c9e0aedae6e912f8615d927c5ef7b9db4929d47b Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Wed, 22 Nov 2023 11:57:55 -0500 Subject: [PATCH 4/4] Add changeset --- .changeset/good-candles-promise.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/good-candles-promise.md diff --git a/.changeset/good-candles-promise.md b/.changeset/good-candles-promise.md new file mode 100644 index 0000000000..cd9296d030 --- /dev/null +++ b/.changeset/good-candles-promise.md @@ -0,0 +1,6 @@ +--- +'@hyperlane-xyz/helloworld': patch +'@hyperlane-xyz/core': patch +--- + +Improve client side StandardHookMetadata library interface