Merge pull request #24 from KintoXYZ/small-fixes

Make sure owner is assigned correctly when created by an EOA
KintoXYZ · Jan 4, 2024 · 2db164e · 2db164e
2 parents 1a879e5 + 9a9bf22
commit 2db164e
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 13 deletions.
diff --git a/script/test.sol b/script/test.sol
@@ -129,7 +129,9 @@ contract KintoDeployTestCounter is AASetup, KYCSignature, UserOp {
         address computed =
             _walletFactory.getContractAddress(bytes32(0), keccak256(abi.encodePacked(type(Counter).creationCode)));
         if (!isContract(computed)) {
-            address created = _walletFactory.deployContract(0, abi.encodePacked(type(Counter).creationCode), bytes32(0));
+            address created = _walletFactory.deployContract(
+                deployerPublicKey, 0, abi.encodePacked(type(Counter).creationCode), bytes32(0)
+            );
             console.log("Deployed Counter contract at", created);
         } else {
             console.log("Counter already deployed at", computed);
@@ -201,8 +203,9 @@ contract KintoDeployETHPriceIsRight is AASetup, KYCSignature, UserOp {
             bytes32(0), keccak256(abi.encodePacked(type(ETHPriceIsRight).creationCode))
         );
         if (!isContract(computed)) {
-            address created =
-                _walletFactory.deployContract(0, abi.encodePacked(type(ETHPriceIsRight).creationCode), bytes32(0));
+            address created = _walletFactory.deployContract(
+                deployerPublicKey, 0, abi.encodePacked(type(ETHPriceIsRight).creationCode), bytes32(0)
+            );
             console.log("Deployed ETHPriceIsRight contract at", created);
         } else {
             console.log("ETHPriceIsRight already deployed at", computed);

diff --git a/src/interfaces/IKintoWalletFactory.sol b/src/interfaces/IKintoWalletFactory.sol
@@ -14,7 +14,10 @@ interface IKintoWalletFactory {
 
     function createAccount(address owner, address recoverer, uint256 salt) external returns (IKintoWallet ret);
 
-    function deployContract(uint256 amount, bytes memory bytecode, bytes32 salt) external payable returns (address);
+    function deployContract(address contractOwner, uint256 amount, bytes memory bytecode, bytes32 salt)
+        external
+        payable
+        returns (address);
 
     function deployContractByWallet(uint256 amount, bytes memory bytecode, bytes32 salt)
         external

diff --git a/src/wallet/KintoWalletFactory.sol b/src/wallet/KintoWalletFactory.sol
@@ -162,22 +162,29 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
      * - `salt` must have not been used for `bytecode` already.
      * - the factory must have a balance of at least `amount`.
      * - if `amount` is non-zero, `bytecode` must have a `payable` constructor.
+     * @param contractOwner The address to be set as owner
+     * @param amount The amount of wei to send with the contract creation
+     * @param bytecode The bytecode of the contract to deploy
+     * @param salt The salt to use for the calculation
      */
-    function deployContract(uint256 amount, bytes calldata bytecode, bytes32 salt)
+    function deployContract(address contractOwner, uint256 amount, bytes calldata bytecode, bytes32 salt)
         external
         payable
         override
         returns (address)
     {
         require(kintoID.isKYC(msg.sender), "KYC required");
-        return _deployAndAssignOwnership(amount, bytecode, salt);
+        return _deployAndAssignOwnership(contractOwner, amount, bytecode, salt);
     }
 
     /**
      * @dev Deploys a contract using `CREATE2`. The address where the contract
      * will be deployed can be known in advance via {computeAddress}.
      *
      * Same as above but this one goes through the Kinto Wallet.
+     * @param amount The amount of wei to send with the contract creation
+     * @param bytecode The bytecode of the contract to deploy
+     * @param salt The salt to use for the calculation
      */
     function deployContractByWallet(uint256 amount, bytes calldata bytecode, bytes32 salt)
         external
@@ -186,7 +193,7 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
         returns (address)
     {
         require(walletTs[msg.sender] > 0 && kintoID.isKYC(KintoWallet(payable(msg.sender)).owners(0)), "KYC required");
-        return _deployAndAssignOwnership(amount, bytecode, salt);
+        return _deployAndAssignOwnership(msg.sender, amount, bytecode, salt);
     }
 
     /**
@@ -262,7 +269,7 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
         }
     }
 
-    function _deployAndAssignOwnership(uint256 amount, bytes calldata bytecode, bytes32 salt)
+    function _deployAndAssignOwnership(address newOwner, uint256 amount, bytes calldata bytecode, bytes32 salt)
         internal
         returns (address)
     {
@@ -272,7 +279,7 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
         // Assign ownership to the deployer if needed
         try OwnableUpgradeable(created).owner() returns (address owner) {
             if (owner == address(this)) {
-                OwnableUpgradeable(created).transferOwnership(msg.sender);
+                OwnableUpgradeable(created).transferOwnership(newOwner);
             }
         } catch {}
         return created;

diff --git a/test/DeployTests.t.sol b/test/DeployTests.t.sol
@@ -80,11 +80,13 @@ contract DeveloperDeployTest is Create2Helper, UserOp, AATestScaffolding {
         deployAAScaffolding(_owner, _kycProvider, _recoverer);
         vm.startPrank(_owner);
 
-        address created = _walletFactory.deployContract(0, abi.encodePacked(type(Counter).creationCode), bytes32(0));
+        address created =
+            _walletFactory.deployContract(_owner, 0, abi.encodePacked(type(Counter).creationCode), bytes32(0));
         _counter = Counter(created);
 
-        created =
-            _walletFactory.deployContract(0, abi.encodePacked(type(CounterInitializable).creationCode), bytes32(0));
+        created = _walletFactory.deployContract(
+            _owner, 0, abi.encodePacked(type(CounterInitializable).creationCode), bytes32(0)
+        );
 
         // deploy _proxy contract and point it to _implementation
         _proxyc = new UUPSProxy{salt: 0}(address(created), "");

diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol
@@ -133,7 +133,8 @@ contract KintoWalletFactoryTest is Create2Helper, UserOp, AATestScaffolding {
         vm.startPrank(_owner);
         address computed =
             _walletFactory.getContractAddress(bytes32(0), keccak256(abi.encodePacked(type(Counter).creationCode)));
-        address created = _walletFactory.deployContract(0, abi.encodePacked(type(Counter).creationCode), bytes32(0));
+        address created =
+            _walletFactory.deployContract(_owner, 0, abi.encodePacked(type(Counter).creationCode), bytes32(0));
         assertEq(computed, created);
         assertEq(Counter(created).count(), 0);
         Counter(created).increment();
@@ -145,6 +146,7 @@ contract KintoWalletFactoryTest is Create2Helper, UserOp, AATestScaffolding {
         vm.startPrank(_owner);
         bytes memory a = abi.encodeWithSelector(KintoWallet.initialize.selector, _owner, _owner);
         _walletFactory.deployContract(
+            _owner,
             0,
             abi.encodePacked(type(SafeBeaconProxy).creationCode, abi.encode(address(_walletFactory.beacon()), a)),
             bytes32(0)