Skip to content

Commit 80f4dd3

Browse files
anajuliabitclaude
andcommitted
optimize bridge validation and add safety checks
- Remove intermediate array allocations for gas savings - Add router contract validation using address.code.length - Add zero bridge cost validation - Add tests for router and zero cost validation - Simplify code by removing _verifyBridgeCalls function 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 7fe82b4 commit 80f4dd3

File tree

3 files changed

+362
-31
lines changed

3 files changed

+362
-31
lines changed

proposals/hooks/BridgeValidationHook.sol

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,51 +24,32 @@ contract BridgeValidationHook {
2424
function _verifyBridgeActions(
2525
ProposalAction[] memory proposal
2626
) internal view {
27-
address[] memory targets = new address[](proposal.length);
28-
uint256[] memory values = new uint256[](proposal.length);
29-
bytes[] memory datas = new bytes[](proposal.length);
30-
31-
for (uint256 i = 0; i < proposal.length; i++) {
32-
targets[i] = proposal[i].target;
33-
values[i] = proposal[i].value;
34-
datas[i] = proposal[i].data;
35-
}
36-
37-
_verifyBridgeCalls(targets, values, datas);
38-
}
39-
40-
/// @notice Internal function to verify bridge calls
41-
/// @param targets Array of target addresses for each action
42-
/// @param values Array of native token values for each action
43-
/// @param datas Array of calldata for each action
44-
function _verifyBridgeCalls(
45-
address[] memory targets,
46-
uint256[] memory values,
47-
bytes[] memory datas
48-
) internal view {
49-
uint256 proposalLength = targets.length;
27+
uint256 proposalLength = proposal.length;
5028

5129
for (uint256 i = 0; i < proposalLength; i++) {
52-
bytes4 selector = bytesToBytes4(datas[i]);
30+
bytes4 selector = bytesToBytes4(proposal[i].data);
5331

5432
// Check if this action is a bridgeToRecipient call
5533
if (selector == BRIDGE_TO_RECIPIENT_SELECTOR) {
56-
address router = targets[i];
57-
uint256 actionValue = values[i];
34+
address router = proposal[i].target;
35+
uint256 actionValue = proposal[i].value;
36+
37+
// Validate router is a contract
38+
_validateRouterIsContract(router);
5839

5940
// Extract wormholeChainId from calldata
6041
// Calldata structure:
6142
// 0-3: function selector
6243
// 4-35: address to (32 bytes)
6344
// 36-67: uint256 amount (32 bytes)
6445
// 68-99: uint16 wormholeChainId (32 bytes, right-padded)
65-
uint16 wormholeChainId = extractUint16FromCalldata(datas[i]);
66-
67-
// Get the actual bridge cost from the router
68-
uint256 bridgeCost = xWELLRouter(router).bridgeCost(
69-
wormholeChainId
46+
uint16 wormholeChainId = extractUint16FromCalldata(
47+
proposal[i].data
7048
);
7149

50+
// Get the actual bridge cost from the router with validation
51+
uint256 bridgeCost = _getBridgeCost(router, wormholeChainId);
52+
7253
// Validate that action value is between 5x and 10x the bridge cost
7354
uint256 minValue = bridgeCost * MIN_BRIDGE_COST_MULTIPLIER;
7455
uint256 maxValue = bridgeCost * MAX_BRIDGE_COST_MULTIPLIER;
@@ -96,6 +77,31 @@ contract BridgeValidationHook {
9677
}
9778
}
9879

80+
/// @notice Validates that the router address is a contract
81+
/// @param router The router address to validate
82+
function _validateRouterIsContract(address router) private view {
83+
require(
84+
router.code.length > 0,
85+
"BridgeValidationHook: router must be a contract"
86+
);
87+
}
88+
89+
/// @notice Gets bridge cost from router and validates it's non-zero
90+
/// @param router The router contract address
91+
/// @param wormholeChainId The destination chain ID
92+
/// @return bridgeCost The validated bridge cost
93+
function _getBridgeCost(
94+
address router,
95+
uint16 wormholeChainId
96+
) private view returns (uint256 bridgeCost) {
97+
bridgeCost = xWELLRouter(router).bridgeCost(wormholeChainId);
98+
99+
require(
100+
bridgeCost > 0,
101+
"BridgeValidationHook: bridge cost must be greater than zero"
102+
);
103+
}
104+
99105
/// @notice Extract uint16 value from calldata at the third parameter position
100106
/// @param input The calldata to extract from
101107
/// @return result The extracted uint16 value

test/integration/BridgeValidationHookIntegration.t.sol

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,38 @@ contract BridgeValidationHookIntegrationTest is Test {
365365

366366
proposal.testVerifyBridgeActions(actions);
367367
}
368+
369+
/// ============ VALIDATION TESTS ============
370+
371+
function testInvalidRouterNotContract() public {
372+
InvalidRouterNotContractProposal proposal = new InvalidRouterNotContractProposal(
373+
actualBridgeCost
374+
);
375+
376+
proposal.build(addresses);
377+
378+
(
379+
address[] memory targets,
380+
uint256[] memory values,
381+
bytes[] memory calldatas,
382+
,
383+
384+
) = proposal.getProposalActionSteps();
385+
386+
ProposalAction[] memory actions = new ProposalAction[](1);
387+
actions[0] = ProposalAction({
388+
target: targets[0],
389+
value: values[0],
390+
data: calldatas[0],
391+
description: "",
392+
actionType: ActionType.Moonbeam
393+
});
394+
395+
// Should fail validation - router is not a contract
396+
vm.expectRevert("BridgeValidationHook: router must be a contract");
397+
398+
proposal.testVerifyBridgeActions(actions);
399+
}
368400
}
369401

370402
/// ============ TEST PROPOSAL CONTRACTS ============
@@ -541,3 +573,28 @@ contract BoundaryAboveMaximumProposal is BaseTestProposal {
541573
);
542574
}
543575
}
576+
577+
/// @notice Invalid proposal - router is not a contract (EOA address)
578+
contract InvalidRouterNotContractProposal is BaseTestProposal {
579+
constructor(uint256 _bridgeCost) BaseTestProposal(_bridgeCost) {}
580+
581+
string public constant override name = "INVALID_ROUTER_NOT_CONTRACT";
582+
583+
function build(Addresses addresses) public override {
584+
// Use an EOA address instead of a contract
585+
address eoaRouter = address(0x1234567890123456789012345678901234567890);
586+
587+
_pushAction(
588+
eoaRouter, // EOA instead of contract
589+
bridgeCost * 7,
590+
abi.encodeWithSignature(
591+
"bridgeToRecipient(address,uint256,uint16)",
592+
TEST_RECIPIENT,
593+
TEST_BRIDGE_AMOUNT,
594+
BASE_WORMHOLE_CHAIN_ID
595+
),
596+
"Bridge WELL to Base via invalid router",
597+
ActionType.Moonbeam
598+
);
599+
}
600+
}

0 commit comments

Comments
 (0)