Move EIP712 dependency from AccountCore to Account (#58)

Co-authored-by: Ernesto García <[email protected]>

Author: Amxx

CHANGELOG.md

@@ -12,7 +12,7 @@
 - `AccountCore`: Added a simple ERC-4337 account implementation with minimal logic to process user operations.
 - `Account`: Extensions of AccountCore with recommended features that most accounts should have.
 - `AbstractSigner`, `SignerECDSA`, `SignerP256`, and `SignerRSA`: Add an abstract contract, and various implementations, for contracts that deal with signature verification. Used by AccountCore and `ERC7739Utils.
-- `AccountSignerERC7702`: Implementation of `AbstractSigner` for ERC-7702 compatible accounts.
+- `SignerERC7702`: Implementation of `AbstractSigner` for Externally Owned Accounts (EOAs). Useful with ERC-7702.
 
 ## 13-12-2024
 

contracts/account/Account.sol

@@ -2,6 +2,8 @@
 
 pragma solidity ^0.8.20;
 
+import {PackedUserOperation} from "@openzeppelin/contracts/interfaces/draft-IERC4337.sol";
+import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 import {ERC721Holder} from "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";
 import {ERC1155Holder} from "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol";
 import {ERC7739} from "../utils/cryptography/ERC7739.sol";
@@ -18,7 +20,39 @@ import {AccountCore} from "./AccountCore.sol";
  * NOTE: To use this contract, the {ERC7739-_rawSignatureValidation} function must be
  * implemented using a specific signature verification algorithm. See {SignerECDSA}, {SignerP256} or {SignerRSA}.
  */
-abstract contract Account is AccountCore, ERC721Holder, ERC1155Holder, ERC7739, ERC7821 {
+abstract contract Account is AccountCore, EIP712, ERC721Holder, ERC1155Holder, ERC7739, ERC7821 {
+    bytes32 internal constant _PACKED_USER_OPERATION =
+        keccak256(
+            "PackedUserOperation(address sender,uint256 nonce,bytes initCode,bytes callData,bytes32 accountGasLimits,uint256 preVerificationGas,bytes32 gasFees,bytes paymasterAndData)"
+        );
+
+    /**
+     * @dev Specialization of {AccountCore-_signableUserOpHash} that returns a typehash following EIP-712 typed data
+     * hashing for readability. This assumes the underlying signature scheme implements `signTypedData`, which will be
+     * the case when combined with {SignerECDSA} or {SignerERC7702}.
+     */
+    function _signableUserOpHash(
+        PackedUserOperation calldata userOp,
+        bytes32 /*userOpHash*/
+    ) internal view virtual override returns (bytes32) {
+        return
+            _hashTypedDataV4(
+                keccak256(
+                    abi.encode(
+                        _PACKED_USER_OPERATION,
+                        userOp.sender,
+                        userOp.nonce,
+                        keccak256(userOp.initCode),
+                        keccak256(userOp.callData),
+                        userOp.accountGasLimits,
+                        userOp.preVerificationGas,
+                        userOp.gasFees,
+                        keccak256(userOp.paymasterAndData)
+                    )
+                )
+            );
+    }
+
     /// @inheritdoc ERC7821
     function _erc7821AuthorizedExecutor(
         address caller,

contracts/account/AccountCore.sol

@@ -4,16 +4,14 @@ pragma solidity ^0.8.20;
 
 import {PackedUserOperation, IAccount, IEntryPoint} from "@openzeppelin/contracts/interfaces/draft-IERC4337.sol";
 import {ERC4337Utils} from "@openzeppelin/contracts/account/utils/draft-ERC4337Utils.sol";
-import {Address} from "@openzeppelin/contracts/utils/Address.sol";
-import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
-import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 import {AbstractSigner} from "../utils/cryptography/AbstractSigner.sol";
 
 /**
  * @dev A simple ERC4337 account implementation. This base implementation only includes the minimal logic to process
  * user operations.
  *
- * Developers must implement the {AbstractSigner-_rawSignatureValidation} function to define the account's validation logic.
+ * Developers must implement the {AccountCore-_signableUserOpHash} and {AbstractSigner-_rawSignatureValidation}
+ * functions to define the account's validation logic.
  *
  * NOTE: This core account doesn't include any mechanism for performing arbitrary external calls. This is an essential
  * feature that all Account should have. We leave it up to the developers to implement the mechanism of their choice.
@@ -23,14 +21,7 @@ import {AbstractSigner} from "../utils/cryptography/AbstractSigner.sol";
  * attacker to bypass the account's security measures. Check out {SignerECDSA}, {SignerP256}, or {SignerRSA} for
  * digital signature validation implementations.
  */
-abstract contract AccountCore is AbstractSigner, EIP712, IAccount {
-    using MessageHashUtils for bytes32;
-
-    bytes32 internal constant _PACKED_USER_OPERATION =
-        keccak256(
-            "PackedUserOperation(address sender,uint256 nonce,bytes initCode,bytes callData,bytes32 accountGasLimits,uint256 preVerificationGas,bytes32 gasFees,bytes paymasterAndData)"
-        );
-
+abstract contract AccountCore is AbstractSigner, IAccount {
     /**
      * @dev Unauthorized call to the account.
      */
@@ -89,34 +80,14 @@ abstract contract AccountCore is AbstractSigner, EIP712, IAccount {
     }
 
     /**
-     * @dev Returns the digest used by an offchain signer instead of the opaque `userOpHash`.
-     *
-     * Given the `userOpHash` calculation is defined by ERC-4337, offchain signers
-     * may need to sign again this hash by rehashing it with other schemes (e.g. ERC-191).
-     *
-     * Returns a typehash following EIP-712 typed data hashing for readability.
+     * @dev Virtual function that returns the signable hash for a user operations. Some implementation may return
+     * `userOpHash` while other may prefer a signer-friendly value such as an EIP-712 hash describing the `userOp`
+     * details.
      */
     function _signableUserOpHash(
         PackedUserOperation calldata userOp,
-        bytes32 /* userOpHash */
-    ) internal view virtual returns (bytes32) {
-        return
-            _hashTypedDataV4(
-                keccak256(
-                    abi.encode(
-                        _PACKED_USER_OPERATION,
-                        userOp.sender,
-                        userOp.nonce,
-                        keccak256(userOp.initCode),
-                        keccak256(userOp.callData),
-                        userOp.accountGasLimits,
-                        userOp.preVerificationGas,
-                        userOp.gasFees,
-                        keccak256(userOp.paymasterAndData)
-                    )
-                )
-            );
-    }
+        bytes32 userOpHash
+    ) internal view virtual returns (bytes32);
 
     /**
      * @dev Sends the missing funds for executing the user operation to the {entrypoint}.

contracts/account/README.adoc

@@ -17,6 +17,6 @@ This directory includes contracts to build accounts for ERC-4337. These include:
 
 == Extensions
 
-{{AccountSignerERC7702}}
+{{SignerERC7702}}
 
 {{ERC7821}}

contracts/mocks/account/AccountERC7702Mock.sol

@@ -3,6 +3,6 @@
 pragma solidity ^0.8.20;
 
 import {Account} from "../../account/Account.sol";
-import {AccountSignerERC7702} from "../../account/extensions/AccountSignerERC7702.sol";
+import {SignerERC7702} from "../../utils/cryptography/SignerERC7702.sol";
 
-abstract contract AccountERC7702Mock is Account, AccountSignerERC7702 {}
+abstract contract AccountERC7702Mock is Account, SignerERC7702 {}

contracts/account/extensions/AccountSignerERC7702.sol renamed to contracts/utils/cryptography/SignerERC7702.sol

@@ -3,12 +3,12 @@
 pragma solidity ^0.8.20;
 
 import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
-import {AccountCore} from "../AccountCore.sol";
+import {AbstractSigner} from "./AbstractSigner.sol";
 
 /**
- * @dev {Account} implementation whose low-level signature validation is done by an EOA.
+ * @dev Implementation of {AbstractSigner} for implementation for an EOA. Useful for ERC-7702 accounts.
  */
-abstract contract AccountSignerERC7702 is AccountCore {
+abstract contract SignerERC7702 is AbstractSigner {
     /**
      * @dev Validates the signature using the EOA's address (ie. `address(this)`).
      */

test/account/Account.behavior.js

@@ -4,7 +4,6 @@ const { setBalance } = require('@nomicfoundation/hardhat-network-helpers');
 
 const { impersonate } = require('@openzeppelin/contracts/test/helpers/account');
 const { SIG_VALIDATION_SUCCESS, SIG_VALIDATION_FAILURE } = require('@openzeppelin/contracts/test/helpers/erc4337');
-const { CALL_TYPE_BATCH, encodeMode, encodeBatch } = require('@openzeppelin/contracts/test/helpers/erc7579');
 const {
   shouldSupportInterfaces,
 } = require('@openzeppelin/contracts/test/utils/introspection/SupportsInterface.behavior');
@@ -149,145 +148,7 @@ function shouldBehaveLikeAccountHolder() {
   });
 }
 
-function shouldBehaveLikeAccountERC7821({ deployable = true } = {}) {
-  describe('execute', function () {
-    beforeEach(async function () {
-      // give eth to the account (before deployment)
-      await setBalance(this.mock.target, ethers.parseEther('1'));
-
-      // account is not initially deployed
-      await expect(ethers.provider.getCode(this.mock)).to.eventually.equal('0x');
-
-      this.encodeUserOpCalldata = (...calls) =>
-        this.mock.interface.encodeFunctionData('execute', [
-          encodeMode({ callType: CALL_TYPE_BATCH }),
-          encodeBatch(...calls),
-        ]);
-    });
-
-    it('should revert if the caller is not the canonical entrypoint or the account itself', async function () {
-      await this.mock.deploy();
-
-      await expect(
-        this.mock.connect(this.other).execute(
-          encodeMode({ callType: CALL_TYPE_BATCH }),
-          encodeBatch({
-            target: this.target,
-            data: this.target.interface.encodeFunctionData('mockFunctionExtra'),
-          }),
-        ),
-      )
-        .to.be.revertedWithCustomError(this.mock, 'AccountUnauthorized')
-        .withArgs(this.other);
-    });
-
-    if (deployable) {
-      describe('when not deployed', function () {
-        it('should be created with handleOps and increase nonce', async function () {
-          const operation = await this.mock
-            .createUserOp({
-              callData: this.encodeUserOpCalldata({
-                target: this.target,
-                value: 17,
-                data: this.target.interface.encodeFunctionData('mockFunctionExtra'),
-              }),
-            })
-            .then(op => op.addInitCode())
-            .then(op => this.signUserOp(op));
-
-          // Can't call the account to get its nonce before it's deployed
-          await expect(entrypoint.getNonce(this.mock.target, 0)).to.eventually.equal(0);
-          await expect(entrypoint.handleOps([operation.packed], this.beneficiary))
-            .to.emit(entrypoint, 'AccountDeployed')
-            .withArgs(operation.hash(), this.mock, this.factory, ethers.ZeroAddress)
-            .to.emit(this.target, 'MockFunctionCalledExtra')
-            .withArgs(this.mock, 17);
-          await expect(this.mock.getNonce()).to.eventually.equal(1);
-        });
-
-        it('should revert if the signature is invalid', async function () {
-          const operation = await this.mock
-            .createUserOp({
-              callData: this.encodeUserOpCalldata({
-                target: this.target,
-                value: 17,
-                data: this.target.interface.encodeFunctionData('mockFunctionExtra'),
-              }),
-            })
-            .then(op => op.addInitCode());
-
-          operation.signature = '0x00';
-
-          await expect(entrypoint.handleOps([operation.packed], this.beneficiary)).to.be.reverted;
-        });
-      });
-    }
-
-    describe('when deployed', function () {
-      beforeEach(async function () {
-        await this.mock.deploy();
-      });
-
-      it('should increase nonce and call target', async function () {
-        const operation = await this.mock
-          .createUserOp({
-            callData: this.encodeUserOpCalldata({
-              target: this.target,
-              value: 42,
-              data: this.target.interface.encodeFunctionData('mockFunctionExtra'),
-            }),
-          })
-          .then(op => this.signUserOp(op));
-
-        await expect(this.mock.getNonce()).to.eventually.equal(0);
-        await expect(entrypoint.handleOps([operation.packed], this.beneficiary))
-          .to.emit(this.target, 'MockFunctionCalledExtra')
-          .withArgs(this.mock, 42);
-        await expect(this.mock.getNonce()).to.eventually.equal(1);
-      });
-
-      it('should support sending eth to an EOA', async function () {
-        const operation = await this.mock
-          .createUserOp({ callData: this.encodeUserOpCalldata({ target: this.other, value }) })
-          .then(op => this.signUserOp(op));
-
-        await expect(this.mock.getNonce()).to.eventually.equal(0);
-        await expect(entrypoint.handleOps([operation.packed], this.beneficiary)).to.changeEtherBalance(
-          this.other,
-          value,
-        );
-        await expect(this.mock.getNonce()).to.eventually.equal(1);
-      });
-
-      it('should support batch execution', async function () {
-        const value1 = 43374337n;
-        const value2 = 69420n;
-
-        const operation = await this.mock
-          .createUserOp({
-            callData: this.encodeUserOpCalldata(
-              { target: this.other, value: value1 },
-              {
-                target: this.target,
-                value: value2,
-                data: this.target.interface.encodeFunctionData('mockFunctionExtra'),
-              },
-            ),
-          })
-          .then(op => this.signUserOp(op));
-
-        await expect(this.mock.getNonce()).to.eventually.equal(0);
-        const tx = entrypoint.handleOps([operation.packed], this.beneficiary);
-        await expect(tx).to.changeEtherBalances([this.other, this.target], [value1, value2]);
-        await expect(tx).to.emit(this.target, 'MockFunctionCalledExtra').withArgs(this.mock, value2);
-        await expect(this.mock.getNonce()).to.eventually.equal(1);
-      });
-    });
-  });
-}
-
 module.exports = {
   shouldBehaveLikeAccountCore,
   shouldBehaveLikeAccountHolder,
-  shouldBehaveLikeAccountERC7821,
 };

test/account/Account.test.js

@@ -3,11 +3,8 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 const { ERC4337Helper } = require('../helpers/erc4337');
 const { NonNativeSigner } = require('../helpers/signers');
 
-const {
-  shouldBehaveLikeAccountCore,
-  shouldBehaveLikeAccountERC7821,
-  shouldBehaveLikeAccountHolder,
-} = require('./Account.behavior');
+const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
+const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior');
 
 async function fixture() {
   // EOAs and environment
@@ -36,6 +33,6 @@ describe('Account', function () {
   });
 
   shouldBehaveLikeAccountCore();
-  shouldBehaveLikeAccountERC7821();
   shouldBehaveLikeAccountHolder();
+  shouldBehaveLikeERC7821();
 });

test/account/AccountECDSA.test.js

@@ -3,12 +3,9 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 const { ERC4337Helper } = require('../helpers/erc4337');
 const { PackedUserOperation } = require('../helpers/eip712-types');
 
-const {
-  shouldBehaveLikeAccountCore,
-  shouldBehaveLikeAccountERC7821,
-  shouldBehaveLikeAccountHolder,
-} = require('./Account.behavior');
+const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
 const { shouldBehaveLikeERC7739 } = require('../utils/cryptography/ERC7739.behavior');
+const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior');
 
 async function fixture() {
   // EOAs and environment
@@ -45,8 +42,8 @@ describe('AccountECDSA', function () {
   });
 
   shouldBehaveLikeAccountCore();
-  shouldBehaveLikeAccountERC7821();
   shouldBehaveLikeAccountHolder();
+  shouldBehaveLikeERC7821();
 
   describe('ERC7739', function () {
     beforeEach(async function () {

test/account/AccountERC7702.test.js

@@ -3,12 +3,9 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 const { ERC4337Helper } = require('../helpers/erc4337');
 const { PackedUserOperation } = require('../helpers/eip712-types');
 
-const {
-  shouldBehaveLikeAccountCore,
-  shouldBehaveLikeAccountERC7821,
-  shouldBehaveLikeAccountHolder,
-} = require('./Account.behavior');
+const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
 const { shouldBehaveLikeERC7739 } = require('../utils/cryptography/ERC7739.behavior');
+const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior');
 
 async function fixture() {
   // EOAs and environment
@@ -45,8 +42,8 @@ describe('AccountERC7702', function () {
   });
 
   shouldBehaveLikeAccountCore();
-  shouldBehaveLikeAccountERC7821({ deployable: false });
   shouldBehaveLikeAccountHolder();
+  shouldBehaveLikeERC7821({ deployable: false });
 
   describe('ERC7739', function () {
     beforeEach(async function () {