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

🔒 H-02 - Prevent Freezing of Funds in Factory Contracts #114

Merged
merged 5 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion contracts/factory/BiconomyMetaFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ contract BiconomyMetaFactory is Stakeable {
/// @return createdAccount The address of the newly created Nexus account.
function deployWithFactory(address factory, bytes calldata factoryData) external payable returns (address payable createdAccount) {
require(factoryWhitelist[address(factory)], FactoryNotWhitelisted());
(bool success, bytes memory returnData) = factory.call{value: msg.value}(factoryData);
(bool success, bytes memory returnData) = factory.call{ value: msg.value }(factoryData);

// Check if the call was successful
require(success, CallToDeployWithFactoryFailed());
Expand Down
6 changes: 6 additions & 0 deletions contracts/factory/K1ValidatorFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ contract K1ValidatorFactory is Stakeable {
/// @notice Error thrown when a zero address is provided for the implementation, K1 validator, or bootstrapper.
error ZeroAddressNotAllowed();

/// @notice Error thrown when an inner call fails.
error InnerCallFailed();

/// @notice Constructor to set the immutable variables.
/// @param implementation The address of the Nexus implementation to be used for all deployments.
/// @param factoryOwner The address of the factory owner.
Expand Down Expand Up @@ -100,6 +103,9 @@ contract K1ValidatorFactory is Stakeable {
if (!alreadyDeployed) {
INexus(account).initializeAccount(initData);
emit AccountCreated(account, eoaOwner, index);
} else if (msg.value > 0) {
(bool success, ) = eoaOwner.call{ value: msg.value }("");
require(success, InnerCallFailed());
}
return payable(account);
}
Expand Down
2 changes: 2 additions & 0 deletions contracts/factory/NexusAccountFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ contract NexusAccountFactory is Stakeable, INexusFactory {
if (!alreadyDeployed) {
INexus(account).initializeAccount(initData);
emit AccountCreated(account, initData, salt);
} else if (msg.value > 0) {
revert AccountAlreadyDeployed(account);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have negative test case for this how entrypoint behaves.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by sending a userop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
return payable(account);
}
Expand Down
2 changes: 2 additions & 0 deletions contracts/factory/RegistryFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ contract RegistryFactory is Stakeable, INexusFactory {
if (!alreadyDeployed) {
INexus(account).initializeAccount(initData);
emit AccountCreated(account, initData, salt);
} else if (msg.value > 0) {
revert AccountAlreadyDeployed(account);
}
return payable(account);
}
Expand Down
7 changes: 7 additions & 0 deletions contracts/interfaces/factory/INexusFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,15 @@ pragma solidity ^0.8.26;
/// Special thanks to the Solady team for foundational contributions: https://github.com/Vectorized/solady
interface INexusFactory {
/// @notice Emitted when a new Smart Account is created.
/// @param account The address of the newly created account.
/// @param initData Initialization data used for the new Smart Account.
/// @param salt Unique salt used during the creation of the Smart Account.
event AccountCreated(address indexed account, bytes indexed initData, bytes32 indexed salt);

/// @notice Error indicating that the account is already deployed
/// @param account The address of the account that is already deployed
error AccountAlreadyDeployed(address account);

/// @notice Error thrown when the owner address is zero.
error ZeroAddressNotAllowed();

Expand Down
Loading