Skip to content

Commit 70350c6

Browse files
tbwebb22grasphoper
tbwebb22
and
grasphoper
authored
add executeExternalCall function (#1171)
* add executeCustomActions function Signed-off-by: Taylor Webb <tbwebb22@gmail.com> * clean up Signed-off-by: Taylor Webb <tbwebb22@gmail.com> * remove nonreentrant modifier from executeCustomActions Signed-off-by: Taylor Webb <tbwebb22@gmail.com> * add tests Signed-off-by: Taylor Webb <tbwebb22@gmail.com> * comment Signed-off-by: Taylor Webb <tbwebb22@gmail.com> * return data from custom action calls Signed-off-by: Taylor Webb <tbwebb22@gmail.com> * remove delegatecall to self Signed-off-by: Taylor Webb <tbwebb22@gmail.com> * rename function to executeExternalCall Signed-off-by: Taylor Webb <tbwebb22@gmail.com> * emit AdminExternalCallExecuted event (#1180) Signed-off-by: Ihor Farion <ihor@umaproject.org> --------- Signed-off-by: Taylor Webb <tbwebb22@gmail.com> Signed-off-by: Ihor Farion <ihor@umaproject.org> Co-authored-by: Ihor Farion <65650773+grasphoper@users.noreply.github.com>
1 parent f6bd400 commit 70350c6

File tree

3 files changed

+208
-0
lines changed

3 files changed

+208
-0
lines changed

‎contracts/SpokePool.sol‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,9 @@ abstract contract SpokePool is
202202
event PausedFills(bool isPaused);
203203
event SetOFTMessenger(address indexed token, address indexed messenger);
204204

205+
/// @notice Emitted when the call to external contract is executed, triggered by an admin action
206+
event AdminExternalCallExecuted(address indexed target, bytes data);
207+
205208
error OFTTokenMismatch();
206209
/// @notice Thrown when the native fee sent by the caller is insufficient to cover the OFT transfer.
207210
error OFTFeeUnderpaid();
@@ -370,6 +373,27 @@ abstract contract SpokePool is
370373
_setOftMessenger(token, messenger);
371374
}
372375

376+
/**
377+
* @notice Execute an external call to a target contract.
378+
* @param message The message containing the target address and calldata to execute.
379+
* @return returnData The return data from the executed call.
380+
*/
381+
function executeExternalCall(
382+
bytes calldata message
383+
) external onlyAdmin nonReentrant returns (bytes memory returnData) {
384+
(address target, bytes memory data) = abi.decode(message, (address, bytes));
385+
386+
if (target == address(0)) revert ZeroAddressTarget();
387+
if (data.length < 4) revert MessageTooShort(); // need at least a selector
388+
389+
// external call to target
390+
bool success;
391+
(success, returnData) = target.call(data);
392+
393+
if (!success) revert ExternalCallExecutionFailed();
394+
emit AdminExternalCallExecuted(target, data);
395+
}
396+
373397
/**************************************
374398
* LEGACY DEPOSITOR FUNCTIONS *
375399
**************************************/

‎contracts/interfaces/SpokePoolInterface.sol‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,7 @@ interface SpokePoolInterface {
8585
error InvalidWithdrawalRecipient();
8686
error DepositsArePaused();
8787
error FillsArePaused();
88+
error ExternalCallExecutionFailed();
89+
error MessageTooShort();
90+
error ZeroAddressTarget();
8891
}

‎test/evm/foundry/local/SpokePoolExternalCall.t.sol‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
pragma solidity ^0.8.0;
3+
4+
import { Test } from "forge-std/Test.sol";
5+
import { MockSpokePool } from "../../../../contracts/test/MockSpokePool.sol";
6+
import { WETH9 } from "../../../../contracts/external/WETH9.sol";
7+
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
8+
import { SpokePoolInterface } from "../../../../contracts/interfaces/SpokePoolInterface.sol";
9+
import { V3SpokePoolInterface } from "../../../../contracts/interfaces/V3SpokePoolInterface.sol";
10+
11+
contract SpokePoolExternalCallTest is Test {
12+
MockSpokePool spokePool;
13+
WETH9 mockWETH;
14+
15+
address owner;
16+
address anon;
17+
18+
function setUp() public {
19+
mockWETH = new WETH9();
20+
21+
owner = vm.addr(1);
22+
anon = vm.addr(2);
23+
24+
vm.startPrank(owner);
25+
ERC1967Proxy proxy = new ERC1967Proxy(
26+
address(new MockSpokePool(address(mockWETH))),
27+
abi.encodeCall(MockSpokePool.initialize, (0, owner, address(420)))
28+
);
29+
spokePool = MockSpokePool(payable(proxy));
30+
vm.stopPrank();
31+
}
32+
33+
// =============== SUCCESS CASES ===============
34+
35+
function testExecuteExternalCall_ExternalCall() public {
36+
// Test calls approve on WETH to test external call
37+
// Initial allowance should be 0
38+
assertEq(mockWETH.allowance(address(spokePool), anon), 0);
39+
40+
// Encode approve(address,uint256) call to WETH
41+
uint256 approvalAmount = 100;
42+
bytes memory data = abi.encodeWithSignature("approve(address,uint256)", anon, approvalAmount);
43+
bytes memory message = abi.encode(address(mockWETH), data);
44+
45+
// Execute external call as owner
46+
vm.prank(owner);
47+
spokePool.executeExternalCall(message);
48+
49+
// Verify approval was set
50+
assertEq(mockWETH.allowance(address(spokePool), anon), approvalAmount);
51+
}
52+
53+
function testExecuteExternalCall_ReturnsDataFromExternalCall() public {
54+
// Test that return data is captured from external calls
55+
// approve() returns a bool, so we should get that back
56+
57+
uint256 approvalAmount = 100;
58+
bytes memory data = abi.encodeWithSignature("approve(address,uint256)", anon, approvalAmount);
59+
bytes memory message = abi.encode(address(mockWETH), data);
60+
61+
vm.prank(owner);
62+
bytes memory returnData = spokePool.executeExternalCall(message);
63+
64+
// Decode the bool return value
65+
bool approveSuccess = abi.decode(returnData, (bool));
66+
assertTrue(approveSuccess);
67+
}
68+
69+
// =============== FAILURE CASES ===============
70+
71+
function testExecuteExternalCall_RevertsWhenNotAdmin() public {
72+
// Try to execute external call as non-owner
73+
bytes memory data = abi.encodeWithSignature("approve(address,uint256)", anon, 100);
74+
bytes memory message = abi.encode(address(mockWETH), data);
75+
76+
vm.prank(anon);
77+
vm.expectRevert();
78+
spokePool.executeExternalCall(message);
79+
}
80+
81+
function testExecuteExternalCall_RevertsOnZeroAddress() public {
82+
vm.prank(owner);
83+
84+
// Target is zero address, should revert
85+
bytes memory data = abi.encodeWithSignature("approve(address,uint256)", anon, 100);
86+
bytes memory message = abi.encode(address(0), data);
87+
88+
vm.expectRevert(SpokePoolInterface.ZeroAddressTarget.selector);
89+
spokePool.executeExternalCall(message);
90+
}
91+
92+
function testExecuteExternalCall_RevertsOnMessageTooShort() public {
93+
vm.prank(owner);
94+
95+
// Message with less than 4 bytes (no valid selector)
96+
bytes memory data = hex"1234"; // Only 2 bytes
97+
bytes memory message = abi.encode(address(spokePool), data);
98+
99+
vm.expectRevert(SpokePoolInterface.MessageTooShort.selector);
100+
spokePool.executeExternalCall(message);
101+
}
102+
103+
function testExecuteExternalCall_RevertsOnMessageEmpty() public {
104+
vm.prank(owner);
105+
106+
bytes memory data = hex"";
107+
bytes memory message = abi.encode(address(spokePool), data);
108+
109+
vm.expectRevert(SpokePoolInterface.MessageTooShort.selector);
110+
spokePool.executeExternalCall(message);
111+
}
112+
113+
function testExecuteExternalCall_RevertsOnExternalCallNonExistentFunction() public {
114+
vm.prank(owner);
115+
116+
// Try to call a non-existent function
117+
bytes memory data = abi.encodeWithSignature("nonExistentFunction()");
118+
bytes memory message = abi.encode(address(spokePool), data);
119+
120+
vm.expectRevert(SpokePoolInterface.ExternalCallExecutionFailed.selector);
121+
spokePool.executeExternalCall(message);
122+
}
123+
124+
function testExecuteExternalCall_RevertsOnExternalCallFailure() public {
125+
// Verify spokePool has no WETH balance
126+
assertEq(mockWETH.balanceOf(address(spokePool)), 0);
127+
128+
// Try to transfer WETH that spokePool doesn't have - should revert
129+
uint256 transferAmount = 100;
130+
bytes memory data = abi.encodeWithSignature("transfer(address,uint256)", anon, transferAmount);
131+
bytes memory message = abi.encode(address(mockWETH), data);
132+
133+
vm.prank(owner);
134+
vm.expectRevert(SpokePoolInterface.ExternalCallExecutionFailed.selector);
135+
spokePool.executeExternalCall(message);
136+
}
137+
138+
function testExecuteExternalCall_RevertsOnInvalidFunctionSelector() public {
139+
vm.prank(owner);
140+
141+
// 4 bytes but invalid selector
142+
bytes memory data = hex"12345678"; // 4 bytes but doesn't match any function
143+
bytes memory message = abi.encode(address(spokePool), data);
144+
145+
vm.expectRevert(SpokePoolInterface.ExternalCallExecutionFailed.selector);
146+
spokePool.executeExternalCall(message);
147+
}
148+
149+
function testExecuteExternalCall_RevertsOnReentrancy() public {
150+
// Test that executeExternalCall cannot be used to reenter another nonReentrant function
151+
// Create a fillRelay call (which has nonReentrant modifier)
152+
V3SpokePoolInterface.V3RelayData memory relayData = V3SpokePoolInterface.V3RelayData({
153+
depositor: bytes32(uint256(uint160(anon))),
154+
recipient: bytes32(uint256(uint160(anon))),
155+
exclusiveRelayer: bytes32(0),
156+
inputToken: bytes32(uint256(uint160(address(mockWETH)))),
157+
outputToken: bytes32(uint256(uint160(address(mockWETH)))),
158+
inputAmount: 100,
159+
outputAmount: 100,
160+
originChainId: 1,
161+
depositId: 1,
162+
fillDeadline: uint32(block.timestamp + 1000),
163+
exclusivityDeadline: 0,
164+
message: ""
165+
});
166+
167+
bytes memory fillRelayData = abi.encodeWithSignature(
168+
"fillRelay((bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,uint256,uint32,uint32,uint32,bytes),uint256,bytes32)",
169+
relayData,
170+
block.chainid,
171+
bytes32(uint256(uint160(anon)))
172+
);
173+
bytes memory message = abi.encode(address(spokePool), fillRelayData);
174+
175+
// This should revert because fillRelay has nonReentrant modifier
176+
// and we're already inside executeExternalCall which also has nonReentrant
177+
vm.prank(owner);
178+
vm.expectRevert(); // Should revert with ReentrancyGuard error
179+
spokePool.executeExternalCall(message);
180+
}
181+
}

0 commit comments

Comments
 (0)