From 3ccab8fc20b56db0eca43a9d1db642c9385260a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Mart=C3=ADn=20Alconada=20Verzini?= Date: Wed, 24 Jan 2024 12:00:15 -0300 Subject: [PATCH 1/5] chore: increase coverage --- script/test.sol | 2 - src/wallet/KintoWallet.sol | 21 ++- test/EngenCredits.t.sol | 3 - test/KYCViewer.t.sol | 52 ++----- test/KintoAppRegistry.t.sol | 4 +- test/KintoEntryPoint.t.sol | 4 +- test/KintoWalletFactory.t.sol | 98 +++++++++++++- test/{KintoWallet.t.sol => SharedSetup.t.sol} | 3 +- test/SponsorPaymastExploit.t.sol | 1 - test/SponsorPaymaster.t.sol | 49 ++++++- test/helpers/AATestScaffolding.sol | 29 ++++ test/wallet/execute/Execute.t.sol | 65 ++++----- test/wallet/executeBatch/ExecuteBatch.t.sol | 128 +++++++++++++++++- test/wallet/funder/Funder.t.sol | 5 +- test/wallet/policy/Policy.t.sol | 12 +- test/wallet/recovery/Recovery.t.sol | 6 +- test/wallet/setAppKey/AppKey.t.sol | 6 +- test/wallet/upgrade/Upgrade.t.sol | 5 +- .../validateSignature/ValidateSignature.t.sol | 50 +++++-- test/wallet/whitelist/Whitelist.t.sol | 5 +- 20 files changed, 406 insertions(+), 142 deletions(-) rename test/{KintoWallet.t.sol => SharedSetup.t.sol} (91%) diff --git a/script/test.sol b/script/test.sol index 9624d79c3..72a62239f 100644 --- a/script/test.sol +++ b/script/test.sol @@ -156,7 +156,6 @@ contract KintoDeployTestCounter is AASetup, KYCSignature, UserOp { ); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(deployerPublicKey)); console.log("After UserOp. Counter:", counter.count()); vm.stopBroadcast(); @@ -234,7 +233,6 @@ contract KintoDeployETHPriceIsRight is AASetup, KYCSignature, UserOp { ); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(deployerPublicKey)); console.log("After UserOp. ETHPriceIsRight guess count", ethpriceisright.guessCount()); console.log("After UserOp. ETHPriceIsRight avg guess", ethpriceisright.avgGuess()); diff --git a/src/wallet/KintoWallet.sol b/src/wallet/KintoWallet.sol index 3cc8a2057..16b0ac12b 100644 --- a/src/wallet/KintoWallet.sol +++ b/src/wallet/KintoWallet.sol @@ -111,7 +111,7 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto for (uint256 i = 0; i < dest.length; i++) { _executeInner(dest[i], values[i], func[i]); } - // If can transact, cancel recovery + // if can transact, cancel recovery inRecovery = 0; } @@ -282,8 +282,8 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto // if app key is not set or signature is not valid, verify signer policy if ( ( - signerPolicy == SINGLE_SIGNER && owners.length == 1 - && _verifySingleSignature(owners[0], hashData, userOp.signature) == SIG_VALIDATION_SUCCESS + signerPolicy == SINGLE_SIGNER + && _verifySingleSignature(hashData, userOp.signature) == SIG_VALIDATION_SUCCESS ) || ( signerPolicy != SINGLE_SIGNER @@ -335,6 +335,20 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto return appRegistry.isSponsored(sponsor, target) || appRegistry.childToParentContract(target) == sponsor; } + // @notice ensures at least 1 signer has signed the hash + // @dev useful when owners.length > 1 + function _verifySingleSignature(bytes32 hashData, bytes memory signature) private view returns (uint256) { + // ensure at least one signer has signed + address recovered = hashData.recover(signature); + for (uint256 i = 0; i < owners.length; i++) { + if (owners[i] == recovered) { + return _packValidationData(false, 0, 0); + } + } + return SIG_VALIDATION_FAILED; + } + + // @notice ensures signer has signed the hash function _verifySingleSignature(address signer, bytes32 hashData, bytes memory signature) private pure @@ -346,6 +360,7 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto return _packValidationData(false, 0, 0); } + // @notice ensures required signers have signed the hash function _verifyMultipleSignatures(bytes32 hashData, bytes memory signature) private view returns (uint256) { // calculate required signers uint256 requiredSigners = diff --git a/test/EngenCredits.t.sol b/test/EngenCredits.t.sol index d63ad34fb..94c5c4f70 100644 --- a/test/EngenCredits.t.sol +++ b/test/EngenCredits.t.sol @@ -139,7 +139,6 @@ contract EngenCreditsTest is UserOp, AATestScaffolding { privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(_engenCredits), address(_paymaster) ); userOps[1] = userOp; - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(_engenCredits.balanceOf(address(_kintoWallet)), 15); vm.stopPrank(); @@ -171,7 +170,6 @@ contract EngenCreditsTest is UserOp, AATestScaffolding { privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(_engenCredits), address(_paymaster) ); userOps[1] = userOp; - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(_engenCredits.balanceOf(address(_kintoWallet)), 20); vm.stopPrank(); @@ -198,7 +196,6 @@ contract EngenCreditsTest is UserOp, AATestScaffolding { privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(_engenCredits), address(_paymaster) ); userOps[1] = userOp; - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(_engenCredits.balanceOf(address(_kintoWallet)), 15); // call again diff --git a/test/KYCViewer.t.sol b/test/KYCViewer.t.sol index fcf5e6c72..61c524503 100644 --- a/test/KYCViewer.t.sol +++ b/test/KYCViewer.t.sol @@ -9,9 +9,8 @@ import "../src/wallet/KintoWalletFactory.sol"; import "../src/KintoID.sol"; import "../src/viewers/KYCViewer.sol"; -import "./helpers/UserOp.sol"; +import "./SharedSetup.t.sol"; import "./helpers/UUPSProxy.sol"; -import {AATestScaffolding} from "./helpers/AATestScaffolding.sol"; contract KYCViewerUpgraded is KYCViewer { function newFunction() external pure returns (uint256) { @@ -21,55 +20,28 @@ contract KYCViewerUpgraded is KYCViewer { constructor(address _kintoWalletFactory) KYCViewer(_kintoWalletFactory) {} } -contract KYCViewerTest is UserOp, AATestScaffolding { - uint256 _chainID = 1; - - UUPSProxy _proxyViewer; - KYCViewer _implkycViewer; - KYCViewerUpgraded _implKYCViewerUpgraded; - KYCViewer _kycViewer; - KYCViewerUpgraded _kycViewer2; - - function setUp() public { - vm.chainId(_chainID); - vm.startPrank(address(1)); - _owner.transfer(1e18); - vm.stopPrank(); - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - vm.startPrank(_owner); - _implkycViewer = new KYCViewer{salt: 0}(address(_walletFactory)); - // deploy _proxy contract and point it to _implementation - _proxyViewer = new UUPSProxy{salt: 0}(address(_implkycViewer), ""); - // wrap in ABI to support easier calls - _kycViewer = KYCViewer(address(_proxyViewer)); - // Initialize kyc viewer _proxy - _kycViewer.initialize(); - vm.stopPrank(); - } - - function testUp() public { - console.log("address owner", address(_owner)); +contract KYCViewerTest is SharedSetup { + function testUp() public override { + super.testUp(); assertEq(_kycViewer.owner(), _owner); assertEq(address(_entryPoint.walletFactory()), address(_kycViewer.walletFactory())); - address kintoID = address(_kycViewer.kintoID()); - assertEq(address(_walletFactory.kintoID()), kintoID); + assertEq(address(_walletFactory.kintoID()), address(_kycViewer.kintoID())); } /* ============ Upgrade Tests ============ */ - function testOwnerCanUpgradeViewer() public { - vm.startPrank(_owner); + function testUpgradeTo_WhenCallerIsOwner() public { KYCViewerUpgraded _implementationV2 = new KYCViewerUpgraded(address(_walletFactory)); + vm.prank(_owner); _kycViewer.upgradeTo(address(_implementationV2)); - // re-wrap the _proxy - _kycViewer2 = KYCViewerUpgraded(address(_kycViewer)); - assertEq(_kycViewer2.newFunction(), 1); - vm.stopPrank(); + assertEq(KYCViewerUpgraded(address(_kycViewer)).newFunction(), 1); } - function test_RevertWhen_OthersCannotUpgradeFactory() public { + function testUpgradeTo_RevertWhen_CallerIsNotOwner(address someone) public { + vm.assume(someone != _owner); KYCViewerUpgraded _implementationV2 = new KYCViewerUpgraded(address(_walletFactory)); vm.expectRevert("only owner"); + vm.prank(someone); _kycViewer.upgradeTo(address(_implementationV2)); } @@ -80,5 +52,7 @@ contract KYCViewerTest is UserOp, AATestScaffolding { assertEq(_kycViewer.isIndividual(address(_kintoWallet)), _kycViewer.isIndividual(_owner)); assertEq(_kycViewer.isCompany(address(_kintoWallet)), false); assertEq(_kycViewer.hasTrait(address(_kintoWallet), 6), false); + assertEq(_kycViewer.isSanctionsSafe(address(_kintoWallet)), true); + assertEq(_kycViewer.isSanctionsSafeIn(address(_kintoWallet), 1), true); } } diff --git a/test/KintoAppRegistry.t.sol b/test/KintoAppRegistry.t.sol index 002847216..5d619658b 100644 --- a/test/KintoAppRegistry.t.sol +++ b/test/KintoAppRegistry.t.sol @@ -6,7 +6,7 @@ import "forge-std/console.sol"; import "../src/apps/KintoAppRegistry.sol"; -import "./KintoWallet.t.sol"; +import "./SharedSetup.t.sol"; contract KintoAppRegistryV2 is KintoAppRegistry { function newFunction() external pure returns (uint256) { @@ -16,7 +16,7 @@ contract KintoAppRegistryV2 is KintoAppRegistry { constructor(IKintoWalletFactory _walletFactory) KintoAppRegistry(_walletFactory) {} } -contract KintoAppRegistryTest is KintoWalletTest { +contract KintoAppRegistryTest is SharedSetup { function testUp() public override { super.testUp(); diff --git a/test/KintoEntryPoint.t.sol b/test/KintoEntryPoint.t.sol index dd1c892c1..7a7bb4d14 100644 --- a/test/KintoEntryPoint.t.sol +++ b/test/KintoEntryPoint.t.sol @@ -4,9 +4,9 @@ pragma solidity ^0.8.18; import "forge-std/Test.sol"; import "forge-std/console.sol"; -import "./KintoWallet.t.sol"; +import "./SharedSetup.t.sol"; -contract KintoEntryPointTest is KintoWalletTest { +contract KintoEntryPointTest is SharedSetup { function testUp() public override { assertEq(_entryPoint.walletFactory(), address(_walletFactory)); } diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol index 69966ca9b..fdbb3621c 100644 --- a/test/KintoWalletFactory.t.sol +++ b/test/KintoWalletFactory.t.sol @@ -13,7 +13,7 @@ import "../src/sample/Counter.sol"; import "../src/interfaces/IKintoWallet.sol"; import "../src/wallet/KintoWallet.sol"; -import "./KintoWallet.t.sol"; +import "./SharedSetup.t.sol"; contract KintoWalletUpgrade is KintoWallet { constructor(IEntryPoint _entryPoint, IKintoID _kintoID, IKintoAppRegistry _kintoAppRegistry) @@ -33,7 +33,7 @@ contract KintoWalletFactoryUpgrade is KintoWalletFactory { } } -contract KintoWalletFactoryTest is KintoWalletTest { +contract KintoWalletFactoryTest is SharedSetup { using ECDSAUpgradeable for bytes32; using SignatureChecker for address; @@ -46,9 +46,26 @@ contract KintoWalletFactoryTest is KintoWalletTest { assertEq(_entryPoint.walletFactory(), address(_walletFactory)); } + /* ============ Create Account Tests ============ */ + + function testCreateAccount() public { + vm.prank(address(_owner)); + _kintoWallet = _walletFactory.createAccount(_owner, _owner, 0); + assertEq(_kintoWallet.owners(0), _owner); + } + + function testCreateAccount_WhenAlreadyExists() public { + vm.prank(address(_owner)); + _kintoWallet = _walletFactory.createAccount(_owner, _owner, 0); + + vm.prank(address(_owner)); + IKintoWallet _kintoWalletAfter = _walletFactory.createAccount(_owner, _owner, 0); + assertEq(address(_kintoWallet), address(_kintoWalletAfter)); + } + /* ============ Upgrade Tests ============ */ - function testOwnerCanUpgradeFactory() public { + function testUpgradeTo_WhenCallerIsOwner() public { vm.startPrank(_owner); KintoWalletFactoryUpgrade _newImplementation = new KintoWalletFactoryUpgrade(_kintoWalletImpl); _walletFactory.upgradeTo(address(_newImplementation)); @@ -58,9 +75,12 @@ contract KintoWalletFactoryTest is KintoWalletTest { vm.stopPrank(); } - function test_RevertWhen_OthersCannotUpgradeFactory() public { + function testUpgradeTo_RevertWhen_CallerIsNotOwner(address someone) public { + vm.assume(someone != _owner); KintoWalletFactoryUpgrade _newImplementation = new KintoWalletFactoryUpgrade(_kintoWalletImpl); + vm.expectRevert("Ownable: caller is not the owner"); + vm.prank(someone); _walletFactory.upgradeTo(address(_newImplementation)); } @@ -147,7 +167,6 @@ contract KintoWalletFactoryTest is KintoWalletTest { ); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); vm.deal(_funder, 1e17); vm.startPrank(_funder); @@ -176,13 +195,78 @@ contract KintoWalletFactoryTest is KintoWalletTest { /* ============ Recovery Tests ============ */ - function testStart_RevertWhen_RecoverNotRecoverer(address someone) public { - vm.assume(someone != address(_walletFactory)); + function testStartWalletRecovery_WhenCallerIsRecoverer() public { + vm.prank(address(_kintoWallet.recoverer())); + _walletFactory.startWalletRecovery(payable(address(_kintoWallet))); + } + + function testStartWalletRecovery_WhenCallerIsRecoverer_RevertWhen_WalletNotExists() public { + vm.prank(address(_kintoWallet.recoverer())); + vm.expectRevert("invalid wallet"); + _walletFactory.startWalletRecovery(payable(address(123))); + } + + function testStartWalletRecovery_RevertWhen_CallerIsNotRecoverer(address someone) public { + vm.assume(someone != address(_kintoWallet.recoverer())); vm.prank(someone); vm.expectRevert("only recoverer"); _walletFactory.startWalletRecovery(payable(address(_kintoWallet))); } + function testCompleteWalletRecovery_WhenCallerIsRecoverer() public { + vm.prank(address(_kintoWallet.recoverer())); + _walletFactory.startWalletRecovery(payable(address(_kintoWallet))); + + vm.warp(block.timestamp + _kintoWallet.RECOVERY_TIME() + 1); + + // approve KYC for _user burn KYC for _owner + revokeKYC(_kycProvider, _owner, _ownerPk); + approveKYC(_kycProvider, _user, _userPk); + + // run monitor + address[] memory users = new address[](1); + users[0] = _user; + IKintoID.MonitorUpdateData[][] memory updates = new IKintoID.MonitorUpdateData[][](1); + updates[0] = new IKintoID.MonitorUpdateData[](1); + updates[0][0] = IKintoID.MonitorUpdateData(true, true, 5); + vm.prank(_kycProvider); + _kintoIDv1.monitor(users, updates); + + vm.prank(address(_kintoWallet.recoverer())); + _walletFactory.completeWalletRecovery(payable(address(_kintoWallet)), users); + } + + function testCompleteWalletRecovery_RevertWhen_WhenCallerIsRecoverer_WalletNotExists() public { + vm.prank(address(_kintoWallet.recoverer())); + vm.expectRevert("invalid wallet"); + _walletFactory.completeWalletRecovery(payable(address(123)), new address[](0)); + } + + function testCompleteWalletRecovery_RevertWhen_CallerIsNotRecoverer(address someone) public { + vm.assume(someone != address(_kintoWallet.recoverer())); + vm.prank(someone); + vm.expectRevert("only recoverer"); + _walletFactory.completeWalletRecovery(payable(address(_kintoWallet)), new address[](0)); + } + + function testChangeWalletRecoverer_WhenCallerIsRecoverer() public { + vm.prank(address(_kintoWallet.recoverer())); + _walletFactory.changeWalletRecoverer(payable(address(_kintoWallet)), payable(address(123))); + } + + function testChangeWalletRecoverer_RevertWhen_CallerIsRecoverer_WhenWalletNotExists() public { + vm.prank(address(_kintoWallet.recoverer())); + vm.expectRevert("invalid wallet"); + _walletFactory.changeWalletRecoverer(payable(address(123)), payable(address(123))); + } + + function testChangeWalletRecoverer_RevertWhen_CallerIsNotRecoverer(address someone) public { + vm.assume(someone != address(_kintoWallet.recoverer())); + vm.prank(someone); + vm.expectRevert("only recoverer"); + _walletFactory.changeWalletRecoverer(payable(address(_kintoWallet)), payable(address(123))); + } + /* ============ Send Money Tests ============ */ function testSendMoneyToAccount_WhenCallerIsKYCd() public { diff --git a/test/KintoWallet.t.sol b/test/SharedSetup.t.sol similarity index 91% rename from test/KintoWallet.t.sol rename to test/SharedSetup.t.sol index 302e6cf0f..f20ddf8ce 100644 --- a/test/KintoWallet.t.sol +++ b/test/SharedSetup.t.sol @@ -16,7 +16,7 @@ import "./harness/SponsorPaymasterHarness.sol"; import {UserOp} from "./helpers/UserOp.sol"; import {AATestScaffolding} from "./helpers/AATestScaffolding.sol"; -contract KintoWalletTest is UserOp, AATestScaffolding { +contract SharedSetup is UserOp, AATestScaffolding { uint256[] privateKeys; Counter counter; @@ -27,6 +27,7 @@ contract KintoWalletTest is UserOp, AATestScaffolding { event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); event RecovererChanged(address indexed newRecoverer, address indexed recoverer); + event PostOpRevertReason(bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason); function setUp() public virtual { deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); diff --git a/test/SponsorPaymastExploit.t.sol b/test/SponsorPaymastExploit.t.sol index 9390e05bd..97c947cb0 100644 --- a/test/SponsorPaymastExploit.t.sol +++ b/test/SponsorPaymastExploit.t.sol @@ -87,7 +87,6 @@ contract SponsorPaymasterExploitTest is MyOpCreator, AATestScaffolding { ); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // Execute the transaction via the entry point uint256 balanceBefore = _owner.balance; console.log("HACKER BALANCE BEFORE", balanceBefore); vm.expectRevert(); diff --git a/test/SponsorPaymaster.t.sol b/test/SponsorPaymaster.t.sol index b4733b413..07e706eb6 100644 --- a/test/SponsorPaymaster.t.sol +++ b/test/SponsorPaymaster.t.sol @@ -12,7 +12,7 @@ import "../src/paymasters/SponsorPaymaster.sol"; import "../src/sample/Counter.sol"; import "../src/interfaces/IKintoWallet.sol"; -import "./KintoWallet.t.sol"; +import "./SharedSetup.t.sol"; contract SponsorPaymasterUpgrade is SponsorPaymaster { constructor(IEntryPoint __entryPoint, address _owner) SponsorPaymaster(__entryPoint) { @@ -25,7 +25,7 @@ contract SponsorPaymasterUpgrade is SponsorPaymaster { } } -contract SponsorPaymasterTest is KintoWalletTest { +contract SponsorPaymasterTest is SharedSetup { function setUp() public override { super.setUp(); vm.deal(_user, 1e20); @@ -131,6 +131,24 @@ contract SponsorPaymasterTest is KintoWalletTest { vm.stopPrank(); } + function testDepositInfo_WhenCallerDepositsToSomeoneElse(address someone) public { + vm.assume(someone != _owner); + vm.prank(_owner); + _paymaster.addDepositFor{value: 5e18}(address(someone)); + + (uint256 amount, uint256 _unlockBlock) = _paymaster.depositInfo(address(someone)); + assertEq(amount, 5e18); + assertEq(_unlockBlock, 0); + } + + function testDepositInfo_WhenCallerDepositsToHimself() public { + vm.prank(_owner); + _paymaster.addDepositFor{value: 5e18}(address(_owner)); + (uint256 amount, uint256 _unlockBlock) = _paymaster.depositInfo(address(_owner)); + assertEq(amount, 5e18); + assertEq(_unlockBlock, 0); + } + /* ============ Per-Op: Global Rate limits ============ */ function testValidatePaymasterUserOp() public { @@ -240,6 +258,24 @@ contract SponsorPaymasterTest is KintoWalletTest { /* ============ Global Rate limits (tx & batched ops rates) ============ */ + function testAppLimits() public { + (uint256 operationCount, uint256 lastOperationTime, uint256 ethCostCount, uint256 costLimitLastOperationTime) = + _paymaster.appUserLimit(address(_kintoWallet), address(counter)); + assertTrue(operationCount == 0); + assertTrue(lastOperationTime == 0); + assertTrue(ethCostCount == 0); + assertTrue(costLimitLastOperationTime == 0); + + _incrementCounterTxs(10, address(counter)); + + (operationCount, lastOperationTime, ethCostCount, costLimitLastOperationTime) = + _paymaster.appUserLimit(address(_kintoWallet), address(counter)); + assertTrue(operationCount > 0); + assertTrue(lastOperationTime > 0); + assertTrue(ethCostCount > 0); + assertTrue(costLimitLastOperationTime > 0); + } + function testValidatePaymasterUserOp_WithinTxRateLimit() public { // fixme: once _setOperationCount works fine, refactor and use _setOperationCount; @@ -512,19 +548,18 @@ contract SponsorPaymasterTest is KintoWalletTest { uint256[4] memory appLimits = _kintoAppRegistry.getContractLimits(address(counter)); uint256 estimatedGasPerTx = 0; uint256 cumulativeGasUsed = 0; + UserOperation[] memory userOps = new UserOperation[](1); while (cumulativeGasUsed < appLimits[3]) { if (cumulativeGasUsed + estimatedGasPerTx >= appLimits[3]) return amt; userOps[0] = _incrementCounterOps(1, app)[0]; // generate 1 user op + uint256 beforeGas = gasleft(); _entryPoint.handleOps(userOps, payable(_owner)); // execute the op - uint256 afterGas = gasleft(); - if (amt == 0) estimatedGasPerTx = (beforeGas - afterGas); + + if (amt == 0) estimatedGasPerTx = (beforeGas - gasleft()); cumulativeGasUsed += estimatedGasPerTx; amt++; } } - - //// events - event PostOpRevertReason(bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason); } diff --git a/test/helpers/AATestScaffolding.sol b/test/helpers/AATestScaffolding.sol index de8d98fd5..bc7f96a60 100644 --- a/test/helpers/AATestScaffolding.sol +++ b/test/helpers/AATestScaffolding.sol @@ -14,6 +14,7 @@ import "../../src/tokens/EngenCredits.sol"; import "../../src/paymasters/SponsorPaymaster.sol"; import "../../src/wallet/KintoWallet.sol"; import "../../src/wallet/KintoWalletFactory.sol"; +import "../../src/viewers/KYCViewer.sol"; import "../helpers/UUPSProxy.sol"; import "../helpers/KYCSignature.sol"; @@ -23,8 +24,10 @@ import {SponsorPaymasterHarness} from "../harness/SponsorPaymasterHarness.sol"; abstract contract AATestScaffolding is KYCSignature { IKintoEntryPoint _entryPoint; + // Kinto Registry KintoAppRegistry _kintoAppRegistry; + // Kinto ID KintoID _implementation; KintoID _kintoIDv1; @@ -33,8 +36,10 @@ abstract contract AATestScaffolding is KYCSignature { KintoWallet _kintoWalletImpl; IKintoWallet _kintoWallet; + // Others EngenCredits _engenCredits; SponsorPaymaster _paymaster; + KYCViewer _kycViewer; // proxies UUPSProxy _proxy; @@ -42,6 +47,7 @@ abstract contract AATestScaffolding is KYCSignature { UUPSProxy _proxyPaymaster; UUPSProxy _proxyCredit; UUPSProxy _proxyRegistry; + UUPSProxy _proxyKYCViewer; function deployAAScaffolding(address _owner, uint256 _ownerPk, address _kycProvider, address _recoverer) public { // Deploy Kinto ID @@ -65,6 +71,9 @@ abstract contract AATestScaffolding is KYCSignature { // Deploy Engen Credits deployEngenCredits(_owner); + // Deploy KYC Viewer + deployKYCViewer(_owner); + vm.prank(_owner); // deploy latest KintoWallet version through wallet factory and initialize it @@ -177,6 +186,24 @@ abstract contract AATestScaffolding is KYCSignature { vm.stopPrank(); } + function deployKYCViewer(address _owner) public { + vm.startPrank(_owner); + + // deploy the Kinto KYC Viewer + _kycViewer = new KYCViewer{salt: 0}(address(_walletFactory)); + + // deploy _proxy contract and point it to _implementation + _proxyKYCViewer = new UUPSProxy{salt: 0}(address(_kycViewer), ""); + + // wrap in ABI to support easier calls + _kycViewer = KYCViewer(address(_proxyKYCViewer)); + + // initialize proxy + _kycViewer.initialize(); + + vm.stopPrank(); + } + function approveKYC(address _kycProvider, address _account, uint256 _accountPk) public { vm.startPrank(_kycProvider); @@ -378,6 +405,8 @@ abstract contract AATestScaffolding is KYCSignature { // clean revert reason & assert string memory cleanRevertReason = _trimToPrefixAndRemoveTrailingNulls(decodedRevertReason, prefixes); + console.log("left: %s", cleanRevertReason); + console.log("right: %s", string(_reasons[idx])); assertEq(cleanRevertReason, string(_reasons[idx]), "Revert reason does not match"); if (_reasons.length > 1) idx++; // if there's only one reason, we always use the same one diff --git a/test/wallet/execute/Execute.t.sol b/test/wallet/execute/Execute.t.sol index 2e99833e2..2e9ce66c2 100644 --- a/test/wallet/execute/Execute.t.sol +++ b/test/wallet/execute/Execute.t.sol @@ -2,21 +2,15 @@ pragma solidity ^0.8.18; import "forge-std/console.sol"; -import "../../KintoWallet.t.sol"; - -contract ExecuteTest is KintoWalletTest { - /* ============ One Signer Account Transaction Tests (execute) ============ */ +import "../../SharedSetup.t.sol"; +contract ExecuteTest is SharedSetup { function testExecute_WhenPaymaster() public { - uint256 nonce = _kintoWallet.getNonce(); - bool[] memory flags = new bool[](1); - flags[0] = true; - UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = _createUserOperation( address(_kintoWallet), address(counter), - nonce, + _kintoWallet.getNonce(), privateKeys, abi.encodeWithSignature("increment()"), address(_paymaster) @@ -39,7 +33,6 @@ contract ExecuteTest is KintoWalletTest { UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // execute the transaction via the entry point vm.expectRevert(abi.encodeWithSignature("FailedOp(uint256,string)", 0, "AA21 didn't pay prefund")); _entryPoint.handleOps(userOps, payable(_owner)); } @@ -58,11 +51,35 @@ contract ExecuteTest is KintoWalletTest { abi.encodeWithSignature("increment()") ); - // execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(counter.count(), 1); } + function testExecute_WhenMultipleOps_WhenPaymaster() public { + uint256 nonce = _kintoWallet.getNonce(); + UserOperation[] memory userOps = new UserOperation[](2); + userOps[0] = _createUserOperation( + address(_kintoWallet), + address(counter), + nonce, + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) + ); + + userOps[1] = _createUserOperation( + address(_kintoWallet), + address(counter), + nonce + 1, + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) + ); + + _entryPoint.handleOps(userOps, payable(_owner)); + assertEq(counter.count(), 2); + } + function testExecute_RevertWhen_AppIsNotWhitelisted() public { // remove app from whitelist whitelistApp(address(counter), false); @@ -88,30 +105,4 @@ contract ExecuteTest is KintoWalletTest { assertRevertReasonEq("KW: contract not whitelisted"); assertEq(counter.count(), 0); } - - function testExecute_WhenMultipleOps_WhenPaymaster() public { - uint256 nonce = _kintoWallet.getNonce(); - UserOperation[] memory userOps = new UserOperation[](2); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(counter), - nonce, - privateKeys, - abi.encodeWithSignature("increment()"), - address(_paymaster) - ); - - userOps[1] = _createUserOperation( - address(_kintoWallet), - address(counter), - nonce + 1, - privateKeys, - abi.encodeWithSignature("increment()"), - address(_paymaster) - ); - - // execute the transaction via the entry point - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 2); - } } diff --git a/test/wallet/executeBatch/ExecuteBatch.t.sol b/test/wallet/executeBatch/ExecuteBatch.t.sol index eb5e76020..58b2e5c92 100644 --- a/test/wallet/executeBatch/ExecuteBatch.t.sol +++ b/test/wallet/executeBatch/ExecuteBatch.t.sol @@ -6,6 +6,130 @@ import "forge-std/console.sol"; import "@aa/interfaces/IEntryPoint.sol"; -import "../../KintoWallet.t.sol"; +import "../../SharedSetup.t.sol"; -contract ExecuteBatchTest is KintoWalletTest {} +contract ExecuteBatchTest is SharedSetup { + function testExecuteBatch_WhenPaymaster() public { + // prep batch + address[] memory targets = new address[](1); + targets[0] = address(counter); + + uint256[] memory values = new uint256[](1); + values[0] = 0; + + // we want to do 3 calls: whitelistApp, increment counter and increment counter2 + bytes[] memory calls = new bytes[](1); + calls[0] = abi.encodeWithSignature("increment()"); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + ); + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = userOp; + + _entryPoint.handleOps(userOps, payable(_owner)); + assertEq(counter.count(), 1); + } + + function testExecuteBatch_RevertWhen_NoPaymasterNorPrefund() public { + // prep batch + address[] memory targets = new address[](1); + targets[0] = address(counter); + + uint256[] memory values = new uint256[](1); + values[0] = 0; + + bytes[] memory calls = new bytes[](1); + calls[0] = abi.encodeWithSignature("increment()"); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + UserOperation memory userOp = + _createUserOperation(address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(0)); + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = userOp; + + vm.expectRevert(abi.encodeWithSignature("FailedOp(uint256,string)", 0, "AA21 didn't pay prefund")); + _entryPoint.handleOps(userOps, payable(_owner)); + } + + function testExecuteBatch_WhenPrefund() public { + // prefund wallet + vm.deal(address(_kintoWallet), 1 ether); + + // prep batch + address[] memory targets = new address[](1); + targets[0] = address(counter); + + uint256[] memory values = new uint256[](1); + values[0] = 0; + + // we want to do 3 calls: whitelistApp, increment counter and increment counter2 + bytes[] memory calls = new bytes[](1); + calls[0] = abi.encodeWithSignature("increment()"); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = + _createUserOperation(address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(0)); + + _entryPoint.handleOps(userOps, payable(_owner)); + assertEq(counter.count(), 1); + } + + function testExecuteBatch_WhenMultipleOps_WhenPaymaster() public { + // prep batch + address[] memory targets = new address[](2); + targets[0] = address(counter); + targets[1] = address(counter); + + uint256[] memory values = new uint256[](2); + values[0] = 0; + + // we want to do 2 calls + bytes[] memory calls = new bytes[](2); + calls[0] = abi.encodeWithSignature("increment()"); + calls[1] = abi.encodeWithSignature("increment()"); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + ); + + _entryPoint.handleOps(userOps, payable(_owner)); + assertEq(counter.count(), 2); + } + + function testExecuteBatch_RevertWhen_AppIsNotWhitelisted() public { + // remove app from whitelist + whitelistApp(address(counter), false); + + // prep batch + address[] memory targets = new address[](2); + targets[0] = address(_kintoWallet); + targets[1] = address(counter); + + uint256[] memory values = new uint256[](2); + values[0] = 0; + values[1] = 0; + + bytes[] memory calls = new bytes[](2); + calls[0] = abi.encodeWithSignature("recoverer()"); + calls[1] = abi.encodeWithSignature("increment()"); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + ); + + vm.expectEmit(true, true, true, false); + emit UserOperationRevertReason( + _entryPoint.getUserOpHash(userOps[0]), userOps[0].sender, userOps[0].nonce, bytes("") + ); + vm.recordLogs(); + _entryPoint.handleOps(userOps, payable(_owner)); + assertRevertReasonEq("KW: contract not whitelisted"); + } +} diff --git a/test/wallet/funder/Funder.t.sol b/test/wallet/funder/Funder.t.sol index dbf30907e..9c3408a1f 100644 --- a/test/wallet/funder/Funder.t.sol +++ b/test/wallet/funder/Funder.t.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.18; import "forge-std/console.sol"; -import "../../KintoWallet.t.sol"; +import "../../SharedSetup.t.sol"; -contract FunderTest is KintoWalletTest { +contract FunderTest is SharedSetup { /* ============ Funder Whitelist ============ */ function testUp() public override { @@ -32,7 +32,6 @@ contract FunderTest is KintoWalletTest { ); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(_kintoWallet.isFunderWhitelisted(address(23)), true); vm.stopPrank(); diff --git a/test/wallet/policy/Policy.t.sol b/test/wallet/policy/Policy.t.sol index 5e80bd92f..b4dd51bfd 100644 --- a/test/wallet/policy/Policy.t.sol +++ b/test/wallet/policy/Policy.t.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.18; import "forge-std/console.sol"; -import "../../KintoWallet.t.sol"; +import "../../SharedSetup.t.sol"; -contract ResetSignerTest is KintoWalletTest { +contract ResetSignerTest is SharedSetup { /* ============ Signers & Policy Tests ============ */ function testAddingOneSigner() public { @@ -24,7 +24,6 @@ contract ResetSignerTest is KintoWalletTest { UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(_kintoWallet.owners(1), _user); vm.stopPrank(); @@ -46,7 +45,6 @@ contract ResetSignerTest is KintoWalletTest { UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // execute the transaction via the entry point // @dev handleOps fails silently (does not revert) vm.expectEmit(true, true, true, false); emit UserOperationRevertReason( @@ -71,7 +69,6 @@ contract ResetSignerTest is KintoWalletTest { UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // execute the transaction via the entry point // @dev handleOps fails silently (does not revert) vm.expectEmit(true, true, true, false); emit UserOperationRevertReason( @@ -100,7 +97,6 @@ contract ResetSignerTest is KintoWalletTest { UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // execute the transaction via the entry point // @dev handleOps fails silently (does not revert) vm.expectEmit(true, true, true, false); emit UserOperationRevertReason( @@ -126,7 +122,6 @@ contract ResetSignerTest is KintoWalletTest { UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); } @@ -145,7 +140,6 @@ contract ResetSignerTest is KintoWalletTest { address(_paymaster) ); - // execute the transaction via the entry point vm.expectEmit(); emit WalletPolicyChanged(_kintoWallet.ALL_SIGNERS(), _kintoWallet.SINGLE_SIGNER()); _entryPoint.handleOps(userOps, payable(_owner)); @@ -171,7 +165,6 @@ contract ResetSignerTest is KintoWalletTest { ); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(_kintoWallet.owners(1), _user); assertEq(_kintoWallet.owners(2), _user2); @@ -221,7 +214,6 @@ contract ResetSignerTest is KintoWalletTest { _entryPoint.getUserOpHash(userOps[1]), userOps[1].sender, userOps[1].nonce, bytes("") ); - // Execute the transaction via the entry point vm.recordLogs(); _entryPoint.handleOps(userOps, payable(_owner)); diff --git a/test/wallet/recovery/Recovery.t.sol b/test/wallet/recovery/Recovery.t.sol index 834d5bb23..cc54d005a 100644 --- a/test/wallet/recovery/Recovery.t.sol +++ b/test/wallet/recovery/Recovery.t.sol @@ -2,12 +2,12 @@ pragma solidity ^0.8.18; import "forge-std/console.sol"; -import "../../KintoWallet.t.sol"; +import "../../SharedSetup.t.sol"; -contract RecoveryTest is KintoWalletTest { +contract RecoveryTest is SharedSetup { /* ============ Recovery Tests ============ */ - function testStartRecovert() public { + function testStartRecovery() public { vm.prank(address(_walletFactory)); _kintoWallet.startRecovery(); assertEq(_kintoWallet.inRecovery(), block.timestamp); diff --git a/test/wallet/setAppKey/AppKey.t.sol b/test/wallet/setAppKey/AppKey.t.sol index 01f6804ec..a957109be 100644 --- a/test/wallet/setAppKey/AppKey.t.sol +++ b/test/wallet/setAppKey/AppKey.t.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.18; import "forge-std/console.sol"; -import "../../KintoWallet.t.sol"; +import "../../SharedSetup.t.sol"; -contract AppKeyTest is KintoWalletTest { +contract AppKeyTest is SharedSetup { /* ============ App Key ============ */ function testSetAppKey() public { @@ -18,7 +18,6 @@ contract AppKeyTest is KintoWalletTest { address(_paymaster) ); - // Execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(_kintoWallet.appSigner(address(counter)), _user); } @@ -39,7 +38,6 @@ contract AppKeyTest is KintoWalletTest { address(_paymaster) ); - // execute the transaction via the entry point address appSignerBefore = _kintoWallet.appSigner(address(_engenCredits)); // @dev handleOps fails silently (does not revert) diff --git a/test/wallet/upgrade/Upgrade.t.sol b/test/wallet/upgrade/Upgrade.t.sol index f2efa8967..5fa24e723 100644 --- a/test/wallet/upgrade/Upgrade.t.sol +++ b/test/wallet/upgrade/Upgrade.t.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.18; import "forge-std/console.sol"; -import "../../KintoWallet.t.sol"; +import "../../SharedSetup.t.sol"; -contract PolicyTest is KintoWalletTest { +contract PolicyTest is SharedSetup { /* ============ Upgrade Tests ============ */ // FIXME: I think these upgrade tests are wrong because, basically, the KintoWallet.sol does not have @@ -59,7 +59,6 @@ contract PolicyTest is KintoWalletTest { UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // execute the transaction via the entry point // @dev handleOps seems to fail silently (does not revert) vm.expectEmit(true, true, true, false); emit UserOperationRevertReason(_entryPoint.getUserOpHash(userOp), userOp.sender, userOp.nonce, bytes("")); diff --git a/test/wallet/validateSignature/ValidateSignature.t.sol b/test/wallet/validateSignature/ValidateSignature.t.sol index 660bc8598..d267ad68e 100644 --- a/test/wallet/validateSignature/ValidateSignature.t.sol +++ b/test/wallet/validateSignature/ValidateSignature.t.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.18; import "forge-std/console.sol"; -import "../../KintoWallet.t.sol"; +import "../../SharedSetup.t.sol"; -contract ValidateSignatureTest is KintoWalletTest { +contract ValidateSignatureTest is SharedSetup { // constants uint256 constant SIG_VALIDATION_FAILED = 1; uint256 constant SIG_VALIDATION_SUCCESS = 0; @@ -210,7 +210,7 @@ contract ValidateSignatureTest is KintoWalletTest { values[2] = 0; bytes[] memory calls = new bytes[](3); - calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[0] = abi.encodeWithSignature("recoverer()"); calls[1] = abi.encodeWithSignature("increment()"); calls[2] = abi.encodeWithSignature("increment()"); @@ -249,7 +249,7 @@ contract ValidateSignatureTest is KintoWalletTest { values[2] = 0; bytes[] memory calls = new bytes[](3); - calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[0] = abi.encodeWithSignature("recoverer()"); calls[1] = abi.encodeWithSignature("increment()"); calls[2] = abi.encodeWithSignature("increment()"); @@ -290,7 +290,7 @@ contract ValidateSignatureTest is KintoWalletTest { values[2] = 0; bytes[] memory calls = new bytes[](3); - calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[0] = abi.encodeWithSignature("recoverer()"); calls[1] = abi.encodeWithSignature("increment()"); calls[2] = abi.encodeWithSignature("increment()"); @@ -339,7 +339,7 @@ contract ValidateSignatureTest is KintoWalletTest { values[3] = 0; bytes[] memory calls = new bytes[](4); - calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[0] = abi.encodeWithSignature("recoverer()"); calls[1] = abi.encodeWithSignature("increment()"); calls[2] = abi.encodeWithSignature("increment()"); calls[3] = abi.encodeWithSignature("increment()"); @@ -374,7 +374,7 @@ contract ValidateSignatureTest is KintoWalletTest { values[2] = 0; bytes[] memory calls = new bytes[](3); - calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[0] = abi.encodeWithSignature("recoverer()"); calls[1] = abi.encodeWithSignature("increment()"); calls[2] = abi.encodeWithSignature("increment()"); @@ -409,7 +409,7 @@ contract ValidateSignatureTest is KintoWalletTest { bytes[] memory calls = new bytes[](limit + 2); for (uint256 i = 0; i < limit + 2; i++) { - calls[i] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[i] = abi.encodeWithSignature("recoverer()"); } calls[limit + 1] = abi.encodeWithSignature("increment()"); @@ -657,7 +657,7 @@ contract ValidateSignatureTest is KintoWalletTest { values[i] = 0; if (i == order) { targets[i] = address(_kintoWallet); - calls[i] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[i] = abi.encodeWithSignature("recoverer()"); } else { targets[i] = address(counter); calls[i] = abi.encodeWithSignature("increment()"); @@ -702,7 +702,7 @@ contract ValidateSignatureTest is KintoWalletTest { values[i] = 0; if (i == order) { targets[i] = address(_kintoWallet); - calls[i] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[i] = abi.encodeWithSignature("recoverer()"); } else { targets[i] = address(counter); calls[i] = abi.encodeWithSignature("increment()"); @@ -780,4 +780,34 @@ contract ValidateSignatureTest is KintoWalletTest { ) ); } + + // special case 2: requiredSigners == 1, owners.length == 3 and the owner 2 is the signer. + function testValidateSignature_SpecialCase2() public { + // reset signers & change policy + address[] memory owners = new address[](3); + owners[0] = _owner; + owners[1] = _user; + owners[2] = _user2; + resetSigners(owners, _kintoWallet.SINGLE_SIGNER()); + + // create user op with owners 1 as signer + privateKeys = new uint256[](1); + privateKeys[0] = _userPk; + + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), + address(counter), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } } diff --git a/test/wallet/whitelist/Whitelist.t.sol b/test/wallet/whitelist/Whitelist.t.sol index 6214d6c92..186367e7b 100644 --- a/test/wallet/whitelist/Whitelist.t.sol +++ b/test/wallet/whitelist/Whitelist.t.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.18; import "forge-std/console.sol"; -import "../../KintoWallet.t.sol"; +import "../../SharedSetup.t.sol"; -contract WhitelistTest is KintoWalletTest { +contract WhitelistTest is SharedSetup { /* ============ Whitelist ============ */ function testWhitelistApp() public { @@ -18,7 +18,6 @@ contract WhitelistTest is KintoWalletTest { privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(counter), address(_paymaster) ); - // execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertTrue(_kintoWallet.appWhitelist(address(counter))); From b6d3d1622206803e6f378e07ea53a4e614027006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Mart=C3=ADn=20Alconada=20Verzini?= Date: Wed, 24 Jan 2024 14:30:57 -0300 Subject: [PATCH 2/5] chore: add more tests --- test/KintoAppRegistry.t.sol | 190 ++++++++++++++--------- test/SharedSetup.t.sol | 1 + test/harness/KintoAppRegistryHarness.sol | 17 ++ test/helpers/AATestScaffolding.sol | 5 + 4 files changed, 142 insertions(+), 71 deletions(-) create mode 100644 test/harness/KintoAppRegistryHarness.sol diff --git a/test/KintoAppRegistry.t.sol b/test/KintoAppRegistry.t.sol index 5d619658b..c354c29fa 100644 --- a/test/KintoAppRegistry.t.sol +++ b/test/KintoAppRegistry.t.sol @@ -19,6 +19,7 @@ contract KintoAppRegistryV2 is KintoAppRegistry { contract KintoAppRegistryTest is SharedSetup { function testUp() public override { super.testUp(); + useHarness(); assertEq(_kintoAppRegistry.owner(), _owner); assertEq(_kintoAppRegistry.name(), "Kinto APP"); @@ -27,6 +28,10 @@ contract KintoAppRegistryTest is SharedSetup { assertEq(_kintoAppRegistry.RATE_LIMIT_THRESHOLD(), 10); assertEq(_kintoAppRegistry.GAS_LIMIT_PERIOD(), 30 days); assertEq(_kintoAppRegistry.GAS_LIMIT_THRESHOLD(), 1e16); + assertEq( + KintoAppRegistryHarness(address(_kintoAppRegistry)).exposed_baseURI(), + "https://kinto.xyz/metadata/kintoapp/" + ); } /* ============ Upgrade Tests ============ */ @@ -102,7 +107,50 @@ contract KintoAppRegistryTest is SharedSetup { assertEq(metadata.name, name); } - function testUpdateApp(string memory name, address parentContract) public { + function testRegisterApp_RevertWhen_ChildrenIsWallet(string memory name, address parentContract) public { + uint256[4] memory appLimits = [uint256(0), uint256(0), uint256(0), uint256(0)]; + address[] memory appContracts = new address[](1); + appContracts[0] = address(_kintoWallet); + + // register app + vm.expectRevert("Wallets can not be registered"); + _kintoAppRegistry.registerApp(name, parentContract, appContracts, appLimits); + } + + function testRegisterApp_RevertWhen_AlreadyRegistered() public { + // register app + string memory name = "app"; + address parentContract = address(_engenCredits); + uint256[4] memory appLimits = [uint256(0), uint256(0), uint256(0), uint256(0)]; + address[] memory appContracts = new address[](0); + + vm.prank(_owner); + _kintoAppRegistry.registerApp(name, parentContract, appContracts, appLimits); + + // try to register again + vm.expectRevert("App already registered"); + _kintoAppRegistry.registerApp(name, parentContract, appContracts, appLimits); + } + + function testRegisterApp_RevertWhen_ParentIsChild() public { + // register app with child address 2 + string memory name = "app"; + address parentContract = address(_engenCredits); + uint256[4] memory appLimits = [uint256(0), uint256(0), uint256(0), uint256(0)]; + address[] memory appContracts = new address[](1); + appContracts[0] = address(2); + + vm.prank(_owner); + _kintoAppRegistry.registerApp(name, parentContract, appContracts, appLimits); + + // registering app "app2" with parent address 2 should revert + parentContract = address(2); + appContracts = new address[](0); + vm.expectRevert("Parent contract already registered as a child"); + _kintoAppRegistry.registerApp(name, parentContract, appContracts, appLimits); + } + + function testUpdateMetadata(string memory name, address parentContract) public { address[] memory appContracts = new address[](1); appContracts[0] = address(8); @@ -135,108 +183,95 @@ contract KintoAppRegistryTest is SharedSetup { assertEq(_kintoAppRegistry.isSponsored(parentContract, address(8)), true); } - function testRegisterApp_RevertWhen_ChildrenIsWallet(string memory name, address parentContract) public { - uint256[4] memory appLimits = [uint256(0), uint256(0), uint256(0), uint256(0)]; - address[] memory appContracts = new address[](1); - appContracts[0] = address(_kintoWallet); + function testUpdateMetadata_RevertWhen_CallerIsNotDeveloper(string memory name, address parentContract) public { + registerApp(_owner, name, parentContract); - // register app - vm.expectRevert("Wallets can not be registered"); - _kintoAppRegistry.registerApp(name, parentContract, appContracts, appLimits); + // update app + address[] memory appContracts = new address[](0); + vm.prank(_user); + vm.expectRevert("Only developer can update metadata"); + _kintoAppRegistry.updateMetadata( + "test2", parentContract, appContracts, [uint256(1), uint256(1), uint256(1), uint256(1)] + ); } /* ============ DSA Test ============ */ function testEnableDSA_WhenCallerIsOwner() public { - address parentContract = address(_engenCredits); - address[] memory appContracts = new address[](1); - appContracts[0] = address(8); - uint256[] memory appLimits = new uint256[](4); - appLimits[0] = _kintoAppRegistry.RATE_LIMIT_PERIOD(); - appLimits[1] = _kintoAppRegistry.RATE_LIMIT_THRESHOLD(); - appLimits[2] = _kintoAppRegistry.GAS_LIMIT_PERIOD(); - appLimits[3] = _kintoAppRegistry.GAS_LIMIT_THRESHOLD(); + registerApp(_owner, "app", address(_engenCredits)); vm.prank(_owner); - _kintoAppRegistry.registerApp( - "", parentContract, appContracts, [appLimits[0], appLimits[1], appLimits[2], appLimits[3]] - ); + _kintoAppRegistry.enableDSA(address(_engenCredits)); - vm.prank(_owner); - _kintoAppRegistry.enableDSA(parentContract); - - IKintoAppRegistry.Metadata memory metadata = _kintoAppRegistry.getAppMetadata(parentContract); + IKintoAppRegistry.Metadata memory metadata = _kintoAppRegistry.getAppMetadata(address(_engenCredits)); assertEq(metadata.dsaEnabled, true); } - function test_Revert_When_User_TriesToEnableDSA() public { - vm.startPrank(_user); - address parentContract = address(_engenCredits); - address[] memory appContracts = new address[](1); - appContracts[0] = address(8); - uint256[] memory appLimits = new uint256[](4); - appLimits[0] = _kintoAppRegistry.RATE_LIMIT_PERIOD(); - appLimits[1] = _kintoAppRegistry.RATE_LIMIT_THRESHOLD(); - appLimits[2] = _kintoAppRegistry.GAS_LIMIT_PERIOD(); - appLimits[3] = _kintoAppRegistry.GAS_LIMIT_THRESHOLD(); - _kintoAppRegistry.registerApp( - "", parentContract, appContracts, [appLimits[0], appLimits[1], appLimits[2], appLimits[3]] - ); + function testEnableDSA_RevertWhen_CallerIsNotOwner() public { + registerApp(_owner, "app", address(_engenCredits)); + vm.expectRevert("Ownable: caller is not the owner"); - _kintoAppRegistry.enableDSA(parentContract); + _kintoAppRegistry.enableDSA(address(_engenCredits)); + } + + function testEnableDSA_RevertWhen_AlreadyEnabled() public { + registerApp(_owner, "app", address(_engenCredits)); + vm.prank(_owner); + _kintoAppRegistry.enableDSA(address(_engenCredits)); + + vm.expectRevert("DSA already enabled"); + _kintoAppRegistry.enableDSA(address(_engenCredits)); } /* ============ Sponsored Contracts Test ============ */ - function testAppCreatorCanSetSponsoredContracts() public { - vm.startPrank(_user); - address parentContract = address(_engenCredits); - address[] memory appContracts = new address[](1); - appContracts[0] = address(8); - uint256[] memory appLimits = new uint256[](4); - appLimits[0] = _kintoAppRegistry.RATE_LIMIT_PERIOD(); - appLimits[1] = _kintoAppRegistry.RATE_LIMIT_THRESHOLD(); - appLimits[2] = _kintoAppRegistry.GAS_LIMIT_PERIOD(); - appLimits[3] = _kintoAppRegistry.GAS_LIMIT_THRESHOLD(); - _kintoAppRegistry.registerApp( - "", parentContract, appContracts, [appLimits[0], appLimits[1], appLimits[2], appLimits[3]] - ); + function testSetSponsoredContracts() public { + registerApp(_owner, "app", address(_engenCredits)); + address[] memory contracts = new address[](2); contracts[0] = address(8); contracts[1] = address(9); + bool[] memory flags = new bool[](2); flags[0] = false; flags[1] = true; - _kintoAppRegistry.setSponsoredContracts(parentContract, contracts, flags); - assertEq(_kintoAppRegistry.isSponsored(parentContract, address(8)), true); // child contracts always sponsored - assertEq(_kintoAppRegistry.isSponsored(parentContract, address(9)), true); - assertEq(_kintoAppRegistry.isSponsored(parentContract, address(10)), false); - vm.stopPrank(); + + vm.prank(_owner); + _kintoAppRegistry.setSponsoredContracts(address(_engenCredits), contracts, flags); + + assertEq(_kintoAppRegistry.isSponsored(address(_engenCredits), address(8)), false); + assertEq(_kintoAppRegistry.isSponsored(address(_engenCredits), address(9)), true); + assertEq(_kintoAppRegistry.isSponsored(address(_engenCredits), address(10)), false); } - function test_Revert_When_NotCreator_TriesToSetSponsoredContracts() public { - vm.startPrank(_user); - address parentContract = address(_engenCredits); - address[] memory appContracts = new address[](1); - appContracts[0] = address(8); - uint256[] memory appLimits = new uint256[](4); - appLimits[0] = _kintoAppRegistry.RATE_LIMIT_PERIOD(); - appLimits[1] = _kintoAppRegistry.RATE_LIMIT_THRESHOLD(); - appLimits[2] = _kintoAppRegistry.GAS_LIMIT_PERIOD(); - appLimits[3] = _kintoAppRegistry.GAS_LIMIT_THRESHOLD(); - _kintoAppRegistry.registerApp( - "", parentContract, appContracts, [appLimits[0], appLimits[1], appLimits[2], appLimits[3]] - ); + function testSetSponsoredContracts_RevertWhen_CallerIsNotCreator() public { + registerApp(_owner, "app", address(_engenCredits)); + address[] memory contracts = new address[](2); contracts[0] = address(8); contracts[1] = address(9); + bool[] memory flags = new bool[](2); flags[0] = false; flags[1] = true; - vm.startPrank(_user2); + + vm.prank(_user); vm.expectRevert("Only developer can set sponsored contracts"); - _kintoAppRegistry.setSponsoredContracts(parentContract, contracts, flags); - vm.stopPrank(); + _kintoAppRegistry.setSponsoredContracts(address(_engenCredits), contracts, flags); + } + + function testSetSponsoredContracts_RevertWhen_LengthMismatch() public { + registerApp(_owner, "app", address(_engenCredits)); + + address[] memory contracts = new address[](1); + contracts[0] = address(8); + + bool[] memory flags = new bool[](2); + flags[0] = false; + flags[1] = true; + + vm.expectRevert("Invalid input"); + _kintoAppRegistry.setSponsoredContracts(address(_engenCredits), contracts, flags); } /* ============ Transfer Test ============ */ @@ -258,4 +293,17 @@ contract KintoAppRegistryTest is SharedSetup { vm.expectRevert("Only mint transfers are allowed"); _kintoAppRegistry.safeTransferFrom(_user, _user2, tokenIdx); } + + /* ============ Supports Interface tests ============ */ + + function testSupportsInterface() public { + bytes4 InterfaceERC721Upgradeable = bytes4(keccak256("balanceOf(address)")) + ^ bytes4(keccak256("ownerOf(uint256)")) ^ bytes4(keccak256("safeTransferFrom(address,address,uint256,bytes)")) + ^ bytes4(keccak256("safeTransferFrom(address,address,uint256)")) + ^ bytes4(keccak256("transferFrom(address,address,uint256)")) ^ bytes4(keccak256("approve(address,uint256)")) + ^ bytes4(keccak256("setApprovalForAll(address,bool)")) ^ bytes4(keccak256("getApproved(uint256)")) + ^ bytes4(keccak256("isApprovedForAll(address,address)")); + + assertTrue(_kintoIDv1.supportsInterface(InterfaceERC721Upgradeable)); + } } diff --git a/test/SharedSetup.t.sol b/test/SharedSetup.t.sol index f20ddf8ce..93dd6974c 100644 --- a/test/SharedSetup.t.sol +++ b/test/SharedSetup.t.sol @@ -13,6 +13,7 @@ import "../src/sample/Counter.sol"; import "./harness/KintoWalletHarness.sol"; import "./harness/SponsorPaymasterHarness.sol"; +import "./harness/KintoAppRegistryHarness.sol"; import {UserOp} from "./helpers/UserOp.sol"; import {AATestScaffolding} from "./helpers/AATestScaffolding.sol"; diff --git a/test/harness/KintoAppRegistryHarness.sol b/test/harness/KintoAppRegistryHarness.sol new file mode 100644 index 000000000..dd6cae8cc --- /dev/null +++ b/test/harness/KintoAppRegistryHarness.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.18; + +import "../../src/interfaces/IKintoEntryPoint.sol"; +import "../../src/interfaces/IKintoID.sol"; +import "../../src/interfaces/IKintoAppRegistry.sol"; + +import {KintoAppRegistry} from "../../src/apps/KintoAppRegistry.sol"; + +// Harness contract to expose internal functions for testing. +contract KintoAppRegistryHarness is KintoAppRegistry { + constructor(IKintoWalletFactory _walletFactory) KintoAppRegistry(_walletFactory) {} + + function exposed_baseURI() public pure returns (string memory) { + return _baseURI(); + } +} diff --git a/test/helpers/AATestScaffolding.sol b/test/helpers/AATestScaffolding.sol index bc7f96a60..8976eb277 100644 --- a/test/helpers/AATestScaffolding.sol +++ b/test/helpers/AATestScaffolding.sol @@ -20,6 +20,7 @@ import "../helpers/UUPSProxy.sol"; import "../helpers/KYCSignature.sol"; import {KintoWalletHarness} from "../harness/KintoWalletHarness.sol"; import {SponsorPaymasterHarness} from "../harness/SponsorPaymasterHarness.sol"; +import {KintoAppRegistryHarness} from "../harness/KintoAppRegistryHarness.sol"; abstract contract AATestScaffolding is KYCSignature { IKintoEntryPoint _entryPoint; @@ -361,6 +362,10 @@ abstract contract AATestScaffolding is KYCSignature { SponsorPaymasterHarness _paymasterImpl = new SponsorPaymasterHarness(_entryPoint); vm.prank(_paymaster.owner()); _paymaster.upgradeTo(address(_paymasterImpl)); + + KintoAppRegistryHarness _registryImpl = new KintoAppRegistryHarness(_walletFactory); + vm.prank(_kintoAppRegistry.owner()); + _kintoAppRegistry.upgradeTo(address(_registryImpl)); } ////// helper methods to assert the revert reason on UserOperationRevertReason events //// From 17818dbffe9e9d4a9361a9cf4b3f84903c65fb5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Mart=C3=ADn=20Alconada=20Verzini?= Date: Wed, 24 Jan 2024 17:24:12 -0300 Subject: [PATCH 3/5] chore: add migration --- script/migrations/21-multiple_upgrade.sol | 6 + script/migrations/22-wallet_v5.sol | 27 ++++ script/migrations/utils/MigrationHelper.sol | 146 ++++++++++++++++++++ src/wallet/KintoWallet.sol | 2 +- 4 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 script/migrations/22-wallet_v5.sol create mode 100644 script/migrations/utils/MigrationHelper.sol diff --git a/script/migrations/21-multiple_upgrade.sol b/script/migrations/21-multiple_upgrade.sol index a9e223ab6..39756ca44 100644 --- a/script/migrations/21-multiple_upgrade.sol +++ b/script/migrations/21-multiple_upgrade.sol @@ -16,6 +16,12 @@ import "../../test/helpers/UserOp.sol"; import "forge-std/Script.sol"; import "forge-std/console.sol"; +contract KintoWalletV4 is KintoWallet { + constructor(IEntryPoint _entryPoint, IKintoID _kintoID, IKintoAppRegistry _appRegistry) + KintoWallet(_entryPoint, _kintoID, _appRegistry) + {} +} + contract KintoMigration21DeployScript is Create2Helper, ArtifactsReader, UserOp { using ECDSAUpgradeable for bytes32; diff --git a/script/migrations/22-wallet_v5.sol b/script/migrations/22-wallet_v5.sol new file mode 100644 index 000000000..2d960a596 --- /dev/null +++ b/script/migrations/22-wallet_v5.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.18; + +import "../../src/wallet/KintoWallet.sol"; +import "./utils/MigrationHelper.sol"; + +contract KintoMigration22DeployScript is MigrationHelper { + using ECDSAUpgradeable for bytes32; + + // NOTE: this migration must be run from the ledger admin + function run() public { + run2(); + + // generate bytecode for KintoWalletV5 + bytes memory bytecode = abi.encodePacked( + type(KintoWalletV5).creationCode, + abi.encode( + _getChainDeployment("EntryPoint"), + IKintoID(_getChainDeployment("KintoID")), + IKintoAppRegistry(_getChainDeployment("KintoAppRegistry")) + ) + ); + + // upgrade KintoWallet to V5 + deployAndUpgrade("KintoWallet", "V5", bytecode); + } +} diff --git a/script/migrations/utils/MigrationHelper.sol b/script/migrations/utils/MigrationHelper.sol new file mode 100644 index 000000000..29da8d9f4 --- /dev/null +++ b/script/migrations/utils/MigrationHelper.sol @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.18; + +import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; + +import "../../../src/wallet/KintoWalletFactory.sol"; + +import "../../../src/interfaces/ISponsorPaymaster.sol"; +import "../../../src/interfaces/IKintoWallet.sol"; + +import "../../../test/helpers/Create2Helper.sol"; +import "../../../test/helpers/ArtifactsReader.sol"; +import "../../../test/helpers/UserOp.sol"; + +import "forge-std/Script.sol"; +import "forge-std/console.sol"; + +interface IInitialize { + function initialize(address anOwner, address _recoverer) external; +} + +contract MigrationHelper is Create2Helper, ArtifactsReader, UserOp { + using ECDSAUpgradeable for bytes32; + + uint256 deployerPrivateKey; + KintoWalletFactory _walletFactory; + + function run2() public { + console.log("Running on chain: ", vm.toString(block.chainid)); + console.log("Executing from address", msg.sender); + + deployerPrivateKey = vm.envUint("PRIVATE_KEY"); + _walletFactory = KintoWalletFactory(payable(_getChainDeployment("KintoWalletFactory"))); + } + + /// @dev deploys contracts from deployer and upgrades them using LEDGER_ADMIN + function deployAndUpgrade(string memory contractName, string memory version, bytes memory bytecode) public { + bool isWallet = keccak256(abi.encodePacked(contractName)) == keccak256(abi.encodePacked("KintoWallet")); + address proxy = _getChainDeployment(contractName); + + if (!isWallet) require(proxy != address(0), "Need to execute main deploy script first"); + + // (1). deploy new implementation via wallet factory + vm.broadcast(deployerPrivateKey); + address _impl = _walletFactory.deployContract(vm.envAddress("LEDGER_ADMIN"), 0, bytecode, bytes32(0)); + + console.log(string.concat(contractName, version, "-impl: ", vm.toString(address(_impl)))); + + // (2). call upgradeTo to set new implementation + if (isWallet) { + vm.broadcast(); // requires LEDGER_ADMIN + // vm.prank(vm.envAddress("LEDGER_ADMIN")); + _walletFactory.upgradeAllWalletImplementations(IKintoWallet(_impl)); + } else { + vm.broadcast(); // requires LEDGER_ADMIN + // vm.prank(vm.envAddress("LEDGER_ADMIN")); + UUPSUpgradeable(proxy).upgradeTo(_impl); + } + } + + // utils for doing actions through EntryPoint + + function _upgradeTo(address _proxy, address _newImpl, uint256 _signerPk) internal { + address payable _from = payable(_getChainDeployment("KintoWallet-admin")); + + // prep upgradeTo user op + uint256 nonce = IKintoWallet(_from).getNonce(); + uint256[] memory privateKeys = new uint256[](1); + privateKeys[0] = _signerPk; + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + block.chainid, + _from, + _proxy, + 0, + nonce, + privateKeys, + abi.encodeWithSelector(UUPSUpgradeable.upgradeTo.selector, address(_newImpl)), + _getChainDeployment("SponsorPaymaster") + ); + + vm.broadcast(deployerPrivateKey); + IEntryPoint(_getChainDeployment("EntryPoint")).handleOps(userOps, payable(vm.addr(_signerPk))); + } + + function _initialize(address _proxy, uint256 _signerPk) internal { + address payable _from = payable(_getChainDeployment("KintoWallet-admin")); + + // fund _proxy in the paymaster + require( + ISponsorPaymaster(payable(_getChainDeployment("SponsorPaymaster"))).balances(_proxy) > 0, + "Need to fund proxy in paymaster" + ); + + // todo: move to a fund function + // ISponsorPaymaster _paymaster = ISponsorPaymaster(_getChainDeployment("SponsorPaymaster")); + // vm.broadcast(deployerPrivateKey); + // _paymaster.addDepositFor{value: 0.00000001 ether}(_proxy); + // assertEq(_paymaster.balances(_proxy), 0.00000001 ether); + + // prep upgradeTo user op + uint256 nonce = IKintoWallet(_from).getNonce(); + uint256[] memory privateKeys = new uint256[](1); + privateKeys[0] = _signerPk; + + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + block.chainid, + _from, + _proxy, + 0, + nonce, + privateKeys, + abi.encodeWithSelector(IInitialize.initialize.selector), + _getChainDeployment("SponsorPaymaster") + ); + + vm.broadcast(deployerPrivateKey); + IEntryPoint(_getChainDeployment("EntryPoint")).handleOps(userOps, payable(vm.addr(_signerPk))); + } + + function _transferOwnership(address _proxy, uint256 _signerPk, address _newOwner) internal { + address payable _from = payable(_getChainDeployment("KintoWallet-admin")); + + // prep upgradeTo user op + uint256 nonce = IKintoWallet(_from).getNonce(); + uint256[] memory privateKeys = new uint256[](1); + privateKeys[0] = _signerPk; + + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + block.chainid, + _from, + _proxy, + 0, + nonce, + privateKeys, + abi.encodeWithSelector(Ownable.transferOwnership.selector, _newOwner), + _getChainDeployment("SponsorPaymaster") + ); + + vm.broadcast(deployerPrivateKey); + IEntryPoint(_getChainDeployment("EntryPoint")).handleOps(userOps, payable(vm.addr(_signerPk))); + } +} diff --git a/src/wallet/KintoWallet.sol b/src/wallet/KintoWallet.sol index 16b0ac12b..9148029ef 100644 --- a/src/wallet/KintoWallet.sol +++ b/src/wallet/KintoWallet.sol @@ -448,7 +448,7 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto } // Upgradeable version of KintoWallet -contract KintoWalletV4 is KintoWallet { +contract KintoWalletV5 is KintoWallet { constructor(IEntryPoint _entryPoint, IKintoID _kintoID, IKintoAppRegistry _appRegistry) KintoWallet(_entryPoint, _kintoID, _appRegistry) {} From 57dcc6fcc86d03cfaf838103093361db465298d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Mart=C3=ADn=20Alconada=20Verzini?= Date: Wed, 24 Jan 2024 17:25:20 -0300 Subject: [PATCH 4/5] chore: remove unused function --- src/paymasters/SponsorPaymaster.sol | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/paymasters/SponsorPaymaster.sol b/src/paymasters/SponsorPaymaster.sol index c7b6fd3a8..10d146a4f 100644 --- a/src/paymasters/SponsorPaymaster.sol +++ b/src/paymasters/SponsorPaymaster.sol @@ -277,25 +277,6 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen ); } - /// Return app's sponsor from the registry - /// @return sponsor - the sponsor address - /// @dev reverts if neither execute nor executeBatch - /// @dev ensures all targets are sponsored by the same app if executeBatch (todo: decouple this) - function _getSponsor(bytes calldata callData) internal view returns (address sponsor) { - bytes4 selector = bytes4(callData[:4]); - if (selector == IKintoWallet.execute.selector) { - (address target,,) = abi.decode(callData[4:], (address, uint256, bytes)); - sponsor = appRegistry.getSponsor(target); - } else if (selector == IKintoWallet.executeBatch.selector) { - // last target is the sponsor - (address[] memory targets,,) = abi.decode(callData[4:], (address[], uint256[], bytes[])); - sponsor = appRegistry.getSponsor(targets[targets.length - 1]); - } else { - // handle unknown function or error - revert("SP: Unknown function selector"); - } - } - // @notice extracts `target` contract from callData // @dev the last op on a batch MUST always be a contract whose sponsor is the one we want to // bear with the gas cost of all ops From ccdf17a882e3867a6961b75df4931961b1329d7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Mart=C3=ADn=20Alconada=20Verzini?= Date: Wed, 24 Jan 2024 17:34:36 -0300 Subject: [PATCH 5/5] chore: fix test --- test/KintoAppRegistry.t.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/test/KintoAppRegistry.t.sol b/test/KintoAppRegistry.t.sol index c354c29fa..3047536bf 100644 --- a/test/KintoAppRegistry.t.sol +++ b/test/KintoAppRegistry.t.sol @@ -220,6 +220,7 @@ contract KintoAppRegistryTest is SharedSetup { _kintoAppRegistry.enableDSA(address(_engenCredits)); vm.expectRevert("DSA already enabled"); + vm.prank(_owner); _kintoAppRegistry.enableDSA(address(_engenCredits)); }