Skip to content

Call disable initializer in smart wallet factory contract constructors #566

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 31, 2023
Merged

Call disable initializer in smart wallet factory contract constructors #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e6e87a
Add _disableInitializers()
nkrishang Oct 31, 2023
b408512
test revert: initialize implementation contract
nkrishang Oct 31, 2023
78cb6c2
Merge branch 'main' into nkrishang/disableInitializer-for-factory-con…
nkrishang Oct 31, 2023
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
4 changes: 3 additions & 1 deletion contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ contract DynamicAccountFactory is Initializable, BaseAccountFactory, ContractMet
address(new DynamicAccount(IEntryPoint(ENTRYPOINT_ADDRESS), _defaultExtensions)),
ENTRYPOINT_ADDRESS
)
{}
{
_disableInitializers();
}

/// @notice Initializes the factory contract.
function initialize(address _defaultAdmin, string memory _contractURI) external initializer {
Expand Down
4 changes: 3 additions & 1 deletion contracts/prebuilts/account/managed/ManagedAccountFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ contract ManagedAccountFactory is
constructor(IEntryPoint _entrypoint, Extension[] memory _defaultExtensions)
BaseRouter(_defaultExtensions)
BaseAccountFactory(address(new ManagedAccount(_entrypoint)), address(_entrypoint))
{}
{
_disableInitializers();
}

/// @notice Initializes the factory contract.
function initialize(address _defaultAdmin, string memory _contractURI) external initializer {
Expand Down
4 changes: 3 additions & 1 deletion contracts/prebuilts/account/non-upgradeable/AccountFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ contract AccountFactory is Initializable, BaseAccountFactory, ContractMetadata,
Constructor
//////////////////////////////////////////////////////////////*/

constructor(IEntryPoint _entrypoint) BaseAccountFactory(address(new Account(_entrypoint)), address(_entrypoint)) {}
constructor(IEntryPoint _entrypoint) BaseAccountFactory(address(new Account(_entrypoint)), address(_entrypoint)) {
_disableInitializers();
}

/// @notice Initializes the factory contract.
function initialize(address _defaultAdmin, string memory _contractURI) external initializer {
Expand Down
9 changes: 8 additions & 1 deletion src/test/smart-wallet/Account.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ contract SimpleAccountTest is BaseTest {

bytes32 private uidCache = bytes32("random uid");

address internal factoryImpl;

event AccountCreated(address indexed account, address indexed accountAdmin);

function _prepareSignature(IAccountPermissions.SignerPermissionRequest memory _req)
Expand Down Expand Up @@ -251,7 +253,7 @@ contract SimpleAccountTest is BaseTest {
// Setup contracts
entrypoint = new EntryPoint();
// deploy account factory
address factoryImpl = address(new AccountFactory(IEntryPoint(payable(address(entrypoint)))));
factoryImpl = address(new AccountFactory(IEntryPoint(payable(address(entrypoint)))));
accountFactory = AccountFactory(
address(
payable(
Expand All @@ -276,6 +278,11 @@ contract SimpleAccountTest is BaseTest {
assertEq(accountFactory.hasRole(0x00, deployer), true);
}

function test_revert_initializeImplementation() public {
vm.expectRevert("Initializable: contract is already initialized");
AccountFactory(factoryImpl).initialize(deployer, "https://example.com");
}

/*///////////////////////////////////////////////////////////////
Test: creating an account
//////////////////////////////////////////////////////////////*/
Expand Down
13 changes: 10 additions & 3 deletions src/test/smart-wallet/DynamicAccount.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ contract DynamicAccountTest is BaseTest {

bytes32 private uidCache = bytes32("random uid");

address internal factoryImpl;

event AccountCreated(address indexed account, address indexed accountAdmin);

function _prepareSignature(IAccountPermissions.SignerPermissionRequest memory _req)
Expand Down Expand Up @@ -320,7 +322,7 @@ contract DynamicAccountTest is BaseTest {
extensions[0] = defaultExtension;

// deploy account factory
address factoryImpl = address(new DynamicAccountFactory(extensions));
factoryImpl = address(new DynamicAccountFactory(extensions));
accountFactory = DynamicAccountFactory(
address(
payable(
Expand All @@ -339,6 +341,11 @@ contract DynamicAccountTest is BaseTest {
Test: creating an account
//////////////////////////////////////////////////////////////*/

function test_revert_initializeImplementation() public {
vm.expectRevert("Initializable: contract is already initialized");
DynamicAccountFactory(factoryImpl).initialize(deployer, "https://example.com");
}

/// @dev benchmark test for deployment gas cost
function test_deploy_dynamicAccount() public {
// Setting up default extension.
Expand Down Expand Up @@ -385,12 +392,12 @@ contract DynamicAccountTest is BaseTest {
extensions[0] = defaultExtension;

// deploy account factory
address factoryImpl = address(new DynamicAccountFactory(extensions));
address factoryImplementation = address(new DynamicAccountFactory(extensions));
DynamicAccountFactory factory = DynamicAccountFactory(
address(
payable(
new TWProxy(
factoryImpl,
factoryImplementation,
abi.encodeWithSignature("initialize(address,string)", deployer, "https://example.com")
)
)
Expand Down
11 changes: 9 additions & 2 deletions src/test/smart-wallet/ManagedAccount.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ contract ManagedAccountTest is BaseTest {

bytes32 private uidCache = bytes32("random uid");

address internal factoryImpl;

event AccountCreated(address indexed account, address indexed accountAdmin);

function _prepareSignature(IAccountPermissions.SignerPermissionRequest memory _req)
Expand Down Expand Up @@ -321,7 +323,7 @@ contract ManagedAccountTest is BaseTest {

// deploy account factory
vm.prank(factoryDeployer);
address factoryImpl = address(new ManagedAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions));
factoryImpl = address(new ManagedAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions));
accountFactory = ManagedAccountFactory(
payable(
address(
Expand All @@ -336,6 +338,11 @@ contract ManagedAccountTest is BaseTest {
numberContract = new Number();
}

function test_revert_initializeImplementation() public {
vm.expectRevert("Initializable: contract is already initialized");
ManagedAccountFactory(payable(factoryImpl)).initialize(deployer, "https://example.com");
}

/// @dev benchmark test for deployment gas cost
function test_deploy_managedAccount() public {
// Setting up default extension.
Expand Down Expand Up @@ -383,7 +390,7 @@ contract ManagedAccountTest is BaseTest {

// deploy account factory
vm.prank(factoryDeployer);
address factoryImpl = address(new ManagedAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions));
factoryImpl = address(new ManagedAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions));
ManagedAccountFactory factory = ManagedAccountFactory(
payable(
address(
Expand Down