♻️ Refactor signature validation and remove fuse

- Remove the creation flow fuse mechanism - Replace fuse checks with nonce checks in signature validation - Adjust initialization tests for SmartAccount
get-smooth · Apr 18, 2024 · d55272e · d55272e
1 parent 5c3c2ea
commit d55272e
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 78 deletions.
diff --git a/src/v1/Account/SmartAccount.sol b/src/v1/Account/SmartAccount.sol
@@ -28,7 +28,6 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
     // ======================================
 
     address internal factoryAddress;
-    uint96 internal creationFlowFuse;
     uint256[50] internal __gap;
 
     // ======================================
@@ -89,9 +88,6 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
 
         // 2. set the first signer
         _addWebAuthnSigner(credIdHash, pubX, pubY, credId);
-
-        // 3. activate the creation flow fuse
-        creationFlowFuse = 1;
     }
 
     // ======================================
@@ -155,16 +151,16 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
     /// @param initCode The initCode field presents in the userOp. It has been used to create the account
     /// @return 0 if the signature is valid, 1 otherwise
     function _validateCreationSignature(
+        uint256 nonce,
         bytes calldata signature,
         bytes calldata initCode
     )
         internal
         virtual
         returns (uint256)
     {
-        // 1. check that we are in the creation flow -- either return failure if not or deactivate the creation flow fuse
-        if (creationFlowFuse != 1) return Signature.State.FAILURE;
-        creationFlowFuse = 0;
+        // 1. check that the nonce is equal to 0
+        if (nonce != 0) return Signature.State.FAILURE;
 
         // 2. get the address of the factory and check it is the expected one
         address accountFactory = address(bytes20(initCode[:20]));
@@ -252,7 +248,7 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
 
         // 1.b check the signature is a "creation" signature (length is checked by the signature library)
         if (signatureType == Signature.Type.CREATION) {
-            return _validateCreationSignature(userOp.signature, userOp.initCode);
+            return _validateCreationSignature(userOp.nonce, userOp.signature, userOp.initCode);
         }
 
         return Signature.State.FAILURE;

diff --git a/test/unit/v1/Account/initialize.t.sol b/test/unit/v1/Account/initialize.t.sol
@@ -8,15 +8,15 @@ import { BaseTest } from "test/BaseTest/BaseTest.sol";
 import { SignerVaultWebAuthnP256R1 } from "src/utils/SignerVaultWebAuthnP256R1.sol";
 import "src/utils/Signature.sol" as Signature;
 
-contract AccountHarness__Initiliaze is BaseTest {
+contract SmartAccount__Initiliaze is BaseTest {
     // @DEV: constant used by the `Initializable` library
     bytes32 private constant INITIALIZABLE_STORAGE = 0xf0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a00;
-    AccountHarness private accountImplementation;
+    SmartAccount private accountImplementation;
     AccountFactoryHarness private factory;
 
     function setUp() external {
         // 1. deploy an implementation of the account
-        accountImplementation = new AccountHarness(makeAddr("entrypoint"), makeAddr("verifier"));
+        accountImplementation = new SmartAccount(makeAddr("entrypoint"), makeAddr("verifier"));
 
         // 2. deploy an implementation of the factory and its instance
         factory = new AccountFactoryHarness(address(accountImplementation), makeAddr("factorySigner"));
@@ -46,7 +46,7 @@ contract AccountHarness__Initiliaze is BaseTest {
         );
 
         // 2. deploy the account and call the initialize function at the same time
-        AccountHarness account = factory.exposed_deployAccount(
+        SmartAccount account = factory.exposed_deployAccount(
             keccak256(createFixtures.signer.credId),
             createFixtures.signer.pubX,
             createFixtures.signer.pubY,
@@ -75,7 +75,7 @@ contract AccountHarness__Initiliaze is BaseTest {
         );
 
         // 2. deploy the account and call the initialize function at the same time
-        AccountHarness account = factory.exposed_deployAccount(
+        SmartAccount account = factory.exposed_deployAccount(
             keccak256(createFixtures.signer.credId),
             createFixtures.signer.pubX,
             createFixtures.signer.pubY,
@@ -90,7 +90,7 @@ contract AccountHarness__Initiliaze is BaseTest {
         // it can be called using a proxy and set version to 1
 
         // 1. deploy the account and call the initialize function at the same time
-        AccountHarness account = factory.exposed_deployAccount(
+        SmartAccount account = factory.exposed_deployAccount(
             keccak256(createFixtures.signer.credId),
             createFixtures.signer.pubX,
             createFixtures.signer.pubY,
@@ -109,22 +109,7 @@ contract AccountHarness__Initiliaze is BaseTest {
         );
     }
 
-    function test_ItSetsTheCreationFuse() external setUpCreateFixture {
-        // it stores the signer
-
-        // 1. deploy the account and call the initialize function at the same time
-        AccountHarness account = factory.exposed_deployAccount(
-            keccak256(createFixtures.signer.credId),
-            createFixtures.signer.pubX,
-            createFixtures.signer.pubY,
-            createFixtures.signer.credId
-        );
-
-        // 2. check the creation fuse has been set
-        assertEq(account.exposed_creationFlowFuse(), 1);
-    }
-
-    // Duplicate of the event in the AccountHarness.sol file
+    // Duplicate of the event in the SmartAccount.sol file
     event SignerAdded(
         bytes1 indexed signatureType, bytes credId, bytes32 indexed credIdHash, uint256 pubKeyX, uint256 pubKeyY
     );
@@ -142,7 +127,7 @@ contract AccountHarness__Initiliaze is BaseTest {
         emit SignerAdded(Signature.Type.WEBAUTHN_P256R1, credId, credIdHash, pubX, pubY);
 
         // 2. deploy the account and call the initialize function at the same time
-        AccountHarness account = factory.exposed_deployAccount(credIdHash, pubX, pubY, credId);
+        SmartAccount account = factory.exposed_deployAccount(credIdHash, pubX, pubY, credId);
 
         // 3. get the starting slot of the signer
         bytes32 startingSlot = SignerVaultWebAuthnP256R1.getSignerStartingSlot(credIdHash);
@@ -154,14 +139,6 @@ contract AccountHarness__Initiliaze is BaseTest {
     }
 }
 
-contract AccountHarness is SmartAccount {
-    constructor(address entrypoint, address verifier) SmartAccount(entrypoint, verifier) { }
-
-    function exposed_creationFlowFuse() external view returns (uint256) {
-        return creationFlowFuse;
-    }
-}
-
 contract AccountFactoryHarness is AccountFactory {
     constructor(address accountImplementation, address operator) AccountFactory(accountImplementation, operator) { }
 
@@ -172,8 +149,8 @@ contract AccountFactoryHarness is AccountFactory {
         bytes calldata credId
     )
         external
-        returns (AccountHarness account)
+        returns (SmartAccount account)
     {
-        return AccountHarness(payable(address(_deployAccount(credIdHash, pubX, pubY, credId))));
+        return SmartAccount(payable(address(_deployAccount(credIdHash, pubX, pubY, credId))));
     }
 }
diff --git a/test/unit/v1/Account/initialize.tree b/test/unit/v1/Account/initialize.tree
@@ -3,5 +3,4 @@ SmartAccount__Initiliaze
 ├── it can be called using a proxy and set version to 1
 ├── it store the initiator
 ├── it store the first signer
-├── it sets the creation fuse
 └── it can not be called twice
diff --git a/test/unit/v1/Account/receive.t.sol b/test/unit/v1/Account/receive.t.sol
@@ -14,7 +14,6 @@ contract SmartAccount__Receive is BaseTest {
     function test_CanReceiveNativeTokens(address sender, uint256 amount) external {
         // it can receive native tokens
 
-        // sender = bound(100, type(address).max);
         amount = bound(amount, 1, type(uint256).max);
 
         // make the sender the caller for the next call and send him n native tokens

diff --git a/test/unit/v1/Account/validateCreationSignature.t.sol b/test/unit/v1/Account/validateCreationSignature.t.sol
@@ -51,16 +51,16 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         );
     }
 
-    function test_FailsIfCalledTwice() external {
+    function test_FailsIfNonceNot0(uint256 fuzzedNonce) external {
         // it fails if called twice
 
+        fuzzedNonce = bound(fuzzedNonce, 1, type(uint256).max);
+
         // 1. get valid initcode and signature
         (bytes memory initCode, bytes memory signature) = _calculateInitCodeAndSignature();
 
-        // 2. check the signature validation is successful
-        assertEq(account.exposed_validateCreationSignature(signature, initCode), Signature.State.SUCCESS);
-        // 3. ensure we cannot calling it twice
-        assertEq(account.exposed_validateCreationSignature(signature, initCode), Signature.State.FAILURE);
+        // 2. check the signature validation is failure -- nonce not equal to 0
+        assertEq(account.exposed_validateCreationSignature(fuzzedNonce, signature, initCode), Signature.State.FAILURE);
     }
 
     function test_RevertsIfTheInitCodeIsNotCorrectlyConstructed() external {
@@ -79,7 +79,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
 
         // 3. check the signature validation is failure
         vm.expectRevert();
-        account.exposed_validateCreationSignature(signature, invalidInitCode);
+        account.exposed_validateCreationSignature(0, signature, invalidInitCode);
     }
 
     function test_FailsIfTheUseropFactoryIsNotCorrect(address incorrectFactory) external {
@@ -99,7 +99,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         );
 
         // 3. check the signature validation is failure
-        assertEq(account.exposed_validateCreationSignature(signature, invalidInitCode), Signature.State.FAILURE);
+        assertEq(account.exposed_validateCreationSignature(0, signature, invalidInitCode), Signature.State.FAILURE);
     }
 
     function test_FailsIfTheAdminOfTheFactoryIsNotCorrect(address incorrectSigner) external {
@@ -117,7 +117,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         );
 
         // 3. check the signature validation is failure
-        assertEq(account.exposed_validateCreationSignature(signature, initCode), Signature.State.FAILURE);
+        assertEq(account.exposed_validateCreationSignature(0, signature, initCode), Signature.State.FAILURE);
     }
 
     function test_FailsIfThePassedSignatureIsNotCorrect(string memory name) external {
@@ -132,7 +132,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         (bytes memory initCode,) = _calculateInitCodeAndSignature();
 
         // 3. check the signature validation is failure
-        assertEq(account.exposed_validateCreationSignature(invalidSignature, initCode), Signature.State.FAILURE);
+        assertEq(account.exposed_validateCreationSignature(0, invalidSignature, initCode), Signature.State.FAILURE);
     }
 
     function test_FailsIfSignatureTypeMissing() external {
@@ -145,7 +145,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         bytes memory invalidSignature = truncBytes(signature, 1, signature.length);
 
         // 3. check the signature validation is failure
-        assertEq(account.exposed_validateCreationSignature(invalidSignature, initCode), Signature.State.FAILURE);
+        assertEq(account.exposed_validateCreationSignature(0, invalidSignature, initCode), Signature.State.FAILURE);
     }
 
     function test_FailsIfTheCredIdDoesNotMatchTheCredIdStored(bytes32 incorrectCredIdHash) external {
@@ -163,7 +163,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         (bytes memory initCode, bytes memory signature) = _calculateInitCodeAndSignature();
 
         // 4. check the signature validation is failure
-        assertEq(account.exposed_validateCreationSignature(signature, initCode), Signature.State.FAILURE);
+        assertEq(account.exposed_validateCreationSignature(0, signature, initCode), Signature.State.FAILURE);
     }
 
     function test_FailsIfThePubKeyXDoesNotMatchThePubKeyXStored(uint256 incorrectPubKeyX) external {
@@ -182,7 +182,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         (bytes memory initCode, bytes memory signature) = _calculateInitCodeAndSignature();
 
         // 4. check the signature validation is failure
-        assertEq(account.exposed_validateCreationSignature(signature, initCode), Signature.State.FAILURE);
+        assertEq(account.exposed_validateCreationSignature(0, signature, initCode), Signature.State.FAILURE);
     }
 
     function test_FailsIfThePubKeyYDoesNotMatchThePubKeyYStored(uint256 incorrectPubKeyY) external {
@@ -201,21 +201,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         (bytes memory initCode, bytes memory signature) = _calculateInitCodeAndSignature();
 
         // 4. check the signature validation is failure
-        assertEq(account.exposed_validateCreationSignature(signature, initCode), Signature.State.FAILURE);
-    }
-
-    function test_SetsTheCreationFuseToZero() external {
-        // it succeed if the signature recovery is correct
-
-        // 1. check that the creation flow fuse is initially set to 1
-        assertEq(account.exposed_creationFlowFuse(), 1);
-
-        // 2. get valid initcode and signature
-        (bytes memory initCode, bytes memory signature) = _calculateInitCodeAndSignature();
-
-        // 3. make sure the creation flow fuse is set to 0 after calling the function
-        account.exposed_validateCreationSignature(signature, initCode);
-        assertEq(account.exposed_creationFlowFuse(), 0);
+        assertEq(account.exposed_validateCreationSignature(0, signature, initCode), Signature.State.FAILURE);
     }
 
     function test_SucceedIfTheSignatureRecoveryIsCorrect() external {
@@ -225,25 +211,22 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
         (bytes memory initCode, bytes memory signature) = _calculateInitCodeAndSignature();
 
         // 2. check the signature validation is successful
-        assertEq(account.exposed_validateCreationSignature(signature, initCode), Signature.State.SUCCESS);
+        assertEq(account.exposed_validateCreationSignature(0, signature, initCode), Signature.State.SUCCESS);
     }
 }
 
 contract SmartAccountHarness is SmartAccount {
     constructor(address entryPoint, address webAuthnVerifier) SmartAccount(entryPoint, webAuthnVerifier) { }
 
     function exposed_validateCreationSignature(
+        uint256 nonce,
         bytes calldata signature,
         bytes calldata initCode
     )
         external
         returns (uint256)
     {
-        return _validateCreationSignature(signature, initCode);
-    }
-
-    function exposed_creationFlowFuse() external view returns (uint256) {
-        return creationFlowFuse;
+        return _validateCreationSignature(nonce, signature, initCode);
     }
 }
 

diff --git a/test/unit/v1/Account/validateCreationSignature.tree b/test/unit/v1/Account/validateCreationSignature.tree
@@ -1,5 +1,5 @@
 SmartAccount__ValidateCreationSignature
-├── it fails if called twice
+├── it fails if nonce not 0
 ├── it reverts if the initCode is not correctly constructed
 ├── it fails if the userop factory is not correct
 ├── it fails if the admin of the factory is not correct
@@ -8,5 +8,4 @@ SmartAccount__ValidateCreationSignature
 ├── it fails if the credId does not match the credId stored
 ├── it fails if the pubKeyX does not match the pubKeyX stored
 ├── it fails if the pubKeyY does not match the pubKeyY stored
-├── it sets the creation fuse to zero
 └── it succeed if the signature recovery is correct