Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remediate Trevor's comments #2835

Merged
merged 6 commits into from
Oct 24, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add tests for hook type
  • Loading branch information
yorhodes committed Oct 24, 2023
commit f195da320df1c1e8b6e28a7845671506b0bb3395
10 changes: 1 addition & 9 deletions solidity/contracts/hooks/OPStackHook.sol
Original file line number Diff line number Diff line change
@@ -58,13 +58,7 @@ contract OPStackHook is AbstractMessageIdAuthHook {
l1Messenger = ICrossDomainMessenger(_l1Messenger);
}

// ============ External functions ============

/// @inheritdoc IPostDispatchHook
function hookType() external pure override returns (uint8) {
return uint8(IPostDispatchHook.Types.OP_STACK);
}

// ============ Internal functions ============
function _quoteDispatch(bytes calldata, bytes calldata)
internal
pure
@@ -74,8 +68,6 @@ contract OPStackHook is AbstractMessageIdAuthHook {
return 0; // gas subsidized by the L2
}

// ============ Internal functions ============

/// @inheritdoc AbstractMessageIdAuthHook
function _sendMessageId(bytes calldata metadata, bytes memory payload)
internal
7 changes: 0 additions & 7 deletions solidity/contracts/hooks/aggregation/ERC5164Hook.sol
Original file line number Diff line number Diff line change
@@ -43,13 +43,6 @@ contract ERC5164Hook is AbstractMessageIdAuthHook {
dispatcher = IMessageDispatcher(_dispatcher);
}

// ============ External Functions ============

/// @inheritdoc IPostDispatchHook
function hookType() external pure override returns (uint8) {
return uint8(IPostDispatchHook.Types.ERC_5164);
}

// ============ Internal Functions ============

function _quoteDispatch(bytes calldata, bytes calldata)
2 changes: 1 addition & 1 deletion solidity/contracts/hooks/igp/InterchainGasPaymaster.sol
Original file line number Diff line number Diff line change
@@ -94,7 +94,7 @@ contract InterchainGasPaymaster is

/// @inheritdoc IPostDispatchHook
function hookType() external pure override returns (uint8) {
return uint8(IPostDispatchHook.Types.IGP);
return uint8(IPostDispatchHook.Types.INTERCHAIN_GAS_PAYMASTER);
}

/**
6 changes: 6 additions & 0 deletions solidity/contracts/hooks/libs/AbstractMessageIdAuthHook.sol
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@ pragma solidity >=0.8.0;
@@@@@@@@@ @@@@@@@@*/

// ============ Internal Imports ============
import {IPostDispatchHook} from "../../interfaces/hooks/IPostDispatchHook.sol";
import {AbstractPostDispatchHook} from "./AbstractPostDispatchHook.sol";
import {AbstractMessageIdAuthorizedIsm} from "../../isms/hook/AbstractMessageIdAuthorizedIsm.sol";
import {TypeCasts} from "../../libs/TypeCasts.sol";
@@ -56,6 +57,11 @@ abstract contract AbstractMessageIdAuthHook is
destinationDomain = _destinationDomain;
}

/// @inheritdoc IPostDispatchHook
function hookType() external pure returns (uint8) {
return uint8(IPostDispatchHook.Types.ID_AUTH_ISM);
}

// ============ Internal functions ============

/// @inheritdoc AbstractPostDispatchHook
5 changes: 2 additions & 3 deletions solidity/contracts/interfaces/hooks/IPostDispatchHook.sol
Original file line number Diff line number Diff line change
@@ -19,10 +19,9 @@ interface IPostDispatchHook {
ROUTING,
AGGREGATION,
MERKLE_TREE,
ERC_5164,
INTERCHAIN_GAS_PAYMASTER,
FALLBACK_ROUTING,
IGP,
OP_STACK,
ID_AUTH_ISM,
PAUSABLE,
PROTOCOL_FEE
}
5 changes: 5 additions & 0 deletions solidity/test/MerkleTreeHook.t.sol
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ import {Message} from "../contracts/libs/Message.sol";
import {TypeCasts} from "../contracts/libs/TypeCasts.sol";
import {TestMerkleTreeHook} from "../contracts/test/TestMerkleTreeHook.sol";
import {TestMailbox} from "../contracts/test/TestMailbox.sol";
import {IPostDispatchHook} from "../contracts/interfaces/hooks/IPostDispatchHook.sol";

contract MerkleTreeHookTest is Test {
using Message for bytes;
@@ -68,4 +69,8 @@ contract MerkleTreeHookTest is Test {
assertEq(hook.quoteDispatch("", currMessage), 0);
}
}

function testHookType() public {
assertEq(hook.hookType(), uint8(IPostDispatchHook.Types.MERKLE_TREE));
}
}
6 changes: 6 additions & 0 deletions solidity/test/hooks/AggregationHook.t.sol
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@ import {Test} from "forge-std/Test.sol";
import {StaticAggregationHook} from "../../contracts/hooks/aggregation/StaticAggregationHook.sol";
import {StaticAggregationHookFactory} from "../../contracts/hooks/aggregation/StaticAggregationHookFactory.sol";
import {TestPostDispatchHook} from "../../contracts/test/TestPostDispatchHook.sol";
import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol";

contract AggregationHookTest is Test {
StaticAggregationHookFactory internal factory;
@@ -88,4 +89,9 @@ contract AggregationHookTest is Test {
address[] memory actualHook = hook.hooks("");
assertEq(actualHook, expectedHooks);
}

function testHookType() public {
deployHooks(1, 0);
assertEq(hook.hookType(), uint8(IPostDispatchHook.Types.AGGREGATION));
}
}
12 changes: 12 additions & 0 deletions solidity/test/hooks/DomainRoutingHook.t.sol
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import {DomainRoutingHook} from "../../contracts/hooks/routing/DomainRoutingHook
import {FallbackDomainRoutingHook} from "../../contracts/hooks/routing/FallbackDomainRoutingHook.sol";
import {TestPostDispatchHook} from "../../contracts/test/TestPostDispatchHook.sol";
import {TestMailbox} from "../../contracts/test/TestMailbox.sol";
import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol";

import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";

@@ -104,6 +105,10 @@ contract DomainRoutingHookTest is Test {
vm.expectRevert();
hook.postDispatch(metadata, testMessage);
}

function testHookType() public virtual {
assertEq(hook.hookType(), uint8(IPostDispatchHook.Types.ROUTING));
}
}

contract FallbackDomainRoutingHookTest is DomainRoutingHookTest {
@@ -162,4 +167,11 @@ contract FallbackDomainRoutingHookTest is DomainRoutingHookTest {
);
hook.postDispatch(metadata, testMessage);
}

function testHookType() public override {
assertEq(
hook.hookType(),
uint8(IPostDispatchHook.Types.FALLBACK_ROUTING)
);
}
}
5 changes: 5 additions & 0 deletions solidity/test/hooks/StaticProtocolFee.t.sol
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ import {Test} from "forge-std/Test.sol";
import {TypeCasts} from "../../contracts/libs/TypeCasts.sol";
import {MessageUtils} from "../isms/IsmTestUtils.sol";
import {StandardHookMetadata} from "../../contracts/hooks/libs/StandardHookMetadata.sol";
import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol";

import {StaticProtocolFee} from "../../contracts/hooks/StaticProtocolFee.sol";

@@ -35,6 +36,10 @@ contract StaticProtocolFeeTest is Test {
assertEq(fees.protocolFee(), FEE);
}

function testHookType() public {
assertEq(fees.hookType(), uint8(IPostDispatchHook.Types.PROTOCOL_FEE));
}

function testSetProtocolFee(uint256 fee) public {
fee = bound(fee, 0, fees.MAX_PROTOCOL_FEE());
fees.setProtocolFee(fee);
8 changes: 8 additions & 0 deletions solidity/test/igps/InterchainGasPaymaster.t.sol
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ import {TypeCasts} from "../../contracts/libs/TypeCasts.sol";
import {InterchainGasPaymaster} from "../../contracts/hooks/igp/InterchainGasPaymaster.sol";
import {StorageGasOracle} from "../../contracts/hooks/igp/StorageGasOracle.sol";
import {IGasOracle} from "../../contracts/interfaces/IGasOracle.sol";
import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol";

contract InterchainGasPaymasterTest is Test {
using StandardHookMetadata for bytes;
@@ -534,6 +535,13 @@ contract InterchainGasPaymasterTest is Test {
assertEq(_igpBalanceAfter - _igpBalanceBefore, _quote);
}

function testHookType() public {
assertEq(
igp.hookType(),
uint8(IPostDispatchHook.Types.INTERCHAIN_GAS_PAYMASTER)
);
}

// ============ claim ============

function testClaim() public {
7 changes: 7 additions & 0 deletions solidity/test/isms/ERC5164ISM.t.sol
Original file line number Diff line number Diff line change
@@ -9,6 +9,8 @@ import {MessageUtils} from "./IsmTestUtils.sol";
import {TypeCasts} from "../../contracts/libs/TypeCasts.sol";

import {IMessageDispatcher} from "../../contracts/interfaces/hooks/IMessageDispatcher.sol";
import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol";
import {IInterchainSecurityModule} from "../../contracts/interfaces/IInterchainSecurityModule.sol";
import {ERC5164Hook} from "../../contracts/hooks/aggregation/ERC5164Hook.sol";
import {AbstractMessageIdAuthorizedIsm} from "../../contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol";
import {ERC5164Ism} from "../../contracts/isms/hook/ERC5164Ism.sol";
@@ -132,6 +134,11 @@ contract ERC5164IsmTest is Test {
hook.postDispatch(bytes(""), encodedMessage);
}

function testTypes() public {
assertEq(hook.hookType(), uint8(IPostDispatchHook.Types.ID_AUTH_ISM));
assertEq(ism.moduleType(), uint8(IInterchainSecurityModule.Types.NULL));
}

function test_postDispatch_RevertWhen_ChainIDNotSupported() public {
encodedMessage = MessageUtils.formatMessage(
VERSION,