Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ remove the upgradeability of the AccountFactory #84

Merged
merged 3 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions script/Account/01_AccountDeployImplementation.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@ contract SmartAccountDeploy is BaseScript {
// 2. Check if the address of the entryPoint is deployed
require(entryPointAddress.code.length > 0, "The entrypoint is not deployed");

// 3. Run the script using the entrypoint address
return run(entryPointAddress);
}

function run(address entryPointAddress) internal broadcast returns (SmartAccount) {
// 3. Get the address of the verifier and check if it is deployed
address verifier = vm.envAddress("WEBAUTHN_VERIFIER");

// 1. Check if the address of the verifier is correct
require(verifier.code.length > 0, "The verifier is not deployed");

// 3. Run the script using the entrypoint address
return run(entryPointAddress, verifier);
}

function run(address entryPointAddress, address verifier) internal broadcast returns (SmartAccount) {
// 2. Deploy the smart account
SmartAccount account = new SmartAccount(entryPointAddress, verifier);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@ import { Metadata } from "src/v1/Metadata.sol";
/// @title FactoryDeployImplementation
/// @notice Deploy an implementation of the account factory
contract FactoryDeployImplementation is BaseScript {
function run() public broadcast returns (AccountFactory) {
function run() public returns (AccountFactory) {
// 1. get the environment variables
address payable accountImplementation = payable(vm.envAddress("ACCOUNT_IMPLEMENTATION"));
address factorySigner = vm.envAddress("FACTORY_SIGNER");

// 1. Check if the account implementation is deployed
// 2. Check if the account implementation is deployed
require(address(accountImplementation).code.length > 0, "Account not deployed");

// 2. Check the version of the account is the expected one
// 3. Check the version of the account is the expected one
require(Metadata.VERSION == SmartAccount(accountImplementation).version(), "Version mismatch");

// 3. Confirm the account implementation address with the user
// 4. Confirm the account implementation address with the user
string memory prompt = string(
abi.encodePacked(
"The account implementation can never be changed in the factory contract."
Expand All @@ -41,8 +43,13 @@ contract FactoryDeployImplementation is BaseScript {
revert("Entrypoint address not approved");
}

// 5. run the script
return run(accountImplementation, factorySigner);
}

function run(address accountImplementation, address factorySigner) internal broadcast returns (AccountFactory) {
// 4. Deploy the account factory
AccountFactory accountFactoryImplementation = new AccountFactory(accountImplementation);
AccountFactory accountFactoryImplementation = new AccountFactory(accountImplementation, factorySigner);

// 5. Check the version of the account factory is the expected one
require(Metadata.VERSION == accountFactoryImplementation.version(), "Version mismatch");
Expand Down
40 changes: 0 additions & 40 deletions script/AccountFactory/02_FactoryDeployInstance.s.sol

This file was deleted.

23 changes: 0 additions & 23 deletions script/AccountFactory/03_FactoryGetProxyAdminInformation.sol

This file was deleted.

18 changes: 0 additions & 18 deletions script/AccountFactory/07_FactoryGetAccountImplem.s.sol

This file was deleted.

42 changes: 20 additions & 22 deletions src/v1/AccountFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,28 @@
pragma solidity >=0.8.20 <0.9.0;

import { ERC1967Proxy } from "@openzeppelin/proxy/ERC1967/ERC1967Proxy.sol";
import { OwnableUpgradeable } from "@openzeppelin-upgradeable/access/OwnableUpgradeable.sol";
import { Initializable } from "@openzeppelin-upgradeable/proxy/utils/Initializable.sol";
import { SmartAccount } from "./Account/SmartAccount.sol";
import { Metadata } from "src/v1/Metadata.sol";
import { SignerVaultWebAuthnP256R1 } from "src/utils/SignerVaultWebAuthnP256R1.sol";
import "src/utils/Signature.sol" as Signature;

// FIXME: createAndInitAccount() Understand the implications of the ban system of the function

/// @title 4337-compliant Account Factory
/// @notice This contract is a 4337-compliant factory for smart-accounts. It is in charge of deploying an account
/// implementation during its construction, then deploying proxies for the users. The proxies are deployed
/// using the CREATE2 opcode and they use the implementation contract deployed on construction as a
/// reference. Once the account has been deployed by the factory, the factory is also in charge of setting
/// the first signer of the account, leading to a fully-setup account for the user.
/// @dev The signature is only used to set the first-signer to the account. It is a EIP-191 message
/// signed by the owner. The message is the keccak256 hash of the login of the account.
/// signed by the operator. The message is the keccak256 hash of the login of the account.
/// As the address of the account is already dependant of the address of the factory, we do not need to
/// include it in the signature.
contract AccountFactory is Initializable, OwnableUpgradeable {
contract AccountFactory {
// ==============================
// ========= CONSTANT ===========
// ==============================

address payable public immutable accountImplementation;
address private immutable operator;

// ==============================
// ======= EVENTS/ERRORS ========
Expand All @@ -36,6 +33,7 @@ contract AccountFactory is Initializable, OwnableUpgradeable {

error InvalidSignature(address accountAddress, bytes authenticatorData, bytes signature);
error InvalidAccountImplementation();
error InvalidSigner();

// ==============================
// ======= CONSTRUCTION =========
Expand All @@ -45,31 +43,26 @@ contract AccountFactory is Initializable, OwnableUpgradeable {
/// implementation will be used as the implementation reference for all the proxies deployed by this
/// factory.
/// @param _accountImplementation The address of the implementation of the smart account. Must never be changed!!
/// @param _operator The address of the operator of the factory
/// @dev All the arguments passed to the constructor function are used to set immutable variables.
constructor(address _accountImplementation) {
constructor(address _accountImplementation, address _operator) {
// 1. set the address of the implementation account -- THIS ADDRESS MUST NEVER BE CHANGED (!!)
if (_accountImplementation == address(0)) revert InvalidAccountImplementation();
accountImplementation = payable(_accountImplementation);

// 2. prevent the implementation contract from being used directly
_disableInitializers();
}

// The initialize function will be used to set up the initial state of the contract.
function initialize(address owner) public virtual reinitializer(1) {
// 1. set the owner
__Ownable_init(owner);
// 2. set the operator
if (_operator == address(0)) revert InvalidSigner();
operator = _operator;
}

// ==============================
// ===== INTERNAL FUNCTIONS =====
// ==============================

/// @notice This function checks if the signature is signed by the operator (owner)
/// @notice This function checks if the signature is signed by the operator
/// @param accountAddress The address of the account that would be deployed
/// @param authenticatorData The authenticatorData field of the WebAuthn response when creating a signer
/// @param signature Signature made off-chain by made the operator of the factory (owner). It gates the use of the
/// factory.
/// @param signature Signature made off-chain by made the operator of the factory. It gates the use of the factory.
/// @return True if the signature is legit, false otherwise
/// @dev Incorrect signatures are expected to lead to a revert by the library used
function _isSignatureLegit(
Expand All @@ -82,11 +75,11 @@ contract AccountFactory is Initializable, OwnableUpgradeable {
virtual
returns (bool)
{
// 1. Recreate the message signed by the operator (owner)
// 1. Recreate the message signed by the operator
bytes memory message = abi.encode(Signature.Type.CREATION, authenticatorData, accountAddress, block.chainid);

// 2. Try to recover the address and return if the signature is legit
return Signature.recover(owner(), message, signature[1:]);
return Signature.recover(operator, message, signature[1:]);
}

function _deployAccount(
Expand Down Expand Up @@ -170,8 +163,7 @@ contract AccountFactory is Initializable, OwnableUpgradeable {
/// @notice This function either deploys an account and sets its first signer or returns the address of an existing
/// account based on the parameter given
/// @param authenticatorData The authenticatorData field of the WebAuthn response when creating a signer
/// @param signature Signature made off-chain by made the operator of the factory (owner). It gates the use of the
/// factory.
/// @param signature Signature made off-chain by made the operator of the factory. It gates the use of the factory.
/// @return The address of the existing account (either deployed by this function or not)
function createAndInitAccount(
bytes calldata authenticatorData,
Expand Down Expand Up @@ -224,6 +216,12 @@ contract AccountFactory is Initializable, OwnableUpgradeable {
function version() external pure virtual returns (uint256) {
return Metadata.VERSION;
}

/// @notice This function returns the owner of the factory
/// @return The owner of the factory
function owner() external view virtual returns (address) {
return operator;
}
}

// NOTE:
Expand Down
8 changes: 0 additions & 8 deletions src/v1/Proxy/TransparentProxy.sol

This file was deleted.

3 changes: 1 addition & 2 deletions test/BaseTest/BaseTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ import { MessageHashUtils } from "@openzeppelin/utils/cryptography/MessageHashUt
import "src/utils/Signature.sol" as Signature;
import { BaseTestUtils } from "test/BaseTest/BaseTestUtils.sol";
import { BaseTestCreateFixtures } from "test/BaseTest/BaseTestCreateFixtures.sol";
import { BaseTestDeployment } from "test/BaseTest/BaseTestDeployment.sol";

/// @title BaseTest
/// @notice This contract override the default Foundry's `Test` contract with some utility functions
contract BaseTest is Test, BaseTestUtils, BaseTestCreateFixtures, BaseTestDeployment {
contract BaseTest is Test, BaseTestUtils, BaseTestCreateFixtures {
// solhint-disable-next-line var-name-mixedcase
VmSafe.Wallet internal SMOOTH_SIGNER;

Expand Down
42 changes: 0 additions & 42 deletions test/BaseTest/BaseTestDeployment.sol

This file was deleted.

3 changes: 1 addition & 2 deletions test/unit/v1/Account/SmartAccountERC1271/EIP191.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { WebAuthn256r1Wrapper } from "script/WebAuthn256r1/WebAuthn256r1Wrapper.sol";
import { SmartAccount } from "src/v1/Account/SmartAccount.sol";
import { AccountFactory } from "src/v1/AccountFactory.sol";
import { EIP1271_VALIDATION_SUCCESS, EIP1271_VALIDATION_FAILURE } from "src/v1/Account/SmartAccountEIP1271.sol";

Check warning on line 8 in test/unit/v1/Account/SmartAccountERC1271/EIP191.t.sol

View workflow job for this annotation

GitHub Actions / lint

imported name EIP1271_VALIDATION_SUCCESS is not used

Check warning on line 8 in test/unit/v1/Account/SmartAccountERC1271/EIP191.t.sol

View workflow job for this annotation

GitHub Actions / lint

imported name EIP1271_VALIDATION_FAILURE is not used

Check warning on line 8 in test/unit/v1/Account/SmartAccountERC1271/EIP191.t.sol

View workflow job for this annotation

GitHub Actions / lint

imported name EIP1271_VALIDATION_SUCCESS is not used

Check warning on line 8 in test/unit/v1/Account/SmartAccountERC1271/EIP191.t.sol

View workflow job for this annotation

GitHub Actions / lint

imported name EIP1271_VALIDATION_FAILURE is not used
import { BaseTest } from "test/BaseTest/BaseTest.sol";
import { SignerVaultWebAuthnP256R1 } from "src/utils/SignerVaultWebAuthnP256R1.sol";

Expand Down Expand Up @@ -34,8 +34,7 @@
SmartAccount accountImplementation = new SmartAccount(address(entrypoint), address(webauthn));

// 4. deploy the factory
address factoryImplementation = address(deployFactoryImplementation(address(accountImplementation)));
factory = deployFactoryInstance(factoryImplementation, makeAddr("proxy_owner"), SMOOTH_SIGNER.addr);
factory = new AccountFactory(address(accountImplementation), SMOOTH_SIGNER.addr);

// 5. set the signer data for the test
// This signer has been generated for the needs of this test file. It is a valid signer.
Expand Down
3 changes: 1 addition & 2 deletions test/unit/v1/Account/SmartAccountERC1271/EIP712.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { WebAuthn256r1Wrapper } from "script/WebAuthn256r1/WebAuthn256r1Wrapper.sol";
import { SmartAccount } from "src/v1/Account/SmartAccount.sol";
import { AccountFactory } from "src/v1/AccountFactory.sol";
import { EIP1271_VALIDATION_SUCCESS, EIP1271_VALIDATION_FAILURE } from "src/v1/Account/SmartAccountEIP1271.sol";

Check warning on line 7 in test/unit/v1/Account/SmartAccountERC1271/EIP712.t.sol

View workflow job for this annotation

GitHub Actions / lint

imported name EIP1271_VALIDATION_SUCCESS is not used

Check warning on line 7 in test/unit/v1/Account/SmartAccountERC1271/EIP712.t.sol

View workflow job for this annotation

GitHub Actions / lint

imported name EIP1271_VALIDATION_FAILURE is not used

Check warning on line 7 in test/unit/v1/Account/SmartAccountERC1271/EIP712.t.sol

View workflow job for this annotation

GitHub Actions / lint

imported name EIP1271_VALIDATION_SUCCESS is not used

Check warning on line 7 in test/unit/v1/Account/SmartAccountERC1271/EIP712.t.sol

View workflow job for this annotation

GitHub Actions / lint

imported name EIP1271_VALIDATION_FAILURE is not used
import { BaseTest } from "test/BaseTest/BaseTest.sol";
import { SignerVaultWebAuthnP256R1 } from "src/utils/SignerVaultWebAuthnP256R1.sol";

Expand Down Expand Up @@ -33,8 +33,7 @@
SmartAccount accountImplementation = new SmartAccount(address(entrypoint), address(webauthn));

// 4. deploy the factory
address factoryImplementation = address(deployFactoryImplementation(address(accountImplementation)));
factory = deployFactoryInstance(factoryImplementation, makeAddr("proxy_owner"), SMOOTH_SIGNER.addr);
factory = new AccountFactory(address(accountImplementation), SMOOTH_SIGNER.addr);

// 5. set the signer data for the test
// This signer has been generated for the needs of this test file. It is a valid signer.
Expand Down
4 changes: 1 addition & 3 deletions test/unit/v1/Account/addWebAuthnP256R1Signer.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ contract SmartAccount__AddWebAuthnP256R1Signer is BaseTest {
SmartAccount accountImplementation = new SmartAccount(entrypoint, makeAddr("verifier"));

// 3. deploy the factory
address factoryImplementation = address(deployFactoryImplementation(address(accountImplementation)));
AccountFactory factory =
deployFactoryInstance(factoryImplementation, makeAddr("proxy_owner"), SMOOTH_SIGNER.addr);
AccountFactory factory = new AccountFactory(address(accountImplementation), SMOOTH_SIGNER.addr);

// 4. calculate the future address of the account
address accountFutureAddress = factory.getAddress(createFixtures.response.authData);
Expand Down
3 changes: 1 addition & 2 deletions test/unit/v1/Account/getSigner.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ contract SmartAccount__GetSigner is BaseTest {
SmartAccount accountImplementation = new SmartAccount(entrypoint, makeAddr("verifier"));

// 3. deploy the factory
address factoryImplementation = address(deployFactoryImplementation(address(accountImplementation)));
factory = deployFactoryInstance(factoryImplementation, makeAddr("proxy_owner"), SMOOTH_SIGNER.addr);
factory = new AccountFactory(address(accountImplementation), SMOOTH_SIGNER.addr);
}

function _deployAndInitValidAccount() internal returns (SmartAccount) {
Expand Down
7 changes: 2 additions & 5 deletions test/unit/v1/Account/initialize.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ contract SmartAccount__Initiliaze is BaseTest {
accountImplementation = new SmartAccount(makeAddr("entrypoint"), makeAddr("verifier"));

// 2. deploy an implementation of the factory and its instance
address factoryImplementation = address(new AccountFactoryHarness(address(accountImplementation)));
factory = AccountFactoryHarness(
address(deployFactoryInstance(factoryImplementation, makeAddr("proxyOwner"), makeAddr("factorySigner")))
);
factory = new AccountFactoryHarness(address(accountImplementation), makeAddr("factorySigner"));
}

function test_RevertsIfCalledDirectly() external {
Expand Down Expand Up @@ -143,7 +140,7 @@ contract SmartAccount__Initiliaze is BaseTest {
}

contract AccountFactoryHarness is AccountFactory {
constructor(address accountImplementation) AccountFactory(accountImplementation) { }
constructor(address accountImplementation, address operator) AccountFactory(accountImplementation, operator) { }

function exposed_deployAccount(
bytes32 credIdHash,
Expand Down
Loading
Loading