identify / multisig with ERC1271

OpenZeppelin · Jul 12, 2024 · add9961 · add9961
1 parent 5745f60
commit add9961
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 87 deletions.
diff --git a/contracts/abstraction/account/modules/recovery/AccountAllSignatures.sol b/contracts/abstraction/account/modules/recovery/AccountAllSignatures.sol
@@ -17,7 +17,6 @@ abstract contract AccountAllSignatures is AccountECDSA, AccountERC1271 {
         bytes32 userOpHash
     ) internal virtual override(AccountECDSA, AccountERC1271) returns (address) {
         (SignatureType sigType, bytes memory sigData) = abi.decode(signature, (SignatureType, bytes));
-
         if (sigType == SignatureType.ECDSA) {
             return AccountECDSA._recoverSigner(sigData, userOpHash);
         } else if (sigType == SignatureType.ERC1271) {

diff --git a/contracts/abstraction/account/modules/recovery/AccountERC1271.sol b/contracts/abstraction/account/modules/recovery/AccountERC1271.sol
@@ -14,8 +14,6 @@ abstract contract AccountERC1271 is Account {
         bytes32 msgHash = MessageHashUtils.toEthSignedMessageHash(userOpHash);
         (address signer, bytes memory sig) = abi.decode(signature, (address, bytes));
 
-        return SignatureChecker.isValidERC1271SignatureNow(signer, msgHash, sig)
-            ? signer
-            : address(0);
+        return SignatureChecker.isValidERC1271SignatureNow(signer, msgHash, sig) ? signer : address(0);
     }
 }
diff --git a/contracts/abstraction/mocks/AdvancedAccount.sol b/contracts/abstraction/mocks/AdvancedAccount.sol
@@ -8,49 +8,9 @@ import {ERC1155Holder} from "../../token/ERC1155/utils/ERC1155Holder.sol";
 import {Account} from "../account/Account.sol";
 import {AccountCommon} from "../account/AccountCommon.sol";
 import {AccountMultisig} from "../account/modules/AccountMultisig.sol";
-import {AccountECDSA} from "../account/modules/recovery/AccountECDSA.sol";
-import {AccountERC1271} from "../account/modules/recovery/AccountERC1271.sol";
+import {AccountAllSignatures} from "../account/modules/recovery/AccountAllSignatures.sol";
 
-contract AdvancedAccountECDSA is AccessControl, AccountCommon, AccountECDSA, AccountMultisig {
-    bytes32 public constant SIGNER_ROLE = keccak256("SIGNER_ROLE");
-    uint256 private _requiredSignatures;
-
-    constructor(
-        IEntryPoint entryPoint_,
-        address admin_,
-        address[] memory signers_,
-        uint256 requiredSignatures_
-    ) AccountCommon(entryPoint_) {
-        _grantRole(DEFAULT_ADMIN_ROLE, admin_);
-        for (uint256 i = 0; i < signers_.length; ++i) {
-            _grantRole(SIGNER_ROLE, signers_[i]);
-        }
-        _requiredSignatures = requiredSignatures_;
-    }
-
-    function supportsInterface(
-        bytes4 interfaceId
-    ) public view virtual override(AccessControl, ERC1155Holder) returns (bool) {
-        return super.supportsInterface(interfaceId);
-    }
-
-    function requiredSignatures() public view virtual override returns (uint256) {
-        return _requiredSignatures;
-    }
-
-    function _isAuthorized(address user) internal view virtual override returns (bool) {
-        return hasRole(SIGNER_ROLE, user);
-    }
-
-    function _processSignature(
-        bytes memory signature,
-        bytes32 userOpHash
-    ) internal virtual override(Account, AccountMultisig) returns (bool, address, uint48, uint48) {
-        return super._processSignature(signature, userOpHash);
-    }
-}
-
-contract AdvancedAccountERC1271 is AccessControl, AccountCommon, AccountERC1271, AccountMultisig {
+contract AdvancedAccount is AccessControl, AccountCommon, AccountAllSignatures, AccountMultisig {
     bytes32 public constant SIGNER_ROLE = keccak256("SIGNER_ROLE");
     uint256 private _requiredSignatures;
 

diff --git a/test/abstraction/accountECDSA.test.js b/test/abstraction/accountECDSA.test.js
@@ -3,16 +3,23 @@ const { expect } = require('chai');
 const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 
 const { ERC4337Helper } = require('../helpers/erc4337');
+const { IdentityHelper } = require('../helpers/identity');
 
 async function fixture() {
   const accounts = await ethers.getSigners();
-  accounts.user = accounts.shift();
+  accounts.relayer = accounts.shift();
   accounts.beneficiary = accounts.shift();
 
-  const target = await ethers.deployContract('CallReceiverMock');
+  // 4337 helper
   const helper = new ERC4337Helper('SimpleAccountECDSA');
-  await helper.wait();
-  const sender = await helper.newAccount(accounts.user);
+  const identity = new IdentityHelper();
+
+  // environment
+  const target = await ethers.deployContract('CallReceiverMock');
+
+  // create 4337 account controlled by ECDSA
+  const signer = await identity.newECDSASigner();
+  const sender = await helper.newAccount(signer);
 
   return {
     accounts,
@@ -31,7 +38,7 @@ describe('AccountECDSA', function () {
 
   describe('execute operation', function () {
     beforeEach('fund account', async function () {
-      await this.accounts.user.sendTransaction({ to: this.sender, value: ethers.parseEther('1') });
+      await this.accounts.relayer.sendTransaction({ to: this.sender, value: ethers.parseEther('1') });
     });
 
     describe('account not deployed yet', function () {
@@ -57,7 +64,7 @@ describe('AccountECDSA', function () {
 
     describe('account already deployed', function () {
       beforeEach(async function () {
-        await this.sender.deploy();
+        await this.sender.deploy(this.accounts.relayer);
       });
 
       it('success: call', async function () {

diff --git a/test/abstraction/accountERC1271.test.js b/test/abstraction/accountERC1271.test.js
@@ -3,26 +3,22 @@ const { expect } = require('chai');
 const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 
 const { ERC4337Helper } = require('../helpers/erc4337');
-const { P256Signer } = require('../helpers/p256');
+const { IdentityHelper } = require('../helpers/identity');
 
 async function fixture() {
   const accounts = await ethers.getSigners();
-  accounts.user = accounts.shift();
+  accounts.relayer = accounts.shift();
   accounts.beneficiary = accounts.shift();
 
   // 4337 helper
   const helper = new ERC4337Helper('SimpleAccountERC1271');
-  await helper.wait();
+  const identity = new IdentityHelper();
 
   // environment
   const target = await ethers.deployContract('CallReceiverMock');
-  const identifyFactory = await ethers.deployContract('IdentityP256Factory');
 
-  // create P256 key and identity contract
-  const signer = P256Signer.random();
-  signer.address = await identifyFactory.predict(signer.publicKey); // override address of the signer
-  signer.sigParams.prefixAddress = true;
-  await identifyFactory.create(signer.publicKey);
+  // create 4337 account controlled by P256
+  const signer = await identity.newP256Signer();
   const sender = await helper.newAccount(signer);
 
   return {
@@ -31,6 +27,7 @@ async function fixture() {
     helper,
     entrypoint: helper.entrypoint,
     factory: helper.factory,
+    signer,
     sender,
   };
 }
@@ -42,7 +39,7 @@ describe('AccountERC1271', function () {
 
   describe('execute operation', function () {
     beforeEach('fund account', async function () {
-      await this.accounts.user.sendTransaction({ to: this.sender, value: ethers.parseEther('1') });
+      await this.accounts.relayer.sendTransaction({ to: this.sender, value: ethers.parseEther('1') });
     });
 
     describe('account not deployed yet', function () {
@@ -68,7 +65,7 @@ describe('AccountERC1271', function () {
 
     describe('account already deployed', function () {
       beforeEach(async function () {
-        await this.sender.deploy(this.accounts.user);
+        await this.sender.deploy(this.accounts.relayer);
       });
 
       it('success: call', async function () {

diff --git a/test/abstraction/accountMultisig.test.js b/test/abstraction/accountMultisig.test.js
@@ -3,26 +3,36 @@ const { expect } = require('chai');
 const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 
 const { ERC4337Helper } = require('../helpers/erc4337');
+const { IdentityHelper } = require('../helpers/identity');
 
 async function fixture() {
   const accounts = await ethers.getSigners();
-  accounts.user = accounts.shift();
+  accounts.relayer = accounts.shift();
   accounts.beneficiary = accounts.shift();
-  accounts.signers = Array(3)
-    .fill()
-    .map(() => accounts.shift());
 
+  // 4337 helper
+  const helper = new ERC4337Helper('AdvancedAccount');
+  const identity = new IdentityHelper();
+
+  // environment
   const target = await ethers.deployContract('CallReceiverMock');
-  const helper = new ERC4337Helper('AdvancedAccountECDSA');
-  await helper.wait();
-  const sender = await helper.newAccount(accounts.user, [accounts.signers, 2]); // 2-of-3
+
+  // create 4337 account controlled by multiple signers
+  const signers = await Promise.all([
+    identity.newECDSASigner(), // secp256k1
+    identity.newP256Signer(), // secp256r1
+    identity.newP256Signer(), // secp256r1
+    identity.newECDSASigner(), // secp256k1
+  ]);
+  const sender = await helper.newAccount(accounts.relayer, [signers, 2]); // 2-of-4
 
   return {
     accounts,
     target,
     helper,
     entrypoint: helper.entrypoint,
     factory: helper.factory,
+    signers,
     sender,
   };
 }
@@ -34,7 +44,7 @@ describe('AccountMultisig', function () {
 
   describe('execute operation', function () {
     beforeEach('fund account', async function () {
-      await this.accounts.user.sendTransaction({ to: this.sender, value: ethers.parseEther('1') });
+      await this.accounts.relayer.sendTransaction({ to: this.sender, value: ethers.parseEther('1') });
     });
 
     describe('account not deployed yet', function () {
@@ -48,7 +58,7 @@ describe('AccountMultisig', function () {
             ]),
           })
           .then(op => op.addInitCode())
-          .then(op => op.sign(this.accounts.signers));
+          .then(op => op.sign(this.signers, true));
 
         await expect(this.entrypoint.handleOps([operation.packed], this.accounts.beneficiary))
           .to.emit(this.entrypoint, 'AccountDeployed')
@@ -72,7 +82,7 @@ describe('AccountMultisig', function () {
               this.target.interface.encodeFunctionData('mockFunctionExtra'),
             ]),
           })
-          .then(op => op.sign(this.accounts.signers));
+          .then(op => op.sign(this.signers, true));
 
         await expect(this.entrypoint.handleOps([operation.packed], this.accounts.beneficiary))
           .to.emit(this.target, 'MockFunctionCalledExtra')
@@ -88,7 +98,7 @@ describe('AccountMultisig', function () {
               this.target.interface.encodeFunctionData('mockFunctionExtra'),
             ]),
           })
-          .then(op => op.sign([this.accounts.signers[0], this.accounts.signers[2]]));
+          .then(op => op.sign([this.signers[0], this.signers[2]], true));
 
         await expect(this.entrypoint.handleOps([operation.packed], this.accounts.beneficiary))
           .to.emit(this.target, 'MockFunctionCalledExtra')
@@ -104,7 +114,7 @@ describe('AccountMultisig', function () {
               this.target.interface.encodeFunctionData('mockFunctionExtra'),
             ]),
           })
-          .then(op => op.sign([this.accounts.signers[2]]));
+          .then(op => op.sign([this.signers[2]], true));
 
         await expect(this.entrypoint.handleOps([operation.packed], this.accounts.beneficiary))
           .to.be.revertedWithCustomError(this.entrypoint, 'FailedOp')
@@ -120,7 +130,7 @@ describe('AccountMultisig', function () {
               this.target.interface.encodeFunctionData('mockFunctionExtra'),
             ]),
           })
-          .then(op => op.sign([this.accounts.user, this.accounts.signers[2]]));
+          .then(op => op.sign([this.accounts.relayer, this.signers[2]], true));
 
         await expect(this.entrypoint.handleOps([operation.packed], this.accounts.beneficiary))
           .to.be.revertedWithCustomError(this.entrypoint, 'FailedOp')

diff --git a/test/helpers/erc4337.js b/test/helpers/erc4337.js
@@ -1,9 +1,12 @@
 const { ethers } = require('hardhat');
 
+const { SignatureType } = require('./identity');
+
 function pack(left, right) {
   return ethers.solidityPacked(['uint128', 'uint128'], [left, right]);
 }
 
+/// Global ERC-4337 environment helper.
 class ERC4337Helper {
   constructor(account = 'SimpleAccountECDSA') {
     this.entrypointAsPromise = ethers.deployContract('EntryPoint');
@@ -20,19 +23,20 @@ class ERC4337Helper {
     return this;
   }
 
-  async newAccount(user, extraArgs = [], salt = ethers.randomBytes(32)) {
+  async newAccount(signer, extraArgs = [], salt = ethers.randomBytes(32)) {
     await this.wait();
     const initCode = await this.account
-      .getDeployTransaction(this.entrypoint, user, ...extraArgs)
+      .getDeployTransaction(this.entrypoint, signer, ...extraArgs)
       .then(tx => this.factory.interface.encodeFunctionData('$deploy', [0, salt, tx.data]))
       .then(deployCode => ethers.concat([this.factory.target, deployCode]));
     const instance = await this.entrypoint.getSenderAddress
       .staticCall(initCode)
-      .then(address => this.account.attach(address).connect(user));
+      .then(address => this.account.attach(address).connect(signer));
     return new AbstractAccount(instance, initCode, this);
   }
 }
 
+/// Represent one ERC-4337 account contract.
 class AbstractAccount extends ethers.BaseContract {
   constructor(instance, initCode, context) {
     super(instance.target, instance.interface, instance.runner, instance.deployTx);
@@ -69,6 +73,7 @@ class AbstractAccount extends ethers.BaseContract {
   }
 }
 
+/// Represent one user operation
 class UserOperation {
   constructor(params) {
     this.sender = params.sender;
@@ -128,14 +133,27 @@ class UserOperation {
     return this;
   }
 
-  async sign(signer = this.sender.runner) {
-    this.signature = await Promise.all(
-      (Array.isArray(signer) ? signer : [signer])
-        .sort((signer1, signer2) => signer1.address - signer2.address)
-        .map(signer => signer.signMessage(ethers.getBytes(this.hash))),
-    ).then(signatures =>
-      Array.isArray(signer) ? ethers.AbiCoder.defaultAbiCoder().encode(['bytes[]'], [signatures]) : signatures[0],
+  async sign(signer = this.sender.runner, withTypePrefix = false) {
+    const signers = (Array.isArray(signer) ? signer : [signer]).sort(
+      (signer1, signer2) => signer1.address - signer2.address,
+    );
+    const signatures = await Promise.all(
+      signers.map(signer =>
+        Promise.resolve(signer.signMessage(ethers.getBytes(this.hash))).then(signature =>
+          withTypePrefix
+            ? ethers.AbiCoder.defaultAbiCoder().encode(
+                ['uint8', 'bytes'],
+                [signer.type ?? SignatureType.ECDSA, signature],
+              )
+            : signature,
+        ),
+      ),
     );
+
+    this.signature = Array.isArray(signer)
+      ? ethers.AbiCoder.defaultAbiCoder().encode(['bytes[]'], [signatures])
+      : signatures[0];
+
     return this;
   }
 }

diff --git a/test/helpers/identity.js b/test/helpers/identity.js
@@ -0,0 +1,43 @@
+const { ethers } = require('hardhat');
+
+const { Enum } = require('./enums');
+const { P256Signer } = require('./p256');
+
+const SignatureType = Enum('ECDSA', 'ERC1271');
+
+class IdentityHelper {
+  constructor() {
+    this.p256FactoryAsPromise = ethers.deployContract('IdentityP256Factory');
+    this.rsaFactoryAsPromise = ethers.deployContract('IdentityRSAFactory');
+  }
+
+  async wait() {
+    this.p256Factory = await this.p256FactoryAsPromise;
+    this.rsaFactory = await this.rsaFactoryAsPromise;
+    return this;
+  }
+
+  async newECDSASigner() {
+    return Object.assign(ethers.Wallet.createRandom(), { type: SignatureType.ECDSA });
+  }
+
+  async newP256Signer(sigParams = { prefixAddress: true }) {
+    await this.wait();
+
+    const signer = P256Signer.random();
+    return Promise.all([this.p256Factory.predict(signer.publicKey), this.p256Factory.create(signer.publicKey)]).then(
+      ([address]) => Object.assign(signer, { address, sigParams, type: SignatureType.ERC1271 }),
+    );
+  }
+
+  async newRSASigner() {
+    await this.wait();
+
+    return Promise.reject('Not implemented yet');
+  }
+}
+
+module.exports = {
+  SignatureType,
+  IdentityHelper,
+};