From ae70644bcdb6b22357750db2a6d10543ea330be3 Mon Sep 17 00:00:00 2001 From: Wilson Cusack Date: Sun, 10 Mar 2024 19:31:23 -0400 Subject: [PATCH] exploring gas of using oz --- .gas-snapshot | 89 ++++++++++--------- .gitmodules | 5 +- lib/openzeppelin-contracts | 1 + src/CoinbaseSmartWallet.sol | 6 +- src/CoinbaseSmartWalletFactory.sol | 20 ++--- .../ExecuteWithoutChainIdValidation.t.sol | 2 +- .../IsValidSignature.t.sol | 2 + 7 files changed, 65 insertions(+), 60 deletions(-) create mode 160000 lib/openzeppelin-contracts diff --git a/.gas-snapshot b/.gas-snapshot index 224c255..6927069 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,50 +1,51 @@ -AddOwnerAddressTest:testEmitsAddOwner() (gas: 92330) -AddOwnerAddressTest:testIncreasesOwnerIndex() (gas: 90548) -AddOwnerAddressTest:testRevertsIfAlreadyOwner() (gas: 93093) -AddOwnerAddressTest:testRevertsIfCalledByNonOwner() (gas: 11732) -AddOwnerAddressTest:testSetsIsOwner() (gas: 90432) -AddOwnerAddressTest:testSetsOwnerAtIndex() (gas: 98831) -AddOwnerPublicKeyTest:testEmitsAddOwner() (gas: 115377) -AddOwnerPublicKeyTest:testRevertsIfAlreadyOwner() (gas: 116169) -AddOwnerPublicKeyTest:testRevertsIfCalledByNonOwner() (gas: 11656) -AddOwnerPublicKeyTest:testSetsIsOwner() (gas: 113698) -AddOwnerPublicKeyTest:testSetsOwnerAtIndex() (gas: 128444) -CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForDeployedAccount() (gas: 308642) -CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForUndeployedAccount() (gas: 291413) -CoinbaseSmartWalletFactoryTest:testDeployDeterministicPassValues() (gas: 267720) -CoinbaseSmartWalletFactoryTest:test_CreateAccount_ReturnsPredeterminedAddress_WhenAccountAlreadyExists() (gas: 287384) -CoinbaseSmartWalletFactoryTest:test_createAccountDeploysToPredeterminedAddress() (gas: 269071) -CoinbaseSmartWalletFactoryTest:test_createAccountSetsOwnersCorrectly() (gas: 277937) +AddOwnerAddressTest:testEmitsAddOwner() (gas: 92374) +AddOwnerAddressTest:testIncreasesOwnerIndex() (gas: 90614) +AddOwnerAddressTest:testRevertsIfAlreadyOwner() (gas: 93137) +AddOwnerAddressTest:testRevertsIfCalledByNonOwner() (gas: 11754) +AddOwnerAddressTest:testSetsIsOwner() (gas: 90476) +AddOwnerAddressTest:testSetsOwnerAtIndex() (gas: 98897) +AddOwnerPublicKeyTest:testEmitsAddOwner() (gas: 115509) +AddOwnerPublicKeyTest:testFuzzIsOwnerPublicKey(bytes32,bytes32) (runs: 256, μ: 115333, ~: 115333) +AddOwnerPublicKeyTest:testRevertsIfAlreadyOwner() (gas: 116301) +AddOwnerPublicKeyTest:testRevertsIfCalledByNonOwner() (gas: 11744) +AddOwnerPublicKeyTest:testSetsIsOwner() (gas: 113775) +AddOwnerPublicKeyTest:testSetsOwnerAtIndex() (gas: 128543) +CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForDeployedAccount() (gas: 283108) +CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForUndeployedAccount() (gas: 265831) +CoinbaseSmartWalletFactoryTest:testDeployDeterministicPassValues() (gas: 249076) +CoinbaseSmartWalletFactoryTest:test_CreateAccount_ReturnsPredeterminedAddress_WhenAccountAlreadyExists() (gas: 261498) +CoinbaseSmartWalletFactoryTest:test_createAccountDeploysToPredeterminedAddress() (gas: 250379) +CoinbaseSmartWalletFactoryTest:test_createAccountSetsOwnersCorrectly() (gas: 259033) CoinbaseSmartWalletFactoryTest:test_revertsIfNoOwners() (gas: 29214) -ERC1271Test:test_returnsExpectedDomainHashWhenProxy() (gas: 15384) -MultiOwnableInitializeTest:testRevertsIfLength32ButLargerThanAddress() (gas: 78994) -MultiOwnableInitializeTest:testRevertsIfLength32NotAddress() (gas: 78975) -MultiOwnableInitializeTest:testRevertsIfLengthNot32Or64() (gas: 101278) -RemoveOwnerAtIndexTest:testEmitsRemoveOwner() (gas: 33281) -RemoveOwnerAtIndexTest:testRemovesOwner() (gas: 32647) -RemoveOwnerAtIndexTest:testRemovesOwnerAtIndex() (gas: 27532) -RemoveOwnerAtIndexTest:testRevertsIfCalledByNonOwner() (gas: 11310) -RemoveOwnerAtIndexTest:testRevertsIfNoOwnerAtIndex() (gas: 16626) -TestCanSkipChainIdValidation:test_approvedSelectorsReturnTrue() (gas: 15889) -TestCanSkipChainIdValidation:test_otherSelectorsReturnFalse() (gas: 12491) -TestExecuteWithoutChainIdValidation:testExecute() (gas: 424683) -TestExecuteWithoutChainIdValidation:testExecuteBatch() (gas: 729020) -TestExecuteWithoutChainIdValidation:testExecuteBatch(uint256) (runs: 256, μ: 3376320, ~: 3249240) -TestExecuteWithoutChainIdValidation:test__codesize() (gas: 48858) -TestExecuteWithoutChainIdValidation:test__codesize() (gas: 49093) -TestExecuteWithoutChainIdValidation:test_canChangeOwnerWithoutChainId() (gas: 287917) -TestExecuteWithoutChainIdValidation:test_cannotCallExec() (gas: 220047) -TestExecuteWithoutChainIdValidation:test_revertsIfCallerNotEntryPoint() (gas: 8620) +ERC1271Test:test_returnsExpectedDomainHashWhenProxy() (gas: 13194) +MultiOwnableInitializeTest:testRevertsIfLength32ButLargerThanAddress() (gas: 78927) +MultiOwnableInitializeTest:testRevertsIfLength32NotAddress() (gas: 78908) +MultiOwnableInitializeTest:testRevertsIfLengthNot32Or64() (gas: 101211) +RemoveOwnerAtIndexTest:testEmitsRemoveOwner() (gas: 33299) +RemoveOwnerAtIndexTest:testRemovesOwner() (gas: 32682) +RemoveOwnerAtIndexTest:testRemovesOwnerAtIndex() (gas: 27567) +RemoveOwnerAtIndexTest:testRevertsIfCalledByNonOwner() (gas: 11332) +RemoveOwnerAtIndexTest:testRevertsIfNoOwnerAtIndex() (gas: 16648) +TestCanSkipChainIdValidation:test_approvedSelectorsReturnTrue() (gas: 15845) +TestCanSkipChainIdValidation:test_otherSelectorsReturnFalse() (gas: 12469) +TestExecuteWithoutChainIdValidation:testExecute() (gas: 424839) +TestExecuteWithoutChainIdValidation:testExecuteBatch() (gas: 729083) +TestExecuteWithoutChainIdValidation:testExecuteBatch(uint256) (runs: 256, μ: 3663189, ~: 3665261) +TestExecuteWithoutChainIdValidation:test__codesize() (gas: 50164) +TestExecuteWithoutChainIdValidation:test__codesize() (gas: 50399) +TestExecuteWithoutChainIdValidation:test_canChangeOwnerWithoutChainId() (gas: 288312) +TestExecuteWithoutChainIdValidation:test_cannotCallExec() (gas: 220443) +TestExecuteWithoutChainIdValidation:test_revertsIfCallerNotEntryPoint() (gas: 8598) TestExecuteWithoutChainIdValidation:test_revertsIfWrongNonceKey() (gas: 62275) TestExecuteWithoutChainIdValidation:test_revertsWithReservedNonce() (gas: 82302) TestInitialize:testInitialize() (gas: 21122) -TestInitialize:test_cannotInitImplementation() (gas: 2678562) -TestIsValidSignature:testReturnsInvalidIfPasskeySigButWrongOwnerLength() (gas: 39464) +TestInitialize:test_cannotInitImplementation() (gas: 2935795) +TestIsValidSignature:testReturnsInvalidIfPasskeySigButWrongOwnerLength() (gas: 42452) TestIsValidSignature:testRevertsIfEthereumSignatureButWrongOwnerLength() (gas: 24046) -TestIsValidSignature:testSmartWalletSigner() (gas: 2951388) -TestIsValidSignature:testValidateSignatureWithEOASigner() (gas: 24917) -TestIsValidSignature:testValidateSignatureWithEOASignerFailsWithWrongSigner() (gas: 23847) -TestIsValidSignature:testValidateSignatureWithPasskeySigner() (gas: 421251) +TestIsValidSignature:testSmartWalletSigner() (gas: 3214255) +TestIsValidSignature:testValidateSignatureWithEOASigner() (gas: 25347) +TestIsValidSignature:testValidateSignatureWithEOASignerFailsWithWrongSigner() (gas: 25481) +TestIsValidSignature:testValidateSignatureWithPasskeySigner() (gas: 421262) TestIsValidSignature:testValidateSignatureWithPasskeySignerFailsBadOwnerIndex() (gas: 34964) -TestIsValidSignature:testValidateSignatureWithPasskeySignerFailsWithWrongBadSignature() (gas: 428717) -TestValidateUserOp:testValidateUserOp() (gas: 447113) \ No newline at end of file +TestIsValidSignature:testValidateSignatureWithPasskeySignerFailsWithWrongBadSignature() (gas: 428728) +TestValidateUserOp:testValidateUserOp() (gas: 449222) \ No newline at end of file diff --git a/.gitmodules b/.gitmodules index 147bd38..695bf8a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -6,7 +6,7 @@ url = https://github.com/daimo-eth/p256-verifier [submodule "lib/account-abstraction"] path = lib/account-abstraction - url = https://github.com/eth-infinitism/account-abstraction + url = https://github.com/eth-finitism/account-abstraction [submodule "lib/FreshCryptoLib"] path = lib/FreshCryptoLib url = https://github.com/rdubois-crypto/FreshCryptoLib @@ -16,3 +16,6 @@ [submodule "lib/webauthn-sol"] path = lib/webauthn-sol url = https://github.com/base-org/webauthn-sol +[submodule "lib/openzeppelin-contracts"] + path = lib/openzeppelin-contracts + url = https://github.com/openzeppelin/openzeppelin-contracts diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts new file mode 160000 index 0000000..dbb6104 --- /dev/null +++ b/lib/openzeppelin-contracts @@ -0,0 +1 @@ +Subproject commit dbb6104ce834628e473d2173bbc9d47f81a9eec3 diff --git a/src/CoinbaseSmartWallet.sol b/src/CoinbaseSmartWallet.sol index 4556be1..545a488 100644 --- a/src/CoinbaseSmartWallet.sol +++ b/src/CoinbaseSmartWallet.sol @@ -2,8 +2,8 @@ pragma solidity 0.8.23; import {Receiver} from "solady/accounts/Receiver.sol"; -import {UUPSUpgradeable} from "solady/utils/UUPSUpgradeable.sol"; -import {SignatureCheckerLib} from "solady/utils/SignatureCheckerLib.sol"; +import {UUPSUpgradeable} from "openzeppelin-contracts/contracts/proxy/utils/UUPSUpgradeable.sol"; +import {SignatureChecker} from "openzeppelin-contracts/contracts/utils/cryptography/SignatureChecker.sol"; import {UserOperation, UserOperationLib} from "account-abstraction/interfaces/UserOperation.sol"; import {WebAuthn} from "webauthn-sol/WebAuthn.sol"; @@ -302,7 +302,7 @@ contract CoinbaseSmartWallet is MultiOwnable, UUPSUpgradeable, Receiver, ERC1271 owner := mload(add(ownerBytes, 32)) } - return SignatureCheckerLib.isValidSignatureNow(owner, message, sigWrapper.signatureData); + return SignatureChecker.isValidSignatureNow(owner, message, sigWrapper.signatureData); } if (ownerBytes.length == 64) { diff --git a/src/CoinbaseSmartWalletFactory.sol b/src/CoinbaseSmartWalletFactory.sol index 2251cba..2c8dae6 100644 --- a/src/CoinbaseSmartWalletFactory.sol +++ b/src/CoinbaseSmartWalletFactory.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.4; -import {LibClone} from "solady/utils/LibClone.sol"; +import {Clones} from "openzeppelin-contracts/contracts/proxy/Clones.sol"; import {CoinbaseSmartWallet} from "./CoinbaseSmartWallet.sol"; /// @title Coinbase Smart Wallet Factory @@ -31,24 +31,22 @@ contract CoinbaseSmartWalletFactory { revert OwnerRequired(); } - (bool alreadyDeployed, address accountAddress) = - LibClone.createDeterministicERC1967(msg.value, implementation, _getSalt(owners, nonce)); + bytes32 salt = _getSalt(owners, nonce); + address accountAddress = Clones.predictDeterministicAddress(implementation, salt, address(this)); account = CoinbaseSmartWallet(payable(accountAddress)); - if (alreadyDeployed == false) { - account.initialize(owners); + if (address(account).code.length > 0) { + return account; } + + Clones.cloneDeterministic(implementation, salt); + account.initialize{value: msg.value}(owners); } /// @dev Returns the deterministic address of the account created via `createAccount`. function getAddress(bytes[] calldata owners, uint256 nonce) external view returns (address predicted) { - predicted = LibClone.predictDeterministicAddress(initCodeHash(), _getSalt(owners, nonce), address(this)); - } - - /// @dev Returns the initialization code hash of the ERC4337 account (a minimal ERC1967 proxy). - function initCodeHash() public view virtual returns (bytes32 result) { - result = LibClone.initCodeHashERC1967(implementation); + predicted = Clones.predictDeterministicAddress(implementation, _getSalt(owners, nonce), address(this)); } /// @dev Returns the salt that will be used for deterministic address diff --git a/test/CoinbaseSmartWallet/ExecuteWithoutChainIdValidation.t.sol b/test/CoinbaseSmartWallet/ExecuteWithoutChainIdValidation.t.sol index 3c2084d..9c1e2c3 100644 --- a/test/CoinbaseSmartWallet/ExecuteWithoutChainIdValidation.t.sol +++ b/test/CoinbaseSmartWallet/ExecuteWithoutChainIdValidation.t.sol @@ -42,7 +42,7 @@ contract TestExecuteWithoutChainIdValidation is SmartWalletTestBase { UserOperation memory userOp = _getUserOpWithSignature(); vm.expectEmit(true, true, true, true); emit IEntryPoint.UserOperationEvent( - entryPoint.getUserOpHash(userOp), userOp.sender, address(0), userOp.nonce, false, 0, 48027 + entryPoint.getUserOpHash(userOp), userOp.sender, address(0), userOp.nonce, false, 0, 48423 ); _sendUserOperation(userOp); } diff --git a/test/CoinbaseSmartWallet/IsValidSignature.t.sol b/test/CoinbaseSmartWallet/IsValidSignature.t.sol index 156934c..f9a80d4 100644 --- a/test/CoinbaseSmartWallet/IsValidSignature.t.sol +++ b/test/CoinbaseSmartWallet/IsValidSignature.t.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +import {SignatureCheckerLib} from "solady/utils/SignatureCheckerLib.sol"; + import "./SmartWalletTestBase.sol"; import "webauthn-sol/../test/Utils.sol";