Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
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
2 changes: 1 addition & 1 deletion .github/workflows/moonbeam-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ jobs:
retry_wait_seconds: 60
timeout_minutes: 20
max_attempts: 3
command: time forge test --match-contract MoonbeamTest --fork-url moonbeam -vvv
command: time forge test --match-contract "MoonbeamTest|IntegrationTest" --fork-url moonbeam -vvv
167 changes: 167 additions & 0 deletions proposals/hooks/BridgeValidationHook.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.19;
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing SPDX license identifier. Add // SPDX-License-Identifier: GPL-3.0-or-later at the top of the file to match the project's license convention.

Copilot uses AI. Check for mistakes.

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
Comment on lines +121 to +123
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect offset calculation. The comment states the total is 100 bytes, but 0x20 + 0x44 = 0x64 which equals 100 in decimal. However, the code adds these offsets to input, which already points to the data. The correct offset from the start of the data should be 0x44 (68 bytes) to skip selector (4 bytes) + first two params (64 bytes). The addition of 0x20 accounts for the length prefix, so the final pointer is at position 100 from the start of the memory location, but only 68 bytes into the actual calldata. This works correctly but the inline comment is confusing as it suggests 0x20 + 0x44 = 100 in hex.

Suggested change
// 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
// To reach the third parameter (uint16 wormholeChainId), skip:
// - 0x20 (32 bytes) for the length prefix
// - 0x04 (4 bytes) for the function selector
// - 0x40 (64 bytes) for the first two parameters
// Total offset: 0x20 + 0x04 + 0x40 = 0x64 (100 decimal) bytes from the start of the bytes array in memory.
// Since input points to the start of the bytes array, add 0x20 for the length prefix, then 0x44 (68 decimal) to reach the third parameter.
let dataPointer := add(add(input, 0x20), 0x44) // 0x20 (32) + 0x44 (68) = 100 (decimal)

Copilot uses AI. Check for mistakes.
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);
}
}
24 changes: 12 additions & 12 deletions proposals/hooks/MarketCreationHook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
36 changes: 35 additions & 1 deletion proposals/proposalTypes/HybridProposal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand All @@ -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;

Expand Down
Loading
Loading