Skip to content

Commit ce7c2de

Browse files
ScreamingHawkScreamingHawk
committed
Correct error handling
1 parent 0fef464 commit ce7c2de

File tree

2 files changed

+166
-18
lines changed

2 files changed

+166
-18
lines changed

‎contracts/hooks/EIP4337Hook.sol‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,20 @@ contract EIP4337Hook is IEIP4337Hook, ModuleNonce {
2929
/**
3030
* Allow the EIP-4337 entrypoint to execute a transaction on the wallet.
3131
* @dev This function does not validate as the entrypoint is trusted to have called validateUserOp.
32-
* @dev Failure handling done by ModuleCalls.
3332
* @notice This functions is only callable by the Entrypoint.
3433
*/
3534
function eip4337SelfExecute(IModuleCalls.Transaction[] calldata txs) external payable onlyEntrypoint {
3635
// Self execute
3736
(bool success, ) = payable(address(this)).call{value: msg.value}(
3837
abi.encodeWithSelector(IModuleCalls.selfExecute.selector, txs)
3938
);
40-
(success);
39+
if (!success) {
40+
// Bubble up revert reason
41+
bytes memory reason = LibOptim.returnData();
42+
assembly {
43+
revert(add(reason, 0x20), mload(reason))
44+
}
45+
}
4146
}
4247

4348
/**
@@ -55,7 +60,8 @@ contract EIP4337Hook is IEIP4337Hook, ModuleNonce {
5560
_validateNonce(userOp.nonce);
5661

5762
// Check signature
58-
bytes32 ethHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", userOpHash));
63+
bytes32 ethHash = keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n32', userOpHash));
64+
// solhint-disable-next-line avoid-low-level-calls
5965
(bool sigSuccess, bytes memory data) = address(this).call{gas: type(uint256).max}(
6066
abi.encodeWithSelector(ERC1271_SELECTOR, ethHash, userOp.signature)
6167
);
@@ -66,7 +72,7 @@ contract EIP4337Hook is IEIP4337Hook, ModuleNonce {
6672

6773
// Pay entrypoint
6874
if (missingAccountFunds != 0) {
69-
(bool success,) = payable(msg.sender).call{value: missingAccountFunds, gas: type(uint256).max}('');
75+
(bool success, ) = payable(msg.sender).call{value: missingAccountFunds, gas: type(uint256).max}('');
7076
(success);
7177
}
7278

‎foundry_test/hooks/EIP4337Hook.t.sol‎

Lines changed: 156 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,44 +3,89 @@ pragma solidity 0.8.18;
33

44
import 'contracts/hooks/EIP4337Hook.sol';
55
import 'contracts/hooks/interfaces/IEIP4337Hook.sol';
6+
import 'contracts/modules/commons/ModuleAuth.sol';
7+
import 'contracts/modules/commons/ModuleCalls.sol';
68
import 'contracts/modules/commons/ModuleHooks.sol';
79
import 'contracts/modules/MainModule.sol';
810
import 'contracts/modules/MainModuleUpgradable.sol';
911
import 'contracts/Factory.sol';
1012

1113
import 'foundry_test/base/AdvTest.sol';
1214

15+
contract MockModules is ModuleAuth, ModuleCalls, ModuleHooks {
16+
function validateNonce(uint256 _rawNonce) external {
17+
_validateNonce(_rawNonce);
18+
}
19+
20+
function writeNonce(uint256 _space, uint256 _nonce) external {
21+
_writeNonce(_space, _nonce);
22+
}
23+
24+
// Module Auth imp
25+
mapping(bytes32 => mapping(bytes => bytes32)) public sigToSubdigest;
26+
mapping(bytes32 => mapping(bytes => bool)) public sigToIsValid;
27+
28+
function _signatureValidation(
29+
bytes32 _digest,
30+
bytes calldata _signature
31+
) internal view override(IModuleAuth, ModuleAuth) returns (bool isValid, bytes32 subdigest) {
32+
subdigest = sigToSubdigest[_digest][_signature];
33+
isValid = sigToIsValid[_digest][_signature];
34+
}
35+
36+
function mockSignature(bytes32 _digest, bytes calldata _signature, bytes32 _subdigest, bool _isValid) external {
37+
sigToSubdigest[_digest][_signature] = _subdigest;
38+
sigToIsValid[_digest][_signature] = _isValid;
39+
}
40+
41+
// solhint-disable no-empty-blocks
42+
function _isValidImage(bytes32) internal view override returns (bool) {}
43+
44+
function _updateImageHash(bytes32) internal override {}
45+
46+
// solhint-enable no-empty-blocks
47+
48+
function supportsInterface(
49+
bytes4 _interfaceID
50+
) public pure virtual override(ModuleAuth, ModuleCalls, ModuleHooks) returns (bool) {
51+
return
52+
ModuleAuth.supportsInterface(_interfaceID) ||
53+
ModuleCalls.supportsInterface(_interfaceID) ||
54+
ModuleHooks.supportsInterface(_interfaceID);
55+
}
56+
}
57+
1358
contract EIP4337HookTest is AdvTest, IEIP4337HookErrors {
14-
MainModule private template;
59+
MockModules private walletMod;
1560
EIP4337Hook private wallet;
1661

1762
address private constant ENTRYPOINT = address(uint160(uint256(keccak256('entrypoint'))));
1863

1964
function setUp() external {
2065
Factory factory = new Factory();
21-
address upgradeable = address(new MainModuleUpgradable());
22-
template = new MainModule(address(factory), upgradeable);
23-
ModuleHooks walletMod = ModuleHooks(payable(factory.deploy(address(template), bytes32(0)))); // Add hook below
24-
vm.label(address(walletMod), "wallet");
66+
ModuleHooks template = new MockModules();
67+
walletMod = MockModules(payable(factory.deploy(address(template), bytes32(0))));
2568
EIP4337Hook hook = new EIP4337Hook(ENTRYPOINT);
2669

27-
// Fund wallet
28-
vm.deal(address(walletMod), 10 ether);
29-
3070
// Add hooks
3171
vm.startPrank(address(walletMod));
3272
walletMod.addHook(IAccount.validateUserOp.selector, address(hook));
3373
walletMod.addHook(IEIP4337Hook.eip4337SelfExecute.selector, address(hook));
3474
vm.stopPrank();
3575

3676
wallet = EIP4337Hook(address(walletMod));
77+
vm.label(address(wallet), 'wallet');
3778
}
3879

3980
struct ToVal {
4081
address target;
4182
uint256 value;
4283
}
4384

85+
//
86+
// Execute
87+
//
88+
4489
function test_4337execute_invalidCaller(ToVal memory sendTx, address sender) external {
4590
vm.assume(sender != ENTRYPOINT);
4691
IModuleCalls.Transaction[] memory txs = new IModuleCalls.Transaction[](1);
@@ -59,14 +104,15 @@ contract EIP4337HookTest is AdvTest, IEIP4337HookErrors {
59104
}
60105

61106
function test_4337execute_sendEth(ToVal memory sendTx) external {
62-
vm.assume(sendTx.target.code.length == 0); // Non contract
63-
uint256 walletBal = address(wallet).balance;
64-
vm.assume(sendTx.value <= walletBal);
107+
_assumeSafeAddress(sendTx.target);
108+
109+
// Give wallet exact funds
110+
vm.deal(address(wallet), sendTx.value);
65111

66112
IModuleCalls.Transaction[] memory txs = new IModuleCalls.Transaction[](1);
67113
txs[0] = IModuleCalls.Transaction({
68114
delegateCall: false,
69-
revertOnError: false,
115+
revertOnError: true,
70116
gasLimit: 0,
71117
target: sendTx.target,
72118
value: sendTx.value,
@@ -76,9 +122,105 @@ contract EIP4337HookTest is AdvTest, IEIP4337HookErrors {
76122
vm.prank(ENTRYPOINT);
77123
wallet.eip4337SelfExecute(txs);
78124

79-
assertEq(address(wallet).balance, walletBal - sendTx.value);
125+
assertEq(address(wallet).balance, 0);
80126
assertEq(sendTx.target.balance, sendTx.value);
81127
}
82128

83-
function test_validateUserOp(ToVal[] memory sendTx) external {}
129+
function test_4337execute_insufficientEth(ToVal memory sendTx) external {
130+
_assumeSafeAddress(sendTx.target);
131+
vm.assume(sendTx.value > 0);
132+
133+
IModuleCalls.Transaction[] memory txs = new IModuleCalls.Transaction[](1);
134+
txs[0] = IModuleCalls.Transaction({
135+
delegateCall: false,
136+
revertOnError: true,
137+
gasLimit: 0,
138+
target: sendTx.target,
139+
value: sendTx.value,
140+
data: ''
141+
});
142+
143+
vm.expectRevert();
144+
vm.prank(ENTRYPOINT);
145+
wallet.eip4337SelfExecute(txs);
146+
}
147+
148+
//
149+
// Validate
150+
//
151+
152+
function test_4337validateUserOp_invalidCaller(
153+
IAccount.UserOperation calldata userOp,
154+
bytes32 userOpHash,
155+
uint256 missingAccountFunds
156+
) external {
157+
vm.expectRevert(InvalidCaller.selector);
158+
wallet.validateUserOp(userOp, userOpHash, missingAccountFunds);
159+
}
160+
161+
function test_4337validateUserOp(
162+
IAccount.UserOperation calldata userOp,
163+
bytes32 userOpHash,
164+
uint256 missingAccountFunds
165+
) external {
166+
vm.assume(userOp.nonce == 0);
167+
168+
// Give wallet enough funds
169+
vm.deal(address(wallet), missingAccountFunds);
170+
171+
// Accept the hash
172+
bytes32 encodedHash = keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n32', userOpHash));
173+
walletMod.mockSignature(encodedHash, userOp.signature, bytes32(0), true);
174+
175+
// Validate
176+
vm.prank(ENTRYPOINT);
177+
uint256 validationData = wallet.validateUserOp(userOp, userOpHash, missingAccountFunds);
178+
179+
assertEq(validationData, 0);
180+
}
181+
182+
function test_4337validateUserOp_failedSignature(
183+
IAccount.UserOperation calldata userOp,
184+
bytes32 userOpHash,
185+
uint256 missingAccountFunds
186+
) external {
187+
vm.assume(userOp.nonce == 0);
188+
189+
// Give wallet enough funds
190+
vm.deal(address(wallet), missingAccountFunds);
191+
192+
// Validate
193+
vm.prank(ENTRYPOINT);
194+
uint256 validationData = wallet.validateUserOp(userOp, userOpHash, missingAccountFunds);
195+
196+
assertEq(validationData, 1);
197+
}
198+
199+
function test_4337validateUserOp_invalidFunds(
200+
IAccount.UserOperation calldata userOp,
201+
bytes32 userOpHash,
202+
uint256 missingAccountFunds
203+
) external {
204+
vm.assume(userOp.nonce == 0);
205+
206+
// Accept the hash
207+
bytes32 encodedHash = keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n32', userOpHash));
208+
walletMod.mockSignature(encodedHash, userOp.signature, bytes32(0), true);
209+
210+
// Validate
211+
vm.prank(ENTRYPOINT);
212+
uint256 validationData = wallet.validateUserOp(userOp, userOpHash, missingAccountFunds);
213+
214+
// Passes. Account doesn't validate
215+
assertEq(validationData, 0);
216+
}
217+
218+
//
219+
// Helpers
220+
//
221+
222+
function _assumeSafeAddress(address addr) private view {
223+
vm.assume(uint160(addr) > 20); // Non precompiled
224+
vm.assume(addr.code.length == 0); // Non contract
225+
}
84226
}

0 commit comments

Comments
 (0)