Skip to content

Commit

Permalink
🔧 Add creationFlowFuse for creation validation
Browse files Browse the repository at this point in the history
- Add internal uint96 for tracking creation process
- Implement logic to activate and deactivate fuse
- Modify _validateCreationSignature to check and
reset creationFlowFuse ensuring single use
- Update unit tests for new creation flow logic
  • Loading branch information
qd-qd committed Apr 15, 2024
1 parent 69845c9 commit b6471ee
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 33 deletions.
11 changes: 7 additions & 4 deletions src/v1/Account/SmartAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
// ======================================

address internal factoryAddress;
uint96 internal creationFlowFuse;

// ======================================
// =========== EVENTS/ERRORS ============
Expand Down Expand Up @@ -87,6 +88,9 @@ 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;
}

// ======================================
Expand Down Expand Up @@ -149,18 +153,17 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
/// @param signature The signature field presents in the userOp.
/// @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
// TODO: use transient storage for the expected signer/factory?
function _validateCreationSignature(
bytes calldata signature,
bytes calldata initCode
)
internal
view
virtual
returns (uint256)
{
// 1. check that the nonce is 0x00. The value of the first key is checked here
if (getNonce() != 0) return Signature.State.FAILURE;
// 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;

// 2. get the address of the factory and check it is the expected one
address accountFactory = address(bytes20(initCode[:20]));
Expand Down
43 changes: 33 additions & 10 deletions test/unit/v1/Account/initialize.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 SmartAccount__Initiliaze is BaseTest {
contract AccountHarness__Initiliaze is BaseTest {
// @DEV: constant used by the `Initializable` library
bytes32 private constant INITIALIZABLE_STORAGE = 0xf0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a00;
SmartAccount private accountImplementation;
AccountHarness private accountImplementation;
AccountFactoryHarness private factory;

function setUp() external {
// 1. deploy an implementation of the account
accountImplementation = new SmartAccount(makeAddr("entrypoint"), makeAddr("verifier"));
accountImplementation = new AccountHarness(makeAddr("entrypoint"), makeAddr("verifier"));

// 2. deploy an implementation of the factory and its instance
factory = new AccountFactoryHarness(address(accountImplementation), makeAddr("factorySigner"));
Expand Down Expand Up @@ -46,7 +46,7 @@ contract SmartAccount__Initiliaze is BaseTest {
);

// 2. deploy the account and call the initialize function at the same time
SmartAccount account = factory.exposed_deployAccount(
AccountHarness account = factory.exposed_deployAccount(
keccak256(createFixtures.signer.credId),
createFixtures.signer.pubX,
createFixtures.signer.pubY,
Expand Down Expand Up @@ -75,7 +75,7 @@ contract SmartAccount__Initiliaze is BaseTest {
);

// 2. deploy the account and call the initialize function at the same time
SmartAccount account = factory.exposed_deployAccount(
AccountHarness account = factory.exposed_deployAccount(
keccak256(createFixtures.signer.credId),
createFixtures.signer.pubX,
createFixtures.signer.pubY,
Expand All @@ -90,7 +90,7 @@ contract SmartAccount__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
SmartAccount account = factory.exposed_deployAccount(
AccountHarness account = factory.exposed_deployAccount(
keccak256(createFixtures.signer.credId),
createFixtures.signer.pubX,
createFixtures.signer.pubY,
Expand All @@ -109,7 +109,22 @@ contract SmartAccount__Initiliaze is BaseTest {
);
}

// Duplicate of the event in the SmartAccount.sol file
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
event SignerAdded(
bytes1 indexed signatureType, bytes credId, bytes32 indexed credIdHash, uint256 pubKeyX, uint256 pubKeyY
);
Expand All @@ -127,7 +142,7 @@ contract SmartAccount__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
SmartAccount account = factory.exposed_deployAccount(credIdHash, pubX, pubY, credId);
AccountHarness account = factory.exposed_deployAccount(credIdHash, pubX, pubY, credId);

// 3. get the starting slot of the signer
bytes32 startingSlot = SignerVaultWebAuthnP256R1.getSignerStartingSlot(credIdHash);
Expand All @@ -139,6 +154,14 @@ contract SmartAccount__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) { }

Expand All @@ -149,8 +172,8 @@ contract AccountFactoryHarness is AccountFactory {
bytes calldata credId
)
external
returns (SmartAccount account)
returns (AccountHarness account)
{
return _deployAccount(credIdHash, pubX, pubY, credId);
return AccountHarness(payable(address(_deployAccount(credIdHash, pubX, pubY, credId))));
}
}
1 change: 1 addition & 0 deletions test/unit/v1/Account/initialize.tree
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ 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
41 changes: 27 additions & 14 deletions test/unit/v1/Account/validateCreationSignature.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,15 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
);
}

function test_FailsIfTheNonceIsNot0(uint256 randomNonce) external {
// it fails if the nonce is not 0
function test_FailsIfCalledTwice() external {
// it fails if called twice

// bound the nonce to a invalid range
randomNonce = bound(randomNonce, 1, type(uint256).max);

// get valid initcode and signature
// 1. get valid initcode and signature
(bytes memory initCode, bytes memory signature) = _calculateInitCodeAndSignature();

// mock the call to the entrypoint to return the random nonce
vm.mockCall(address(entrypoint), abi.encodeWithSelector(MockEntryPoint.getNonce.selector), abi.encode(1));

// assert that the signature validation fails if the nonce is not equal to zero
// 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);
}

Expand All @@ -86,7 +82,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
account.exposed_validateCreationSignature(signature, invalidInitCode);
}

function test_FailsIfTheUseropFactoryIsNotCorrect(address incorrectFactory) external view {
function test_FailsIfTheUseropFactoryIsNotCorrect(address incorrectFactory) external {
// it fails if the userop factory is not correct

vm.assume(incorrectFactory != address(factory));
Expand Down Expand Up @@ -139,7 +135,7 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
assertEq(account.exposed_validateCreationSignature(invalidSignature, initCode), Signature.State.FAILURE);
}

function test_FailsIfSignatureTypeMissing() external view {
function test_FailsIfSignatureTypeMissing() external {
// it fails if the passed signature is not correct

// 1. get valid initcode and signature
Expand Down Expand Up @@ -208,7 +204,21 @@ contract SmartAccount__ValidateCreationSignature is BaseTest {
assertEq(account.exposed_validateCreationSignature(signature, initCode), Signature.State.FAILURE);
}

function test_SucceedIfTheSignatureRecoveryIsCorrect() external view {
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);
}

function test_SucceedIfTheSignatureRecoveryIsCorrect() external {
// it succeed if the signature recovery is correct

// 1. get valid initcode and signature
Expand All @@ -227,11 +237,14 @@ contract SmartAccountHarness is SmartAccount {
bytes calldata initCode
)
external
view
returns (uint256)
{
return _validateCreationSignature(signature, initCode);
}

function exposed_creationFlowFuse() external view returns (uint256) {
return creationFlowFuse;
}
}

contract AccountFactoryHarness is AccountFactory {
Expand Down
3 changes: 2 additions & 1 deletion test/unit/v1/Account/validateCreationSignature.tree
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
SmartAccount__ValidateCreationSignature
├── it fails if the nonce is not 0
├── it fails if called twice
├── 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
Expand All @@ -8,4 +8,5 @@ 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
4 changes: 0 additions & 4 deletions test/unit/v1/AccountFactory/constructor.t.sol
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
// SPDX-License-Identifier: APACHE-2.0
pragma solidity >=0.8.20;

import { Initializable } from "@openzeppelin/proxy/utils/Initializable.sol";
import { AccountFactory } from "src/v1/AccountFactory.sol";
import { BaseTest } from "test/BaseTest/BaseTest.sol";
import { Metadata } from "src/v1/Metadata.sol";

// @DEV: constant used by the `Initializable` library
bytes32 constant INITIALIZABLE_STORAGE = 0xf0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a00;

contract AccountFactory__Constructor is BaseTest {
function test_RevertIfAccountImplementationIs0() external {
// it revert if account implementation is 0
Expand Down

0 comments on commit b6471ee

Please sign in to comment.