From cc537c52275d06ed3e37ae208593d1fa41fdadcd Mon Sep 17 00:00:00 2001 From: Ramon Recuero Date: Mon, 8 Jan 2024 10:29:59 -0800 Subject: [PATCH] Fixes app whitelist and child parent req --- src/apps/KintoAppRegistry.sol | 2 +- src/wallet/KintoWallet.sol | 1 + test/KintoWallet.t.sol | 31 ++++++++++++++++++++++++++++-- test/helpers/AATestScaffolding.sol | 15 +++++++++++++++ 4 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/apps/KintoAppRegistry.sol b/src/apps/KintoAppRegistry.sol index 02299b0c..b597fdb7 100644 --- a/src/apps/KintoAppRegistry.sol +++ b/src/apps/KintoAppRegistry.sol @@ -107,7 +107,7 @@ contract KintoAppRegistry is address[] calldata appContracts, uint256[4] calldata appLimits ) external override { - require(appLimits.length == 4, "Invalid app limits"); + require(childToParentContract[parentContract] == address(0), "Parent contract already registered as a child"); _updateMetadata(_name, parentContract, appContracts, appLimits); appCount++; _safeMint(msg.sender, appCount); diff --git a/src/wallet/KintoWallet.sol b/src/wallet/KintoWallet.sol index 1f72827c..ef08e2bf 100644 --- a/src/wallet/KintoWallet.sol +++ b/src/wallet/KintoWallet.sol @@ -181,6 +181,7 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto function setAppWhitelist(address[] calldata apps, bool[] calldata flags) external override onlySelf { require(apps.length == flags.length, "KW-apw: invalid array"); for (uint256 i = 0; i < apps.length; i++) { + require(appRegistry.getAppMetadata(apps[i]).admin != address(0), "KW-apw: app must be registered"); appWhitelist[apps[i]] = flags[i]; } } diff --git a/test/KintoWallet.t.sol b/test/KintoWallet.t.sol index 67db8d20..ccf69d56 100644 --- a/test/KintoWallet.t.sol +++ b/test/KintoWallet.t.sol @@ -161,6 +161,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { // deploy the counter contract Counter counter = new Counter(); assertEq(counter.count(), 0); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); // send a transaction to the counter contract through our wallet // without a paymaster and without prefunding the wallet @@ -185,7 +187,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { // deploy the counter contract Counter counter = new Counter(); assertEq(counter.count(), 0); - + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); // prefund wallet vm.deal(address(_kintoWalletv1), 1 ether); @@ -257,6 +260,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { assertEq(counter.count(), 0); vm.stopPrank(); _fundPaymasterForContract(address(counter)); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); vm.startPrank(_owner); // Let's send a transaction to the counter contract through our wallet uint256 nonce = _kintoWalletv1.getNonce(); @@ -295,6 +300,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { assertEq(counter.count(), 0); uint256 nonce = _kintoWalletv1.getNonce(); vm.stopPrank(); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); vm.startPrank(_owner); _fundPaymasterForContract(address(counter)); // Let's send a transaction to the counter contract through our wallet @@ -337,6 +344,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { assertEq(counter.count(), 0); uint256 nonce = _kintoWalletv1.getNonce(); vm.stopPrank(); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); vm.startPrank(_owner); _fundPaymasterForContract(address(counter)); address[] memory targets = new address[](3); @@ -378,6 +387,9 @@ contract KintoWalletTest is AATestScaffolding, UserOp { // only fund counter _fundPaymasterForContract(address(counter)); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); + // prep batch address[] memory targets = new address[](3); targets[0] = address(_kintoWalletv1); @@ -709,6 +721,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { // (4). fund paymaster for Counter contract _fundPaymasterForContract(address(counter)); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); // (5). Set private keys privateKeys = new uint256[](2); @@ -776,6 +790,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { // (4). fund paymaster for Counter contract _fundPaymasterForContract(address(counter)); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); // (5). Create 2 user ops: userOps = new UserOperation[](2); @@ -841,6 +857,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { // (4). fund paymaster for Counter contract _fundPaymasterForContract(address(counter)); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); // (5). Set private keys privateKeys = new uint256[](3); @@ -883,6 +901,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { // (2). fund paymaster for Counter contract _fundPaymasterForContract(address(counter)); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); // (3). Create 2 user ops: UserOperation[] memory userOps = new UserOperation[](1); @@ -949,6 +969,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { // (4). fund paymaster for Counter contract _fundPaymasterForContract(address(counter)); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); // (5). Set private keys privateKeys = new uint256[](2); @@ -1168,6 +1190,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { function test_RevertWhen_SettingAppKeyNoWhitelist() public { address app = address(_engenCredits); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(_engenCredits), childContracts); UserOperation memory userOp = this.createUserOperationWithPaymaster( _chainID, address(_kintoWalletv1), @@ -1197,6 +1221,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { function testSettingAppKey() public { address app = address(_engenCredits); uint256 nonce = _kintoWalletv1.getNonce(); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(_engenCredits), childContracts); UserOperation[] memory userOps = new UserOperation[](2); userOps[0] = createWhitelistAppOp( @@ -1221,7 +1247,6 @@ contract KintoWalletTest is AATestScaffolding, UserOp { function testMultisigTransactionWith2SignersWithAppkey() public { vm.startPrank(_owner); - // set 2 owners address[] memory owners = new address[](2); owners[0] = _owner; @@ -1250,6 +1275,8 @@ contract KintoWalletTest is AATestScaffolding, UserOp { Counter counter = new Counter(); assertEq(counter.count(), 0); vm.stopPrank(); + address[] memory childContracts = new address[](0); + createApp(_owner, "test", address(counter), childContracts); // Fund counter contract vm.startPrank(_owner); diff --git a/test/helpers/AATestScaffolding.sol b/test/helpers/AATestScaffolding.sol index 66043e5a..54a1f32c 100644 --- a/test/helpers/AATestScaffolding.sol +++ b/test/helpers/AATestScaffolding.sol @@ -206,6 +206,21 @@ abstract contract AATestScaffolding is KYCSignature { vm.stopPrank(); } + function createApp(address _owner, string memory name, address parentContract, address[] memory appContracts) + public + { + vm.startPrank(_owner); + uint256[] memory appLimits = new uint256[](4); + appLimits[0] = _kintoApp.RATE_LIMIT_PERIOD(); + appLimits[1] = _kintoApp.RATE_LIMIT_THRESHOLD(); + appLimits[2] = _kintoApp.GAS_LIMIT_PERIOD(); + appLimits[3] = _kintoApp.GAS_LIMIT_THRESHOLD(); + _kintoApp.registerApp( + name, parentContract, appContracts, [appLimits[0], appLimits[1], appLimits[2], appLimits[3]] + ); + vm.stopPrank(); + } + ////// helper methods to assert the revert reason on UserOperationRevertReason events //// // string reasons