Skip to content
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.

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

Custom errors in ERC1271 util + static test for replaySafeHash #32

Merged
merged 1 commit into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions .gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ AddOwnerPublicKeyTest:testRevertsIfAlreadyOwner() (gas: 116339)
AddOwnerPublicKeyTest:testRevertsIfCalledByNonOwner() (gas: 11754)
AddOwnerPublicKeyTest:testSetsIsOwner() (gas: 113794)
AddOwnerPublicKeyTest:testSetsOwnerAtIndex() (gas: 128562)
CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForDeployedAccount() (gas: 308658)
CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForUndeployedAccount() (gas: 291429)
CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForDeployedAccount() (gas: 308670)
CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForUndeployedAccount() (gas: 291699)
CoinbaseSmartWalletFactoryTest:testDeployDeterministicPassValues() (gas: 267742)
CoinbaseSmartWalletFactoryTest:test_CreateAccount_ReturnsPredeterminedAddress_WhenAccountAlreadyExists() (gas: 287406)
CoinbaseSmartWalletFactoryTest:test_createAccountDeploysToPredeterminedAddress() (gas: 269093)
CoinbaseSmartWalletFactoryTest:test_createAccountSetsOwnersCorrectly() (gas: 277981)
CoinbaseSmartWalletFactoryTest:test_implementation_returnsExpectedAddress() (gas: 7676)
CoinbaseSmartWalletFactoryTest:test_revertsIfNoOwners() (gas: 29214)
ERC1271Test:test_returnsExpectedDomainHashWhenProxy() (gas: 15406)
ERC1271Test:test_returnsExpectedDomainHashWhenProxy() (gas: 15384)
ERC1271Test:test_static() (gas: 3057560)
MultiOwnableInitializeTest:testRevertsIfLength32ButLargerThanAddress() (gas: 78927)
MultiOwnableInitializeTest:testRevertsIfLength32NotAddress() (gas: 78908)
MultiOwnableInitializeTest:testRevertsIfLengthNot32Or64() (gas: 101211)
Expand Down
21 changes: 19 additions & 2 deletions src/utils/ERC1271InputGenerator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ import {CoinbaseSmartWallet} from "../CoinbaseSmartWallet.sol";
///
/// @author Coinbase (https://github.com/coinbase/smart-wallet)
contract ERC1271InputGenerator {
/// @notice Thrown when call to `accountFactory` with `factoryCalldata` fails.
error AccountDeploymentFailed();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

want to align with custom errors we use elsewhere


/// @notice Thrown when the address returned from call to `accountFactory` does not
/// match passed account
///
/// @param account The passed account
/// @param returned The returned account
error ReturnedAddressDoesNotMatchAccount(address account, address returned);

/// @notice Computes and returns the expected ERC-1271 replay-safe hash for a CoinbaseSmartWallet.
///
/// @dev `accountFactory` can be any address if the account is already deployed.
Expand Down Expand Up @@ -60,8 +70,15 @@ contract ERC1271InputGenerator {
}

// Deploy the account.
(bool success,) = accountFactory.call(factoryCalldata);
require(success, "CoinbaseSmartWallet1271InputGenerator: deployment");
(bool success, bytes memory result) = accountFactory.call(factoryCalldata);
if (!success) {
revert AccountDeploymentFailed();
}

address returnAddress = abi.decode(result, (address));
if (returnAddress != address(account)) {
revert ReturnedAddressDoesNotMatchAccount(address(account), returnAddress);
}

// Call and return replaySafeHash on the now-deployed account.
return account.replaySafeHash(hash);
Expand Down
19 changes: 19 additions & 0 deletions test/ERC1271.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {Test, console2} from "forge-std/Test.sol";
import "../src/ERC1271.sol";
import "../src/CoinbaseSmartWalletFactory.sol";

import {MockCoinbaseSmartWallet} from "./mocks/MockCoinbaseSmartWallet.sol";

contract ERC1271Test is Test {
CoinbaseSmartWalletFactory factory;
CoinbaseSmartWallet account;
Expand Down Expand Up @@ -33,4 +35,21 @@ contract ERC1271Test is Test {
);
assertEq(expected, account.domainSeparator());
}

/// @dev a test for a static output, for reference with a javascript test out of this repo
function test_static() public {
vm.chainId(84532);

owners.push(
hex"66efa90a7c6a9fe2f4472dc80307116577be940f06f4b81b3cce9207d0d35ebdd420af05337a40c253b6a37144c30ba22bbd54c71af9e4457774d790b34c8227"
);
CoinbaseSmartWallet a = new MockCoinbaseSmartWallet();
vm.etch(0x2Af621c1B01466256393EBA6BF183Ac2962fd98C, address(a).code);
a.initialize(owners);
bytes32 expected = 0x1b03b7e3bddbb2f9b5080f154cf33fcbed9b9cd42c98409fb0730369426a0a69;
bytes32 actual = CoinbaseSmartWallet(payable(0x2Af621c1B01466256393EBA6BF183Ac2962fd98C)).replaySafeHash(
0x9ef3f7124243b092c883252302a74d4ed968efc9f612396f1a82bbeef8931328
);
assertEq(expected, actual);
}
}