Skip to content

Commit 5c14279

Browse files
amiecorsoamiecorso
committed
add ecrecover check to isValidSignature and tests
1 parent c306de8 commit 5c14279

File tree

3 files changed

+224
-17
lines changed

3 files changed

+224
-17
lines changed

‎src/EIP7702Proxy.sol‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ import {Address} from "@openzeppelin/contracts/utils/Address.sol";
1111
/// @dev Implements ERC-1967, but with an initial implementation.
1212
/// @dev Guards the initializer function, requiring a signed payload by the wallet to call it.
1313
contract EIP7702Proxy is Proxy {
14+
// ERC1271 interface constants
15+
bytes4 internal constant ERC1271_MAGIC_VALUE = 0x1626ba7e;
16+
bytes4 internal constant ERC1271_ISVALIDSIGNATURE_SELECTOR = 0x1626ba7e;
17+
bytes4 internal constant ERC1271_FAIL_VALUE = 0xffffffff;
18+
1419
address immutable proxy;
1520
address immutable initialImplementation;
1621
bytes4 immutable guardedInitializer;
@@ -56,6 +61,46 @@ contract EIP7702Proxy is Proxy {
5661
function _fallback() internal override {
5762
// block guarded initializer from being called
5863
if (msg.sig == guardedInitializer) revert InvalidInitializer();
64+
65+
// Special handling for isValidSignature
66+
if (msg.sig == ERC1271_ISVALIDSIGNATURE_SELECTOR) {
67+
(bytes32 hash, bytes memory signature) = abi.decode(
68+
msg.data[4:],
69+
(bytes32, bytes)
70+
);
71+
72+
// First try delegatecall to implementation
73+
(bool success, bytes memory result) = _implementation()
74+
.delegatecall(msg.data);
75+
76+
// If delegatecall succeeded and returned magic value, return that
77+
if (
78+
success &&
79+
result.length == 32 &&
80+
abi.decode(result, (bytes4)) == ERC1271_MAGIC_VALUE
81+
) {
82+
assembly {
83+
mstore(0, ERC1271_MAGIC_VALUE)
84+
return(0, 32)
85+
}
86+
}
87+
88+
// Otherwise try ecrecover
89+
address recovered = ECDSA.recover(hash, signature);
90+
if (recovered == address(this)) {
91+
assembly {
92+
mstore(0, ERC1271_MAGIC_VALUE)
93+
return(0, 32)
94+
}
95+
}
96+
97+
// If all checks fail, return failure value
98+
assembly {
99+
mstore(0, ERC1271_FAIL_VALUE)
100+
return(0, 32)
101+
}
102+
}
103+
59104
_delegate(_implementation());
60105
}
61106
}

‎test/EIP7702Proxy/isValidSignature.t.sol‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// SPDX-License-Identifier: UNLICENSED
2+
pragma solidity ^0.8.23;
3+
4+
import {EIP7702ProxyBase} from "../base/EIP7702ProxyBase.sol";
5+
import {EIP7702Proxy} from "../../src/EIP7702Proxy.sol";
6+
import {CoinbaseSmartWallet} from "../../lib/smart-wallet/src/CoinbaseSmartWallet.sol";
7+
import {ECDSA} from "openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol";
8+
9+
contract IsValidSignatureTest is EIP7702ProxyBase {
10+
bytes4 constant ERC1271_MAGIC_VALUE = 0x1626ba7e;
11+
bytes4 constant ERC1271_FAIL_VALUE = 0xffffffff;
12+
13+
bytes32 testHash;
14+
CoinbaseSmartWallet wallet;
15+
16+
function setUp() public override {
17+
super.setUp();
18+
19+
// Initialize the wallet with a contract owner
20+
bytes memory initArgs = _createInitArgs(_newOwner);
21+
bytes memory signature = _signInitData(_EOA_PRIVATE_KEY, initArgs);
22+
vm.prank(_eoa);
23+
EIP7702Proxy(_eoa).initialize(initArgs, signature);
24+
25+
wallet = CoinbaseSmartWallet(payable(_eoa));
26+
testHash = keccak256("test message");
27+
28+
// Verify owner was set correctly
29+
assertTrue(
30+
wallet.isOwnerAddress(_newOwner),
31+
"New owner should be set after initialization"
32+
);
33+
assertEq(
34+
wallet.ownerAtIndex(0),
35+
abi.encode(_newOwner),
36+
"Owner at index 0 should be new owner"
37+
);
38+
}
39+
40+
function testValidContractOwnerSignature() public {
41+
// Create signature from contract owner
42+
bytes memory signature = _createOwnerSignature(
43+
testHash,
44+
address(wallet),
45+
_NEW_OWNER_PRIVATE_KEY,
46+
0 // First owner
47+
);
48+
49+
bytes4 result = wallet.isValidSignature(testHash, signature);
50+
assertEq(
51+
result,
52+
ERC1271_MAGIC_VALUE,
53+
"Should accept valid contract owner signature"
54+
);
55+
}
56+
57+
function testValidEOASignature() public {
58+
// Create signature from original EOA
59+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(_EOA_PRIVATE_KEY, testHash);
60+
bytes memory signature = abi.encodePacked(r, s, v);
61+
62+
bytes4 result = wallet.isValidSignature(testHash, signature);
63+
assertEq(
64+
result,
65+
ERC1271_MAGIC_VALUE,
66+
"Should accept valid EOA signature"
67+
);
68+
}
69+
70+
function testInvalidEOASignature() public {
71+
// Create signature from wrong EOA
72+
uint256 wrongPk = 0xB0B;
73+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(wrongPk, testHash);
74+
bytes memory signature = abi.encodePacked(r, s, v);
75+
76+
bytes4 result = wallet.isValidSignature(testHash, signature);
77+
assertEq(
78+
result,
79+
ERC1271_FAIL_VALUE,
80+
"Should reject invalid EOA signature"
81+
);
82+
}
83+
84+
function testInvalidOwnerSignature() public {
85+
// Create a valid format signature but with wrong signer
86+
uint256 wrongPk = 0xBADBAD; // Different from both EOA and new owner (0xB0B)
87+
bytes memory signature = _createOwnerSignature(
88+
testHash,
89+
address(wallet),
90+
wrongPk,
91+
0
92+
);
93+
94+
bytes4 result = wallet.isValidSignature(testHash, signature);
95+
assertEq(
96+
result,
97+
ERC1271_FAIL_VALUE,
98+
"Should reject signature from non-owner"
99+
);
100+
}
101+
102+
function testEmptySignature() public {
103+
bytes memory emptySignature = "";
104+
105+
vm.expectRevert();
106+
bytes4 result = wallet.isValidSignature(testHash, emptySignature);
107+
}
108+
}

‎test/base/EIP7702ProxyBase.sol‎

Lines changed: 71 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,39 @@ abstract contract EIP7702ProxyBase is Test {
1515
// Test accounts
1616
uint256 internal constant _EOA_PRIVATE_KEY = 0xA11CE;
1717
address payable internal _eoa;
18-
18+
1919
uint256 internal constant _NEW_OWNER_PRIVATE_KEY = 0xB0B;
2020
address payable internal _newOwner;
21-
21+
2222
// Contracts
2323
EIP7702Proxy internal _proxy;
2424
CoinbaseSmartWallet internal _implementation;
25-
25+
2626
// Common test data
2727
bytes4 internal _initSelector;
28-
28+
2929
function setUp() public virtual {
3030
// Set up test accounts
3131
_eoa = payable(vm.addr(_EOA_PRIVATE_KEY));
3232
vm.deal(_eoa, 100 ether);
33-
33+
3434
_newOwner = payable(vm.addr(_NEW_OWNER_PRIVATE_KEY));
3535
vm.deal(_newOwner, 100 ether);
36-
36+
3737
// Deploy implementation
3838
_implementation = new CoinbaseSmartWallet();
3939
_initSelector = CoinbaseSmartWallet.initialize.selector;
40-
40+
4141
// Deploy proxy normally first to get the correct immutable values
42-
_proxy = new EIP7702Proxy(
43-
address(_implementation),
44-
_initSelector
45-
);
46-
42+
_proxy = new EIP7702Proxy(address(_implementation), _initSelector);
43+
4744
// Get the proxy's runtime code
4845
bytes memory proxyCode = address(_proxy).code;
49-
46+
5047
// Etch the proxy code at the EOA's address to simulate EIP-7702 upgrade
5148
vm.etch(_eoa, proxyCode);
5249
}
53-
50+
5451
/**
5552
* @dev Helper to generate initialization signature
5653
* @param signerPk Private key of the signer
@@ -66,15 +63,72 @@ abstract contract EIP7702ProxyBase is Test {
6663
(uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, initHash);
6764
return abi.encodePacked(r, s, v);
6865
}
69-
66+
7067
/**
7168
* @dev Helper to create initialization args with a single owner
7269
* @param owner Address to set as owner
7370
* @return Encoded initialization arguments
7471
*/
75-
function _createInitArgs(address owner) internal pure returns (bytes memory) {
72+
function _createInitArgs(
73+
address owner
74+
) internal pure returns (bytes memory) {
7675
bytes[] memory owners = new bytes[](1);
7776
owners[0] = abi.encode(owner);
7877
return abi.encode(owners);
7978
}
80-
}
79+
80+
function _sign(
81+
uint256 pk,
82+
bytes32 hash
83+
) internal pure returns (bytes memory signature) {
84+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, hash);
85+
return abi.encodePacked(r, s, v);
86+
}
87+
88+
function _createOwnerSignature(
89+
bytes32 message,
90+
address smartWallet,
91+
uint256 ownerPk,
92+
uint256 ownerIndex
93+
) internal view returns (bytes memory) {
94+
bytes32 replaySafeHash = CoinbaseSmartWallet(payable(smartWallet))
95+
.replaySafeHash(message);
96+
bytes memory signature = _sign(ownerPk, replaySafeHash);
97+
bytes memory wrappedSignature = _applySignatureWrapper(
98+
ownerIndex,
99+
signature
100+
);
101+
return wrappedSignature;
102+
}
103+
104+
function _applySignatureWrapper(
105+
uint256 ownerIndex,
106+
bytes memory signatureData
107+
) internal pure returns (bytes memory) {
108+
return
109+
abi.encode(
110+
CoinbaseSmartWallet.SignatureWrapper(ownerIndex, signatureData)
111+
);
112+
}
113+
114+
/**
115+
* @dev Helper to deploy a proxy and etch its code at a target address
116+
* @param target The address where the proxy code should be etched
117+
* @return The target address (for convenience)
118+
*/
119+
function _deployProxy(address target) internal returns (address) {
120+
// Deploy proxy normally first to get the correct immutable values
121+
EIP7702Proxy proxy = new EIP7702Proxy(
122+
address(_implementation),
123+
_initSelector
124+
);
125+
126+
// Get the proxy's runtime code
127+
bytes memory proxyCode = address(proxy).code;
128+
129+
// Etch the proxy code at the target address
130+
vm.etch(target, proxyCode);
131+
132+
return target;
133+
}
134+
}

0 commit comments

Comments
 (0)