Skip to content

Commit c899b94

Browse files
amiecorsoamiecorso
committed
cleanup
1 parent 458fd89 commit c899b94

File tree

6 files changed

+160
-104
lines changed

6 files changed

+160
-104
lines changed

‎test/EIP7702Proxy/coinbaseImplementation.t.sol‎

Lines changed: 66 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ import {EIP7702Proxy} from "../../src/EIP7702Proxy.sol";
66
import {CoinbaseSmartWallet} from "../../lib/smart-wallet/src/CoinbaseSmartWallet.sol";
77
import {ECDSA} from "openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol";
88

9+
/**
10+
* @title CoinbaseImplementationTest
11+
* @dev Tests specific to the CoinbaseSmartWallet implementation
12+
*/
913
contract CoinbaseImplementationTest is Test {
1014
uint256 constant _EOA_PRIVATE_KEY = 0xA11CE;
1115
address payable _eoa;
@@ -44,6 +48,11 @@ contract CoinbaseImplementationTest is Test {
4448
}
4549

4650
// ======== Utility Functions ========
51+
/**
52+
* @dev Creates initialization arguments for CoinbaseSmartWallet
53+
* @param owner Address to set as the initial owner
54+
* @return Encoded initialization arguments for CoinbaseSmartWallet
55+
*/
4756
function _createInitArgs(
4857
address owner
4958
) internal pure returns (bytes memory) {
@@ -52,16 +61,72 @@ contract CoinbaseImplementationTest is Test {
5261
return abi.encode(owners);
5362
}
5463

64+
/**
65+
* @dev Signs initialization data for CoinbaseSmartWallet that will be verified by the proxy
66+
* @param signerPk Private key of the signer
67+
* @param initArgs Initialization arguments to sign
68+
* @return Signature bytes
69+
*/
5570
function _signInitData(
5671
uint256 signerPk,
5772
bytes memory initArgs
5873
) internal view returns (bytes memory) {
59-
// Use the EOA address in the hash since that's where our proxy lives
6074
bytes32 initHash = keccak256(abi.encode(proxy, initArgs));
6175
(uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, initHash);
6276
return abi.encodePacked(r, s, v);
6377
}
6478

79+
/**
80+
* @dev Helper to create ECDSA signatures
81+
* @param pk Private key to sign with
82+
* @param hash Message hash to sign
83+
* @return signature Encoded signature bytes
84+
*/
85+
function _sign(
86+
uint256 pk,
87+
bytes32 hash
88+
) internal pure returns (bytes memory signature) {
89+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, hash);
90+
return abi.encodePacked(r, s, v);
91+
}
92+
93+
/**
94+
* @dev Creates a signature from a wallet owner for CoinbaseSmartWallet validation
95+
* @param message Message to sign
96+
* @param smartWallet Address of the wallet contract
97+
* @param ownerPk Private key of the owner
98+
* @param ownerIndex Index of the owner in the wallet's owner list
99+
* @return Wrapped signature bytes
100+
*/
101+
function _createOwnerSignature(
102+
bytes32 message,
103+
address smartWallet,
104+
uint256 ownerPk,
105+
uint256 ownerIndex
106+
) internal view returns (bytes memory) {
107+
bytes32 replaySafeHash = CoinbaseSmartWallet(payable(smartWallet))
108+
.replaySafeHash(message);
109+
bytes memory signature = _sign(ownerPk, replaySafeHash);
110+
return _applySignatureWrapper(ownerIndex, signature);
111+
}
112+
113+
/**
114+
* @dev Wraps a signature with owner index for CoinbaseSmartWallet validation
115+
* @param ownerIndex Index of the owner in the wallet's owner list
116+
* @param signatureData Raw signature bytes to wrap
117+
* @return Encoded signature wrapper
118+
*/
119+
function _applySignatureWrapper(
120+
uint256 ownerIndex,
121+
bytes memory signatureData
122+
) internal pure returns (bytes memory) {
123+
return
124+
abi.encode(
125+
CoinbaseSmartWallet.SignatureWrapper(ownerIndex, signatureData)
126+
);
127+
}
128+
129+
// ======== Tests ========
65130
function testCoinbaseInitializeSetsOwner() public {
66131
assertTrue(
67132
wallet.isOwnerAddress(_newOwner),
@@ -132,36 +197,4 @@ contract CoinbaseImplementationTest is Test {
132197
vm.expectRevert(CoinbaseSmartWallet.Initialized.selector);
133198
EIP7702Proxy(_eoa).initialize(initArgs, signature);
134199
}
135-
136-
// ======== Utility Functions ========
137-
138-
function _sign(
139-
uint256 pk,
140-
bytes32 hash
141-
) internal pure returns (bytes memory signature) {
142-
(uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, hash);
143-
return abi.encodePacked(r, s, v);
144-
}
145-
146-
function _createOwnerSignature(
147-
bytes32 message,
148-
address smartWallet,
149-
uint256 ownerPk,
150-
uint256 ownerIndex
151-
) internal view returns (bytes memory) {
152-
bytes32 replaySafeHash = CoinbaseSmartWallet(payable(smartWallet))
153-
.replaySafeHash(message);
154-
bytes memory signature = _sign(ownerPk, replaySafeHash);
155-
return _applySignatureWrapper(ownerIndex, signature);
156-
}
157-
158-
function _applySignatureWrapper(
159-
uint256 ownerIndex,
160-
bytes memory signatureData
161-
) internal pure returns (bytes memory) {
162-
return
163-
abi.encode(
164-
CoinbaseSmartWallet.SignatureWrapper(ownerIndex, signatureData)
165-
);
166-
}
167200
}

‎test/EIP7702Proxy/initialize.t.sol‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ pragma solidity ^0.8.23;
33

44
import {EIP7702ProxyBase} from "../base/EIP7702ProxyBase.sol";
55
import {EIP7702Proxy} from "../../src/EIP7702Proxy.sol";
6-
import {MockImplementation, RevertingInitializerMockImplementation} from "../mocks/MockImplementation.sol";
6+
import {MockImplementation} from "../mocks/MockImplementation.sol";
77
import {ECDSA} from "openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol";
88

99
contract InitializeTest is EIP7702ProxyBase {
@@ -13,14 +13,37 @@ contract InitializeTest is EIP7702ProxyBase {
1313

1414
EIP7702Proxy(_eoa).initialize(initArgs, signature);
1515

16-
// Verify initialization through implementation
16+
// Verify implementation was set
1717
assertEq(
18-
MockImplementation(payable(_eoa)).owner(),
19-
_newOwner,
20-
"Owner should be set after initialization"
18+
_getERC1967Implementation(address(_eoa)),
19+
address(_implementation),
20+
"Implementation should be set after initialization"
21+
);
22+
}
23+
24+
function testSetsERC1967ImplementationSlot() public {
25+
bytes memory initArgs = _createInitArgs(_newOwner);
26+
bytes memory signature = _signInitData(_EOA_PRIVATE_KEY, initArgs);
27+
28+
EIP7702Proxy(_eoa).initialize(initArgs, signature);
29+
30+
address storedImpl = _getERC1967Implementation(address(_eoa));
31+
assertEq(
32+
storedImpl,
33+
address(_implementation),
34+
"ERC1967 implementation slot should store implementation address"
2135
);
2236
}
2337

38+
function testEmitsUpgradedEvent() public {
39+
bytes memory initArgs = _createInitArgs(_newOwner);
40+
bytes memory signature = _signInitData(_EOA_PRIVATE_KEY, initArgs);
41+
42+
vm.expectEmit(true, false, false, false, address(_eoa));
43+
emit EIP7702Proxy.Upgraded(address(_implementation));
44+
EIP7702Proxy(_eoa).initialize(initArgs, signature);
45+
}
46+
2447
function testRevertsWithInvalidSignatureLength() public {
2548
bytes memory initArgs = _createInitArgs(_newOwner);
2649
bytes memory signature = hex"deadbeef"; // Too short to be valid ECDSA signature
@@ -50,27 +73,4 @@ contract InitializeTest is EIP7702ProxyBase {
5073
vm.expectRevert(EIP7702Proxy.InvalidSignature.selector);
5174
EIP7702Proxy(_eoa).initialize(initArgs, signature);
5275
}
53-
54-
function testSetsERC1967ImplementationSlot() public {
55-
bytes memory initArgs = _createInitArgs(_newOwner);
56-
bytes memory signature = _signInitData(_EOA_PRIVATE_KEY, initArgs);
57-
58-
EIP7702Proxy(_eoa).initialize(initArgs, signature);
59-
60-
address storedImpl = _getERC1967Implementation(address(_eoa));
61-
assertEq(
62-
storedImpl,
63-
address(_implementation),
64-
"ERC1967 implementation slot should store implementation address"
65-
);
66-
}
67-
68-
function testEmitsUpgradedEvent() public {
69-
bytes memory initArgs = _createInitArgs(_newOwner);
70-
bytes memory signature = _signInitData(_EOA_PRIVATE_KEY, initArgs);
71-
72-
vm.expectEmit(true, false, false, false, address(_eoa));
73-
emit EIP7702Proxy.Upgraded(address(_implementation));
74-
EIP7702Proxy(_eoa).initialize(initArgs, signature);
75-
}
7676
}

‎test/EIP7702Proxy/isValidSignature.t.sol‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ import {EIP7702Proxy} from "../../src/EIP7702Proxy.sol";
66
import {MockImplementation, FailingSignatureImplementation} from "../mocks/MockImplementation.sol";
77
import {ECDSA} from "openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol";
88

9+
/**
10+
* @title IsValidSignatureTestBase
11+
* @dev Base contract for testing ERC-1271 isValidSignature behavior
12+
*/
913
abstract contract IsValidSignatureTestBase is EIP7702ProxyBase {
1014
bytes4 constant ERC1271_MAGIC_VALUE = 0x1626ba7e;
1115
bytes4 constant ERC1271_FAIL_VALUE = 0xffffffff;
@@ -45,18 +49,24 @@ abstract contract IsValidSignatureTestBase is EIP7702ProxyBase {
4549
assertEq(
4650
result,
4751
expectedInvalidSignatureResult(),
48-
"Should handle invalid EOA signature correctly"
52+
"Should handle invalid signature according to whether `isValidSignature` succeeds or fails"
4953
);
5054
}
5155

52-
// Abstract function that each implementation test must define
56+
/**
57+
* @dev Abstract function that each implementation test must define
58+
* @return Expected result for invalid signature tests
59+
*/
5360
function expectedInvalidSignatureResult()
5461
internal
5562
pure
5663
virtual
5764
returns (bytes4);
5865
}
5966

67+
/**
68+
* @dev Tests isValidSignature behavior with failing implementation
69+
*/
6070
contract FailingImplementationTest is IsValidSignatureTestBase {
6171
function setUp() public override {
6272
// Override base setup to use FailingSignatureImplementation
@@ -99,6 +109,9 @@ contract FailingImplementationTest is IsValidSignatureTestBase {
99109
}
100110
}
101111

112+
/**
113+
* @dev Tests isValidSignature behavior with succeeding implementation
114+
*/
102115
contract SucceedingImplementationTest is IsValidSignatureTestBase {
103116
function setUp() public override {
104117
// Override base implementation with standard MockImplementation (always succeeds)
@@ -140,7 +153,7 @@ contract SucceedingImplementationTest is IsValidSignatureTestBase {
140153
assertEq(
141154
result,
142155
ERC1271_MAGIC_VALUE,
143-
"Should return success for any EOA signature if implementation `isValidSignature` always succeeds"
156+
"Should return success for any EOA signature if implementation since `isValidSignature` always succeeds"
144157
);
145158
}
146159
}

‎test/EIP7702Proxy/upgradeToAndCall.t.sol‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@ pragma solidity ^0.8.23;
33

44
import {EIP7702ProxyBase} from "../base/EIP7702ProxyBase.sol";
55
import {EIP7702Proxy} from "../../src/EIP7702Proxy.sol";
6-
import {MockImplementation, RevertingMockImplementation} from "../mocks/MockImplementation.sol";
6+
import {MockImplementation} from "../mocks/MockImplementation.sol";
77

8+
/**
9+
* @title UpgradeToAndCallTest
10+
* @dev Tests upgradeability functionality of EIP7702Proxy
11+
*/
812
contract UpgradeToAndCallTest is EIP7702ProxyBase {
913
MockImplementation newImplementation;
1014

@@ -46,7 +50,7 @@ contract UpgradeToAndCallTest is EIP7702ProxyBase {
4650

4751
function testUpgradeToAndCall_revertsForNonOwner() public {
4852
vm.prank(address(0xBAD));
49-
vm.expectRevert("Unauthorized"); // From MockImplementation
53+
vm.expectRevert(MockImplementation.Unauthorized.selector); // From MockImplementation
5054
MockImplementation(payable(_eoa)).upgradeToAndCall(
5155
address(newImplementation),
5256
""

‎test/base/EIP7702ProxyBase.sol‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,34 +9,31 @@ import {ECDSA} from "openzeppelin-contracts/contracts/utils/cryptography/ECDSA.s
99
/**
1010
* @title EIP7702ProxyBase
1111
* @dev Base contract containing shared setup and utilities for EIP7702Proxy tests.
12-
* This contract should not contain any actual tests.
1312
*/
1413
abstract contract EIP7702ProxyBase is Test {
15-
// Add ERC1967 implementation slot constant
14+
/// @dev Storage slot with the address of the current implementation (ERC1967)
1615
bytes32 internal constant IMPLEMENTATION_SLOT =
1716
0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
1817

19-
// Test accounts
18+
/// @dev Test account private keys and addresses
2019
uint256 internal constant _EOA_PRIVATE_KEY = 0xA11CE;
2120
address payable internal _eoa;
2221

2322
uint256 internal constant _NEW_OWNER_PRIVATE_KEY = 0xB0B;
2423
address payable internal _newOwner;
2524

26-
// Contracts
25+
/// @dev Core contract instances
2726
EIP7702Proxy internal _proxy;
2827
MockImplementation internal _implementation;
2928

30-
// Common test data
29+
/// @dev Function selector for initialization
3130
bytes4 internal _initSelector;
3231

3332
function setUp() public virtual {
3433
// Set up test accounts
3534
_eoa = payable(vm.addr(_EOA_PRIVATE_KEY));
36-
vm.deal(_eoa, 100 ether);
3735

3836
_newOwner = payable(vm.addr(_NEW_OWNER_PRIVATE_KEY));
39-
vm.deal(_newOwner, 100 ether);
4037

4138
// Deploy implementation
4239
_implementation = new MockImplementation();
@@ -62,7 +59,6 @@ abstract contract EIP7702ProxyBase is Test {
6259
uint256 signerPk,
6360
bytes memory initArgs
6461
) internal view returns (bytes memory) {
65-
// Use the EOA address in the hash since that's where our proxy lives
6662
bytes32 initHash = keccak256(abi.encode(_proxy, initArgs));
6763
(uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, initHash);
6864
return abi.encodePacked(r, s, v);

0 commit comments

Comments
 (0)