Skip to content

Commit 48f8884

Browse files
Agusx1211claude
andcommitted
Merge branch 'master' into pausable-sapient
Resolves Allowlist.sol conflict: keep master's F-09 fix (constructor emits AddressAdded for initial entries) and graft on the renounceOwnership override from the pausable branch. All unit (159) + integration (5) tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 parents 58bf498 + 3611e8a commit 48f8884

7 files changed

Lines changed: 225 additions & 16 deletions

File tree

itest/SequenceV3Integration.t.sol

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@ contract SequenceV3IntegrationTest is Test {
9191
}
9292
}
9393

94+
function _abiDynamicArgDataOffset(bytes memory callData, uint256 argIndex) private pure returns (uint256 offset) {
95+
uint256 tailOffset;
96+
assembly ("memory-safe") {
97+
tailOffset := mload(add(add(callData, 36), shl(5, argIndex)))
98+
}
99+
return 4 + tailOffset + 32;
100+
}
101+
94102
function _randomBytes(uint256 len, bytes32 seed) private pure returns (bytes memory data) {
95103
data = new bytes(len);
96104

@@ -339,6 +347,88 @@ contract SequenceV3IntegrationTest is Test {
339347
assertEq(receiver.lastData(), dataB);
340348
}
341349

350+
function test_integration_sapientStaticHydrateRange_isNotShiftedByPrependedTypeZeroCommand() external {
351+
TrailsUtils trailsUtils = new TrailsUtils();
352+
353+
RecordingReceiver receiver = new RecordingReceiver();
354+
address signedReceiver = makeAddr("signedReceiver");
355+
address relayer = makeAddr("relayer");
356+
address origin = makeAddr("origin");
357+
358+
LocalPayload.Call[] memory innerCalls = new LocalPayload.Call[](1);
359+
innerCalls[0] = LocalPayload.Call({
360+
to: address(receiver),
361+
value: 0,
362+
data: new bytes(20),
363+
gasLimit: 0,
364+
delegateCall: false,
365+
onlyFallback: false,
366+
behaviorOnError: LocalPayload.BEHAVIOR_REVERT_ON_ERROR
367+
});
368+
369+
bytes memory innerPacked = innerCalls.packCalls();
370+
bytes memory hydratePayload = bytes.concat(
371+
abi.encodePacked(uint8(0)),
372+
abi.encodePacked(uint8(0x13), signedReceiver, uint16(0)),
373+
abi.encodePacked(uint8(0x00))
374+
);
375+
bytes memory shiftedHydratePayload = bytes.concat(
376+
abi.encodePacked(uint8(0)),
377+
abi.encodePacked(uint8(0x01), uint16(0xBEEF)),
378+
abi.encodePacked(uint8(0x13), signedReceiver, uint16(0)),
379+
abi.encodePacked(uint8(0x00))
380+
);
381+
382+
bytes memory outerData = abi.encodeWithSelector(HydrateProxy.hydrateExecute.selector, innerPacked, hydratePayload);
383+
uint16 signedRangeOffset = uint16(_abiDynamicArgDataOffset(outerData, 1) + 2);
384+
bytes memory sapientSig = abi.encodePacked(uint8(0), signedRangeOffset, uint16(20));
385+
386+
LocalPayload.Call[] memory outerCalls = new LocalPayload.Call[](1);
387+
outerCalls[0] = LocalPayload.Call({
388+
to: address(trailsUtils),
389+
value: 0,
390+
data: outerData,
391+
gasLimit: 0,
392+
delegateCall: true,
393+
onlyFallback: false,
394+
behaviorOnError: LocalPayload.BEHAVIOR_REVERT_ON_ERROR
395+
});
396+
397+
bytes memory shiftedOuterData =
398+
abi.encodeWithSelector(HydrateProxy.hydrateExecute.selector, innerPacked, shiftedHydratePayload);
399+
LocalPayload.Call[] memory shiftedOuterCalls = new LocalPayload.Call[](1);
400+
shiftedOuterCalls[0] = LocalPayload.Call({
401+
to: address(trailsUtils),
402+
value: 0,
403+
data: shiftedOuterData,
404+
gasLimit: 0,
405+
delegateCall: true,
406+
onlyFallback: false,
407+
behaviorOnError: LocalPayload.BEHAVIOR_REVERT_ON_ERROR
408+
});
409+
410+
bytes memory outerPacked = outerCalls.packCalls();
411+
bytes memory shiftedOuterPacked = shiftedOuterCalls.packCalls();
412+
413+
SeqPayload.Decoded memory payload = _asSeqPayload(outerCalls, 0, 0);
414+
bytes32 sapientImageHash = ISapient(address(trailsUtils)).recoverSapientSignature(payload, sapientSig);
415+
address wallet = _deployWalletWithSapient(address(trailsUtils), sapientImageHash);
416+
bytes memory signature = _buildSapientSignature(address(trailsUtils), 1, 1, sapientSig);
417+
418+
vm.prank(relayer, origin);
419+
vm.expectRevert();
420+
SeqStage1Module(payable(wallet)).execute(shiftedOuterPacked, signature);
421+
422+
assertEq(SeqStage1Module(payable(wallet)).readNonce(0), 0);
423+
424+
vm.prank(relayer, origin);
425+
SeqStage1Module(payable(wallet)).execute(outerPacked, signature);
426+
427+
assertEq(receiver.lastSender(), wallet);
428+
assertEq(_readAddress(receiver.lastData(), 0), signedReceiver);
429+
assertEq(SeqStage1Module(payable(wallet)).readNonce(0), 1);
430+
}
431+
342432
function test_integration_walletDelegatecallsHydrateProxy_andHydrates() external {
343433
TrailsUtils trailsUtils = new TrailsUtils();
344434

src/autoRecovery/Allowlist.sol

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,21 @@ contract Allowlist is Ownable {
3030
/// @param initial The initial addresses to mark as allowed.
3131
constructor(address owner_, address[] memory initial) Ownable(owner_) {
3232
for (uint256 i; i < initial.length; i++) {
33-
_add(initial[i], false);
33+
_add(initial[i]);
3434
}
3535
}
3636

3737
/// @notice Adds `addr` to the allowlist.
3838
/// @param addr The address to add.
3939
function add(address addr) external onlyOwner {
40-
_add(addr, true);
40+
_add(addr);
4141
}
4242

4343
/// @notice Adds each address in `addrs` to the allowlist.
4444
/// @param addrs The addresses to add.
4545
function add(address[] calldata addrs) external onlyOwner {
4646
for (uint256 i; i < addrs.length; i++) {
47-
_add(addrs[i], true);
47+
_add(addrs[i]);
4848
}
4949
}
5050

@@ -53,15 +53,15 @@ contract Allowlist is Ownable {
5353
/// @param index Optional index hint into `getAllowed()`. Pass `0` to use search mode.
5454
/// @dev Removal uses swap-and-pop, so `getAllowed()` ordering is not stable.
5555
function remove(address addr, uint256 index) external onlyOwner {
56-
_remove(addr, index, true);
56+
_remove(addr, index);
5757
}
5858

5959
/// @notice Removes each address in `addrs` from the allowlist using search mode.
6060
/// @param addrs The addresses to remove.
6161
/// @dev Removal uses swap-and-pop, so `getAllowed()` ordering is not stable.
6262
function remove(address[] calldata addrs) external onlyOwner {
6363
for (uint256 i; i < addrs.length; i++) {
64-
_remove(addrs[i], 0, true);
64+
_remove(addrs[i], 0);
6565
}
6666
}
6767

@@ -81,17 +81,17 @@ contract Allowlist is Ownable {
8181
revert OwnershipRenunciationDisabled();
8282
}
8383

84-
function _add(address addr, bool emitEvent) private {
84+
function _add(address addr) private {
8585
if (addr == address(0)) revert ZeroAddress();
8686
if (_allowed[addr]) revert AlreadyAllowed(addr);
8787

8888
_allowed[addr] = true;
8989
_entries.push(addr);
9090

91-
if (emitEvent) emit AddressAdded(addr);
91+
emit AddressAdded(addr);
9292
}
9393

94-
function _remove(address addr, uint256 index, bool emitEvent) private {
94+
function _remove(address addr, uint256 index) private {
9595
if (!_allowed[addr]) revert NotAllowed(addr);
9696
_allowed[addr] = false;
9797

@@ -110,6 +110,6 @@ contract Allowlist is Ownable {
110110
}
111111
}
112112

113-
if (emitEvent) emit AddressRemoved(addr);
113+
emit AddressRemoved(addr);
114114
}
115115
}

src/modules/HydrateProxy.sol

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,10 @@ contract HydrateProxy is Sweepable, IDelegatedExtension {
192192
address valueAddress;
193193
(valueAddress, rindex) = _getAddressFromFlag(valueFlag, hydratePayload, rindex);
194194

195+
if (typeFlag == 0) {
196+
revert UnknownHydrateTypeCommand(typeFlag);
197+
}
198+
195199
if (typeFlag <= HYDRATE_TYPE_DATA_ERC20_ALLOWANCE) {
196200
// Data hydration commands mutate `decoded.calls[tindex].data` in-place.
197201
(cindex, rindex) = hydratePayload.readUint16(rindex);

src/modules/Sweepable.sol

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ contract Sweepable {
3131
// Sweep all token addresses specified
3232
for (uint256 i = 0; i < tokensToSweep.length; ++i) {
3333
uint256 balance = IERC20(tokensToSweep[i]).balanceOf(address(this));
34-
if (balance > 0) {
35-
IERC20(tokensToSweep[i]).safeTransfer(sweepToAddress, balance);
34+
if (balance > 0 && IERC20(tokensToSweep[i]).trySafeTransfer(sweepToAddress, balance)) {
3635
emit Sweep(tokensToSweep[i], sweepToAddress, balance);
3736
}
3837
}

test/Allowlist.t.sol

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
pragma solidity ^0.8.27;
33

44
import {Test} from "forge-std/Test.sol";
5+
import {Vm} from "forge-std/Vm.sol";
56
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
67

78
import {Allowlist} from "src/autoRecovery/Allowlist.sol";
@@ -39,6 +40,41 @@ contract AllowlistTest is Test {
3940
assertEq(all[1], second);
4041
}
4142

43+
function testFuzz_constructor_emitsAddressAddedForInitial(address owner_, address first, address second) external {
44+
vm.assume(owner_ != address(0));
45+
_assumeDistinctNonZero(first, second);
46+
47+
address[] memory initial = new address[](2);
48+
initial[0] = first;
49+
initial[1] = second;
50+
51+
vm.recordLogs();
52+
new Allowlist(owner_, initial);
53+
Vm.Log[] memory entries = vm.getRecordedLogs();
54+
55+
bytes32 addressAddedTopic = keccak256("AddressAdded(address)");
56+
uint256 matches;
57+
58+
for (uint256 i; i < entries.length; i++) {
59+
if (entries[i].topics.length > 0 && entries[i].topics[0] == addressAddedTopic) {
60+
assertEq(entries[i].topics.length, 2);
61+
address actual = address(uint160(uint256(entries[i].topics[1])));
62+
63+
if (matches == 0) {
64+
assertEq(actual, first);
65+
} else if (matches == 1) {
66+
assertEq(actual, second);
67+
} else {
68+
fail();
69+
}
70+
71+
matches++;
72+
}
73+
}
74+
75+
assertEq(matches, 2);
76+
}
77+
4278
function testFuzz_constructor_revertsZeroAddress(address owner_, address first) external {
4379
vm.assume(owner_ != address(0));
4480
vm.assume(first != address(0));

test/HydrateProxy.t.sol

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -644,13 +644,13 @@ contract HydrateProxyTest is Test {
644644
}
645645

646646
function testFuzz_hydrateExecute_unknownHydrateFlag_reverts(uint8 flag) external {
647-
// With nibble-based encoding, valid flags are 0x00-0x63
648-
// Test invalid flags: either invalid value flag (bottom nibble > 0x03) or invalid type flag (top nibble > 0x06)
647+
// With nibble-based encoding, the valid command flags are 0x10-0x63.
648+
// 0x00 is the terminator; every other 0x0X value is invalid.
649649
uint8 valueFlag = flag & 0x0F;
650650
uint8 typeFlag = flag >> 4;
651651

652-
// Skip valid flags (0x00-0x63)
653-
vm.assume(flag > 0x63 || (valueFlag > 0x03) || (typeFlag > 0x06 && valueFlag <= 0x03));
652+
bool validFlag = flag == 0x00 || (typeFlag >= 0x01 && typeFlag <= 0x06 && valueFlag <= 0x03);
653+
vm.assume(!validFlag);
654654

655655
HydrateProxy proxy = new HydrateProxy();
656656
RecordingReceiver receiver = new RecordingReceiver();
@@ -678,6 +678,31 @@ contract HydrateProxyTest is Test {
678678
proxy.hydrateExecute(calls.packCalls(), hydratePayload);
679679
}
680680

681+
function test_hydrateExecute_typeZeroFlags_revert() external {
682+
HydrateProxy proxy = new HydrateProxy();
683+
RecordingReceiver receiver = new RecordingReceiver();
684+
685+
Payload.Call[] memory calls = new Payload.Call[](1);
686+
calls[0] = Payload.Call({
687+
to: address(receiver),
688+
value: 0,
689+
data: hex"01",
690+
gasLimit: 0,
691+
delegateCall: false,
692+
onlyFallback: false,
693+
behaviorOnError: Payload.BEHAVIOR_IGNORE_ERROR
694+
});
695+
696+
vm.expectRevert(abi.encodeWithSelector(HydrateProxy.UnknownHydrateTypeCommand.selector, uint256(0)));
697+
proxy.hydrateExecute(calls.packCalls(), abi.encodePacked(uint8(0), uint8(0x01)));
698+
699+
vm.expectRevert(abi.encodeWithSelector(HydrateProxy.UnknownHydrateTypeCommand.selector, uint256(0)));
700+
proxy.hydrateExecute(calls.packCalls(), abi.encodePacked(uint8(0), uint8(0x02)));
701+
702+
vm.expectRevert(abi.encodeWithSelector(HydrateProxy.UnknownHydrateTypeCommand.selector, uint256(0)));
703+
proxy.hydrateExecute(calls.packCalls(), abi.encodePacked(uint8(0), uint8(0x03), makeAddr("hydrated-owner")));
704+
}
705+
681706
function testFuzz_hydrateExecute_delegateCallNotAllowed_reverts(bytes calldata data) external {
682707
HydrateProxy proxy = new HydrateProxy();
683708

test/Sweepable.t.sol

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,37 @@ import {Test} from "forge-std/Test.sol";
66
import {Sweepable} from "src/modules/Sweepable.sol";
77
import {MockERC20} from "test/helpers/Mocks.sol";
88

9+
contract ReturnBombERC20 {
10+
mapping(address => uint256) public balanceOf;
11+
12+
uint256 internal immutable bombSize;
13+
14+
constructor(uint256 bombSize_) {
15+
bombSize = bombSize_;
16+
}
17+
18+
function mint(address to, uint256 amount) external {
19+
balanceOf[to] += amount;
20+
}
21+
22+
function transfer(address, uint256) external view returns (bool) {
23+
uint256 size = bombSize;
24+
25+
assembly ("memory-safe") {
26+
let ptr := mload(0x40)
27+
mstore(0x40, add(ptr, size))
28+
revert(ptr, size)
29+
}
30+
}
31+
}
32+
933
contract SweepableTest is Test {
34+
uint256 internal constant GOOD_TOKEN_BALANCE = 2 ether;
35+
uint256 internal constant BAD_TOKEN_BALANCE = 1 ether;
36+
uint256 internal constant NATIVE_BALANCE = 3 ether;
37+
uint256 internal constant RETURN_BOMB_SIZE = 1 << 20;
38+
uint256 internal constant RETURN_BOMB_GAS_LIMIT = 3_500_000;
39+
1040
Sweepable public sweepable;
1141

1242
function setUp() public {
@@ -83,5 +113,30 @@ contract SweepableTest is Test {
83113
assertEq(sweepTarget.balance, 0);
84114
assertEq(address(sweepable).balance, 0);
85115
}
86-
}
87116

117+
function test_sweepReturnBombSkipsFailedTokenAndContinues() external {
118+
MockERC20 goodToken = new MockERC20();
119+
ReturnBombERC20 badToken = new ReturnBombERC20(RETURN_BOMB_SIZE);
120+
address recipient = makeAddr("recipient");
121+
122+
goodToken.mint(address(sweepable), GOOD_TOKEN_BALANCE);
123+
badToken.mint(address(sweepable), BAD_TOKEN_BALANCE);
124+
vm.deal(address(sweepable), NATIVE_BALANCE);
125+
126+
address[] memory tokensToSweep = new address[](2);
127+
tokensToSweep[0] = address(badToken);
128+
tokensToSweep[1] = address(goodToken);
129+
130+
(bool success,) = address(sweepable).call{gas: RETURN_BOMB_GAS_LIMIT}(
131+
abi.encodeCall(Sweepable.sweep, (recipient, tokensToSweep, true))
132+
);
133+
134+
assertTrue(success);
135+
assertEq(badToken.balanceOf(address(sweepable)), BAD_TOKEN_BALANCE);
136+
assertEq(badToken.balanceOf(recipient), 0);
137+
assertEq(goodToken.balanceOf(address(sweepable)), 0);
138+
assertEq(goodToken.balanceOf(recipient), GOOD_TOKEN_BALANCE);
139+
assertEq(address(sweepable).balance, 0);
140+
assertEq(recipient.balance, NATIVE_BALANCE);
141+
}
142+
}

0 commit comments

Comments
 (0)