Skip to content

Refactor TrailsRouter to accept Multicall3 address as a constructor argument #83

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.

Sign up for GitHub

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

Merged
shunkakinoki merged 5 commits into from
Nov 27, 2025
+46 −29
Merged

Refactor TrailsRouter to accept Multicall3 address as a constructor argument #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
88d6545
Refactor TrailsRouter to accept Multicall3 address as a constructor a…
shunkakinoki Nov 27, 2025
4a2588a
Update gas snapshots and modify expectedRouterAddress to include Mult…
shunkakinoki Nov 27, 2025
be3f011
Change MULTICALL3 to immutable in TrailsRouter contract
shunkakinoki Nov 27, 2025
a0193b2
Update gas snapshots for TrailsRouter and TrailsRouterTest
shunkakinoki Nov 27, 2025
230b419
Update TrailsRouter contract to rename state variables to immutable v…
shunkakinoki Nov 27, 2025
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
20 changes: 10 additions & 10 deletions .gas-snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,20 @@ TrailsIntentEntrypointTest:testNonceIncrementsOnDeposit() (gas: 91830)
TrailsIntentEntrypointTest:testPermitAmountExcessiveWithFee() (gas: 61879)
TrailsIntentEntrypointTest:testPermitAmountInsufficientWithFee() (gas: 60889)
TrailsIntentEntrypointTest:testVersionConstant() (gas: 10506)
TrailsRouterDeploymentTest:test_DeployTrailsRouter_SameAddress() (gas: 1856117)
TrailsRouterDeploymentTest:test_DeployTrailsRouter_Success() (gas: 1846511)
TrailsRouterDeploymentTest:test_DeployedRouter_HasCorrectConfiguration() (gas: 1846307)
TrailsRouterShimDeploymentTest:test_DeployRouterShim_SameAddress() (gas: 7530847)
TrailsRouterShimDeploymentTest:test_DeployRouterShim_Success() (gas: 4873481)
TrailsRouterShimDeploymentTest:test_DeployedContract_HasCorrectConfiguration() (gas: 4873526)
TrailsRouterDeploymentTest:test_DeployTrailsRouter_SameAddress() (gas: 1864438)
TrailsRouterDeploymentTest:test_DeployTrailsRouter_Success() (gas: 1852254)
TrailsRouterDeploymentTest:test_DeployedRouter_HasCorrectConfiguration() (gas: 1852050)
TrailsRouterShimDeploymentTest:test_DeployRouterShim_SameAddress() (gas: 7613006)
TrailsRouterShimDeploymentTest:test_DeployRouterShim_Success() (gas: 4916218)
TrailsRouterShimDeploymentTest:test_DeployedContract_HasCorrectConfiguration() (gas: 4916264)
TrailsRouterShimTest:testConstructorValidation() (gas: 69336)
TrailsRouterShimTest:testForwardToRouterReturnValue() (gas: 713355)
TrailsRouterShimTest:testRouterAddressImmutable() (gas: 1391679)
TrailsRouterShimTest:test_constructor_revert_zeroRouter() (gas: 68984)
TrailsRouterShimTest:test_delegatecall_forwards_and_sets_sentinel_sstore_inactive() (gas: 1848663)
TrailsRouterShimTest:test_delegatecall_forwards_and_sets_sentinel_sstore_inactive() (gas: 1848930)
TrailsRouterShimTest:test_delegatecall_forwards_and_sets_sentinel_tstore_active() (gas: 38392)
TrailsRouterShimTest:test_delegatecall_router_revert_bubbles_as_RouterCallFailed() (gas: 82109)
TrailsRouterShimTest:test_delegatecall_sets_sentinel_with_sstore_when_no_tstore() (gas: 1830888)
TrailsRouterShimTest:test_delegatecall_sets_sentinel_with_sstore_when_no_tstore() (gas: 1831155)
TrailsRouterShimTest:test_delegatecall_sets_sentinel_with_tstore_when_supported() (gas: 20706)
TrailsRouterShimTest:test_direct_handleSequenceDelegateCall_reverts_not_delegatecall() (gas: 9840)
TrailsRouterShimTest:test_forwardToRouter_return_data_handling() (gas: 729052)
Expand All @@ -78,7 +78,7 @@ TrailsRouterShimTest:test_handleSequenceDelegateCall_with_eth_value() (gas: 3210
TrailsRouterShimTest:test_handleSequenceDelegateCall_zero_call_value() (gas: 26873)
TrailsRouterShimTest:test_sentinel_setting_with_different_op_hashes() (gas: 36405)
TrailsRouterTest:testDelegateCallWithETH() (gas: 326226)
TrailsRouterTest:testExecute_WithFailingMulticall() (gas: 450176)
TrailsRouterTest:testExecute_WithFailingMulticall() (gas: 156745)
TrailsRouterTest:testHandleSequenceDelegateCall_InjectAndCall() (gas: 72655)
TrailsRouterTest:testHandleSequenceDelegateCall_RefundAndSweep() (gas: 85388)
TrailsRouterTest:testHandleSequenceDelegateCall_Sweep() (gas: 48693)
Expand Down Expand Up @@ -133,7 +133,7 @@ TrailsRouterTest:test_pullAmountAndExecute_WithToken_ShouldTransferAndExecute()
TrailsRouterTest:test_pullAmountAndExecute_WithValidToken_ShouldTransferAndExecute() (gas: 75490)
TrailsRouterTest:test_pullAndExecute_WithETH_NoEthSent() (gas: 18454)
TrailsRouterTest:test_pullAndExecute_WithETH_ShouldTransferAndExecute() (gas: 72439)
TrailsRouterTest:test_pullAndExecute_WithFailingMulticall() (gas: 494404)
TrailsRouterTest:test_pullAndExecute_WithFailingMulticall() (gas: 202286)
TrailsRouterTest:test_pullAndExecute_WithToken_IncorrectValue() (gas: 60552)
TrailsRouterTest:test_pullAndExecute_WithValidToken_ShouldTransferFullBalanceAndExecute() (gas: 77324)
TrailsRouterTest:test_refundAndSweep_erc20_partialRefund() (gas: 110365)
Expand Down
5 changes: 3 additions & 2 deletions script/TrailsRouter.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ contract Deploy is SingletonDeployer {

function deployRouter(uint256 pk) public returns (address) {
bytes32 salt = bytes32(0);
address multicall3 = 0xcA11bde05977b3631167028862bE2a173976CA11;

// Deploy TrailsRouter
bytes memory initCode = type(TrailsRouter).creationCode;
// Deploy TrailsRouter with constructor arguments
bytes memory initCode = abi.encodePacked(type(TrailsRouter).creationCode, abi.encode(multicall3));
address router = _deployIfNotAlready("TrailsRouter", initCode, salt, pk);

return router;
Expand Down
10 changes: 9 additions & 1 deletion src/TrailsRouter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ contract TrailsRouter is IDelegatedExtension, ITrailsRouter, DelegatecallGuard,
// Immutable Variables
// -------------------------------------------------------------------------

address public immutable MULTICALL3 = 0xcA11bde05977b3631167028862bE2a173976CA11;
address public immutable MULTICALL3;

// -------------------------------------------------------------------------
// Constructor
// -------------------------------------------------------------------------

constructor(address _multicall3) {
MULTICALL3 = _multicall3;
}
Comment on lines +33 to +35
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

The constructor parameter should accept a immutable assignment instead of a mutable state variable. With immutable variables, the address is encoded in the deployed bytecode rather than stored in contract storage. This is critical for a contract primarily used via delegatecall, as it prevents storage slot conflicts with the calling contract. Consider: address public immutable MULTICALL3; constructor(address _multicall3) { MULTICALL3 = _multicall3; }

Copilot uses AI. Check for mistakes.
Comment on lines +33 to +35
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

Missing input validation: The constructor should validate that _multicall3 is not the zero address to prevent deployment with an invalid multicall address. Consider adding: if (_multicall3 == address(0)) revert InvalidMulticall3Address(); This pattern is already used in TrailsRouterShim (see TrailsRouterShim.sol:34).

Copilot uses AI. Check for mistakes.

// -------------------------------------------------------------------------
// Errors
Expand Down
28 changes: 16 additions & 12 deletions test/TrailsRouter.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ contract RevertingReceiver {
}
}

// Mock multicall that always fails (for testing storage conflicts)
contract AlwaysFailingMulticall3 {
function aggregate3(IMulticall3.Call3[] calldata) external payable {
revert("MockMulticall3: forced failure");
}

function aggregate3Value(IMulticall3.Call3Value[] calldata) external payable {
revert("MockMulticall3: forced failure");
}
}

contract MockTarget {
uint256 public lastAmount;
bool public shouldRevert;
Expand Down Expand Up @@ -199,7 +210,7 @@ contract TrailsRouterTest is Test {
MockMulticall3 mockMulticall3 = new MockMulticall3();
vm.etch(0xcA11bde05977b3631167028862bE2a173976CA11, address(mockMulticall3).code);

router = new TrailsRouter();
router = new TrailsRouter(0xcA11bde05977b3631167028862bE2a173976CA11);
getter = new MockSenderGetter();
mockToken = new MockERC20("MockToken", "MTK", 18);
failingToken = new FailingToken();
Expand Down Expand Up @@ -865,17 +876,13 @@ contract TrailsRouterTest is Test {
// Save original multicall code
bytes memory originalCode = 0xcA11bde05977b3631167028862bE2a173976CA11.code;

// Deploy and etch failing multicall
MockMulticall3 failingMulticall = new MockMulticall3();
// Deploy and etch always-failing multicall (doesn't use storage, always reverts)
AlwaysFailingMulticall3 failingMulticall = new AlwaysFailingMulticall3();
vm.etch(0xcA11bde05977b3631167028862bE2a173976CA11, address(failingMulticall).code);

// Verify the etch worked
assertEq(keccak256(0xcA11bde05977b3631167028862bE2a173976CA11.code), keccak256(address(failingMulticall).code));

// Set the failure flag directly in storage since delegatecall uses caller's storage
// The shouldFail variable is at slot 0 in MockMulticall3
vm.store(address(router), bytes32(0), bytes32(uint256(1))); // Set shouldFail = true in router's storage

IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1);
calls[0] = IMulticall3.Call3({
target: address(getter), allowFailure: false, callData: abi.encodeWithSignature("getSender()")
Expand All @@ -901,13 +908,10 @@ contract TrailsRouterTest is Test {
// Save original multicall code
bytes memory originalCode = 0xcA11bde05977b3631167028862bE2a173976CA11.code;

// Mock multicall3 to return failure
MockMulticall3 failingMulticall = new MockMulticall3();
// Deploy and etch always-failing multicall (doesn't use storage, always reverts)
AlwaysFailingMulticall3 failingMulticall = new AlwaysFailingMulticall3();
vm.etch(0xcA11bde05977b3631167028862bE2a173976CA11, address(failingMulticall).code);

// Set the failure flag directly in storage since delegatecall uses caller's storage
vm.store(address(router), bytes32(0), bytes32(uint256(1))); // Set shouldFail = true in router's storage

IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1);
calls[0] = IMulticall3.Call3({
target: address(getter), allowFailure: false, callData: abi.encodeWithSignature("getSender()")
Expand Down
4 changes: 2 additions & 2 deletions test/TrailsRouterShim.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ contract TrailsRouterShimTest is Test {

// Verify sentinel by re-etching TrailsRouter and validating via delegated entrypoint
bytes memory original = address(shimImpl).code;
vm.etch(holder, address(new TrailsRouter()).code);
vm.etch(holder, address(new TrailsRouter(0xcA11bde05977b3631167028862bE2a173976CA11)).code);
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

Critical bug: vm.etch only copies runtime bytecode, not storage. Since MULTICALL3 is now a storage variable (not immutable), it will be uninitialized (address(0)) at the holder address after this etch operation. This means any subsequent calls to validateOpHashAndSweep that perform multicall operations will fail or behave unexpectedly. The original code worked because immutable variables are embedded in the bytecode itself. Either revert MULTICALL3 to immutable, or explicitly set the storage slot after etching: vm.store(holder, bytes32(uint256(0)), bytes32(uint256(uint160(0xcA11bde05977b3631167028862bE2a173976CA11))));

Suggested change
vm.etch(holder, address(new TrailsRouter(0xcA11bde05977b3631167028862bE2a173976CA11)).code);
vm.etch(holder, address(new TrailsRouter(0xcA11bde05977b3631167028862bE2a173976CA11)).code);
// Set MULTICALL3 storage slot (slot 0) to the correct address after etching
vm.store(holder, bytes32(uint256(0)), bytes32(uint256(uint160(0xcA11bde05977b3631167028862bE2a173976CA11))));

Copilot uses AI. Check for mistakes.

address payable recipient = payable(address(0x111));
vm.deal(holder, callValue);
Expand Down Expand Up @@ -309,7 +309,7 @@ contract TrailsRouterShimTest is Test {

// Verify via TrailsRouter delegated validation
bytes memory original = address(shimImpl).code;
vm.etch(holder, address(new TrailsRouter()).code);
vm.etch(holder, address(new TrailsRouter(0xcA11bde05977b3631167028862bE2a173976CA11)).code);
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

Critical bug: vm.etch only copies runtime bytecode, not storage. Since MULTICALL3 is now a storage variable (not immutable), it will be uninitialized (address(0)) at the holder address after this etch operation. Any subsequent operations that rely on MULTICALL3 will fail. The original code worked because immutable variables are embedded in the bytecode itself. Either revert MULTICALL3 to immutable, or explicitly set the storage slot after etching: vm.store(holder, bytes32(uint256(0)), bytes32(uint256(uint160(0xcA11bde05977b3631167028862bE2a173976CA11))));

Suggested change
vm.etch(holder, address(new TrailsRouter(0xcA11bde05977b3631167028862bE2a173976CA11)).code);
vm.etch(holder, address(new TrailsRouter(0xcA11bde05977b3631167028862bE2a173976CA11)).code);
// Explicitly set MULTICALL3 storage slot after etch
vm.store(holder, bytes32(uint256(0)), bytes32(uint256(uint160(0xcA11bde05977b3631167028862bE2a173976CA11))));

Copilot uses AI. Check for mistakes.
address payable recipient = payable(address(0x112));
vm.deal(holder, 1 ether);
bytes memory data =
Expand Down
4 changes: 3 additions & 1 deletion test/script/TrailsRouter.s.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ contract TrailsRouterDeploymentTest is Test {

// Expected predetermined address (calculated using CREATE2)
function expectedRouterAddress() internal pure returns (address payable) {
return Create2Utils.calculateCreate2Address(type(TrailsRouter).creationCode, Create2Utils.standardSalt());
address multicall3 = 0xcA11bde05977b3631167028862bE2a173976CA11;
bytes memory initCode = abi.encodePacked(type(TrailsRouter).creationCode, abi.encode(multicall3));
return Create2Utils.calculateCreate2Address(initCode, Create2Utils.standardSalt());
}

// -------------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion test/script/TrailsRouterShim.s.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ contract TrailsRouterShimDeploymentTest is Test {

// Expected predetermined addresses (calculated using CREATE2)
function expectedRouterAddress() internal pure returns (address payable) {
return Create2Utils.calculateCreate2Address(type(TrailsRouter).creationCode, Create2Utils.standardSalt());
address multicall3 = 0xcA11bde05977b3631167028862bE2a173976CA11;
bytes memory initCode = abi.encodePacked(type(TrailsRouter).creationCode, abi.encode(multicall3));
return Create2Utils.calculateCreate2Address(initCode, Create2Utils.standardSalt());
}

function expectedShimAddress() internal pure returns (address payable) {
Expand Down