Merge branch 'master' of github.com:gonzaotc/openzeppelin-community-contracts into feature/SocialRecoveryExecutor

Author: gonzaotc

FUNDING.json

@@ -0,0 +1,10 @@
+{
+  "drips": {
+    "ethereum": {
+      "ownedBy": "0xAeb37910f93486C85A1F8F994b67E8187554d664"
+    },
+  },
+  "opRetro": {
+    "projectId": "0x939241afa4c4b9e1dda6b8250baa8f04fa8b0debce738cfd324c0b18f9926d25"
+  }
+}

contracts/account/Account.sol

@@ -3,7 +3,6 @@
 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";
@@ -21,8 +20,8 @@ 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, EIP712, ERC721Holder, ERC1155Holder, ERC7739 {
-    bytes32 internal constant _PACKED_USER_OPERATION =
+abstract contract Account is AccountCore, ERC721Holder, ERC1155Holder, ERC7739 {
+    bytes32 private constant PACKED_USER_OPERATION =
         keccak256(
             "PackedUserOperation(address sender,uint256 nonce,bytes initCode,bytes callData,bytes32 accountGasLimits,uint256 preVerificationGas,bytes32 gasFees,bytes paymasterAndData)"
         );
@@ -40,7 +39,7 @@ abstract contract Account is AccountCore, EIP712, ERC721Holder, ERC1155Holder, E
             _hashTypedDataV4(
                 keccak256(
                     abi.encode(
-                        _PACKED_USER_OPERATION,
+                        PACKED_USER_OPERATION,
                         userOp.sender,
                         userOp.nonce,
                         keccak256(userOp.initCode),

contracts/account/extensions/AccountERC7579.sol

@@ -11,7 +11,6 @@ import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol";
 import {Packing} from "@openzeppelin/contracts/utils/Packing.sol";
 import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 import {Calldata} from "@openzeppelin/contracts/utils/Calldata.sol";
-import {ERC7739} from "../../utils/cryptography/ERC7739.sol";
 import {AccountCore} from "../AccountCore.sol";
 
 /**
@@ -43,7 +42,7 @@ import {AccountCore} from "../AccountCore.sol";
  */
 abstract contract AccountERC7579 is
     AccountCore,
-    ERC7739,
+    IERC1271,
     IERC7579Execution,
     IERC7579AccountConfig,
     IERC7579ModuleConfig
@@ -145,12 +144,12 @@ abstract contract AccountERC7579 is
         return false;
     }
 
-    /// @dev Executes a transaction from the entry point or the account itself. See {_execute}.
+    /// @inheritdoc IERC7579Execution
     function execute(bytes32 mode, bytes calldata executionCalldata) public payable virtual onlyEntryPointOrSelf {
         _execute(Mode.wrap(mode), executionCalldata);
     }
 
-    /// @dev Executes a transaction from the executor module. See {_execute}.
+    /// @inheritdoc IERC7579Execution
     function executeFromExecutor(
         bytes32 mode,
         bytes calldata executionCalldata
@@ -164,6 +163,30 @@ abstract contract AccountERC7579 is
         return _execute(Mode.wrap(mode), executionCalldata);
     }
 
+    /**
+     * @dev Implement ERC-1271 through IERC7579Validator modules. If module based validation fails, fallback to
+     * "native" validation by the abstract signer.
+     *
+     * NOTE: when combined with {ERC7739} (for example through {Account}), resolution ordering may have an impact
+     * ({ERC7739} does not call super). Manual resolution might be necessary.
+     */
+    function isValidSignature(bytes32 hash, bytes calldata signature) public view virtual override returns (bytes4) {
+        // check signature length is enough for extraction
+        if (signature.length >= 20) {
+            (address module, bytes calldata innerSignature) = _extractSignatureValidator(signature);
+            // if module is not installed, skip
+            if (isModuleInstalled(MODULE_TYPE_VALIDATOR, module, Calldata.emptyBytes())) {
+                // try validation, skip any revert
+                try IERC7579Validator(module).isValidSignatureWithSender(address(this), hash, innerSignature) returns (
+                    bytes4 magic
+                ) {
+                    if (magic == IERC1271.isValidSignature.selector) return magic;
+                } catch {}
+            }
+        }
+        return bytes4(0xffffffff);
+    }
+
     /**
      * @dev Validates a user operation with {_signableUserOpHash} and returns the validation data
      * if the module specified by the first 20 bytes of the nonce key is installed. Falls back to
@@ -182,26 +205,6 @@ abstract contract AccountERC7579 is
                 : super._validateUserOp(userOp, userOpHash);
     }
 
-    /**
-     * @dev Lowest-level signature validation function. See {ERC7739-_rawSignatureValidation}.
-     *
-     * This function delegates the signature validation to a validation module if the first 20 bytes of the
-     * signature correspond to an installed validator module.
-     *
-     * See {_extractSignatureValidator} for the module extraction logic.
-     */
-    function _rawSignatureValidation(
-        bytes32 hash,
-        bytes calldata signature
-    ) internal view virtual override returns (bool) {
-        if (signature.length < 20) return false;
-        (address module, bytes calldata innerSignature) = _extractSignatureValidator(signature);
-        return
-            isModuleInstalled(MODULE_TYPE_VALIDATOR, module, Calldata.emptyBytes()) &&
-            IERC7579Validator(module).isValidSignatureWithSender(address(this), hash, innerSignature) ==
-            IERC1271.isValidSignature.selector;
-    }
-
     /**
      * @dev ERC-7579 execution logic. See {supportsExecutionMode} for supported modes.
      *
@@ -387,4 +390,12 @@ abstract contract AccountERC7579 is
     ) internal pure virtual returns (bytes4 selector, bytes memory remaining) {
         return (bytes4(data), data.slice(4));
     }
+
+    /// @dev By default, only use the modules for validation of userOp and signature. Disable raw signatures.
+    function _rawSignatureValidation(
+        bytes32 /*hash*/,
+        bytes calldata /*signature*/
+    ) internal view virtual override returns (bool) {
+        return false;
+    }
 }

contracts/mocks/account/AccountERC7702WithModulesMock.sol

@@ -8,6 +8,7 @@ import {Account} from "../../account/Account.sol";
 import {AccountERC7579} from "../../account/extensions/AccountERC7579.sol";
 import {ERC7821} from "../../account/extensions/ERC7821.sol";
 import {AbstractSigner} from "../../utils/cryptography/AbstractSigner.sol";
+import {ERC7739} from "../../utils/cryptography/ERC7739.sol";
 import {SignerERC7702} from "../../utils/cryptography/SignerERC7702.sol";
 
 abstract contract AccountERC7702WithModulesMock is Account, AccountERC7579, SignerERC7702 {
@@ -18,13 +19,22 @@ abstract contract AccountERC7702WithModulesMock is Account, AccountERC7579, Sign
         return super._validateUserOp(userOp, userOpHash);
     }
 
+    /// @dev Resolve implementation of ERC-1271 by both ERC7739 and AccountERC7579 to support both schemes.
+    function isValidSignature(
+        bytes32 hash,
+        bytes calldata signature
+    ) public view virtual override(ERC7739, AccountERC7579) returns (bytes4) {
+        // ERC-7739 can return the fn selector (success), 0xffffffff (invalid) or 0x77390001 (detection).
+        // If the return is 0xffffffff, we fallback to validation using ERC-7579 modules.
+        bytes4 erc7739magic = ERC7739.isValidSignature(hash, signature);
+        return erc7739magic == bytes4(0xffffffff) ? AccountERC7579.isValidSignature(hash, signature) : erc7739magic;
+    }
+
+    /// @dev Enable signature using the ERC-7702 signer.
     function _rawSignatureValidation(
         bytes32 hash,
         bytes calldata signature
     ) internal view virtual override(AbstractSigner, AccountERC7579, SignerERC7702) returns (bool) {
-        // Try ERC-7702 first, and fallback to ERC-7579
-        return
-            SignerERC7702._rawSignatureValidation(hash, signature) ||
-            AccountERC7579._rawSignatureValidation(hash, signature);
+        return SignerERC7702._rawSignatureValidation(hash, signature);
     }
 }

contracts/mocks/account/AccountMock.sol

@@ -9,11 +9,8 @@ import {Account} from "../../account/Account.sol";
 
 abstract contract AccountMock is Account, ERC7821 {
     /// Validates a user operation with a boolean signature.
-    function _rawSignatureValidation(
-        bytes32 /* userOpHash */,
-        bytes calldata signature
-    ) internal pure override returns (bool) {
-        return bytes1(signature[0:1]) == bytes1(0x01);
+    function _rawSignatureValidation(bytes32 hash, bytes calldata signature) internal pure override returns (bool) {
+        return signature.length >= 32 && bytes32(signature) == hash;
     }
 
     /// @inheritdoc ERC7821

test/account/Account.test.js

@@ -1,9 +1,11 @@
 const { ethers } = require('hardhat');
 const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 const { ERC4337Helper } = require('../helpers/erc4337');
+const { PackedUserOperation } = require('../helpers/eip712-types');
 const { NonNativeSigner } = require('../helpers/signers');
 
 const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
+const { shouldBehaveLikeERC1271 } = require('../utils/cryptography/ERC1271.behavior');
 const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior');
 
 async function fixture() {
@@ -12,19 +14,27 @@ async function fixture() {
   const target = await ethers.deployContract('CallReceiverMockExtended');
 
   // ERC-4337 signer
-  const signer = new NonNativeSigner({ sign: () => ({ serialized: '0x01' }) });
+  const signer = new NonNativeSigner({ sign: hash => ({ serialized: hash }) });
 
   // ERC-4337 account
   const helper = new ERC4337Helper();
   const env = await helper.wait();
   const mock = await helper.newAccount('$AccountMock', ['Account', '1']);
 
-  const signUserOp = async userOp => {
-    userOp.signature = await signer.signMessage(userOp.hash());
-    return userOp;
+  // domain cannot be fetched using getDomain(mock) before the mock is deployed
+  const domain = {
+    name: 'Account',
+    version: '1',
+    chainId: env.chainId,
+    verifyingContract: mock.address,
   };
 
-  return { ...env, mock, signer, target, beneficiary, other, signUserOp };
+  const signUserOp = async userOp =>
+    signer
+      .signTypedData(domain, { PackedUserOperation }, userOp.packed)
+      .then(signature => Object.assign(userOp, { signature }));
+
+  return { ...env, mock, domain, signer, target, beneficiary, other, signUserOp };
 }
 
 describe('Account', function () {
@@ -34,5 +44,6 @@ describe('Account', function () {
 
   shouldBehaveLikeAccountCore();
   shouldBehaveLikeAccountHolder();
+  shouldBehaveLikeERC1271({ erc7739: true });
   shouldBehaveLikeERC7821();
 });

test/account/AccountECDSA.test.js

@@ -4,7 +4,7 @@ const { ERC4337Helper } = require('../helpers/erc4337');
 const { PackedUserOperation } = require('../helpers/eip712-types');
 
 const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
-const { shouldBehaveLikeERC7739 } = require('../utils/cryptography/ERC7739.behavior');
+const { shouldBehaveLikeERC1271 } = require('../utils/cryptography/ERC1271.behavior');
 const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior');
 
 async function fixture() {
@@ -43,13 +43,6 @@ describe('AccountECDSA', function () {
 
   shouldBehaveLikeAccountCore();
   shouldBehaveLikeAccountHolder();
+  shouldBehaveLikeERC1271({ erc7739: true });
   shouldBehaveLikeERC7821();
-
-  describe('ERC7739', function () {
-    beforeEach(async function () {
-      this.mock = await this.mock.deploy();
-    });
-
-    shouldBehaveLikeERC7739();
-  });
 });

test/account/AccountERC7702.test.js

@@ -4,7 +4,7 @@ const { ERC4337Helper } = require('../helpers/erc4337');
 const { PackedUserOperation } = require('../helpers/eip712-types');
 
 const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
-const { shouldBehaveLikeERC7739 } = require('../utils/cryptography/ERC7739.behavior');
+const { shouldBehaveLikeERC1271 } = require('../utils/cryptography/ERC1271.behavior');
 const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior');
 
 async function fixture() {
@@ -43,13 +43,6 @@ describe('AccountERC7702', function () {
 
   shouldBehaveLikeAccountCore();
   shouldBehaveLikeAccountHolder();
+  shouldBehaveLikeERC1271({ erc7739: true });
   shouldBehaveLikeERC7821({ deployable: false });
-
-  describe('ERC7739', function () {
-    beforeEach(async function () {
-      this.mock = await this.mock.deploy();
-    });
-
-    shouldBehaveLikeERC7739();
-  });
 });

test/account/AccountP256.test.js

@@ -5,7 +5,7 @@ const { NonNativeSigner, P256SigningKey } = require('../helpers/signers');
 const { PackedUserOperation } = require('../helpers/eip712-types');
 
 const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
-const { shouldBehaveLikeERC7739 } = require('../utils/cryptography/ERC7739.behavior');
+const { shouldBehaveLikeERC1271 } = require('../utils/cryptography/ERC1271.behavior');
 const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior');
 
 async function fixture() {
@@ -49,13 +49,6 @@ describe('AccountP256', function () {
 
   shouldBehaveLikeAccountCore();
   shouldBehaveLikeAccountHolder();
+  shouldBehaveLikeERC1271({ erc7739: true });
   shouldBehaveLikeERC7821();
-
-  describe('ERC7739', function () {
-    beforeEach(async function () {
-      this.mock = await this.mock.deploy();
-    });
-
-    shouldBehaveLikeERC7739();
-  });
 });

test/account/AccountRSA.test.js

@@ -5,7 +5,7 @@ const { NonNativeSigner, RSASHA256SigningKey } = require('../helpers/signers');
 const { PackedUserOperation } = require('../helpers/eip712-types');
 
 const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
-const { shouldBehaveLikeERC7739 } = require('../utils/cryptography/ERC7739.behavior');
+const { shouldBehaveLikeERC1271 } = require('../utils/cryptography/ERC1271.behavior');
 const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior');
 
 async function fixture() {
@@ -49,13 +49,6 @@ describe('AccountRSA', function () {
 
   shouldBehaveLikeAccountCore();
   shouldBehaveLikeAccountHolder();
+  shouldBehaveLikeERC1271({ erc7739: true });
   shouldBehaveLikeERC7821();
-
-  describe('ERC7739', function () {
-    beforeEach(async function () {
-      this.mock = await this.mock.deploy();
-    });
-
-    shouldBehaveLikeERC7739();
-  });
 });