From cb0f1da4cb6bc0115f295c04d6841cfc73c8ec91 Mon Sep 17 00:00:00 2001 From: Yash <72552910+kumaryash90@users.noreply.github.com> Date: Thu, 26 Oct 2023 22:45:45 +0530 Subject: [PATCH] Account factory: update constructor params (#556) * update constructor params * fix tests --- .../account/dynamic/DynamicAccountFactory.sol | 11 ++++++++--- .../account/managed/ManagedAccountFactory.sol | 10 +++++++--- .../account/non-upgradeable/AccountFactory.sol | 4 ++-- src/test/benchmark/AccountBenchmark.t.sol | 2 +- src/test/smart-wallet/Account.t.sol | 2 +- src/test/smart-wallet/AccountVulnPOC.t.sol | 2 +- src/test/smart-wallet/DynamicAccount.t.sol | 12 +++++------- src/test/smart-wallet/ManagedAccount.t.sol | 7 ++++++- .../smart-wallet/account-core/isValidSigner.t.sol | 2 +- .../setPermissionsForSigner.t.sol | 7 ++++--- 10 files changed, 36 insertions(+), 23 deletions(-) diff --git a/contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol b/contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol index 4763f97aa..ea5c320da 100644 --- a/contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol +++ b/contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol @@ -22,14 +22,19 @@ import { DynamicAccount, IEntryPoint } from "./DynamicAccount.sol"; // \____/ \__| \__|\__|\__| \_______| \_____\____/ \_______|\_______/ contract DynamicAccountFactory is BaseAccountFactory, ContractMetadata, PermissionsEnumerable { + address public constant ENTRYPOINT_ADDRESS = 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789; + /*/////////////////////////////////////////////////////////////// Constructor //////////////////////////////////////////////////////////////*/ - constructor(IEntryPoint _entrypoint, IExtension.Extension[] memory _defaultExtensions) - BaseAccountFactory(payable(address(new DynamicAccount(_entrypoint, _defaultExtensions))), address(_entrypoint)) + constructor(address _defaultAdmin, IExtension.Extension[] memory _defaultExtensions) + BaseAccountFactory( + payable(address(new DynamicAccount(IEntryPoint(ENTRYPOINT_ADDRESS), _defaultExtensions))), + ENTRYPOINT_ADDRESS + ) { - _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); + _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin); } /*/////////////////////////////////////////////////////////////// diff --git a/contracts/prebuilts/account/managed/ManagedAccountFactory.sol b/contracts/prebuilts/account/managed/ManagedAccountFactory.sol index 789ec4c3b..f46e1f2dd 100644 --- a/contracts/prebuilts/account/managed/ManagedAccountFactory.sol +++ b/contracts/prebuilts/account/managed/ManagedAccountFactory.sol @@ -26,15 +26,19 @@ contract ManagedAccountFactory is BaseAccountFactory, ContractMetadata, Permissi Constructor //////////////////////////////////////////////////////////////*/ - constructor(IEntryPoint _entrypoint, Extension[] memory _defaultExtensions) + constructor( + address _defaultAdmin, + IEntryPoint _entrypoint, + Extension[] memory _defaultExtensions + ) BaseRouter(_defaultExtensions) BaseAccountFactory(payable(address(new ManagedAccount(_entrypoint, address(this)))), address(_entrypoint)) { __BaseRouter_init(); - _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); + _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin); bytes32 _extensionRole = keccak256("EXTENSION_ROLE"); - _setupRole(_extensionRole, msg.sender); + _setupRole(_extensionRole, _defaultAdmin); _setRoleAdmin(_extensionRole, _extensionRole); } diff --git a/contracts/prebuilts/account/non-upgradeable/AccountFactory.sol b/contracts/prebuilts/account/non-upgradeable/AccountFactory.sol index 909309685..e570f0b4b 100644 --- a/contracts/prebuilts/account/non-upgradeable/AccountFactory.sol +++ b/contracts/prebuilts/account/non-upgradeable/AccountFactory.sol @@ -30,10 +30,10 @@ contract AccountFactory is BaseAccountFactory, ContractMetadata, PermissionsEnum Constructor //////////////////////////////////////////////////////////////*/ - constructor(IEntryPoint _entrypoint) + constructor(address _defaultAdmin, IEntryPoint _entrypoint) BaseAccountFactory(address(new Account(_entrypoint, address(this))), address(_entrypoint)) { - _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); + _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin); } /*/////////////////////////////////////////////////////////////// diff --git a/src/test/benchmark/AccountBenchmark.t.sol b/src/test/benchmark/AccountBenchmark.t.sol index 914915a6e..0a7ce86a3 100644 --- a/src/test/benchmark/AccountBenchmark.t.sol +++ b/src/test/benchmark/AccountBenchmark.t.sol @@ -181,7 +181,7 @@ contract AccountBenchmarkTest is BaseTest { // Setup contracts entrypoint = new EntryPoint(); // deploy account factory - accountFactory = new AccountFactory(IEntryPoint(payable(address(entrypoint)))); + accountFactory = new AccountFactory(deployer, IEntryPoint(payable(address(entrypoint)))); // deploy dummy contract numberContract = new Number(); } diff --git a/src/test/smart-wallet/Account.t.sol b/src/test/smart-wallet/Account.t.sol index fdd04e811..309ccf27d 100644 --- a/src/test/smart-wallet/Account.t.sol +++ b/src/test/smart-wallet/Account.t.sol @@ -188,7 +188,7 @@ contract SimpleAccountTest is BaseTest { // Setup contracts entrypoint = new EntryPoint(); // deploy account factory - accountFactory = new AccountFactory(IEntryPoint(payable(address(entrypoint)))); + accountFactory = new AccountFactory(deployer, IEntryPoint(payable(address(entrypoint)))); // deploy dummy contract numberContract = new Number(); } diff --git a/src/test/smart-wallet/AccountVulnPOC.t.sol b/src/test/smart-wallet/AccountVulnPOC.t.sol index 4de05daf2..3d6def6fc 100644 --- a/src/test/smart-wallet/AccountVulnPOC.t.sol +++ b/src/test/smart-wallet/AccountVulnPOC.t.sol @@ -212,7 +212,7 @@ contract SimpleAccountVulnPOCTest is BaseTest { // Setup contracts entrypoint = new EntryPoint(); // deploy account factory - accountFactory = new AccountFactory(IEntryPoint(payable(address(entrypoint)))); + accountFactory = new AccountFactory(deployer, IEntryPoint(payable(address(entrypoint)))); // deploy dummy contract numberContract = new Number(); } diff --git a/src/test/smart-wallet/DynamicAccount.t.sol b/src/test/smart-wallet/DynamicAccount.t.sol index 33f2a8585..92d505e6c 100644 --- a/src/test/smart-wallet/DynamicAccount.t.sol +++ b/src/test/smart-wallet/DynamicAccount.t.sol @@ -46,7 +46,7 @@ contract NFTRejector { contract DynamicAccountTest is BaseTest { // Target contracts - EntryPoint private entrypoint; + EntryPoint private constant entrypoint = EntryPoint(payable(0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789)); DynamicAccountFactory private accountFactory; // Mocks @@ -202,7 +202,8 @@ contract DynamicAccountTest is BaseTest { nonSigner = vm.addr(nonSignerPKey); // Setup contracts - entrypoint = new EntryPoint(); + address _deployedEntrypoint = address(new EntryPoint()); + vm.etch(address(entrypoint), bytes(_deployedEntrypoint.code)); // Setting up default extension. IExtension.Extension memory defaultExtension; @@ -248,7 +249,7 @@ contract DynamicAccountTest is BaseTest { extensions[0] = defaultExtension; // deploy account factory - accountFactory = new DynamicAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions); + accountFactory = new DynamicAccountFactory(deployer, extensions); // deploy dummy contract numberContract = new Number(); } @@ -303,10 +304,7 @@ contract DynamicAccountTest is BaseTest { extensions[0] = defaultExtension; // deploy account factory - DynamicAccountFactory factory = new DynamicAccountFactory( - IEntryPoint(payable(address(entrypoint))), - extensions - ); + DynamicAccountFactory factory = new DynamicAccountFactory(deployer, extensions); } /// @dev Create an account by directly calling the factory. diff --git a/src/test/smart-wallet/ManagedAccount.t.sol b/src/test/smart-wallet/ManagedAccount.t.sol index 8900950c5..10678ac99 100644 --- a/src/test/smart-wallet/ManagedAccount.t.sol +++ b/src/test/smart-wallet/ManagedAccount.t.sol @@ -250,7 +250,11 @@ contract ManagedAccountTest is BaseTest { // deploy account factory vm.prank(factoryDeployer); - accountFactory = new ManagedAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions); + accountFactory = new ManagedAccountFactory( + factoryDeployer, + IEntryPoint(payable(address(entrypoint))), + extensions + ); // deploy dummy contract numberContract = new Number(); } @@ -303,6 +307,7 @@ contract ManagedAccountTest is BaseTest { // deploy account factory vm.prank(factoryDeployer); ManagedAccountFactory factory = new ManagedAccountFactory( + factoryDeployer, IEntryPoint(payable(address(entrypoint))), extensions ); diff --git a/src/test/smart-wallet/account-core/isValidSigner.t.sol b/src/test/smart-wallet/account-core/isValidSigner.t.sol index 453a8a630..c289ca237 100644 --- a/src/test/smart-wallet/account-core/isValidSigner.t.sol +++ b/src/test/smart-wallet/account-core/isValidSigner.t.sol @@ -190,7 +190,7 @@ contract AccountCoreTest_isValidSigner is BaseTest { IExtension.Extension[] memory extensions; // deploy account factory - accountFactory = new DynamicAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions); + accountFactory = new DynamicAccountFactory(deployer, extensions); // deploy dummy contract numberContract = new Number(); diff --git a/src/test/smart-wallet/account-permissions/setPermissionsForSigner.t.sol b/src/test/smart-wallet/account-permissions/setPermissionsForSigner.t.sol index 170db6fcd..daf984e9f 100644 --- a/src/test/smart-wallet/account-permissions/setPermissionsForSigner.t.sol +++ b/src/test/smart-wallet/account-permissions/setPermissionsForSigner.t.sol @@ -54,7 +54,7 @@ contract AccountPermissionsTest_setPermissionsForSigner is BaseTest { ); // Target contracts - EntryPoint private entrypoint; + EntryPoint private constant entrypoint = EntryPoint(payable(0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789)); DynamicAccountFactory private accountFactory; // Mocks @@ -220,7 +220,8 @@ contract AccountPermissionsTest_setPermissionsForSigner is BaseTest { nonSigner = vm.addr(nonSignerPKey); // Setup contracts - entrypoint = new EntryPoint(); + address _deployedEntrypoint = address(new EntryPoint()); + vm.etch(address(entrypoint), bytes(_deployedEntrypoint.code)); // Setting up default extension. IExtension.Extension memory defaultExtension; @@ -266,7 +267,7 @@ contract AccountPermissionsTest_setPermissionsForSigner is BaseTest { extensions[0] = defaultExtension; // deploy account factory - accountFactory = new DynamicAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions); + accountFactory = new DynamicAccountFactory(deployer, extensions); // deploy dummy contract numberContract = new Number(); }