From 29014953078bae6509befdb2af6945233a998677 Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Tue, 30 Jan 2024 10:55:47 -0800 Subject: [PATCH 1/9] Send money to account check --- src/wallet/KintoWalletFactory.sol | 11 ++++----- test/KintoWalletFactory.t.sol | 38 +++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/wallet/KintoWalletFactory.sol b/src/wallet/KintoWalletFactory.sol index 53775a690..c4c4692ca 100644 --- a/src/wallet/KintoWalletFactory.sol +++ b/src/wallet/KintoWalletFactory.sol @@ -198,15 +198,14 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl } /** - * @dev Send money to an account from privileged accounts + * @dev Send money to an account from privileged accounts or from kyc accounts to kyc accounts or contracts. * @param target The target address */ function sendMoneyToAccount(address target) external payable override { - require( - owner() == msg.sender || kintoID.isKYC(msg.sender) - || IAccessControl(address(kintoID)).hasRole(kintoID.KYC_PROVIDER_ROLE(), msg.sender), - "KYC or Provider role required" - ); + bool isPrivileged = owner() == msg.sender || IAccessControl(address(kintoID)).hasRole(kintoID.KYC_PROVIDER_ROLE(), msg.sender); + require(isPrivileged || kintoID.isKYC(msg.sender), "KYC or Provider role required"); + bool isContract = target.code.length > 0; + require(target != address(0) && ((kintoID.isKYC(target) || isContract) || isPrivileged), "Invalid target address"); (bool sent,) = target.call{value: msg.value}(""); require(sent, "Failed to send Ether"); } diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol index 471346054..04fa48561 100644 --- a/test/KintoWalletFactory.t.sol +++ b/test/KintoWalletFactory.t.sol @@ -321,10 +321,20 @@ contract KintoWalletFactoryTest is SharedSetup { function testSendMoneyToAccount_WhenCallerIsKYCd() public { approveKYC(_kycProvider, _user, _userPk); + approveKYC(_kycProvider, _user2, _user2Pk); + vm.deal(_user, 1 ether); vm.prank(_user); - _walletFactory.sendMoneyToAccount{value: 1e18}(address(123)); - assertEq(address(123).balance, 1e18); + _walletFactory.sendMoneyToAccount{value: 1e18}(address(_user2)); + assertEq(address(_user2).balance, 1e18); + } + + function testSendMoneyToAccount_WhenCallerIsKYCdAndTargetIsContract() public { + approveKYC(_kycProvider, _user, _userPk); + vm.deal(_user, 1 ether); + vm.prank(_user); + _walletFactory.sendMoneyToAccount{value: 1e18}(address(_kintoWallet)); + assertEq(address(_kintoWallet).balance, 1e18); } function testSendMoneyToAccount_WhenCallerIsOwner() public { @@ -334,6 +344,14 @@ contract KintoWalletFactoryTest is SharedSetup { assertEq(address(123).balance, 1e18); } + function testSendMoneyToAccount_WhenCallerIsOwnerAndTargetIsKYC() public { + approveKYC(_kycProvider, _user, _userPk); + revokeKYC(_kycProvider, _owner, _ownerPk); + vm.prank(_owner); + _walletFactory.sendMoneyToAccount{value: 1e18}(address(_user)); + assertEq(address(_user).balance, 1e18); + } + function testSendMoneyToAccount_WhenCallerIsKYCProvider() public { vm.deal(_kycProvider, 1 ether); vm.prank(_kycProvider); @@ -353,6 +371,14 @@ contract KintoWalletFactoryTest is SharedSetup { assertEq(address(123).balance, 1e18); } + function testSendMoneyToAccount_When_CallerKYCAndTargetContract() public { + vm.deal(_kycProvider, 1 ether); + vm.prank(_kycProvider); + uint beforeBalance = address(_kintoWallet).balance; + _walletFactory.sendMoneyToAccount{value: 1e18}(address(_kintoWallet)); + assertEq(address(_kintoWallet).balance, beforeBalance + 1e18); + } + function testSendMoneyToAccount_RevertWhen_CallerIsNotAllowed() public { vm.deal(address(123), 1 ether); vm.prank(address(123)); @@ -360,6 +386,14 @@ contract KintoWalletFactoryTest is SharedSetup { _walletFactory.sendMoneyToAccount{value: 1e18}(address(123)); } + function testSendMoneyToAccount_RevertWhen_CallerKYCButTargetisNot() public { + approveKYC(_kycProvider, _user, _userPk); + vm.deal(_user, 1 ether); + vm.prank(_user); + vm.expectRevert("Invalid target address"); + _walletFactory.sendMoneyToAccount{value: 1e18}(address(123)); + } + /* ============ Claim From Faucet tests ============ */ function testClaimFromFaucet_WhenCallerIsKYCd() public { From 9a9ec9cae4249abeaa6492dc7b604243c57f5336 Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Tue, 30 Jan 2024 10:56:36 -0800 Subject: [PATCH 2/9] format --- src/wallet/KintoWalletFactory.sol | 7 +++++-- test/KintoWalletFactory.t.sol | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/wallet/KintoWalletFactory.sol b/src/wallet/KintoWalletFactory.sol index c4c4692ca..0f0d7e3a7 100644 --- a/src/wallet/KintoWalletFactory.sol +++ b/src/wallet/KintoWalletFactory.sol @@ -202,10 +202,13 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl * @param target The target address */ function sendMoneyToAccount(address target) external payable override { - bool isPrivileged = owner() == msg.sender || IAccessControl(address(kintoID)).hasRole(kintoID.KYC_PROVIDER_ROLE(), msg.sender); + bool isPrivileged = + owner() == msg.sender || IAccessControl(address(kintoID)).hasRole(kintoID.KYC_PROVIDER_ROLE(), msg.sender); require(isPrivileged || kintoID.isKYC(msg.sender), "KYC or Provider role required"); bool isContract = target.code.length > 0; - require(target != address(0) && ((kintoID.isKYC(target) || isContract) || isPrivileged), "Invalid target address"); + require( + target != address(0) && ((kintoID.isKYC(target) || isContract) || isPrivileged), "Invalid target address" + ); (bool sent,) = target.call{value: msg.value}(""); require(sent, "Failed to send Ether"); } diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol index 04fa48561..c4ae0536d 100644 --- a/test/KintoWalletFactory.t.sol +++ b/test/KintoWalletFactory.t.sol @@ -374,7 +374,7 @@ contract KintoWalletFactoryTest is SharedSetup { function testSendMoneyToAccount_When_CallerKYCAndTargetContract() public { vm.deal(_kycProvider, 1 ether); vm.prank(_kycProvider); - uint beforeBalance = address(_kintoWallet).balance; + uint256 beforeBalance = address(_kintoWallet).balance; _walletFactory.sendMoneyToAccount{value: 1e18}(address(_kintoWallet)); assertEq(address(_kintoWallet).balance, beforeBalance + 1e18); } From 2ff1b5b16dda2b2007323919db7d3f27f113428c Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Wed, 31 Jan 2024 07:53:27 -0800 Subject: [PATCH 3/9] Update test/KintoWalletFactory.t.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Martín Alconada Verzini --- test/KintoWalletFactory.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol index c4ae0536d..fbd6aeced 100644 --- a/test/KintoWalletFactory.t.sol +++ b/test/KintoWalletFactory.t.sol @@ -371,7 +371,7 @@ contract KintoWalletFactoryTest is SharedSetup { assertEq(address(123).balance, 1e18); } - function testSendMoneyToAccount_When_CallerKYCAndTargetContract() public { + function testSendMoneyToAccount_WhenCallerIsKYCd_WhenTargetIsContract() public { vm.deal(_kycProvider, 1 ether); vm.prank(_kycProvider); uint256 beforeBalance = address(_kintoWallet).balance; From 4503615e00cf717cd5261a353f4f7bcafb02a75b Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Wed, 31 Jan 2024 07:53:37 -0800 Subject: [PATCH 4/9] Update test/KintoWalletFactory.t.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Martín Alconada Verzini --- test/KintoWalletFactory.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol index fbd6aeced..b0559413c 100644 --- a/test/KintoWalletFactory.t.sol +++ b/test/KintoWalletFactory.t.sol @@ -344,7 +344,7 @@ contract KintoWalletFactoryTest is SharedSetup { assertEq(address(123).balance, 1e18); } - function testSendMoneyToAccount_WhenCallerIsOwnerAndTargetIsKYC() public { + function testSendMoneyToAccount_WhenCallerIsOwner_WhenTargetIsKYC() public { approveKYC(_kycProvider, _user, _userPk); revokeKYC(_kycProvider, _owner, _ownerPk); vm.prank(_owner); From d5e206871ec7fe8abcdfe67f1fe148e3a3943fb3 Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Wed, 31 Jan 2024 07:53:43 -0800 Subject: [PATCH 5/9] Update test/KintoWalletFactory.t.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Martín Alconada Verzini --- test/KintoWalletFactory.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol index b0559413c..89ed5b65b 100644 --- a/test/KintoWalletFactory.t.sol +++ b/test/KintoWalletFactory.t.sol @@ -386,7 +386,7 @@ contract KintoWalletFactoryTest is SharedSetup { _walletFactory.sendMoneyToAccount{value: 1e18}(address(123)); } - function testSendMoneyToAccount_RevertWhen_CallerKYCButTargetisNot() public { + function testSendMoneyToAccount_RevertWhen_CallerIsKYCd_WhenTargetisNotKYCd() public { approveKYC(_kycProvider, _user, _userPk); vm.deal(_user, 1 ether); vm.prank(_user); From c1a4700c796662ecdbe2692b624d4af501b194e0 Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Wed, 31 Jan 2024 07:53:52 -0800 Subject: [PATCH 6/9] Update src/wallet/KintoWalletFactory.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Martín Alconada Verzini --- src/wallet/KintoWalletFactory.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/KintoWalletFactory.sol b/src/wallet/KintoWalletFactory.sol index 0f0d7e3a7..db7918080 100644 --- a/src/wallet/KintoWalletFactory.sol +++ b/src/wallet/KintoWalletFactory.sol @@ -205,7 +205,7 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl bool isPrivileged = owner() == msg.sender || IAccessControl(address(kintoID)).hasRole(kintoID.KYC_PROVIDER_ROLE(), msg.sender); require(isPrivileged || kintoID.isKYC(msg.sender), "KYC or Provider role required"); - bool isContract = target.code.length > 0; + bool isValidTarget = kintoID.isKYC(target) || target.code.length > 0; require( target != address(0) && ((kintoID.isKYC(target) || isContract) || isPrivileged), "Invalid target address" ); From 48458ac9696c19039f2b3df4a9b0ef21a79fd2d1 Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Wed, 31 Jan 2024 07:54:05 -0800 Subject: [PATCH 7/9] Update src/wallet/KintoWalletFactory.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Martín Alconada Verzini --- src/wallet/KintoWalletFactory.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/KintoWalletFactory.sol b/src/wallet/KintoWalletFactory.sol index db7918080..a5b605206 100644 --- a/src/wallet/KintoWalletFactory.sol +++ b/src/wallet/KintoWalletFactory.sol @@ -207,7 +207,7 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl require(isPrivileged || kintoID.isKYC(msg.sender), "KYC or Provider role required"); bool isValidTarget = kintoID.isKYC(target) || target.code.length > 0; require( - target != address(0) && ((kintoID.isKYC(target) || isContract) || isPrivileged), "Invalid target address" + require(isValidTarget || isPrivileged, "Target is not valid"); ); (bool sent,) = target.call{value: msg.value}(""); require(sent, "Failed to send Ether"); From 7cfcbd1cce1a69d93ea8b6d89f6e4a4f72c63f38 Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Wed, 31 Jan 2024 07:54:11 -0800 Subject: [PATCH 8/9] Update src/wallet/KintoWalletFactory.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Martín Alconada Verzini --- src/wallet/KintoWalletFactory.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/KintoWalletFactory.sol b/src/wallet/KintoWalletFactory.sol index a5b605206..9e417498b 100644 --- a/src/wallet/KintoWalletFactory.sol +++ b/src/wallet/KintoWalletFactory.sol @@ -202,6 +202,7 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl * @param target The target address */ function sendMoneyToAccount(address target) external payable override { + require(target != address(0), "Invalid target: zero address"); bool isPrivileged = owner() == msg.sender || IAccessControl(address(kintoID)).hasRole(kintoID.KYC_PROVIDER_ROLE(), msg.sender); require(isPrivileged || kintoID.isKYC(msg.sender), "KYC or Provider role required"); From 4fa310cde723d882fc93149c0165fd7e921fb16b Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Wed, 31 Jan 2024 07:58:52 -0800 Subject: [PATCH 9/9] send money comments --- src/wallet/KintoWalletFactory.sol | 6 ++---- test/KintoWalletFactory.t.sol | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/wallet/KintoWalletFactory.sol b/src/wallet/KintoWalletFactory.sol index 9e417498b..e95b358f4 100644 --- a/src/wallet/KintoWalletFactory.sol +++ b/src/wallet/KintoWalletFactory.sol @@ -202,14 +202,12 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl * @param target The target address */ function sendMoneyToAccount(address target) external payable override { - require(target != address(0), "Invalid target: zero address"); + require(target != address(0), "Invalid target: zero address"); bool isPrivileged = owner() == msg.sender || IAccessControl(address(kintoID)).hasRole(kintoID.KYC_PROVIDER_ROLE(), msg.sender); require(isPrivileged || kintoID.isKYC(msg.sender), "KYC or Provider role required"); bool isValidTarget = kintoID.isKYC(target) || target.code.length > 0; - require( - require(isValidTarget || isPrivileged, "Target is not valid"); - ); + require(isValidTarget || isPrivileged, "Target is not valid"); (bool sent,) = target.call{value: msg.value}(""); require(sent, "Failed to send Ether"); } diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol index 89ed5b65b..1d1825d58 100644 --- a/test/KintoWalletFactory.t.sol +++ b/test/KintoWalletFactory.t.sol @@ -390,7 +390,7 @@ contract KintoWalletFactoryTest is SharedSetup { approveKYC(_kycProvider, _user, _userPk); vm.deal(_user, 1 ether); vm.prank(_user); - vm.expectRevert("Invalid target address"); + vm.expectRevert("Target is not valid"); _walletFactory.sendMoneyToAccount{value: 1e18}(address(123)); }