✨ Multi Owned ECDSA Module

bcnmy · Oct 17, 2023 · b6b179a · b6b179a
1 parent 376053b
commit b6b179a
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 77 deletions.
diff --git a/contracts/smart-account/interfaces/modules/IMultiOwnedECDSAModule.sol b/contracts/smart-account/interfaces/modules/IMultiOwnedECDSAModule.sol
@@ -31,7 +31,9 @@ interface IMultiOwnedECDSAModule {
      * Should be used at a time of first enabling the module for a Smart Account.
      * @param eoaOwners The owner of the Smart Account. Should be EOA!
      */
-    function initForSmartAccount(address[] calldata eoaOwners) external returns (address);
+    function initForSmartAccount(
+        address[] calldata eoaOwners
+    ) external returns (address);
 
     /**
      * @dev Sets/changes an for a Smart Account.

diff --git a/contracts/smart-account/modules/MultiOwnedECDSAModule.sol b/contracts/smart-account/modules/MultiOwnedECDSAModule.sol
@@ -34,21 +34,24 @@ contract MultiOwnedECDSAModule is
     string public constant VERSION = "0.2.0";
 
     // owner => smartAccount => isOwner
-    mapping(address => mapping (address => bool)) internal _smartAccountOwners;
-    mapping (address => uint256) internal numberOfOwners;
+    mapping(address => mapping(address => bool)) internal _smartAccountOwners;
+    mapping(address => uint256) internal numberOfOwners;
 
     /// @inheritdoc IMultiOwnedECDSAModule
     function initForSmartAccount(
         address[] calldata eoaOwners
     ) external returns (address) {
-
         if (numberOfOwners[msg.sender] != 0) {
             revert AlreadyInitedForSmartAccount(msg.sender);
         }
-        for(uint256 i; i < eoaOwners.length; ) {
-            if (eoaOwners[i] == address(0)) revert ZeroAddressNotAllowedAsOwner();
+        for (uint256 i; i < eoaOwners.length; ) {
+            if (eoaOwners[i] == address(0))
+                revert ZeroAddressNotAllowedAsOwner();
             if (_smartAccountOwners[eoaOwners[i]][msg.sender])
-                revert OwnerAlreadyUsedForSmartAccount(eoaOwners[i], msg.sender);
+                revert OwnerAlreadyUsedForSmartAccount(
+                    eoaOwners[i],
+                    msg.sender
+                );
 
             _smartAccountOwners[eoaOwners[i]][msg.sender] = true;
             numberOfOwners[msg.sender]++;
@@ -60,11 +63,15 @@ contract MultiOwnedECDSAModule is
     }
 
     /// @inheritdoc IMultiOwnedECDSAModule
-    function transferOwnership(address owner, address newOwner) external override {
+    function transferOwnership(
+        address owner,
+        address newOwner
+    ) external override {
         if (_isSmartContract(newOwner)) revert NotEOA(owner);
         if (newOwner == address(0)) revert ZeroAddressNotAllowedAsOwner();
         if (owner == address(0)) revert ZeroAddressNotAllowedAsOwner();
-        if (owner == newOwner) revert OwnerAlreadyUsedForSmartAccount(newOwner, msg.sender);
+        if (owner == newOwner)
+            revert OwnerAlreadyUsedForSmartAccount(newOwner, msg.sender);
         _transferOwnership(msg.sender, owner, newOwner);
     }
 
@@ -83,7 +90,6 @@ contract MultiOwnedECDSAModule is
     function removeOwner(address owner) external override {
         _transferOwnership(msg.sender, owner, address(0));
         --numberOfOwners[msg.sender];
-
     }
 
     /// @inheritdoc IMultiOwnedECDSAModule
@@ -171,11 +177,17 @@ contract MultiOwnedECDSAModule is
         address recovered = (dataHash.toEthSignedMessageHash()).recover(
             signature
         );
-        if (recovered != address(0) && _smartAccountOwners[recovered][smartAccount]) {
+        if (
+            recovered != address(0) &&
+            _smartAccountOwners[recovered][smartAccount]
+        ) {
             return true;
         }
         recovered = dataHash.recover(signature);
-        if (recovered != address(0) && _smartAccountOwners[recovered][smartAccount]) {
+        if (
+            recovered != address(0) &&
+            _smartAccountOwners[recovered][smartAccount]
+        ) {
             return true;
         }
         return false;

diff --git a/test/bundler-integration/module/MultiOwnedECDSA.Module.specs.ts b/test/bundler-integration/module/MultiOwnedECDSA.Module.specs.ts
@@ -14,7 +14,13 @@ import { BundlerTestEnvironment } from "../environment/bundlerEnvironment";
 import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
 
 describe("MultiOwned ECDSA Module (with Bundler):", async () => {
-  let [deployer, smartAccountOwner1, smartAccountOwner2, smartAccountOwner3, eve] = [] as SignerWithAddress[];
+  let [
+    deployer,
+    smartAccountOwner1,
+    smartAccountOwner2,
+    smartAccountOwner3,
+    eve,
+  ] = [] as SignerWithAddress[];
   const smartAccountDeploymentIndex = 0;
   const SIG_VALIDATION_SUCCESS = 0;
   let environment: BundlerTestEnvironment;
@@ -29,7 +35,13 @@ describe("MultiOwned ECDSA Module (with Bundler):", async () => {
   });
 
   beforeEach(async function () {
-    [deployer, smartAccountOwner1, smartAccountOwner2, smartAccountOwner3, eve] = await ethers.getSigners();
+    [
+      deployer,
+      smartAccountOwner1,
+      smartAccountOwner2,
+      smartAccountOwner3,
+      eve,
+    ] = await ethers.getSigners();
   });
 
   afterEach(async function () {
@@ -55,9 +67,16 @@ describe("MultiOwned ECDSA Module (with Bundler):", async () => {
     const mockToken = await getMockToken();
 
     const ecdsaOwnershipSetupData =
-      multiOwnedECDSAModule.interface.encodeFunctionData("initForSmartAccount", [
-        [smartAccountOwner1.address, smartAccountOwner2.address, smartAccountOwner3.address],
-      ]);
+      multiOwnedECDSAModule.interface.encodeFunctionData(
+        "initForSmartAccount",
+        [
+          [
+            smartAccountOwner1.address,
+            smartAccountOwner2.address,
+            smartAccountOwner3.address,
+          ],
+        ]
+      );
 
     const deploymentData = saFactory.interface.encodeFunctionData(
       "deployCounterFactualAccount",
@@ -141,7 +160,7 @@ describe("MultiOwned ECDSA Module (with Bundler):", async () => {
         "execute_ncC",
         [multiOwnedECDSAModule.address, 0, txnData1],
         userSA.address,
-        smartAccountOwner1, //can be signed by any owner
+        smartAccountOwner1, // can be signed by any owner
         entryPoint,
         multiOwnedECDSAModule.address,
         {
@@ -150,76 +169,81 @@ describe("MultiOwned ECDSA Module (with Bundler):", async () => {
       );
 
       await environment.sendUserOperation(userOp, entryPoint.address);
-      expect(await multiOwnedECDSAModule.isOwner(userSA.address, eve.address)).to.be.true;
+      expect(
+        await multiOwnedECDSAModule.isOwner(userSA.address, eve.address)
+      ).to.equal(true);
     });
   });
 
   describe("removeOwner():", async () => {
     it("Should be able to renounce ownership and the new owner should be address(0)", async () => {
-        const { multiOwnedECDSAModule, entryPoint, userSA } = await setupTests();
-        const txnData1 = multiOwnedECDSAModule.interface.encodeFunctionData(
-          "removeOwner",
-          [smartAccountOwner2.address]
-        );
-        const userOp = await makeEcdsaModuleUserOp(
-          "execute_ncC",
-          [multiOwnedECDSAModule.address, 0, txnData1],
+      const { multiOwnedECDSAModule, entryPoint, userSA } = await setupTests();
+      const txnData1 = multiOwnedECDSAModule.interface.encodeFunctionData(
+        "removeOwner",
+        [smartAccountOwner2.address]
+      );
+      const userOp = await makeEcdsaModuleUserOp(
+        "execute_ncC",
+        [multiOwnedECDSAModule.address, 0, txnData1],
+        userSA.address,
+        smartAccountOwner3, // any owner can sign
+        entryPoint,
+        multiOwnedECDSAModule.address,
+        {
+          preVerificationGas: 50000,
+        }
+      );
+
+      await environment.sendUserOperation(userOp, entryPoint.address);
+      expect(
+        await multiOwnedECDSAModule.isOwner(
           userSA.address,
-          smartAccountOwner3, //any owner can sign
-          entryPoint,
-          multiOwnedECDSAModule.address,
-          {
-            preVerificationGas: 50000,
-          }
-        );
-
-        await environment.sendUserOperation(userOp, entryPoint.address);
-        expect(
-          await multiOwnedECDSAModule.isOwner(userSA.address, smartAccountOwner2.address)
-        ).to.be.false;
+          smartAccountOwner2.address
+        )
+      ).to.equal(false);
     });
   });
 
   describe("validateUserOp(): ", async () => {
     it("Returns SIG_VALIDATION_SUCCESS for a valid UserOp and valid userOpHash and allows to handle userOp", async () => {
-        const { multiOwnedECDSAModule, entryPoint, userSA, mockToken } =
-          await setupTests();
-        const userSABalanceBefore = await mockToken.balanceOf(userSA.address);
-        const eveBalanceBefore = await mockToken.balanceOf(eve.address);
-        const tokenAmountToTransfer = ethers.utils.parseEther("3.5672");
-
-        const txnData = mockToken.interface.encodeFunctionData("transfer", [
-          eve.address,
-          tokenAmountToTransfer.toString(),
-        ]);
-        const userOp = await makeEcdsaModuleUserOp(
-          "execute_ncC",
-          [mockToken.address, 0, txnData],
-          userSA.address,
-          smartAccountOwner2, //any owner can sign
-          entryPoint,
-          multiOwnedECDSAModule.address,
-          {
-            preVerificationGas: 50000,
-          }
-        );
-        // Construct userOpHash
-        const provider = entryPoint?.provider;
-        const chainId = await provider!.getNetwork().then((net) => net.chainId);
-        const userOpHash = getUserOpHash(userOp, entryPoint.address, chainId);
-
-        const res = await multiOwnedECDSAModule.validateUserOp(
-          userOp,
-          userOpHash
-        );
-        expect(res).to.be.equal(SIG_VALIDATION_SUCCESS);
-        await environment.sendUserOperation(userOp, entryPoint.address);
-        expect(await mockToken.balanceOf(eve.address)).to.equal(
-          eveBalanceBefore.add(tokenAmountToTransfer)
-        );
-        expect(await mockToken.balanceOf(userSA.address)).to.equal(
-          userSABalanceBefore.sub(tokenAmountToTransfer)
-        );
+      const { multiOwnedECDSAModule, entryPoint, userSA, mockToken } =
+        await setupTests();
+      const userSABalanceBefore = await mockToken.balanceOf(userSA.address);
+      const eveBalanceBefore = await mockToken.balanceOf(eve.address);
+      const tokenAmountToTransfer = ethers.utils.parseEther("3.5672");
+
+      const txnData = mockToken.interface.encodeFunctionData("transfer", [
+        eve.address,
+        tokenAmountToTransfer.toString(),
+      ]);
+      const userOp = await makeEcdsaModuleUserOp(
+        "execute_ncC",
+        [mockToken.address, 0, txnData],
+        userSA.address,
+        smartAccountOwner2, // any owner can sign
+        entryPoint,
+        multiOwnedECDSAModule.address,
+        {
+          preVerificationGas: 50000,
+        }
+      );
+      // Construct userOpHash
+      const provider = entryPoint?.provider;
+      const chainId = await provider!.getNetwork().then((net) => net.chainId);
+      const userOpHash = getUserOpHash(userOp, entryPoint.address, chainId);
+
+      const res = await multiOwnedECDSAModule.validateUserOp(
+        userOp,
+        userOpHash
+      );
+      expect(res).to.be.equal(SIG_VALIDATION_SUCCESS);
+      await environment.sendUserOperation(userOp, entryPoint.address);
+      expect(await mockToken.balanceOf(eve.address)).to.equal(
+        eveBalanceBefore.add(tokenAmountToTransfer)
+      );
+      expect(await mockToken.balanceOf(userSA.address)).to.equal(
+        userSABalanceBefore.sub(tokenAmountToTransfer)
+      );
     });
   });
 });