fix: handle inner call failure and msgvalue in factory contracts

bcnmy · Jul 29, 2024 · c10eb71 · c10eb71
1 parent 482e6cc
commit c10eb71
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 1 deletion.
diff --git a/contracts/factory/BiconomyMetaFactory.sol b/contracts/factory/BiconomyMetaFactory.sol
@@ -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());

diff --git a/contracts/factory/K1ValidatorFactory.sol b/contracts/factory/K1ValidatorFactory.sol
@@ -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.
@@ -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);
     }

diff --git a/contracts/factory/NexusAccountFactory.sol b/contracts/factory/NexusAccountFactory.sol
@@ -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);
         }
         return payable(account);
     }

diff --git a/contracts/factory/RegistryFactory.sol b/contracts/factory/RegistryFactory.sol
@@ -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);
     }

diff --git a/contracts/interfaces/factory/INexusFactory.sol b/contracts/interfaces/factory/INexusFactory.sol
@@ -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();