diff --git a/.github/workflows/hook-tests.yml b/.github/workflows/hook-tests.yml new file mode 100644 index 000000000..44a78aacb --- /dev/null +++ b/.github/workflows/hook-tests.yml @@ -0,0 +1,35 @@ +name: Hook Tests + +on: + pull_request: + push: + branches: + - main + workflow_dispatch: + +env: + MOONBEAM_RPC_URL: ${{secrets.MOONBEAM_RPC_URL}} + FOUNDRY_PROFILE: ci + +jobs: + hook-tests: + name: Hook Tests (Unit + Integration) + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v2 + with: + submodules: recursive + + - name: Setup Environment + uses: ./.github/actions + + - name: Run Hook Tests + uses: nick-fields/retry@v3 + with: + polling_interval_seconds: 30 + retry_wait_seconds: 60 + timeout_minutes: 20 + max_attempts: 3 + command: time forge test --match-contract Hook --fork-url moonbeam -vvv diff --git a/proposals/hooks/BridgeValidationHook.sol b/proposals/hooks/BridgeValidationHook.sol new file mode 100644 index 000000000..615649900 --- /dev/null +++ b/proposals/hooks/BridgeValidationHook.sol @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity 0.8.19; + +import {xWELLRouter} from "@protocol/xWELL/xWELLRouter.sol"; +import {ProposalAction} from "@proposals/proposalTypes/IProposal.sol"; + +/// @title BridgeValidationHook +/// @notice Hook to validate bridgeToRecipient calls in proposals +/// @dev Ensures that the native value sent with bridge calls is between 5x and 10x +/// the actual bridge cost returned by router.bridgeCost(destinationChain) +abstract contract BridgeValidationHook { + /// @notice Function selector for bridgeToRecipient(address,uint256,uint16) + bytes4 private constant BRIDGE_TO_RECIPIENT_SELECTOR = + xWELLRouter.bridgeToRecipient.selector; + + /// @notice Minimum multiplier for bridge cost (5x) + uint256 private constant MIN_BRIDGE_COST_MULTIPLIER = 5; + + /// @notice Maximum multiplier for bridge cost (10x) + uint256 private constant MAX_BRIDGE_COST_MULTIPLIER = 10; + + /// @notice Verify bridge-related proposal actions before execution + /// @dev Called by inheriting contracts to validate bridge cost parameters + /// @param proposal Array of proposal actions to validate + function _verifyBridgeActions( + ProposalAction[] memory proposal + ) internal view { + uint256 proposalLength = proposal.length; + + for (uint256 i = 0; i < proposalLength; i++) { + bytes4 selector = bytesToBytes4(proposal[i].data); + + // Check if this action is a bridgeToRecipient call + if (selector == BRIDGE_TO_RECIPIENT_SELECTOR) { + address router = proposal[i].target; + uint256 actionValue = proposal[i].value; + + // Validate router is a contract + _validateRouterIsContract(router); + + // Extract wormholeChainId from calldata + // Calldata structure: + // 0-3: function selector + // 4-35: address to (32 bytes) + // 36-67: uint256 amount (32 bytes) + // 68-99: uint16 wormholeChainId (32 bytes, right-padded) + uint16 wormholeChainId = extractUint16FromCalldata( + proposal[i].data + ); + + // Get the actual bridge cost from the router with validation + uint256 bridgeCost = _getBridgeCost(router, wormholeChainId); + + // Validate that action value is between 5x and 10x the bridge cost + uint256 minValue = bridgeCost * MIN_BRIDGE_COST_MULTIPLIER; + uint256 maxValue = bridgeCost * MAX_BRIDGE_COST_MULTIPLIER; + + require( + actionValue >= minValue, + string.concat( + "BridgeValidationHook: bridge value too low. Expected >= ", + _toString(minValue), + ", got ", + _toString(actionValue) + ) + ); + + require( + actionValue <= maxValue, + string.concat( + "BridgeValidationHook: bridge value too high. Expected <= ", + _toString(maxValue), + ", got ", + _toString(actionValue) + ) + ); + } + } + } + + /// @notice Validates that the router address is a contract + /// @param router The router address to validate + function _validateRouterIsContract(address router) private view { + require( + router.code.length > 0, + "BridgeValidationHook: router must be a contract" + ); + } + + /// @notice Gets bridge cost from router and validates it's non-zero + /// @param router The router contract address + /// @param wormholeChainId The destination chain ID + /// @return bridgeCost The validated bridge cost + function _getBridgeCost( + address router, + uint16 wormholeChainId + ) private view returns (uint256 bridgeCost) { + bridgeCost = xWELLRouter(router).bridgeCost(wormholeChainId); + + require( + bridgeCost > 0, + "BridgeValidationHook: bridge cost must be greater than zero" + ); + } + + /// @notice Extract uint16 value from calldata at the third parameter position + /// @param input The calldata to extract from + /// @return result The extracted uint16 value + function extractUint16FromCalldata( + bytes memory input + ) public pure returns (uint16 result) { + require( + input.length >= 100, + "BridgeValidationHook: invalid calldata length" + ); + + // The uint16 wormholeChainId is the third parameter, starting at byte 68 + // It's stored in the last 2 bytes of a 32-byte word + bytes32 rawBytes; + assembly { + // Skip 32 bytes (array length) + 4 bytes (selector) + 64 bytes (first two params) + // = 100 bytes total, so we load from position 68 after the length prefix + let dataPointer := add(add(input, 0x20), 0x44) // 0x20 (32) + 0x44 (68) = 100 + rawBytes := mload(dataPointer) // Load 32 bytes + } + + // Extract the uint16 from the rightmost 2 bytes + result = uint16(uint256(rawBytes)); + } + + /// @notice Extract the first 4 bytes (function selector) from calldata + /// @dev This function must be implemented by inheriting contracts + /// @param toSlice The bytes to extract from + /// @return functionSignature The extracted function selector + function bytesToBytes4( + bytes memory toSlice + ) public pure virtual returns (bytes4 functionSignature); + + /// @notice Convert uint256 to string + /// @param value The uint256 value to convert + /// @return str The string representation + function _toString( + uint256 value + ) internal pure returns (string memory str) { + if (value == 0) { + return "0"; + } + + uint256 temp = value; + uint256 digits; + + while (temp != 0) { + digits++; + temp /= 10; + } + + bytes memory buffer = new bytes(digits); + + while (value != 0) { + digits -= 1; + buffer[digits] = bytes1(uint8(48 + uint256(value % 10))); + value /= 10; + } + + return string(buffer); + } +} diff --git a/proposals/hooks/MarketCreationHook.sol b/proposals/hooks/MarketCreationHook.sol index b874db235..be66bd315 100644 --- a/proposals/hooks/MarketCreationHook.sol +++ b/proposals/hooks/MarketCreationHook.sol @@ -7,7 +7,7 @@ import {MToken} from "@protocol/MToken.sol"; import {Comptroller} from "@protocol/Comptroller.sol"; import {ProposalAction} from "@proposals/proposalTypes/IProposal.sol"; -contract MarketCreationHook { +abstract contract MarketCreationHook { /// private so that contracts that inherit cannot write to functionDetectors bytes4 private constant detector = Comptroller._supportMarket.selector; @@ -31,7 +31,12 @@ contract MarketCreationHook { /// @notice comptroller address as specified by an mToken being created address private comptroller; - function _verifyActionsPreRun(ProposalAction[] memory proposal) internal { + /// @notice Verify market creation actions in proposals + /// @dev Called by inheriting contracts to validate market listing pattern + /// @param proposal Array of proposal actions to validate + function _verifyMarketCreationActions( + ProposalAction[] memory proposal + ) internal { address[] memory targets = new address[](proposal.length); uint256[] memory values = new uint256[](proposal.length); bytes[] memory datas = new bytes[](proposal.length); @@ -197,18 +202,13 @@ contract MarketCreationHook { result = address(uint160(uint256(rawBytes))); } - /// @notice function to grab the first 4 bytes of calldata payload + /// @notice Extract the first 4 bytes (function selector) from calldata + /// @dev This function must be implemented by inheriting contracts + /// @param toSlice The bytes to extract from + /// @return functionSignature The extracted function selector function bytesToBytes4( bytes memory toSlice - ) public pure returns (bytes4 functionSignature) { - if (toSlice.length < 4) { - return bytes4(0); - } - - assembly { - functionSignature := mload(add(toSlice, 0x20)) - } - } + ) public pure virtual returns (bytes4 functionSignature); /// Credit ethereum stackexchange https://ethereum.stackexchange.com/a/58341 function toString(bytes memory data) public pure returns (string memory) { diff --git a/proposals/proposalTypes/HybridProposal.sol b/proposals/proposalTypes/HybridProposal.sol index 769f46f0a..d5131a5a2 100644 --- a/proposals/proposalTypes/HybridProposal.sol +++ b/proposals/proposalTypes/HybridProposal.sol @@ -17,6 +17,7 @@ import {ProposalActions} from "@proposals/utils/ProposalActions.sol"; import {ProposalChecker} from "@proposals/utils/ProposalChecker.sol"; import {ITemporalGovernor} from "@protocol/governance/TemporalGovernor.sol"; import {MarketCreationHook} from "@proposals/hooks/MarketCreationHook.sol"; +import {BridgeValidationHook} from "@proposals/hooks/BridgeValidationHook.sol"; import {ProposalAction, ActionType} from "@proposals/proposalTypes/IProposal.sol"; import {AllChainAddresses as Addresses} from "@proposals/Addresses.sol"; import {MultichainGovernor, IMultichainGovernor} from "@protocol/governance/multichain/MultichainGovernor.sol"; @@ -31,7 +32,8 @@ import {MultichainGovernor, IMultichainGovernor} from "@protocol/governance/mult abstract contract HybridProposal is Proposal, ProposalChecker, - MarketCreationHook + MarketCreationHook, + BridgeValidationHook { using Strings for string; using Address for address; @@ -44,6 +46,38 @@ abstract contract HybridProposal is /// @notice instant finality on moonbeam https://book.wormhole.com/wormhole/3_coreLayerContracts.html?highlight=consiste#consistency-levels uint8 public constant consistencyLevel = 200; + /// @notice Verify all proposal actions before execution + /// @dev Calls both market creation and bridge validation hooks + /// @param proposal Array of proposal actions to validate + function _verifyActionsPreRun(ProposalAction[] memory proposal) internal { + // Validate market creation actions + _verifyMarketCreationActions(proposal); + + // Validate bridge actions + _verifyBridgeActions(proposal); + } + + /// @notice Extract the first 4 bytes (function selector) from calldata + /// @dev Provides single implementation for both hooks to avoid duplication + /// @param toSlice The bytes to extract from + /// @return functionSignature The extracted function selector + function bytesToBytes4( + bytes memory toSlice + ) + public + pure + override(MarketCreationHook, BridgeValidationHook) + returns (bytes4 functionSignature) + { + if (toSlice.length < 4) { + return bytes4(0); + } + + assembly { + functionSignature := mload(add(toSlice, 0x20)) + } + } + /// @notice actions to run against contracts ProposalAction[] public actions; diff --git a/test/integration/BridgeValidationHookIntegration.t.sol b/test/integration/BridgeValidationHookIntegration.t.sol new file mode 100644 index 000000000..264270723 --- /dev/null +++ b/test/integration/BridgeValidationHookIntegration.t.sol @@ -0,0 +1,600 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity 0.8.19; + +import "@forge-std/Test.sol"; + +import {Configs} from "@proposals/Configs.sol"; +import {xWELLRouter} from "@protocol/xWELL/xWELLRouter.sol"; +import {ProposalActions} from "@proposals/utils/ProposalActions.sol"; +import {WormholeBridgeAdapter} from "@protocol/xWELL/WormholeBridgeAdapter.sol"; +import {HybridProposal, ActionType} from "@proposals/proposalTypes/HybridProposal.sol"; +import {ProposalAction} from "@proposals/proposalTypes/IProposal.sol"; +import {AllChainAddresses as Addresses} from "@proposals/Addresses.sol"; +import {ChainIds, MOONBEAM_FORK_ID, BASE_WORMHOLE_CHAIN_ID} from "@utils/ChainIds.sol"; + +/// @notice Integration test for BridgeValidationHook using real forked contracts +contract BridgeValidationHookIntegrationTest is Test { + using ChainIds for uint256; + + Addresses public addresses; + xWELLRouter public router; + WormholeBridgeAdapter public wormholeAdapter; + uint256 public actualBridgeCost; + + function setUp() public { + // Create Moonbeam fork + vm.createSelectFork(vm.envString("MOONBEAM_RPC_URL")); + + // Initialize addresses + addresses = new Addresses(); + + // Get real deployed contracts + router = xWELLRouter(addresses.getAddress("xWELL_ROUTER")); + wormholeAdapter = WormholeBridgeAdapter( + addresses.getAddress("WORMHOLE_BRIDGE_ADAPTER_PROXY") + ); + + // Query actual bridge cost from the real router + actualBridgeCost = router.bridgeCost(BASE_WORMHOLE_CHAIN_ID); + + // Ensure we have a valid bridge cost + require(actualBridgeCost > 0, "Bridge cost should be greater than 0"); + } + + /// ============ SUCCESS TESTS ============ + + function testValidBridgeAtMinimum() public { + ValidBridgeMinimumProposal proposal = new ValidBridgeMinimumProposal( + actualBridgeCost + ); + + // This should pass validation + proposal.build(addresses); + + // Verify the proposal was built correctly + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + assertEq(targets.length, 1, "Should have 1 action"); + assertEq( + values[0], + actualBridgeCost * 5, + "Value should be 5x bridge cost" + ); + } + + function testValidBridgeAtMaximum() public { + ValidBridgeMaximumProposal proposal = new ValidBridgeMaximumProposal( + actualBridgeCost + ); + + // This should pass validation + proposal.build(addresses); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + assertEq(targets.length, 1, "Should have 1 action"); + assertEq( + values[0], + actualBridgeCost * 10, + "Value should be 10x bridge cost" + ); + } + + function testValidBridgeInRange() public { + ValidBridgeProposal proposal = new ValidBridgeProposal( + actualBridgeCost + ); + + // This should pass validation + proposal.build(addresses); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + assertEq(targets.length, 1, "Should have 1 action"); + assertEq( + values[0], + actualBridgeCost * 7, + "Value should be 7x bridge cost" + ); + } + + function testMultipleBridgeCalls() public { + MultipleBridgesProposal proposal = new MultipleBridgesProposal( + actualBridgeCost + ); + + // This should pass validation + proposal.build(addresses); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + assertEq(targets.length, 3, "Should have 3 actions"); + assertEq(values[0], actualBridgeCost * 6, "First value should be 6x"); + assertEq(values[1], actualBridgeCost * 7, "Second value should be 7x"); + assertEq(values[2], actualBridgeCost * 8, "Third value should be 8x"); + } + + /// ============ FAILURE TESTS ============ + + function testFailInvalidBridgeTooLow() public { + InvalidBridgeTooLowProposal proposal = new InvalidBridgeTooLowProposal( + actualBridgeCost + ); + + // Calculate expected values for error message + uint256 minValue = actualBridgeCost * 5; + uint256 actualValue = actualBridgeCost * 4; + + // Build expected error message + string memory expectedError = string.concat( + "BridgeValidationHook: bridge value too low. Expected >= ", + vm.toString(minValue), + ", got ", + vm.toString(actualValue) + ); + + // This should fail validation with specific message + vm.expectRevert(bytes(expectedError)); + proposal.build(addresses); + } + + function testInvalidBridgeTooLowRevertMessage() public { + InvalidBridgeTooLowProposal proposal = new InvalidBridgeTooLowProposal( + actualBridgeCost + ); + + // Build the proposal + proposal.build(addresses); + + // Get the proposal actions + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + // Create ProposalAction array + ProposalAction[] memory actions = new ProposalAction[](targets.length); + for (uint256 i = 0; i < targets.length; i++) { + actions[i] = ProposalAction({ + target: targets[i], + value: values[i], + data: calldatas[i], + description: "", + actionType: ActionType.Moonbeam + }); + } + + // Calculate expected values for error message + uint256 minValue = actualBridgeCost * 5; + uint256 actualValue = actualBridgeCost * 4; + + // Build expected error message + string memory expectedError = string.concat( + "BridgeValidationHook: bridge value too low. Expected >= ", + vm.toString(minValue), + ", got ", + vm.toString(actualValue) + ); + + // Try to validate - should revert with specific message + vm.expectRevert(bytes(expectedError)); + + // This will trigger the hook validation + proposal.testVerifyBridgeActions(actions); + } + + function testFailInvalidBridgeTooHigh() public { + InvalidBridgeTooHighProposal proposal = new InvalidBridgeTooHighProposal( + actualBridgeCost + ); + + // Calculate expected values for error message + uint256 maxValue = actualBridgeCost * 10; + uint256 actualValue = actualBridgeCost * 11; + + // Build expected error message + string memory expectedError = string.concat( + "BridgeValidationHook: bridge value too high. Expected <= ", + vm.toString(maxValue), + ", got ", + vm.toString(actualValue) + ); + + // Build and expect revert during validation with specific message + vm.expectRevert(bytes(expectedError)); + proposal.build(addresses); + } + + function testInvalidBridgeTooHighRevertMessage() public { + InvalidBridgeTooHighProposal proposal = new InvalidBridgeTooHighProposal( + actualBridgeCost + ); + + // Build the proposal + proposal.build(addresses); + + // Get the proposal actions + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + ProposalAction[] memory actions = new ProposalAction[](targets.length); + for (uint256 i = 0; i < targets.length; i++) { + actions[i] = ProposalAction({ + target: targets[i], + value: values[i], + data: calldatas[i], + description: "", + actionType: ActionType.Moonbeam + }); + } + + // Calculate expected values for error message + uint256 maxValue = actualBridgeCost * 10; + uint256 actualValue = actualBridgeCost * 11; + + // Build expected error message + string memory expectedError = string.concat( + "BridgeValidationHook: bridge value too high. Expected <= ", + vm.toString(maxValue), + ", got ", + vm.toString(actualValue) + ); + + // Try to validate - should revert with specific message + vm.expectRevert(bytes(expectedError)); + + proposal.testVerifyBridgeActions(actions); + } + + /// ============ EDGE CASE TESTS ============ + + function testBoundaryConditionJustBelowMinimum() public { + // Test with 4.99x (just below minimum) + BoundaryBelowMinimumProposal proposal = new BoundaryBelowMinimumProposal( + actualBridgeCost + ); + + proposal.build(addresses); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + ProposalAction[] memory actions = new ProposalAction[](1); + actions[0] = ProposalAction({ + target: targets[0], + value: values[0], + data: calldatas[0], + description: "", + actionType: ActionType.Moonbeam + }); + + // Calculate expected values for error message + uint256 minValue = actualBridgeCost * 5; + uint256 actualValue = (actualBridgeCost * 499) / 100; // 4.99x + + // Build expected error message + string memory expectedError = string.concat( + "BridgeValidationHook: bridge value too low. Expected >= ", + vm.toString(minValue), + ", got ", + vm.toString(actualValue) + ); + + // Should fail validation with specific message + vm.expectRevert(bytes(expectedError)); + + proposal.testVerifyBridgeActions(actions); + } + + function testBoundaryConditionJustAboveMaximum() public { + // Test with 10.01x (just above maximum) + BoundaryAboveMaximumProposal proposal = new BoundaryAboveMaximumProposal( + actualBridgeCost + ); + + proposal.build(addresses); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + ProposalAction[] memory actions = new ProposalAction[](1); + actions[0] = ProposalAction({ + target: targets[0], + value: values[0], + data: calldatas[0], + description: "", + actionType: ActionType.Moonbeam + }); + + // Calculate expected values for error message + uint256 maxValue = actualBridgeCost * 10; + uint256 actualValue = (actualBridgeCost * 1001) / 100; // 10.01x + + // Build expected error message + string memory expectedError = string.concat( + "BridgeValidationHook: bridge value too high. Expected <= ", + vm.toString(maxValue), + ", got ", + vm.toString(actualValue) + ); + + // Should fail validation with specific message + vm.expectRevert(bytes(expectedError)); + + proposal.testVerifyBridgeActions(actions); + } + + /// ============ VALIDATION TESTS ============ + + function testInvalidRouterNotContract() public { + InvalidRouterNotContractProposal proposal = new InvalidRouterNotContractProposal( + actualBridgeCost + ); + + proposal.build(addresses); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + , + + ) = proposal.getProposalActionSteps(); + + ProposalAction[] memory actions = new ProposalAction[](1); + actions[0] = ProposalAction({ + target: targets[0], + value: values[0], + data: calldatas[0], + description: "", + actionType: ActionType.Moonbeam + }); + + // Should fail validation - router is not a contract + vm.expectRevert("BridgeValidationHook: router must be a contract"); + + proposal.testVerifyBridgeActions(actions); + } +} + +/// ============ TEST PROPOSAL CONTRACTS ============ + +/// @notice Base contract for test proposals with helper functions +abstract contract BaseTestProposal is Configs, HybridProposal { + using ChainIds for uint256; + using ProposalActions for *; + + /// Test constants + uint256 public constant TEST_BRIDGE_AMOUNT = 1_000_000 * 1e18; + address public constant TEST_RECIPIENT = address(0x123456); + + uint256 public bridgeCost; + + constructor(uint256 _bridgeCost) { + bridgeCost = _bridgeCost; + bytes memory proposalDescription = abi.encodePacked( + "Test proposal for BridgeValidationHook" + ); + _setProposalDescription(proposalDescription); + } + + function primaryForkId() public pure override returns (uint256) { + return MOONBEAM_FORK_ID; + } + + /// Helper function to expose _verifyBridgeActions for testing + function testVerifyBridgeActions( + ProposalAction[] memory proposal + ) external view { + _verifyBridgeActions(proposal); + } + + /// Helper to create bridge action + function createBridgeAction( + Addresses addresses, + uint256 value, + uint256 amount + ) internal { + _pushAction( + addresses.getAddress("xWELL_ROUTER"), + value, + abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + TEST_RECIPIENT, // recipient + amount, // amount to bridge + BASE_WORMHOLE_CHAIN_ID // destination chain + ), + "Bridge WELL to Base via xWELL Router", + ActionType.Moonbeam + ); + } + + function run(Addresses, address) public pure override {} + + function validate(Addresses, address) public pure override {} +} + +/// @notice Valid proposal with bridge value at minimum (5x) +contract ValidBridgeMinimumProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "VALID_BRIDGE_MINIMUM"; + + function build(Addresses addresses) public override { + createBridgeAction( + addresses, + bridgeCost * 5, // 5x bridge cost (minimum) + TEST_BRIDGE_AMOUNT + ); + } +} + +/// @notice Valid proposal with bridge value at maximum (10x) +contract ValidBridgeMaximumProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "VALID_BRIDGE_MAXIMUM"; + + function build(Addresses addresses) public override { + createBridgeAction( + addresses, + bridgeCost * 10, // 10x bridge cost (maximum) + TEST_BRIDGE_AMOUNT + ); + } +} + +/// @notice Valid proposal with bridge value in range (7x) +contract ValidBridgeProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "VALID_BRIDGE_IN_RANGE"; + + function build(Addresses addresses) public override { + createBridgeAction( + addresses, + bridgeCost * 7, // 7x bridge cost (mid-range) + TEST_BRIDGE_AMOUNT + ); + } +} + +/// @notice Proposal with multiple valid bridge calls +contract MultipleBridgesProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "MULTIPLE_BRIDGES"; + + function build(Addresses addresses) public override { + createBridgeAction(addresses, bridgeCost * 6, TEST_BRIDGE_AMOUNT); + createBridgeAction(addresses, bridgeCost * 7, TEST_BRIDGE_AMOUNT * 2); + createBridgeAction(addresses, bridgeCost * 8, TEST_BRIDGE_AMOUNT * 3); + } +} + +/// @notice Invalid proposal - bridge value too low (4x) +contract InvalidBridgeTooLowProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "INVALID_BRIDGE_TOO_LOW"; + + function build(Addresses addresses) public override { + createBridgeAction( + addresses, + bridgeCost * 4, // 4x bridge cost (below minimum of 5x) + TEST_BRIDGE_AMOUNT + ); + } +} + +/// @notice Invalid proposal - bridge value too high (11x) +contract InvalidBridgeTooHighProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "INVALID_BRIDGE_TOO_HIGH"; + + function build(Addresses addresses) public override { + createBridgeAction( + addresses, + bridgeCost * 11, // 11x bridge cost (above maximum of 10x) + TEST_BRIDGE_AMOUNT + ); + } +} + +/// @notice Boundary test - just below minimum (4.99x) +contract BoundaryBelowMinimumProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "BOUNDARY_BELOW_MINIMUM"; + + function build(Addresses addresses) public override { + createBridgeAction( + addresses, + (bridgeCost * 499) / 100, // 4.99x bridge cost + TEST_BRIDGE_AMOUNT + ); + } +} + +/// @notice Boundary test - just above maximum (10.01x) +contract BoundaryAboveMaximumProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "BOUNDARY_ABOVE_MAXIMUM"; + + function build(Addresses addresses) public override { + createBridgeAction( + addresses, + (bridgeCost * 1001) / 100, // 10.01x bridge cost + TEST_BRIDGE_AMOUNT + ); + } +} + +/// @notice Invalid proposal - router is not a contract (EOA address) +contract InvalidRouterNotContractProposal is BaseTestProposal { + constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {} + + string public constant override name = "INVALID_ROUTER_NOT_CONTRACT"; + + function build(Addresses) public override { + // Use an EOA address instead of a contract + address eoaRouter = address(0x1234567890123456789012345678901234567890); + + _pushAction( + eoaRouter, // EOA instead of contract + bridgeCost * 7, + abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + TEST_RECIPIENT, + TEST_BRIDGE_AMOUNT, + BASE_WORMHOLE_CHAIN_ID + ), + "Bridge WELL to Base via invalid router", + ActionType.Moonbeam + ); + } +} diff --git a/test/unit/BridgeValidationHookUnit.t.sol b/test/unit/BridgeValidationHookUnit.t.sol new file mode 100644 index 000000000..00f2a6014 --- /dev/null +++ b/test/unit/BridgeValidationHookUnit.t.sol @@ -0,0 +1,281 @@ +//SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity 0.8.19; + +import "@forge-std/Test.sol"; + +import {BridgeValidationHook} from "@proposals/hooks/BridgeValidationHook.sol"; +import {ProposalAction} from "@proposals/proposalTypes/IProposal.sol"; +import {ActionType} from "@proposals/proposalTypes/HybridProposal.sol"; + +/// @notice Unit tests for BridgeValidationHook calldata extraction +contract BridgeValidationHookUnitTest is Test { + TestHook hook; + + function setUp() public { + hook = new TestHook(); + } + + /// @notice Test that uint16 extraction works correctly for various chain IDs + function testExtractUint16FromCalldata() public { + // Test with BASE_WORMHOLE_CHAIN_ID = 30 (0x001e) + bytes memory calldata1 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x123), + 1000000 * 1e18, + uint16(30) // BASE chain ID + ); + + uint16 extracted1 = hook.extractUint16FromCalldata(calldata1); + assertEq(extracted1, 30, "Should extract chain ID 30"); + + // Test with MOONBEAM_WORMHOLE_CHAIN_ID = 16 (0x0010) + bytes memory calldata2 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x456), + 2000000 * 1e18, + uint16(16) // Moonbeam chain ID + ); + + uint16 extracted2 = hook.extractUint16FromCalldata(calldata2); + assertEq(extracted2, 16, "Should extract chain ID 16"); + + // Test with ETHEREUM_WORMHOLE_CHAIN_ID = 2 (0x0002) + bytes memory calldata3 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x789), + 3000000 * 1e18, + uint16(2) // Ethereum chain ID + ); + + uint16 extracted3 = hook.extractUint16FromCalldata(calldata3); + assertEq(extracted3, 2, "Should extract chain ID 2"); + + // Test with max uint16 value + bytes memory calldata4 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0xabc), + 4000000 * 1e18, + type(uint16).max // 65535 + ); + + uint16 extracted4 = hook.extractUint16FromCalldata(calldata4); + assertEq(extracted4, type(uint16).max, "Should extract max uint16"); + + // Test with zero + bytes memory calldata5 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0xdef), + 5000000 * 1e18, + uint16(0) + ); + + uint16 extracted5 = hook.extractUint16FromCalldata(calldata5); + assertEq(extracted5, 0, "Should extract zero"); + } + + /// @notice Test calldata length validation + function testExtractUint16RevertsOnShortCalldata() public { + // Create calldata that's too short (only 50 bytes) + bytes memory shortCalldata = new bytes(50); + + vm.expectRevert("BridgeValidationHook: invalid calldata length"); + hook.extractUint16FromCalldata(shortCalldata); + } + + /// @notice Test with exact minimum length (100 bytes) + function testExtractUint16WithExactLength() public { + bytes memory calldata1 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x123), + 1000 * 1e18, + uint16(30) + ); + + // Should be exactly 100 bytes: 4 (selector) + 32 (address) + 32 (uint256) + 32 (uint16) + assertEq(calldata1.length, 100, "Calldata should be 100 bytes"); + + uint16 extracted = hook.extractUint16FromCalldata(calldata1); + assertEq( + extracted, + 30, + "Should extract chain ID from 100-byte calldata" + ); + } + + /// @notice Test bytesToBytes4 function + function testBytesToBytes4() public { + bytes memory data = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x123), + 1000 * 1e18, + uint16(30) + ); + + bytes4 selector = hook.bytesToBytes4(data); + bytes4 expectedSelector = bytes4( + keccak256("bridgeToRecipient(address,uint256,uint16)") + ); + + assertEq(selector, expectedSelector, "Should extract correct selector"); + } + + /// @notice Test bytesToBytes4 with short data + function testBytesToBytes4WithShortData() public { + bytes memory shortData = new bytes(2); + bytes4 selector = hook.bytesToBytes4(shortData); + assertEq(selector, bytes4(0), "Should return zero for short data"); + } + + /// @notice Fuzz test for extractUint16 with random values + function testFuzzExtractUint16(uint16 chainId) public { + bytes memory calldata1 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x123), + 1000000 * 1e18, + chainId + ); + + uint16 extracted = hook.extractUint16FromCalldata(calldata1); + assertEq(extracted, chainId, "Fuzz: Should extract any uint16 value"); + } + + /// @notice Test with different addresses and amounts (shouldn't affect uint16 extraction) + function testExtractUint16WithVariedParameters( + address recipient, + uint256 amount, + uint16 chainId + ) public { + vm.assume(amount < type(uint128).max); // Keep amounts reasonable + + bytes memory calldata1 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + recipient, + amount, + chainId + ); + + uint16 extracted = hook.extractUint16FromCalldata(calldata1); + assertEq( + extracted, + chainId, + "Chain ID extraction should be independent of other params" + ); + } + + /// @notice Verify ABI encoding behavior and document the extraction logic + function testABIEncodingBehavior() public view { + // Create calldata with known chain ID + bytes memory calldata1 = abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x1234567890123456789012345678901234567890), + uint256(1000000000000000000000000), // 1M tokens + uint16(30) // BASE_WORMHOLE_CHAIN_ID = 0x001e + ); + + // Calldata structure (100 bytes total): + // Bytes 0-3: Function selector (4 bytes) + // Bytes 4-35: Address parameter (32 bytes, left-padded) + // Bytes 36-67: uint256 parameter (32 bytes) + // Bytes 68-99: uint16 parameter (32 bytes, LEFT-padded with zeros) + // + // For uint16(30) = 0x001e: + // ABI encoding stores it as: 0x000000000000000000000000000000000000000000000000000000000000001e + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ zeros ^^^^^^ ^^value^^ + // (left-padded) (rightmost 2 bytes) + // + // Therefore, extracting the RIGHTMOST 2 bytes gives us the correct value. + + assertEq(calldata1.length, 100, "Total calldata should be 100 bytes"); + + // The current implementation correctly extracts from the rightmost position + console.log( + "ABI encoding is LEFT-padded (zeros on left, value on right)" + ); + console.log( + "Current extraction: uint16(uint256(rawBytes)) takes RIGHTMOST 2 bytes" + ); + console.log("This is CORRECT for left-padded ABI encoding"); + } + + /// @notice Test zero bridge cost validation + function testZeroBridgeCostValidation() public { + // Create a mock router that returns zero bridge cost + MockZeroCostRouter mockRouter = new MockZeroCostRouter(); + + // Create proposal action with the mock router + ProposalAction[] memory actions = new ProposalAction[](1); + actions[0] = ProposalAction({ + target: address(mockRouter), + value: 1 ether, + data: abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x123), + 1000 * 1e18, + uint16(30) + ), + description: "Test bridge with zero cost router", + actionType: ActionType.Moonbeam + }); + + // Should revert with zero bridge cost error + vm.expectRevert( + "BridgeValidationHook: bridge cost must be greater than zero" + ); + hook.verifyBridgeActions(actions); + } + + /// @notice Test router validation with EOA + function testRouterMustBeContract() public { + // Use an EOA address + address eoaRouter = address(0x1234567890123456789012345678901234567890); + + // Create proposal action with EOA router + ProposalAction[] memory actions = new ProposalAction[](1); + actions[0] = ProposalAction({ + target: eoaRouter, + value: 1 ether, + data: abi.encodeWithSignature( + "bridgeToRecipient(address,uint256,uint16)", + address(0x123), + 1000 * 1e18, + uint16(30) + ), + description: "Test bridge with EOA router", + actionType: ActionType.Moonbeam + }); + + // Should revert with router validation error + vm.expectRevert("BridgeValidationHook: router must be a contract"); + hook.verifyBridgeActions(actions); + } +} + +/// @notice Test harness to expose internal functions +contract TestHook is BridgeValidationHook { + // Expose _verifyBridgeActions for testing + function verifyBridgeActions( + ProposalAction[] memory proposal + ) external view { + _verifyBridgeActions(proposal); + } + + // Implement bytesToBytes4 required by abstract parent + function bytesToBytes4( + bytes memory toSlice + ) public pure override returns (bytes4 functionSignature) { + if (toSlice.length < 4) { + return bytes4(0); + } + + assembly { + functionSignature := mload(add(toSlice, 0x20)) + } + } +} + +/// @notice Mock router that returns zero bridge cost +contract MockZeroCostRouter { + function bridgeCost(uint16) external pure returns (uint256) { + return 0; + } +}