diff --git a/src/wallet/KintoWallet.sol b/src/wallet/KintoWallet.sol index 38fd4013d..6f52c9a2a 100644 --- a/src/wallet/KintoWallet.sol +++ b/src/wallet/KintoWallet.sol @@ -8,6 +8,7 @@ import "@openzeppelin/contracts/interfaces/IERC20.sol"; import "@aa/core/BaseAccount.sol"; import "@aa/samples/callback/TokenCallbackHandler.sol"; +import "forge-std/console.sol"; import "../interfaces/IKintoID.sol"; import "../interfaces/IKintoEntryPoint.sol"; @@ -191,7 +192,7 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto function setAppKey(address app, address signer) external override onlySelf { // Allow 0 in signer to allow revoking the appkey require(app != address(0), "KW-apk: invalid address"); - require(appWhitelist[app], "KW-apk: contract not whitelisted"); + require(appWhitelist[app], "KW-apk: contract not whitelisted"); // todo: i don't think we need to check this here require(appSigner[app] != signer, "KW-apk: same key"); appSigner[app] = signer; // todo: emit event @@ -271,8 +272,13 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto bytes32 hash = userOpHash.toEthSignedMessageHash(); // If there is only one signature and there is an app Key, check it address app = _getAppContract(userOp.callData); + console.log("APP", app); + console.log("appWhitelist[app]", appWhitelist[app]); + console.log("appSigner[app]", appSigner[app]); + console.log(userOp.signature.length == 65); if (userOp.signature.length == 65 && appWhitelist[app] && appSigner[app] != address(0)) { if (appSigner[app] == hash.recover(userOp.signature)) { + console.log("SIII"); return _packValidationData(false, 0, 0); } } @@ -355,6 +361,8 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto } // Function to extract the first target contract + /// @dev the last op on a batch MUST always be a contract whose sponsor is the one we want to pay + // all other ops function _getAppContract(bytes calldata callData) private view returns (address) { // Extract the function selector from the callData bytes4 selector = bytes4(callData[:4]); @@ -368,6 +376,7 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto // App signer should only be valid for the app itself and its children // It is important that wallet calls are not allowed through the app signer if (!appRegistry.isContractSponsored(lastTargetContract, targets[i])) { + console.log("ENTREEEEE"); return address(0); } } diff --git a/test/EngenCredits.t.sol b/test/EngenCredits.t.sol index 4702b710b..d63ad34fb 100644 --- a/test/EngenCredits.t.sol +++ b/test/EngenCredits.t.sol @@ -30,8 +30,8 @@ contract EngenCreditsTest is UserOp, AATestScaffolding { _owner.transfer(1e18); vm.stopPrank(); deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - _fundSponsorForApp(address(_engenCredits)); - _fundSponsorForApp(address(_kintoWallet)); + fundSponsorForApp(address(_engenCredits)); + fundSponsorForApp(address(_kintoWallet)); vm.startPrank(_owner); _kintoAppRegistry.registerApp( "engen credits", address(_engenCredits), new address[](0), [uint256(0), uint256(0), uint256(0), uint256(0)] diff --git a/test/KintoEntryPoint.t.sol b/test/KintoEntryPoint.t.sol index 34b0b63b8..dd1c892c1 100644 --- a/test/KintoEntryPoint.t.sol +++ b/test/KintoEntryPoint.t.sol @@ -4,21 +4,10 @@ pragma solidity ^0.8.18; import "forge-std/Test.sol"; import "forge-std/console.sol"; -import {UserOp} from "./helpers/UserOp.sol"; -import {AATestScaffolding} from "./helpers/AATestScaffolding.sol"; +import "./KintoWallet.t.sol"; -contract KintoEntryPointTest is AATestScaffolding, UserOp { - uint256 _chainID = 1; - - function setUp() public { - vm.chainId(_chainID); - vm.startPrank(address(1)); - _owner.transfer(1e18); - vm.stopPrank(); - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - } - - function testUp() public { +contract KintoEntryPointTest is KintoWalletTest { + function testUp() public override { assertEq(_entryPoint.walletFactory(), address(_walletFactory)); } diff --git a/test/KintoWallet.t.sol b/test/KintoWallet.t.sol index 6940f69c7..68f6dd2fd 100644 --- a/test/KintoWallet.t.sol +++ b/test/KintoWallet.t.sol @@ -15,11 +15,9 @@ import "./harness/KintoWalletHarness.sol"; import {UserOp} from "./helpers/UserOp.sol"; import {AATestScaffolding} from "./helpers/AATestScaffolding.sol"; -contract UpgradeToTest is AATestScaffolding, UserOp { +contract KintoWalletTest is UserOp, AATestScaffolding { uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; + Counter counter; // events event UserOperationRevertReason( @@ -29,1076 +27,28 @@ contract UpgradeToTest is AATestScaffolding, UserOp { event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - function setUp() public { + function setUp() public virtual { deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); + // add paymaster to _kintoWallet + fundSponsorForApp(address(_kintoWallet)); - // Default tests to use 1 private key for simplicity + // all tests will use 1 private key (_ownerPk) unless otherwise specified privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - - /* ============ Upgrade Tests ============ */ - - // FIXME: I think these upgrade tests are wrong because, basically, the KintoWallet.sol does not have - // an upgrade function. The upgrade function is in the UUPSUpgradeable.sol contract. - function test_RevertWhen_OwnerCannotUpgrade() public { - // deploy a new implementation - KintoWallet _newImplementation = new KintoWallet(_entryPoint, _kintoIDv1, _kintoAppRegistry); - - // try calling upgradeTo from _owner wallet to upgrade _owner wallet - UserOperation memory userOp = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("upgradeTo(address)", address(_newImplementation)), - address(_paymaster) - ); - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = userOp; - - // execute the transaction via the entry point and expect a revert event - // @dev handleOps fails silently (does not revert) - vm.expectEmit(true, true, true, false); - emit UserOperationRevertReason(_entryPoint.getUserOpHash(userOp), userOp.sender, userOp.nonce, bytes("")); - vm.recordLogs(); - _entryPoint.handleOps(userOps, payable(_owner)); - assertRevertReasonEq("Address: low-level call with value failed"); - } - - function test_RevertWhen_OthersCannotUpgrade() public { - // create a wallet for _user - approveKYC(_kycProvider, _user, _userPk); - vm.broadcast(_user); - IKintoWallet userWallet = _walletFactory.createAccount(_user, _recoverer, 0); - - // deploy a new implementation - KintoWallet _newImplementation = new KintoWallet(_entryPoint, _kintoIDv1, _kintoAppRegistry); - - // try calling upgradeTo from _user wallet to upgrade _owner wallet - uint256 nonce = userWallet.getNonce(); - privateKeys[0] = _userPk; - - UserOperation memory userOp = _createUserOperation( - address(userWallet), - address(_kintoWallet), - nonce, - privateKeys, - abi.encodeWithSignature("upgradeTo(address)", address(_newImplementation)), - address(_paymaster) - ); - - 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("")); - - vm.recordLogs(); - _entryPoint.handleOps(userOps, payable(_owner)); - assertRevertReasonEq("KW: contract not whitelisted"); - - vm.stopPrank(); - } - - /* ============ One Signer Account Transaction Tests (executeBatch) ============ */ - - function testExecuteBatch() public { - // deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - registerApp(_owner, "test", address(counter)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counter); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - bytes[] memory calls = new bytes[](3); - - address[] memory apps = new address[](1); - apps[0] = address(counter); - - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp and increment counter (two times) - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = 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) - ); - - // send all transactions via batch - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 2); - } - - function testExecuteBatch_RevertWhen_TargetsBelongToDifferentApps() public { - // deploy the counter contract - Counter counter = new Counter(); - Counter counter2 = new Counter(); - assertEq(counter.count(), 0); - assertEq(counter2.count(), 0); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - registerApp(_owner, "counter app", address(counter)); - registerApp(_owner, "counter2 app", address(counter2)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counter2); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - address[] memory apps = new address[](1); - apps[0] = address(counter); - - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp, increment counter and increment counter2 - bytes[] memory calls = new bytes[](3); - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("increment()"); - - // send all transactions via batch - 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; - - // execute the transaction via the entry point - - // prepare expected error message - uint256 expectedOpIndex = 0; // Adjust as needed - string memory expectedMessage = "AA33 reverted"; - bytes memory additionalMessage = - abi.encodePacked("SP: executeBatch targets must be sponsored by the contract or be the sender wallet"); - bytes memory expectedAdditionalData = abi.encodeWithSelector( - bytes4(keccak256("Error(string)")), // Standard error selector - additionalMessage - ); - - // encode the entire revert reason - bytes memory expectedRevertReason = abi.encodeWithSignature( - "FailedOpWithRevert(uint256,string,bytes)", expectedOpIndex, expectedMessage, expectedAdditionalData - ); - - vm.expectRevert(expectedRevertReason); - _entryPoint.handleOps(userOps, payable(_owner)); - } - - // we want to test that we can execute a batch of user ops where all targets are app children - // except for the first and the last ones which are the wallet itself - function testExecuteBatch_TopAndBottomWalletOps() public { - // deploy the counter contract - Counter counter = new Counter(); - Counter counterRelatedContract = new Counter(); - assertEq(counter.count(), 0); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - address[] memory appContracts = new address[](1); - appContracts[0] = address(counterRelatedContract); - registerApp(_owner, "counter app", address(counter), appContracts); - - // prep batch - address[] memory targets = new address[](4); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counterRelatedContract); - targets[3] = address(_kintoWallet); - - uint256[] memory values = new uint256[](4); - values[0] = 0; - values[1] = 0; - values[2] = 0; - values[3] = 0; - - // whitelistApp params - address[] memory apps = new address[](1); - apps[0] = address(counter); - - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 4 calls batch: whitelistApp, increment counter and increment counter2 and whitelistApp - bytes[] memory calls = new bytes[](4); - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("increment()"); - calls[3] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - - // send all transactions via batch - 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) - ); - - // execute the transaction via the entry point - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 1); - assertEq(counterRelatedContract.count(), 1); - } - - /* ============ Recovery Tests ============ */ - - function testStartRecovert() public { - vm.prank(address(_walletFactory)); - _kintoWallet.startRecovery(); - assertEq(_kintoWallet.inRecovery(), block.timestamp); - } - - function testStartRecovery_RevertWhen_DirectCall(address someone) public { - vm.assume(someone != address(_walletFactory)); - vm.prank(someone); - vm.expectRevert("KW: only factory"); - _kintoWallet.startRecovery(); - } - - function testRecoverAccountSuccessfully() public { - assertEq(_kintoWallet.owners(0), _owner); - - // start Recovery - vm.prank(address(_walletFactory)); - _kintoWallet.startRecovery(); - assertEq(_kintoWallet.inRecovery(), block.timestamp); - - // mint NFT to new owner and burn old - IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoIDv1, _user, _user, 3, block.timestamp + 1000); - uint16[] memory traits = new uint16[](0); - - vm.startPrank(_kycProvider); - _kintoIDv1.mintIndividualKyc(sigdata, traits); - sigdata = _auxCreateSignature(_kintoIDv1, _owner, _owner, 1, block.timestamp + 1000); - _kintoIDv1.burnKYC(sigdata); - vm.stopPrank(); - - assertEq(_kintoIDv1.isKYC(_user), true); - - // pass recovery time - vm.warp(block.timestamp + _kintoWallet.RECOVERY_TIME() + 1); - - 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(_walletFactory)); - _kintoWallet.finishRecovery(users); - - assertEq(_kintoWallet.inRecovery(), 0); - assertEq(_kintoWallet.owners(0), _user); - } - - function testComplete_RevertWhen_RecoverWithoutBurningOldOwner() public { - assertEq(_kintoWallet.owners(0), _owner); - - // start recovery - vm.prank(address(_walletFactory)); - _kintoWallet.startRecovery(); - assertEq(_kintoWallet.inRecovery(), block.timestamp); - - // approve KYC for _user (mint NFT) - approveKYC(_kycProvider, _user, _userPk); - assertEq(_kintoIDv1.isKYC(_user), true); - - // pass recovery time - vm.warp(block.timestamp + _kintoWallet.RECOVERY_TIME() + 1); - - // monitor AML - IKintoID.MonitorUpdateData[][] memory updates = new IKintoID.MonitorUpdateData[][](1); - updates[0] = new IKintoID.MonitorUpdateData[](1); - updates[0][0] = IKintoID.MonitorUpdateData(true, true, 5); - - address[] memory users = new address[](1); - users[0] = _user; - - vm.prank(_kycProvider); - _kintoIDv1.monitor(users, updates); - - // complete recovery - vm.prank(address(_walletFactory)); - vm.expectRevert("KW-fr: Old KYC must be burned"); - _kintoWallet.finishRecovery(users); - } - - function testComplete_RevertWhen_RecoverWithoutNewOwnerKYCd() public { - assertEq(_kintoWallet.owners(0), _owner); - - // start recovery - vm.prank(address(_walletFactory)); - _kintoWallet.startRecovery(); - assertEq(_kintoWallet.inRecovery(), block.timestamp); - - // burn old owner NFT - revokeKYC(_kycProvider, _owner, _ownerPk); - assertEq(_kintoIDv1.isKYC(_owner), false); - - // pass recovery time - vm.warp(block.timestamp + _kintoWallet.RECOVERY_TIME() + 1); - - // complete recovery - assertEq(_kintoIDv1.isKYC(_user), false); // new owner is not KYC'd - address[] memory users = new address[](1); - users[0] = _user; - vm.prank(address(_walletFactory)); - vm.expectRevert("KW-rs: KYC Required"); - _kintoWallet.finishRecovery(users); - } - - function testComplete_RevertWhen_RecoverNotEnoughTime() public { - assertEq(_kintoWallet.owners(0), _owner); - - // start recovery - vm.prank(address(_walletFactory)); - _kintoWallet.startRecovery(); - assertEq(_kintoWallet.inRecovery(), block.timestamp); - - // burn old owner NFT - revokeKYC(_kycProvider, _owner, _ownerPk); - assertEq(_kintoIDv1.isKYC(_owner), false); - - // approve KYC for _user (mint NFT) - approveKYC(_kycProvider, _user, _userPk); - assertEq(_kintoIDv1.isKYC(_user), true); - - // pass recovery time (not enough) - vm.warp(block.timestamp + _kintoWallet.RECOVERY_TIME() - 1); - - // monitor AML - IKintoID.MonitorUpdateData[][] memory updates = new IKintoID.MonitorUpdateData[][](1); - updates[0] = new IKintoID.MonitorUpdateData[](1); - updates[0][0] = IKintoID.MonitorUpdateData(true, true, 5); - - address[] memory users = new address[](1); - users[0] = _user; - - vm.prank(_kycProvider); - _kintoIDv1.monitor(users, updates); - - // complete recovery - vm.prank(address(_walletFactory)); - vm.expectRevert("KW-fr: too early"); - _kintoWallet.finishRecovery(users); - } - - function testCancelRecovery() public { - vm.prank(address(_walletFactory)); - _kintoWallet.startRecovery(); - assertEq(_kintoWallet.inRecovery(), block.timestamp); - - vm.prank(address(_kintoWallet)); - _kintoWallet.cancelRecovery(); - } - - function testCancelRecovery_RevertWhen_CallerIsNotWallet() public { - vm.prank(address(_walletFactory)); - _kintoWallet.startRecovery(); - assertEq(_kintoWallet.inRecovery(), block.timestamp); - - vm.expectRevert("KW: only self"); - _kintoWallet.cancelRecovery(); - } - - function testChangeRecoverer_RevertWhen_CallerIsNotFactory(address someone) public { - vm.assume(someone != address(_walletFactory)); - vm.expectRevert("KW: only factory"); - _kintoWallet.changeRecoverer(payable(address(_kintoWallet))); - } - - function testChangeRecoverer_RevertWhen_SameRecoverer() public { - address recoverer = _kintoWallet.recoverer(); - vm.prank(address(_walletFactory)); - vm.expectRevert("KW-cr: invalid address"); - _kintoWallet.changeRecoverer(payable(recoverer)); - } - - function testChangeRecoverer_RevertWhen_ZeroAddress() public { - vm.prank(address(_walletFactory)); - vm.expectRevert("KW-cr: invalid address"); - _kintoWallet.changeRecoverer(payable(address(0))); - } - - /* ============ Funder Whitelist ============ */ - - function testWalletOwnersAreWhitelisted() public { - vm.startPrank(_owner); - assertEq(_kintoWallet.isFunderWhitelisted(_owner), true); - assertEq(_kintoWallet.isFunderWhitelisted(_user), false); - assertEq(_kintoWallet.isFunderWhitelisted(_user2), false); - vm.stopPrank(); - } - - function testAddingOneFunder() public { - vm.startPrank(_owner); - address[] memory funders = new address[](1); - funders[0] = address(23); - uint256 nonce = _kintoWallet.getNonce(); - bool[] memory flags = new bool[](1); - flags[0] = true; - UserOperation memory userOp = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - nonce, - privateKeys, - abi.encodeWithSignature("setFunderWhitelist(address[],bool[])", funders, flags), - address(_paymaster) - ); - 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(); - } - - /* ============ App Key ============ */ - - function testSetAppKey() public { - address app = address(_engenCredits); - uint256 nonce = _kintoWallet.getNonce(); - registerApp(_owner, "test", address(_engenCredits)); - - UserOperation[] memory userOps = new UserOperation[](2); - userOps[0] = - _whitelistAppOp(privateKeys, address(_kintoWallet), nonce, address(_engenCredits), address(_paymaster)); - - userOps[1] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - nonce + 1, - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", app, _user), - address(_paymaster) - ); - - // Execute the transaction via the entry point - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(_kintoWallet.appSigner(app), _user); - } - - // execute - function testSetAppKey_RevertWhen_AppIsNotWhitelisted() public { - registerApp(_owner, "test", address(_engenCredits)); - - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(_engenCredits), _user), - address(_paymaster) - ); - - // execute the transaction via the entry point - address appSignerBefore = _kintoWallet.appSigner(address(_engenCredits)); - - // @dev handleOps fails silently (does not revert) - 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-apk: contract not whitelisted"); - assertEq(_kintoWallet.appSigner(address(_engenCredits)), appSignerBefore); - } - - function testExecute_When2Signers_WhenUsingAppkey() public { - // set 2 owners - address[] memory owners = new address[](2); - owners[0] = _owner; - owners[1] = _user2; - - // generate the user operation which changes the policy to ALL_SIGNERS - UserOperation memory userOp = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("resetSigners(address[],uint8)", owners, _kintoWallet.ALL_SIGNERS()), - address(_paymaster) - ); - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = userOp; - - // execute the transaction via the entry point - vm.prank(_owner); - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(_kintoWallet.owners(1), _user2); - assertEq(_kintoWallet.signerPolicy(), _kintoWallet.ALL_SIGNERS()); // deploy Counter contract - Counter counter = new Counter(); + counter = new Counter(); assertEq(counter.count(), 0); - // register app - registerApp(_owner, "test", address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // prep user ops (whitelist and set app key) - privateKeys = new uint256[](2); - privateKeys[0] = _ownerPk; - privateKeys[1] = _user2Pk; - - userOps = new UserOperation[](2); - userOps[0] = _whitelistAppOp( - privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(counter), address(_paymaster) - ); - - // set app key signature - userOps[1] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce() + 1, - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - vm.prank(_owner); - _entryPoint.handleOps(userOps, payable(_owner)); - - // create counter increment transaction - userOps = new UserOperation[](1); - uint256[] memory privateKeysApp = new uint256[](1); - privateKeysApp[0] = _userPk; - - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(counter), - _kintoWallet.getNonce(), - privateKeysApp, - abi.encodeWithSignature("increment()"), - address(_paymaster) - ); - - // execute - vm.prank(_owner); - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 1); - } - - function testExecute_WhenUsingAppkey_WhenSignerIsOwner() public { - // deploy Counter contract - Counter counter = new Counter(); - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - - _entryPoint.handleOps(userOps, payable(_owner)); - - // create Counter increment transaction - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(counter), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("increment()"), - address(_paymaster) - ); - - // execute - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 1); - } - - function testExecute_WhenUsingAppkey_WhenSignerIsOwner_WhenAppNotWhitelisted() public { - // deploy Counter contract - Counter counter = new Counter(); - registerApp(_owner, "test", address(counter)); - // whitelistApp(address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - - _entryPoint.handleOps(userOps, payable(_owner)); - - // create Counter increment transaction - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(counter), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("increment()"), - address(_paymaster) - ); - - // 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(userOps[0]), userOps[0].sender, userOps[0].nonce, bytes("") - ); - - vm.recordLogs(); - _entryPoint.handleOps(userOps, payable(_owner)); - assertRevertReasonEq("KW: contract not whitelisted"); - } - - function testExecute_WhenUsingAppkey_WhenCallToWallet_WhenSignerIsOwner() public { - // should skip the verification through app key and just use the policy of the wallet - - // deploy Counter contract - Counter counter = new Counter(); - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - - _entryPoint.handleOps(userOps, payable(_owner)); - - // try doing a wallet call and it should work - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = false; - - userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags), - address(_paymaster) - ); - - // execute - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(_kintoWallet.appWhitelist(address(counter)), false); - } - - function testExecute_Revert_WhenUsingAppkey_WhenSignerIsAppKey_WhenCallToWallet() public { - // deploy Counter contract - Counter counter = new Counter(); - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - _entryPoint.handleOps(userOps, payable(_owner)); - - // create Counter increment transaction - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("isFunderWhitelisted()"), - address(_paymaster) - ); - - // execute - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 1); - } - - // execute batch - function testExecuteBatch_WhenUsingAppkey_WhenSignerIsOwner_WhenNonWalletOps() public {} // should skip the verification through app key and just use the policy of the wallet - - function testExecuteBatch_WhenUsingAppkey_WhenSignerIsOwner_WhenAllWalletOps() public {} // should skip the verification through app key and just use the policy of the wallet - - function testExecuteBatch_WhenUsingAppkey_WhenSignerIsAppKey_WhenNonWalletOps() public {} // should use the app key - - function testExecuteBatch_Revert_WhenUsingAppkey_WhenSignerIsAppKey_WhenFirstOpIsWallet() public { - // should skip app key verification because one of the targets is wallet and this is not allowed - // and then revert with AA24 signature error because signer is not the owner - - // deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - - console.log("COUNTERRR", address(counter)); - console.log("WALLETTT", address(_kintoWallet)); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - _entryPoint.handleOps(userOps, payable(_owner)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counter); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - bytes[] memory calls = new bytes[](3); - - // whitelist app params - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp and increment counter (two times) - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("increment()"); - - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) - ); - - vm.expectRevert(abi.encodeWithSignature("FailedOp(uint256,string)", 0, "AA24 signature error")); - _entryPoint.handleOps(userOps, payable(_owner)); - } - - function testExecuteBatch_Revert_WhenUsingAppkey_WhenSignerIsAppKey_WhenLastOpIsWallet() public { - // should skip app key verification because one of the targets is wallet and this is not allowed - // and then revert with AA24 signature error because signer is not the owner - - // FIXME: what is actually happening is that it is failing on the paymaster because, when it gets the sponsor, - // either this the above should be fixed so they are consistent. - - // deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - console.log("COUNTERRR", address(counter)); - console.log("WALLETTT", address(_kintoWallet)); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - registerApp(_owner, "test", address(counter)); whitelistApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - _entryPoint.handleOps(userOps, payable(_owner)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(counter); - targets[1] = address(counter); - targets[2] = address(_kintoWallet); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - bytes[] memory calls = new bytes[](3); - - // whitelist app params - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp and increment counter (two times) - calls[0] = abi.encodeWithSignature("increment()"); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) - ); - - // prepare expected error message - uint256 expectedOpIndex = 0; // Adjust as needed - string memory expectedMessage = "AA33 reverted"; - bytes memory additionalMessage = - abi.encodePacked("SP: executeBatch targets must be sponsored by the contract or be the sender wallet"); - bytes memory expectedAdditionalData = abi.encodeWithSelector( - bytes4(keccak256("Error(string)")), // Standard error selector - additionalMessage - ); - - // encode the entire revert reason - bytes memory expectedRevertReason = abi.encodeWithSignature( - "FailedOpWithRevert(uint256,string,bytes)", expectedOpIndex, expectedMessage, expectedAdditionalData - ); - - vm.expectRevert(expectedRevertReason); - _entryPoint.handleOps(userOps, payable(_owner)); + fundSponsorForApp(address(counter)); } - function testExecuteBatch_Revert_WhenUsingAppkey_WhenSignerIsAppKey_WhenLastOpIsWallet_Exploit() public { - // deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - console.log("COUNTERRR", address(counter)); - console.log("WALLETTT", address(_kintoWallet)); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - // register app passing wallet as a child - address[] memory appContracts = new address[](0); - appContracts[0] = address(_kintoWallet); - registerApp(_owner, "test", address(counter), appContracts); - - whitelistApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - _entryPoint.handleOps(userOps, payable(_owner)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(counter); - targets[1] = address(counter); - targets[2] = address(_kintoWallet); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - bytes[] memory calls = new bytes[](3); - - // whitelist app params - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp and increment counter (two times) - calls[0] = abi.encodeWithSignature("increment()"); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) - ); - - // send all transactions via batch - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 2); - } - - /* ============ Whitelist ============ */ - - function testWhitelistRegisteredApp() public { - // (1). deploy Counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - - // (2). fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - // (3). register app - registerApp(_owner, "test", address(counter)); - - // (4). Create whitelist app user op - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _whitelistAppOp( - privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(counter), address(_paymaster) - ); - - // (5). execute the transaction via the entry point - _entryPoint.handleOps(userOps, payable(_owner)); - } - - // function testWhitelist_revertWhen_AppNotRegistered() public { - // // (1). deploy Counter contract - // Counter counter = new Counter(); - // assertEq(counter.count(), 0); - - // // (2). fund paymaster for Counter contract - // _fundSponsorForApp(address(counter)); - - // // (3). Create whitelist app user op - // UserOperation[] memory userOps = new UserOperation[](1); - // userOps[0] = _whitelistAppOp( - // privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(counter), address(_paymaster) - // ); - - // // (4). execute the transaction via the entry point and expect a revert event - // 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-apw: app must be registered"); - // } - - /* ============ Getters ============ */ - - function testGetOwnersCount() public { + function testUp() public virtual { + assertEq(_kintoWallet.owners(0), _owner); + assertEq(_entryPoint.walletFactory(), address(_walletFactory)); assertEq(_kintoWallet.getOwnersCount(), 1); } - - /* ============ _validateSignature ============ */ - - function testValidateSignature_RevertWhen_OwnerIsNotKYCd() public { - useHarness(); - revokeKYC(_kycProvider, _owner, _ownerPk); - - UserOperation memory userOp; - assertEq( - SIG_VALIDATION_FAILED, - KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature(userOp, bytes32(0)) - ); - } - - function testValidateSignature_RevertWhen_SignatureLengthMismatch() public { - useHarness(); - revokeKYC(_kycProvider, _owner, _ownerPk); - - UserOperation memory userOp; - assertEq( - SIG_VALIDATION_FAILED, - KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature(userOp, bytes32(0)) - ); - } - - /* ============ _getAppContract ============ */ - function testGetAppContract() public {} } diff --git a/test/KintoWalletFactory.t.sol b/test/KintoWalletFactory.t.sol index 1e524a253..b35f78b59 100644 --- a/test/KintoWalletFactory.t.sol +++ b/test/KintoWalletFactory.t.sol @@ -136,7 +136,7 @@ contract KintoWalletFactoryTest is UserOp, AATestScaffolding { function testWhitelistedSignerCanFundWallet() public { vm.startPrank(_owner); - _fundSponsorForApp(address(_kintoWallet)); + fundSponsorForApp(address(_kintoWallet)); uint256 nonce = _kintoWallet.getNonce(); address[] memory funders = new address[](1); funders[0] = _funder; diff --git a/test/SponsorPaymastExploit.t.sol b/test/SponsorPaymastExploit.t.sol index 0778efa50..9390e05bd 100644 --- a/test/SponsorPaymastExploit.t.sol +++ b/test/SponsorPaymastExploit.t.sol @@ -58,8 +58,8 @@ contract SponsorPaymasterExploitTest is MyOpCreator, AATestScaffolding { _owner.transfer(1e18); vm.stopPrank(); deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - _fundSponsorForApp(address(_engenCredits)); - _fundSponsorForApp(address(_kintoWallet)); + fundSponsorForApp(address(_engenCredits)); + fundSponsorForApp(address(_kintoWallet)); } function testExploit() public { @@ -68,7 +68,7 @@ contract SponsorPaymasterExploitTest is MyOpCreator, AATestScaffolding { Counter counter = new Counter(); assertEq(counter.count(), 0); vm.stopPrank(); - _fundSponsorForApp(address(counter)); + fundSponsorForApp(address(counter)); vm.startPrank(_owner); // Let's send a transaction to the counter contract through our wallet uint256 nonce = _kintoWallet.getNonce(); diff --git a/test/SponsorPaymaster.t.sol b/test/SponsorPaymaster.t.sol index 175f85430..41144a593 100644 --- a/test/SponsorPaymaster.t.sol +++ b/test/SponsorPaymaster.t.sol @@ -39,7 +39,7 @@ contract SponsorPaymasterTest is KYCSignature, UserOp, AATestScaffolding { deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); + fundSponsorForApp(address(_kintoWallet)); // Default tests to use 1 private key for simplicity privateKeys = new uint256[](1); @@ -462,10 +462,69 @@ contract SponsorPaymasterTest is KYCSignature, UserOp, AATestScaffolding { assertRevertReasonEq("SP: Kinto Gas App limit exceeded"); } + function testValidatePaymasterUserOp_RevertWhen_WhenNotRelated_WhenBatch() public { + // if intermediate calls are neither sponsored contracts nor children of an app, it should NOT validate the signature + + // deploy a counter contract + Counter counter = new Counter(); + assertEq(counter.count(), 0); + + // deploy a second counter contract + Counter unknown = new Counter(); + assertEq(unknown.count(), 0); + + // prep batch + address[] memory targets = new address[](3); + targets[0] = address(_kintoWallet); + targets[1] = address(unknown); + targets[2] = address(counter); + + uint256[] memory values = new uint256[](3); + values[0] = 0; + values[1] = 0; + values[2] = 0; + + // we want to do 3 calls: whitelistApp, increment counter and increment counter2 + bytes[] memory calls = new bytes[](3); + calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[1] = abi.encodeWithSignature("increment()"); + calls[2] = abi.encodeWithSignature("increment()"); + + // send all transactions via batch + 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; + + // execute the transaction via the entry point + + // prepare expected error message + uint256 expectedOpIndex = 0; // Adjust as needed + string memory expectedMessage = "AA33 reverted"; + bytes memory additionalMessage = + abi.encodePacked("SP: executeBatch targets must be sponsored by the contract or be the sender wallet"); + bytes memory expectedAdditionalData = abi.encodeWithSelector( + bytes4(keccak256("Error(string)")), // Standard error selector + additionalMessage + ); + + // encode the entire revert reason + bytes memory expectedRevertReason = abi.encodeWithSignature( + "FailedOpWithRevert(uint256,string,bytes)", expectedOpIndex, expectedMessage, expectedAdditionalData + ); + + vm.expectRevert(expectedRevertReason); + _entryPoint.handleOps(userOps, payable(_owner)); + } + // TODO: // reset gas limits after periods // test doing txs in different days + /* ============ Helpers ============ */ + // fixme: somehow not working function _setOperationCount(SponsorPaymaster paymaster, address account, uint256 operationCount) internal { uint256 globalRateLimitSlot = 5; // slot number for the "globalRateLimit" mapping itself @@ -488,7 +547,7 @@ contract SponsorPaymasterTest is KYCSignature, UserOp, AATestScaffolding { app = address(new Counter()); // (2). fund paymaster for Counter contract - _fundSponsorForApp(app); + fundSponsorForApp(app); // (3). register the app registerApp(_owner, "CounterApp", app, appLimits); diff --git a/test/helpers/AATestScaffolding.sol b/test/helpers/AATestScaffolding.sol index 374bbdc7f..704c7afb5 100644 --- a/test/helpers/AATestScaffolding.sol +++ b/test/helpers/AATestScaffolding.sol @@ -73,7 +73,7 @@ abstract contract AATestScaffolding is KYCSignature { vm.deal(_owner, 1e20); } - function _fundSponsorForApp(address _contract) internal { + function fundSponsorForApp(address _contract) internal { // we add the deposit to the counter contract in the paymaster _paymaster.addDepositFor{value: 1e19}(address(_contract)); } @@ -208,7 +208,7 @@ abstract contract AATestScaffolding is KYCSignature { } // register app helpers - // fixme: this should go through entrypoint + // fixme: these should go through entrypoint function registerApp(address _owner, string memory name, address parentContract) public { address[] memory appContracts = new address[](0); @@ -240,6 +240,7 @@ abstract contract AATestScaffolding is KYCSignature { registerApp(_owner, name, parentContract, appContracts, appLimits); } + // fixme: this should go through entrypoint function registerApp( address _owner, string memory name, @@ -253,22 +254,58 @@ abstract contract AATestScaffolding is KYCSignature { ); } - // fixme: this should go through entrypoint + function updateMetadata(address _owner, string memory name, address parentContract, address[] memory appContracts) + public + { + uint256[4] memory appLimits = [ + _kintoAppRegistry.RATE_LIMIT_PERIOD(), + _kintoAppRegistry.RATE_LIMIT_THRESHOLD(), + _kintoAppRegistry.GAS_LIMIT_PERIOD(), + _kintoAppRegistry.GAS_LIMIT_THRESHOLD() + ]; + vm.prank(_owner); + _kintoAppRegistry.updateMetadata(name, parentContract, appContracts, appLimits); + } + + function setSponsoredContracts(address _owner, address app, address[] memory contracts, bool[] memory sponsored) + public + { + vm.prank(_owner); + _kintoAppRegistry.setSponsoredContracts(app, contracts, sponsored); + } + function whitelistApp(address app) public { + whitelistApp(app, true); + } + + function whitelistApp(address app, bool whitelist) public { address[] memory targets = new address[](1); targets[0] = address(app); bool[] memory flags = new bool[](1); - flags[0] = true; + flags[0] = whitelist; vm.prank(address(_kintoWallet)); _kintoWallet.whitelistApp(targets, flags); } - // fixme: this should go through entrypoint function setAppKey(address app, address signer) public { vm.prank(address(_kintoWallet)); _kintoWallet.setAppKey(app, signer); } + function resetSigners(address[] memory newSigners, uint8 policy) public { + vm.prank(address(_kintoWallet)); + _kintoWallet.resetSigners(newSigners, policy); + assertEq(_kintoWallet.owners(0), newSigners[0]); + assertEq(_kintoWallet.owners(1), newSigners[1]); + assertEq(_kintoWallet.signerPolicy(), policy); + } + + function setSignerPolicy(uint8 policy) public { + vm.prank(address(_kintoWallet)); + _kintoWallet.setSignerPolicy(policy); + assertEq(_kintoWallet.signerPolicy(), policy); + } + function useHarness() public { KintoWalletHarness _impl = new KintoWalletHarness(_entryPoint, _kintoIDv1, _kintoAppRegistry); vm.prank(_walletFactory.owner()); diff --git a/test/wallet/appKey/AppKey.t.sol b/test/wallet/appKey/AppKey.t.sol deleted file mode 100644 index ba817bedb..000000000 --- a/test/wallet/appKey/AppKey.t.sol +++ /dev/null @@ -1,574 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.18; - -import "forge-std/Test.sol"; -import "forge-std/console.sol"; - -import "@aa/interfaces/IEntryPoint.sol"; - -import "../../../src/interfaces/IKintoWallet.sol"; - -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; - -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; - -contract AppKeyTest is AATestScaffolding, UserOp { - uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; - - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); - - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified - privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - - /* ============ App Key ============ */ - - function testSetAppKey() public { - address app = address(_engenCredits); - uint256 nonce = _kintoWallet.getNonce(); - registerApp(_owner, "test", address(_engenCredits)); - - UserOperation[] memory userOps = new UserOperation[](2); - userOps[0] = - _whitelistAppOp(privateKeys, address(_kintoWallet), nonce, address(_engenCredits), address(_paymaster)); - - userOps[1] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - nonce + 1, - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", app, _user), - address(_paymaster) - ); - - // Execute the transaction via the entry point - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(_kintoWallet.appSigner(app), _user); - } - - // execute - function testSetAppKey_RevertWhen_AppIsNotWhitelisted() public { - registerApp(_owner, "test", address(_engenCredits)); - - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(_engenCredits), _user), - address(_paymaster) - ); - - // execute the transaction via the entry point - address appSignerBefore = _kintoWallet.appSigner(address(_engenCredits)); - - // @dev handleOps fails silently (does not revert) - 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-apk: contract not whitelisted"); - assertEq(_kintoWallet.appSigner(address(_engenCredits)), appSignerBefore); - } - - function testExecute_When2Signers_WhenUsingAppkey() public { - // set 2 owners - address[] memory owners = new address[](2); - owners[0] = _owner; - owners[1] = _user2; - - // generate the user operation which changes the policy to ALL_SIGNERS - UserOperation memory userOp = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("resetSigners(address[],uint8)", owners, _kintoWallet.ALL_SIGNERS()), - address(_paymaster) - ); - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = userOp; - - // execute the transaction via the entry point - vm.prank(_owner); - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(_kintoWallet.owners(1), _user2); - assertEq(_kintoWallet.signerPolicy(), _kintoWallet.ALL_SIGNERS()); - - // deploy Counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - - // register app - registerApp(_owner, "test", address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // prep user ops (whitelist and set app key) - privateKeys = new uint256[](2); - privateKeys[0] = _ownerPk; - privateKeys[1] = _user2Pk; - - userOps = new UserOperation[](2); - userOps[0] = _whitelistAppOp( - privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(counter), address(_paymaster) - ); - - // set app key signature - userOps[1] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce() + 1, - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - vm.prank(_owner); - _entryPoint.handleOps(userOps, payable(_owner)); - - // create counter increment transaction - userOps = new UserOperation[](1); - uint256[] memory privateKeysApp = new uint256[](1); - privateKeysApp[0] = _userPk; - - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(counter), - _kintoWallet.getNonce(), - privateKeysApp, - abi.encodeWithSignature("increment()"), - address(_paymaster) - ); - - // execute - vm.prank(_owner); - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 1); - } - - function testExecute_WhenUsingAppkey_WhenSignerIsOwner() public { - // deploy Counter contract - Counter counter = new Counter(); - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - - _entryPoint.handleOps(userOps, payable(_owner)); - - // create Counter increment transaction - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(counter), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("increment()"), - address(_paymaster) - ); - - // execute - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 1); - } - - function testExecute_WhenUsingAppkey_WhenSignerIsOwner_WhenAppNotWhitelisted() public { - // deploy Counter contract - Counter counter = new Counter(); - registerApp(_owner, "test", address(counter)); - // whitelistApp(address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - - _entryPoint.handleOps(userOps, payable(_owner)); - - // create Counter increment transaction - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(counter), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("increment()"), - address(_paymaster) - ); - - // 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(userOps[0]), userOps[0].sender, userOps[0].nonce, bytes("") - ); - - vm.recordLogs(); - _entryPoint.handleOps(userOps, payable(_owner)); - assertRevertReasonEq("KW: contract not whitelisted"); - } - - function testExecute_WhenUsingAppkey_WhenCallToWallet_WhenSignerIsOwner() public { - // should skip the verification through app key and just use the policy of the wallet - - // deploy Counter contract - Counter counter = new Counter(); - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - - _entryPoint.handleOps(userOps, payable(_owner)); - - // try doing a wallet call and it should work - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = false; - - userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags), - address(_paymaster) - ); - - // execute - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(_kintoWallet.appWhitelist(address(counter)), false); - } - - function testExecute_Revert_WhenUsingAppkey_WhenSignerIsAppKey_WhenCallToWallet() public { - // deploy Counter contract - Counter counter = new Counter(); - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // fund counter contract - _fundSponsorForApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - _entryPoint.handleOps(userOps, payable(_owner)); - - // create Counter increment transaction - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("isFunderWhitelisted()"), - address(_paymaster) - ); - - // execute - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 1); - } - - // execute batch - function testExecuteBatch_WhenUsingAppkey_WhenSignerIsOwner_WhenNonWalletOps() public {} // should skip the verification through app key and just use the policy of the wallet - - function testExecuteBatch_WhenUsingAppkey_WhenSignerIsOwner_WhenAllWalletOps() public {} // should skip the verification through app key and just use the policy of the wallet - - function testExecuteBatch_WhenUsingAppkey_WhenSignerIsAppKey_WhenNonWalletOps() public {} // should use the app key - - function testExecuteBatch_Revert_WhenUsingAppkey_WhenSignerIsAppKey_WhenFirstOpIsWallet() public { - // should skip app key verification because one of the targets is wallet and this is not allowed - // and then revert with AA24 signature error because signer is not the owner - - // deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - - console.log("COUNTERRR", address(counter)); - console.log("WALLETTT", address(_kintoWallet)); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - _entryPoint.handleOps(userOps, payable(_owner)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counter); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - bytes[] memory calls = new bytes[](3); - - // whitelist app params - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp and increment counter (two times) - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("increment()"); - - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) - ); - - vm.expectRevert(abi.encodeWithSignature("FailedOp(uint256,string)", 0, "AA24 signature error")); - _entryPoint.handleOps(userOps, payable(_owner)); - } - - function testExecuteBatch_Revert_WhenUsingAppkey_WhenSignerIsAppKey_WhenLastOpIsWallet() public { - // should skip app key verification because one of the targets is wallet and this is not allowed - // and then revert with AA24 signature error because signer is not the owner - - // FIXME: what is actually happening is that it is failing on the paymaster because, when it gets the sponsor, - // either this the above should be fixed so they are consistent. - - // deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - console.log("COUNTERRR", address(counter)); - console.log("WALLETTT", address(_kintoWallet)); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - registerApp(_owner, "test", address(counter)); - whitelistApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - _entryPoint.handleOps(userOps, payable(_owner)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(counter); - targets[1] = address(counter); - targets[2] = address(_kintoWallet); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - bytes[] memory calls = new bytes[](3); - - // whitelist app params - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp and increment counter (two times) - calls[0] = abi.encodeWithSignature("increment()"); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) - ); - - // prepare expected error message - uint256 expectedOpIndex = 0; // Adjust as needed - string memory expectedMessage = "AA33 reverted"; - bytes memory additionalMessage = - abi.encodePacked("SP: executeBatch targets must be sponsored by the contract or be the sender wallet"); - bytes memory expectedAdditionalData = abi.encodeWithSelector( - bytes4(keccak256("Error(string)")), // Standard error selector - additionalMessage - ); - - // encode the entire revert reason - bytes memory expectedRevertReason = abi.encodeWithSignature( - "FailedOpWithRevert(uint256,string,bytes)", expectedOpIndex, expectedMessage, expectedAdditionalData - ); - - vm.expectRevert(expectedRevertReason); - _entryPoint.handleOps(userOps, payable(_owner)); - } - - function testExecuteBatch_Revert_WhenUsingAppkey_WhenSignerIsAppKey_WhenLastOpIsWallet_Exploit() public { - // deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - console.log("COUNTERRR", address(counter)); - console.log("WALLETTT", address(_kintoWallet)); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - // register app passing wallet as a child - address[] memory appContracts = new address[](0); - appContracts[0] = address(_kintoWallet); - registerApp(_owner, "test", address(counter), appContracts); - - whitelistApp(address(counter)); - - // set app key signature - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = _createUserOperation( - address(_kintoWallet), - address(_kintoWallet), - _kintoWallet.getNonce(), - privateKeys, - abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), - address(_paymaster) - ); - _entryPoint.handleOps(userOps, payable(_owner)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(counter); - targets[1] = address(counter); - targets[2] = address(_kintoWallet); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - bytes[] memory calls = new bytes[](3); - - // whitelist app params - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp and increment counter (two times) - calls[0] = abi.encodeWithSignature("increment()"); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); - userOps = new UserOperation[](1); - privateKeys = new uint256[](1); - privateKeys[0] = _userPk; // we want to make use of the app key - - userOps[0] = _createUserOperation( - address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) - ); - - // send all transactions via batch - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 2); - } -} diff --git a/test/wallet/execute/Execute.t.sol b/test/wallet/execute/Execute.t.sol index abbc2c418..2e99833e2 100644 --- a/test/wallet/execute/Execute.t.sol +++ b/test/wallet/execute/Execute.t.sol @@ -1,215 +1,117 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.18; -import "forge-std/Test.sol"; import "forge-std/console.sol"; +import "../../KintoWallet.t.sol"; -import "@aa/interfaces/IEntryPoint.sol"; - -import "../../../src/interfaces/IKintoWallet.sol"; - -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; - -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; - -contract ExecuteBatchTest is AATestScaffolding, UserOp { - uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; - - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); - - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified - privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - - /* ============ One Signer Account Transaction Tests (executeBatch) ============ */ - - function testExecuteBatch() public { - // deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - registerApp(_owner, "test", address(counter)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counter); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - bytes[] memory calls = new bytes[](3); - - address[] memory apps = new address[](1); - apps[0] = address(counter); +contract ExecuteTest is KintoWalletTest { + /* ============ One Signer Account Transaction Tests (execute) ============ */ + function testExecute_WhenPaymaster() public { + uint256 nonce = _kintoWallet.getNonce(); bool[] memory flags = new bool[](1); flags[0] = true; - // 3 calls batch: whitelistApp and increment counter (two times) - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = 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) + address(_kintoWallet), + address(counter), + nonce, + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) ); - // send all transactions via batch + // execute the transactions via the entry point _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 2); + assertEq(counter.count(), 1); } - function testExecuteBatch_RevertWhen_TargetsBelongToDifferentApps() public { - // deploy the counter contract - Counter counter = new Counter(); - Counter counter2 = new Counter(); - assertEq(counter.count(), 0); - assertEq(counter2.count(), 0); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - registerApp(_owner, "counter app", address(counter)); - registerApp(_owner, "counter2 app", address(counter2)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counter2); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - address[] memory apps = new address[](1); - apps[0] = address(counter); - - bool[] memory flags = new bool[](1); - flags[0] = true; - - // 3 calls batch: whitelistApp, increment counter and increment counter2 - bytes[] memory calls = new bytes[](3); - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("increment()"); - - // send all transactions via batch - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + function testExecute_RevertWhen_NoPaymasterNorPrefund() public { + // send a transaction to the counter contract through our wallet without a paymaster and without prefunding the wallet UserOperation memory userOp = _createUserOperation( - address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + address(_kintoWallet), + address(counter), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("increment()") ); 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)); + } - // prepare expected error message - uint256 expectedOpIndex = 0; // Adjust as needed - string memory expectedMessage = "AA33 reverted"; - bytes memory additionalMessage = - abi.encodePacked("SP: executeBatch targets must be sponsored by the contract or be the sender wallet"); - bytes memory expectedAdditionalData = abi.encodeWithSelector( - bytes4(keccak256("Error(string)")), // Standard error selector - additionalMessage - ); + function testExecute_WhenPrefund() public { + // prefund wallet + vm.deal(address(_kintoWallet), 1 ether); - // encode the entire revert reason - bytes memory expectedRevertReason = abi.encodeWithSignature( - "FailedOpWithRevert(uint256,string,bytes)", expectedOpIndex, expectedMessage, expectedAdditionalData + // send op without a paymaster but prefunding the wallet + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + address(_kintoWallet), + address(counter), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("increment()") ); - vm.expectRevert(expectedRevertReason); + // execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); + assertEq(counter.count(), 1); } - // we want to test that we can execute a batch of user ops where all targets are app children - // except for the first and the last ones which are the wallet itself - function testExecuteBatch_TopAndBottomWalletOps() public { - // deploy the counter contract - Counter counter = new Counter(); - Counter counterRelatedContract = new Counter(); - assertEq(counter.count(), 0); - - // fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - address[] memory appContracts = new address[](1); - appContracts[0] = address(counterRelatedContract); - registerApp(_owner, "counter app", address(counter), appContracts); - - // prep batch - address[] memory targets = new address[](4); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counterRelatedContract); - targets[3] = address(_kintoWallet); - - uint256[] memory values = new uint256[](4); - values[0] = 0; - values[1] = 0; - values[2] = 0; - values[3] = 0; + function testExecute_RevertWhen_AppIsNotWhitelisted() public { + // remove app from whitelist + whitelistApp(address(counter), false); - // whitelistApp params - address[] memory apps = new address[](1); - apps[0] = address(counter); - - bool[] memory flags = new bool[](1); - flags[0] = true; + // create Counter increment user op + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + address(_kintoWallet), + address(counter), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) + ); - // 4 calls batch: whitelistApp, increment counter and increment counter2 and whitelistApp - bytes[] memory calls = new bytes[](4); - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("increment()"); - calls[3] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); + // execute transaction + 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"); + assertEq(counter.count(), 0); + } - // send all transactions via batch - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); - UserOperation[] memory userOps = new UserOperation[](1); + function testExecute_WhenMultipleOps_WhenPaymaster() public { + uint256 nonce = _kintoWallet.getNonce(); + UserOperation[] memory userOps = new UserOperation[](2); userOps[0] = _createUserOperation( - address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + 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(), 1); - assertEq(counterRelatedContract.count(), 1); + assertEq(counter.count(), 2); } } diff --git a/test/wallet/executeBatch/ExecuteBatch.t.sol b/test/wallet/executeBatch/ExecuteBatch.t.sol index 6142a00cd..eb5e76020 100644 --- a/test/wallet/executeBatch/ExecuteBatch.t.sol +++ b/test/wallet/executeBatch/ExecuteBatch.t.sol @@ -6,147 +6,6 @@ import "forge-std/console.sol"; import "@aa/interfaces/IEntryPoint.sol"; -import "../../../src/interfaces/IKintoWallet.sol"; +import "../../KintoWallet.t.sol"; -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; - -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; - -contract ExecuteBatchTest is AATestScaffolding, UserOp { - uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; - - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); - - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified - privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - - /* ============ One Signer Account Transaction Tests (executeBatch) ============ */ - - function testMultipleTransactionsExecuteBatchPaymaster() public { - vm.startPrank(_owner); - // Let's deploy the counter contract - Counter counter = new Counter(); - assertEq(counter.count(), 0); - uint256 nonce = _kintoWallet.getNonce(); - vm.stopPrank(); - registerApp(_owner, "test", address(counter)); - vm.startPrank(_owner); - _fundSponsorForApp(address(counter)); - address[] memory targets = new address[](3); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counter); - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - bytes[] memory calls = new bytes[](3); - address[] memory apps = new address[](1); - apps[0] = address(counter); - bool[] memory flags = new bool[](1); - flags[0] = true; - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("increment()"); - - OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); - UserOperation memory userOp = - _createUserOperation(address(_kintoWallet), nonce, privateKeys, opParams, address(_paymaster)); - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = userOp; - // Execute the transaction via the entry point - _entryPoint.handleOps(userOps, payable(_owner)); - assertEq(counter.count(), 2); - vm.stopPrank(); - } - - function test_RevertWhen_MultipleTransactionsExecuteBatchPaymasterRefuses() public { - // deploy the counter contract - Counter counter = new Counter(); - Counter counter2 = new Counter(); - assertEq(counter.count(), 0); - assertEq(counter2.count(), 0); - - // only fund counter - _fundSponsorForApp(address(counter)); - - registerApp(_owner, "test", address(counter)); - - // prep batch - address[] memory targets = new address[](3); - targets[0] = address(_kintoWallet); - targets[1] = address(counter); - targets[2] = address(counter2); - - uint256[] memory values = new uint256[](3); - values[0] = 0; - values[1] = 0; - values[2] = 0; - - address[] memory apps = new address[](1); - apps[0] = address(counter); - - bool[] memory flags = new bool[](1); - flags[0] = true; - - // we want to do 3 calls: whitelistApp, increment counter and increment counter2 - bytes[] memory calls = new bytes[](3); - calls[0] = abi.encodeWithSignature("whitelistApp(address[],bool[])", apps, flags); - calls[1] = abi.encodeWithSignature("increment()"); - calls[2] = abi.encodeWithSignature("increment()"); - - // send all transactions via batch - 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; - - // execute the transaction via the entry point - - // prepare expected error message - uint256 expectedOpIndex = 0; // Adjust as needed - string memory expectedMessage = "AA33 reverted"; - bytes memory additionalMessage = - abi.encodePacked("SP: executeBatch targets must be sponsored by the contract or be the sender wallet"); - bytes memory expectedAdditionalData = abi.encodeWithSelector( - bytes4(keccak256("Error(string)")), // Standard error selector - additionalMessage - ); - - // encode the entire revert reason - bytes memory expectedRevertReason = abi.encodeWithSignature( - "FailedOpWithRevert(uint256,string,bytes)", expectedOpIndex, expectedMessage, expectedAdditionalData - ); - - vm.expectRevert(expectedRevertReason); - _entryPoint.handleOps(userOps, payable(_owner)); - } -} +contract ExecuteBatchTest is KintoWalletTest {} diff --git a/test/wallet/funder/Funder.t.sol b/test/wallet/funder/Funder.t.sol index 60805e1f5..dbf30907e 100644 --- a/test/wallet/funder/Funder.t.sol +++ b/test/wallet/funder/Funder.t.sol @@ -1,62 +1,21 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.18; -import "forge-std/Test.sol"; import "forge-std/console.sol"; +import "../../KintoWallet.t.sol"; -import "@aa/interfaces/IEntryPoint.sol"; - -import "../../../src/interfaces/IKintoWallet.sol"; - -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; - -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; - -contract FunderTest is AATestScaffolding, UserOp { - uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; - - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); - - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified - privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - +contract FunderTest is KintoWalletTest { /* ============ Funder Whitelist ============ */ - function testWalletOwnersAreWhitelisted() public { - vm.startPrank(_owner); + function testUp() public override { + super.testUp(); + assertEq(_kintoWallet.isFunderWhitelisted(_owner), true); assertEq(_kintoWallet.isFunderWhitelisted(_user), false); assertEq(_kintoWallet.isFunderWhitelisted(_user2), false); - vm.stopPrank(); } - function testAddingOneFunder() public { + function testSetFunderWhitelist() public { vm.startPrank(_owner); address[] memory funders = new address[](1); funders[0] = address(23); diff --git a/test/wallet/policy/Policy.t.sol b/test/wallet/policy/Policy.t.sol index b09cb18ec..5e80bd92f 100644 --- a/test/wallet/policy/Policy.t.sol +++ b/test/wallet/policy/Policy.t.sol @@ -1,51 +1,10 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.18; -import "forge-std/Test.sol"; import "forge-std/console.sol"; +import "../../KintoWallet.t.sol"; -import "@aa/interfaces/IEntryPoint.sol"; - -import "../../../src/interfaces/IKintoWallet.sol"; - -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; - -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; - -contract ResetSignerTest is AATestScaffolding, UserOp { - uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; - - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); - - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified - privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - +contract ResetSignerTest is KintoWalletTest { /* ============ Signers & Policy Tests ============ */ function testAddingOneSigner() public { @@ -64,7 +23,8 @@ contract ResetSignerTest is AATestScaffolding, UserOp { ); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - // Execute the transaction via the entry point + + // execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); assertEq(_kintoWallet.owners(1), _user); vm.stopPrank(); @@ -171,26 +131,27 @@ contract ResetSignerTest is AATestScaffolding, UserOp { } function testChangingPolicyWithTwoSigners() public { - vm.startPrank(_owner); address[] memory owners = new address[](2); owners[0] = _owner; owners[1] = _user; - uint256 nonce = _kintoWallet.getNonce(); - UserOperation memory userOp = _createUserOperation( + + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( address(_kintoWallet), address(_kintoWallet), - nonce, + _kintoWallet.getNonce(), privateKeys, abi.encodeWithSignature("resetSigners(address[],uint8)", owners, _kintoWallet.ALL_SIGNERS()), address(_paymaster) ); - UserOperation[] memory userOps = new UserOperation[](1); - userOps[0] = userOp; - // Execute the transaction via the entry point + + // execute the transaction via the entry point + vm.expectEmit(); + emit WalletPolicyChanged(_kintoWallet.ALL_SIGNERS(), _kintoWallet.SINGLE_SIGNER()); _entryPoint.handleOps(userOps, payable(_owner)); + assertEq(_kintoWallet.owners(1), _user); assertEq(_kintoWallet.signerPolicy(), _kintoWallet.ALL_SIGNERS()); - vm.stopPrank(); } function testChangingPolicyWithThreeSigners() public { @@ -218,6 +179,7 @@ contract ResetSignerTest is AATestScaffolding, UserOp { vm.stopPrank(); } + // todo: separate into 2 different tests function test_RevertWhen_ChangingPolicyWithoutRightSigners() public { address[] memory owners = new address[](2); owners[0] = _owner; diff --git a/test/wallet/recovery/Recovery.t.sol b/test/wallet/recovery/Recovery.t.sol index c461a3596..2a0a9dd64 100644 --- a/test/wallet/recovery/Recovery.t.sol +++ b/test/wallet/recovery/Recovery.t.sol @@ -1,51 +1,10 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.18; -import "forge-std/Test.sol"; import "forge-std/console.sol"; +import "../../KintoWallet.t.sol"; -import "@aa/interfaces/IEntryPoint.sol"; - -import "../../../src/interfaces/IKintoWallet.sol"; - -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; - -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; - -contract RecoveryTest is AATestScaffolding, UserOp { - uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; - - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); - - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified - privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - +contract RecoveryTest is KintoWalletTest { /* ============ Recovery Tests ============ */ function testStartRecovert() public { @@ -228,4 +187,12 @@ contract RecoveryTest is AATestScaffolding, UserOp { vm.expectRevert("KW-cr: invalid address"); _kintoWallet.changeRecoverer(payable(address(0))); } + + // todo: change recoverer to a non KKYC'd address + // todo: we don't want to allow recoverer to be a non KYC'd, right? + function testChangeRecoverer_RevertWhen_RecovererIsNotKYCd() public { + vm.prank(address(_walletFactory)); + vm.expectRevert("KW-cr: invalid address"); + _kintoWallet.changeRecoverer(payable(address(123))); + } } diff --git a/test/wallet/set-app-key/AppKey.t.sol b/test/wallet/set-app-key/AppKey.t.sol new file mode 100644 index 000000000..01f6804ec --- /dev/null +++ b/test/wallet/set-app-key/AppKey.t.sol @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.18; + +import "forge-std/console.sol"; +import "../../KintoWallet.t.sol"; + +contract AppKeyTest is KintoWalletTest { + /* ============ App Key ============ */ + + function testSetAppKey() public { + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + address(_kintoWallet), + address(_kintoWallet), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("setAppKey(address,address)", address(counter), _user), + address(_paymaster) + ); + + // Execute the transaction via the entry point + _entryPoint.handleOps(userOps, payable(_owner)); + assertEq(_kintoWallet.appSigner(address(counter)), _user); + } + + // todo: we may want to remove this requirement from the KintoWallet since anyways + // we always check for the app to be whitelisted regardless if using app key or not + function testSetAppKey_RevertWhen_AppIsNotWhitelisted() public { + // remove app from whitelist + whitelistApp(address(counter), false); + + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = _createUserOperation( + address(_kintoWallet), + address(_kintoWallet), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("setAppKey(address,address)", address(_engenCredits), _user), + address(_paymaster) + ); + + // execute the transaction via the entry point + address appSignerBefore = _kintoWallet.appSigner(address(_engenCredits)); + + // @dev handleOps fails silently (does not revert) + 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-apk: contract not whitelisted"); + assertEq(_kintoWallet.appSigner(address(_engenCredits)), appSignerBefore); + } +} diff --git a/test/wallet/upgrade/Upgrade.t.sol b/test/wallet/upgrade/Upgrade.t.sol index 1555a0e0e..f2efa8967 100644 --- a/test/wallet/upgrade/Upgrade.t.sol +++ b/test/wallet/upgrade/Upgrade.t.sol @@ -1,55 +1,14 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.18; -import "forge-std/Test.sol"; import "forge-std/console.sol"; +import "../../KintoWallet.t.sol"; -import "@aa/interfaces/IEntryPoint.sol"; - -import "../../../src/interfaces/IKintoWallet.sol"; - -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; - -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; - -contract PolicyTest is AATestScaffolding, UserOp { - uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; - - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); - - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified - privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - +contract PolicyTest is KintoWalletTest { /* ============ Upgrade Tests ============ */ // FIXME: I think these upgrade tests are wrong because, basically, the KintoWallet.sol does not have - // an upgrade function. The upgrade function is in the UUPSUpgradeable.sol contract. + // an upgrade function. The upgrade function is in the UUPSUpgradeable.sol contract and the wallet uses the Beacon proxy. function test_RevertWhen_OwnerCannotUpgrade() public { // deploy a new implementation KintoWallet _newImplementation = new KintoWallet(_entryPoint, _kintoIDv1, _kintoAppRegistry); diff --git a/test/wallet/validateSignature/ValidateSignature.t.sol b/test/wallet/validateSignature/ValidateSignature.t.sol index 588289222..1402848f2 100644 --- a/test/wallet/validateSignature/ValidateSignature.t.sol +++ b/test/wallet/validateSignature/ValidateSignature.t.sol @@ -1,73 +1,887 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.18; -import "forge-std/Test.sol"; import "forge-std/console.sol"; +import "../../KintoWallet.t.sol"; -import "@aa/interfaces/IEntryPoint.sol"; +contract ValidateSignatureTest is KintoWalletTest { + // constants + uint256 constant SIG_VALIDATION_FAILED = 1; + uint256 constant SIG_VALIDATION_SUCCESS = 0; -import "../../../src/interfaces/IKintoWallet.sol"; + function setUp() public override { + super.setUp(); + useHarness(); + } -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; + function testValidateSignature_RevertWhen_OwnerIsNotKYCd() public { + revokeKYC(_kycProvider, _owner, _ownerPk); -import "../../harness/KintoWalletHarness.sol"; -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; + UserOperation memory userOp; + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } -contract ValidateSignatureTest is AATestScaffolding, UserOp { - uint256[] privateKeys; + function testValidateSignature_RevertWhen_SignatureLengthMismatch() public { + revokeKYC(_kycProvider, _owner, _ownerPk); - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; + UserOperation memory userOp; + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_RevertWhen_UsingAppKey_SignatureLengthMismatch() public { + revokeKYC(_kycProvider, _owner, _ownerPk); - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); + UserOperation memory userOp; + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + /* ============ with wallet policy (when execute) ============ */ + + /* ============ single-signer ops (todo) ============ */ + + function testValidateSignature_WhenMultipleOwners_When1SignerPolicy() public { + 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) + ) + ); + } - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); + /* ============ multi-signer ops ============ */ - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); + // todo: make these tests with fuzzing - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); + function testValidateSignature_When2Owners_WhenAllSignersPolicy() public { + // generate resetSigners UserOp to set 2 owners + address[] memory owners = new address[](2); + owners[0] = _owner; + owners[1] = _user; + resetSigners(owners, _kintoWallet.ALL_SIGNERS()); - // Default tests to use _ownerPk unless otherwise specified + // set private keys + privateKeys = new uint256[](2); privateKeys[0] = _ownerPk; + privateKeys[1] = _userPk; + + // create increment user op + 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) + ) + ); } - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); + function testValidateSignature_When3Owners_WhenAllSignersPolicy() public { + address[] memory owners = new address[](3); + owners[0] = _owner; + owners[1] = _user; + owners[2] = _user2; + + resetSigners(owners, _kintoWallet.ALL_SIGNERS()); + + // use only 2 signers + privateKeys = new uint256[](2); + privateKeys[0] = _ownerPk; + privateKeys[1] = _userPk; + + // create op with wrong private keys + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), + address(counter), + _kintoWallet.getNonce() + 1, + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); } - /* ============ _validateSignature ============ */ + function testValidateSignature_When2Owners_WhenAllSignersPolicy_WhenWrongSigners() public { + // change policy to 2 owners and all signers policy + address[] memory owners = new address[](2); + owners[0] = _owner; + owners[1] = _user; + resetSigners(owners, _kintoWallet.ALL_SIGNERS()); - function testValidateSignature_RevertWhen_OwnerIsNotKYCd() public { - useHarness(); - revokeKYC(_kycProvider, _owner, _ownerPk); + // create op with wrong private keys + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), + address(counter), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) + ); - UserOperation memory userOp; assertEq( SIG_VALIDATION_FAILED, - KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature(userOp, bytes32(0)) + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) ); } - function testValidateSignature_RevertWhen_SignatureLengthMismatch() public { - useHarness(); - revokeKYC(_kycProvider, _owner, _ownerPk); + function testValidateSignature_When3Owners_WhenAllSignersPolicy_WhenWrongSigners() public { + // change policy to 3 owners and all signers policy + address[] memory owners = new address[](3); + owners[0] = _owner; + owners[1] = _user; + owners[2] = _user2; + resetSigners(owners, _kintoWallet.ALL_SIGNERS()); + + // set private keys + privateKeys = new uint256[](3); + privateKeys[0] = _ownerPk; + privateKeys[1] = _userPk; + privateKeys[2] = _user2Pk; + + // create op with wrong private keys + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), + address(counter), + _kintoWallet.getNonce() + 1, + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + /* ============ with wallet policy (when executeBatch) ============ */ + + /* ============ single-signer ops ============ */ + + function testValidateSignature_WhenExecuteBatch() public { + address[] memory targets = new address[](3); + targets[0] = address(_kintoWallet); + targets[1] = address(counter); + targets[2] = address(counter); + + uint256[] memory values = new uint256[](3); + values[0] = 0; + values[1] = 0; + values[2] = 0; + + bytes[] memory calls = new bytes[](3); + calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[1] = abi.encodeWithSignature("increment()"); + calls[2] = 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) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenExecuteBatch_WhenChildren() public { + // if intermediate calls are children of an app, it should validate the signature and sponsor will be the app + + // deploy new app + Counter child = new Counter(); + + // update app's metadata adding child + address[] memory appContracts = new address[](1); + appContracts[0] = address(child); + updateMetadata(_owner, "test", address(counter), appContracts); + + address[] memory targets = new address[](3); + targets[0] = address(_kintoWallet); + targets[1] = address(child); + targets[2] = address(counter); + + uint256[] memory values = new uint256[](3); + values[0] = 0; + values[1] = 0; + values[2] = 0; + + bytes[] memory calls = new bytes[](3); + calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[1] = abi.encodeWithSignature("increment()"); + calls[2] = 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) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenExecuteBatch_WhenSponsoredContract() public { + // if intermediate calls are sponsored contracts of an app, it should validate the signature and sponsor will be the app + + // deploy new app + Counter sponsoredContract = new Counter(); + + // add sponsored contract + address[] memory contracts = new address[](1); + contracts[0] = address(sponsoredContract); + bool[] memory sponsored = new bool[](1); + sponsored[0] = true; + setSponsoredContracts(_owner, address(counter), contracts, sponsored); + + address[] memory targets = new address[](3); + targets[0] = address(_kintoWallet); + targets[1] = address(sponsoredContract); + targets[2] = address(counter); + + uint256[] memory values = new uint256[](3); + values[0] = 0; + values[1] = 0; + values[2] = 0; + + bytes[] memory calls = new bytes[](3); + calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[1] = abi.encodeWithSignature("increment()"); + calls[2] = 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) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenExecuteBatch_WhenChildrenAndSponsoredContract() public { + // if intermediate calls are either sponsored contracts or children of an app, it should validate the signature and sponsor will be the app + + // deploy new app + Counter sponsoredContract = new Counter(); + Counter child = new Counter(); + + // add sponsored contract + address[] memory contracts = new address[](1); + contracts[0] = address(sponsoredContract); + bool[] memory sponsored = new bool[](1); + sponsored[0] = true; + setSponsoredContracts(_owner, address(counter), contracts, sponsored); + + // update app's metadata passing a child + address[] memory appContracts = new address[](1); + appContracts[0] = address(child); + updateMetadata(_owner, "test", address(counter), appContracts); + + address[] memory targets = new address[](4); + targets[0] = address(_kintoWallet); + targets[1] = address(child); + targets[2] = address(sponsoredContract); + targets[3] = address(counter); + + uint256[] memory values = new uint256[](4); + values[0] = 0; + values[1] = 0; + values[2] = 0; + values[3] = 0; + + bytes[] memory calls = new bytes[](4); + calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[1] = abi.encodeWithSignature("increment()"); + calls[2] = abi.encodeWithSignature("increment()"); + calls[3] = 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) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenExecuteBatch_WhenNotRelated() public { + // if intermediate calls are neither sponsored contracts nor children of an app, it should NOT validate the signature + + // deploy new app + Counter unknown = new Counter(); + + address[] memory targets = new address[](3); + targets[0] = address(_kintoWallet); + targets[1] = address(unknown); + targets[2] = address(counter); + + uint256[] memory values = new uint256[](3); + values[0] = 0; + values[1] = 0; + values[2] = 0; + + bytes[] memory calls = new bytes[](3); + calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[1] = abi.encodeWithSignature("increment()"); + calls[2] = 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) + ); + + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenExecuteBatch_WhenMoreThan3CallsToWallet() public { + // if there are more than 3 calls to the wallet, it should NOT validate the signature + + address[] memory targets = new address[](5); + targets[0] = address(_kintoWallet); + targets[1] = address(_kintoWallet); + targets[2] = address(_kintoWallet); + targets[3] = address(_kintoWallet); + targets[4] = address(counter); + + uint256[] memory values = new uint256[](5); + values[0] = 0; + values[1] = 0; + values[2] = 0; + values[3] = 0; + values[4] = 0; + + bytes[] memory calls = new bytes[](5); + calls[0] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[1] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[2] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[3] = abi.encodeWithSignature("isFunderWhitelisted()"); + calls[4] = 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) + ); + + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + /* ============ multi-signer ops ============ */ + + /* ============ using app key (when execute) ============ */ + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsAppKey() public { + // it should skip the policy verification and use the app key + // should validate the signature + + setAppKey(address(counter), _user); + + // create user op with app key as signer + 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) + ) + ); + // todo: assert that it has used the app key and not the wallet policy + } + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsAppKey_WhenWalletHas2Signers() public { + // it should skip the policy verification and use the app key + // should validate the signature + + setAppKey(address(counter), _user); + + // reset signers & change policy + address[] memory owners = new address[](2); + owners[0] = _owner; + owners[1] = _user2; + uint8 newPolicy = _kintoWallet.ALL_SIGNERS(); + resetSigners(owners, newPolicy); + + // create user op with the app key as signer + 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) + ) + ); + // todo: assert that it has used the app key and not the wallet policy + } + + // todo: we may want to remove this requirement from the KintoWallet since anyways + // we always check for the app to be whitelisted regardless if using app key or not + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsAppKey_WhenAppNotWhitelisted() public { + // it should skip the app key verification and use the policy of the wallet + // it should revert because app is not whitelisted + + setAppKey(address(counter), _user); + + // remove app from whitelist + whitelistApp(address(counter), false); + + // create Counter increment transaction + privateKeys[0] = _userPk; // we want to make use of the app key + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), + address(counter), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("increment()"), + address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsAppKey_WhenCallToWallet() public { + // since we are not allowing calls to the wallet, this would make the app key verification + // to be skipped and will try to use the policy of the wallet + // which will fail because the iSigner is not the owner + // fixme: probably better to just make the signature fail when we realised that the signer is the app key and the call is to the wallet + + setAppKey(address(counter), _user); + + // create Counter increment transaction + privateKeys[0] = _userPk; // we want to make use of the app key + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), + address(_kintoWallet), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("isFunderWhitelisted()"), + address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsOwner() public { + // it should skip the app key verification and use the policy of the wallet + // should validate the signature + + setAppKey(address(counter), _user); + + // create user op with the owner as signer + 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) + ) + ); + // todo: we should be able to assert that it has used the policy of the wallet and not the app key + } + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsOwner_WhenCallToWallet() public { + // it should skip the app key verification and use wallet policy + // should validate the signature + + setAppKey(address(counter), _user); + + // try doing a wallet call and it should work + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), + address(_kintoWallet), + _kintoWallet.getNonce(), + privateKeys, + abi.encodeWithSignature("isFunderWhitelisted()"), + address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + /* ============ using app key (when executeBatch) ============ */ + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsAppKey_WhenExecuteBatch() public { + setAppKey(address(counter), _user); + + address[] memory targets = new address[](2); + targets[0] = address(counter); + 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("increment()"); + calls[1] = abi.encodeWithSignature("increment()"); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + privateKeys[0] = _userPk; // we want to make use of the app key + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsAppKey_WhenWalletHas2Signers_WhenExecuteBatch() public { + // it should skip the policy verification and use the app key + // should validate the signature + + // reset signers & change policy + address[] memory owners = new address[](2); + owners[0] = _owner; + owners[1] = _user2; + + resetSigners(owners, _kintoWallet.ALL_SIGNERS()); + + setAppKey(address(counter), _user); + + address[] memory targets = new address[](2); + targets[0] = address(counter); + 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("increment()"); + calls[1] = abi.encodeWithSignature("increment()"); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + privateKeys[0] = _userPk; // we want to make use of the app key + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + // todo: we may want to remove the whitelist check from here + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsAppKey_WhenAppNotWhitelisted_WhenExecuteBatch() public { + setAppKey(address(counter), _user); + + // remove app from whitelist + whitelistApp(address(counter), false); + + address[] memory targets = new address[](2); + targets[0] = address(counter); + 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("increment()"); + calls[1] = abi.encodeWithSignature("increment()"); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + privateKeys[0] = _userPk; // we want to make use of the app key + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + } + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsAppKey_WhenCallToWallet_WhenExecuteBatch(uint256 order) + public + { + // it should skip the app key verification and use wallet policy + // should validate the signature + + uint256 CALLS_NUMER = 5; + vm.assume(order < CALLS_NUMER); + // it should skip the wallet policy and use the app key verification + // should NOT validate the signature since it's forbidden to call the wallet + + setAppKey(address(counter), _user); + + // prep batch: 5 calls in a batch which will include a call to the wallet (in different orders) + address[] memory targets = new address[](CALLS_NUMER); + uint256[] memory values = new uint256[](CALLS_NUMER); + bytes[] memory calls = new bytes[](CALLS_NUMER); + + for (uint256 i = 0; i < CALLS_NUMER; i++) { + values[i] = 0; + if (i == order) { + targets[i] = address(_kintoWallet); + calls[i] = abi.encodeWithSignature("isFunderWhitelisted()"); + } else { + targets[i] = address(counter); + calls[i] = abi.encodeWithSignature("increment()"); + } + } + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + privateKeys[0] = _userPk; // we want to make use of the app key + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + ); + + assertEq( + SIG_VALIDATION_FAILED, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + // todo: assert that it has used the app key and not the wallet policy + } + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsOwner_WhenExecuteBatch() public {} + + function testValidateSignature_WhenAppKeyIsSet_WhenSignerIsOwner_WhenCallToWallet_WhenExecuteBatch(uint256 order) + public + { + // it should skip the app key verification and use wallet policy + // should validate the signature + + uint256 CALLS_NUMER = 5; + vm.assume(order < CALLS_NUMER); + // it should skip the wallet policy and use the app key verification + // should NOT validate the signature since it's forbidden to call the wallet + + setAppKey(address(counter), _user); + + // prep batch: 5 calls in a batch which will include a call to the wallet (in different orders) + address[] memory targets = new address[](CALLS_NUMER); + uint256[] memory values = new uint256[](CALLS_NUMER); + bytes[] memory calls = new bytes[](CALLS_NUMER); + + for (uint256 i = 0; i < CALLS_NUMER; i++) { + values[i] = 0; + if (i == order) { + targets[i] = address(_kintoWallet); + calls[i] = abi.encodeWithSignature("isFunderWhitelisted()"); + } else { + targets[i] = address(counter); + calls[i] = 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) + ); + + assertEq( + SIG_VALIDATION_SUCCESS, + KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature( + userOp, _entryPoint.getUserOpHash(userOp) + ) + ); + // todo: assert that it has used the app key and not the wallet policy + } + + /* ============ special cases ============ */ + + // special case 1: requiredSigners == 2, owners.length == 3 and the owners 1 and 2 provided their signatures. + function testValidateSignature_SpecialCase1() public { + // reset signers & change policy + address[] memory owners = new address[](3); + owners[0] = _owner; + owners[1] = _user; + owners[2] = _user2; + uint8 newPolicy = _kintoWallet.MINUS_ONE_SIGNER(); + resetSigners(owners, newPolicy); + + // create user op with owners 1 and 2 as signers + privateKeys = new uint256[](2); + privateKeys[0] = _userPk; + privateKeys[1] = _user2Pk; + + 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) + ) + ); + + // create user op with owners 1 and 2 as signers but in reverse order + privateKeys[0] = _user2Pk; + privateKeys[1] = _userPk; + + 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) + ) + ); + } + + // special case 2: register app having wallet as child and try to make a call to the wallet using the app key + // fixme: this would be fixed if we forbid registerApp to have wallets as children + function testValidateSignature_SpecialCase2() public { + setAppKey(address(counter), _user); + + // update app's metadata passing wallet as a child + address[] memory appContracts = new address[](1); + appContracts[0] = address(_kintoWallet); + updateMetadata(_owner, "test", address(counter), appContracts); + + // prep batch: 3 calls including a resetSigners call to change the signer to be the app key + address[] memory targets = new address[](3); + targets[0] = address(counter); + targets[1] = address(counter); + targets[2] = address(_kintoWallet); + + uint256[] memory values = new uint256[](3); + values[0] = 0; + values[1] = 0; + values[2] = 0; + + // reset signers params + address[] memory owners = new address[](1); + owners[0] = _user; + + bytes[] memory calls = new bytes[](3); + calls[0] = abi.encodeWithSignature("increment()"); + calls[1] = abi.encodeWithSignature("increment()"); + calls[2] = abi.encodeWithSignature("resetSigners(address[],uint8)", owners, _kintoWallet.ALL_SIGNERS()); + + OperationParamsBatch memory opParams = OperationParamsBatch({targets: targets, values: values, bytesOps: calls}); + privateKeys[0] = _userPk; // we want to make use of the app key + UserOperation memory userOp = _createUserOperation( + address(_kintoWallet), _kintoWallet.getNonce(), privateKeys, opParams, address(_paymaster) + ); - UserOperation memory userOp; assertEq( SIG_VALIDATION_FAILED, - KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature(userOp, bytes32(0)) + 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 fffafbebb..6214d6c92 100644 --- a/test/wallet/whitelist/Whitelist.t.sol +++ b/test/wallet/whitelist/Whitelist.t.sol @@ -1,81 +1,41 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.18; -import "forge-std/Test.sol"; import "forge-std/console.sol"; +import "../../KintoWallet.t.sol"; -import "@aa/interfaces/IEntryPoint.sol"; - -import "../../../src/interfaces/IKintoWallet.sol"; - -import "../../../src/wallet/KintoWallet.sol"; -import "../../../src/sample/Counter.sol"; - -import {UserOp} from "../../helpers/UserOp.sol"; -import {AATestScaffolding} from "../../helpers/AATestScaffolding.sol"; - -contract WhitelistTest is AATestScaffolding, UserOp { - uint256[] privateKeys; - - // constants - uint256 constant SIG_VALIDATION_FAILED = 1; - - // events - event UserOperationRevertReason( - bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason - ); - event KintoWalletInitialized(IEntryPoint indexed entryPoint, address indexed owner); - event WalletPolicyChanged(uint256 newPolicy, uint256 oldPolicy); - event RecovererChanged(address indexed newRecoverer, address indexed recoverer); - - function setUp() public { - deployAAScaffolding(_owner, 1, _kycProvider, _recoverer); - - // Add paymaster to _kintoWallet - _fundSponsorForApp(address(_kintoWallet)); - - // Default tests to use 1 private key for simplicity - privateKeys = new uint256[](1); - - // Default tests to use _ownerPk unless otherwise specified - privateKeys[0] = _ownerPk; - } - - function testUp() public { - assertEq(_kintoWallet.owners(0), _owner); - assertEq(_entryPoint.walletFactory(), address(_walletFactory)); - } - +contract WhitelistTest is KintoWalletTest { /* ============ Whitelist ============ */ - function testWhitelistRegisteredApp() public { - // (1). deploy Counter contract + function testWhitelistApp() public { + // deploy an app Counter counter = new Counter(); assertEq(counter.count(), 0); - // (2). fund paymaster for Counter contract - _fundSponsorForApp(address(counter)); - - // (3). register app - registerApp(_owner, "test", address(counter)); - - // (4). Create whitelist app user op + // create whitelist app user op UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = _whitelistAppOp( privateKeys, address(_kintoWallet), _kintoWallet.getNonce(), address(counter), address(_paymaster) ); - // (5). execute the transaction via the entry point + // execute the transaction via the entry point _entryPoint.handleOps(userOps, payable(_owner)); + + assertTrue(_kintoWallet.appWhitelist(address(counter))); + } + + function testWhitelistApp_RevertWhen_AlreadyWhitelisted() public { + //re-register app + whitelistApp(address(counter), true); } - // function testWhitelist_revertWhen_AppNotRegistered() public { + // function testWhitelist_RevertWhen_AppIsNotRegistered() public { // // (1). deploy Counter contract // Counter counter = new Counter(); // assertEq(counter.count(), 0); // // (2). fund paymaster for Counter contract - // _fundSponsorForApp(address(counter)); + // fundSponsorForApp(address(counter)); // // (3). Create whitelist app user op // UserOperation[] memory userOps = new UserOperation[](1);