diff --git a/gas/ecdsa/report.txt b/gas/ecdsa/report.txt index 61d06f51..f8521e58 100644 --- a/gas/ecdsa/report.txt +++ b/gas/ecdsa/report.txt @@ -1,19 +1,19 @@ No files changed, compilation skipped Running 8 tests for test/foundry/Kernel.t.sol:KernelTest -[PASS] test_disable_mode() (gas: 162901) -[PASS] test_external_call_default() (gas: 28867) -[PASS] test_external_call_execution() (gas: 453373) -[PASS] test_initialize_twice() (gas: 20885) -[PASS] test_set_default_validator() (gas: 361374) -[PASS] test_set_execution() (gas: 411708) -[PASS] test_validate_signature() (gas: 163680) -[PASS] test_validate_userOp() (gas: 1727059) -Test result: ok. 8 passed; 0 failed; 0 skipped; finished in 2.90ms +[PASS] test_disable_mode() (gas: 162839) +[PASS] test_external_call_default() (gas: 28889) +[PASS] test_external_call_execution() (gas: 453341) +[PASS] test_initialize_twice() (gas: 20907) +[PASS] test_set_default_validator() (gas: 361312) +[PASS] test_set_execution() (gas: 411646) +[PASS] test_validate_signature() (gas: 163702) +[PASS] test_validate_userOp() (gas: 1727980) +Test result: ok. 8 passed; 0 failed; 0 skipped; finished in 2.81ms | src/Kernel.sol:Kernel contract | | | | | | |--------------------------------|-----------------|-------|--------|-------|---------| | Deployment Cost | Deployment Size | | | | | -| 1584184 | 8321 | | | | | +| 1585184 | 8326 | | | | | | Function Name | min | avg | median | max | # calls | | disableMode | 3765 | 3765 | 3765 | 3765 | 1 | | getDefaultValidator | 341 | 341 | 341 | 341 | 1 | diff --git a/src/test/TestERC20.sol b/src/test/TestERC20.sol new file mode 100644 index 00000000..d45c78bd --- /dev/null +++ b/src/test/TestERC20.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import "solady/tokens/ERC20.sol"; + +contract TestERC20 is ERC20 { + constructor() ERC20() {} + + function name() public pure override returns (string memory) { + return "TestERC20"; + } + + function symbol() public pure override returns (string memory) { + return "TST"; + } + + function mint(address _to, uint256 _amount) external { + _mint(_to, _amount); + } +} diff --git a/src/test/TestERC721.sol b/src/test/TestERC721.sol index fffe626e..7ebceb04 100644 --- a/src/test/TestERC721.sol +++ b/src/test/TestERC721.sol @@ -1,10 +1,22 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "openzeppelin-contracts/contracts/token/ERC721/ERC721.sol"; +import "solady/tokens/ERC721.sol"; contract TestERC721 is ERC721 { - constructor() ERC721("TestERC721", "TEST") {} + constructor() ERC721() {} + + function name() public pure override returns (string memory) { + return "TestERC721"; + } + + function symbol() public pure override returns (string memory) { + return "TEST"; + } + + function tokenURI(uint256 _id) public pure override returns (string memory) { + return ""; + } function mint(address _to, uint256 _id) external { _mint(_to, _id); diff --git a/src/validator/SessionKeyValidator.sol b/src/validator/SessionKeyValidator.sol index de37b795..2f371ea5 100644 --- a/src/validator/SessionKeyValidator.sol +++ b/src/validator/SessionKeyValidator.sol @@ -30,10 +30,11 @@ contract ExecuteSessionKeyValidator is IKernelValidator { // TODO : gas spending limit struct SessionData { - bool enabled; - uint48 validUntil; - uint48 validAfter; bytes32 merkleRoot; + uint48 validAfter; + uint48 validUntil; + address paymaster; + bool enabled; } mapping(address sessionKey => mapping(address kernel => SessionData)) public sessionData; @@ -41,10 +42,11 @@ contract ExecuteSessionKeyValidator is IKernelValidator { function enable(bytes calldata _data) external payable { address sessionKey = address(bytes20(_data[0:20])); bytes32 merkleRoot = bytes32(_data[20:52]); - uint48 validUntil = uint48(bytes6(_data[52:58])); - uint48 validAfter = uint48(bytes6(_data[58:64])); + uint48 validAfter = uint48(bytes6(_data[52:58])); + uint48 validUntil = uint48(bytes6(_data[58:64])); + address paymaster = address(bytes20(_data[64:84])); - sessionData[sessionKey][msg.sender] = SessionData(true, validUntil, validAfter, merkleRoot); + sessionData[sessionKey][msg.sender] = SessionData(merkleRoot, validAfter, validUntil, paymaster, true); } function disable(bytes calldata _data) external payable { @@ -58,27 +60,35 @@ contract ExecuteSessionKeyValidator is IKernelValidator { payable returns (uint256) { - // userOp.signature = signature + permission + merkleProof - bytes calldata signature = userOp.signature[0:65]; // this may be problematic with stackup - address sessionKey = ECDSA.recover(userOpHash, signature); + // userOp.signature = signer + signature + permission + merkleProof + address sessionKey = address(bytes20(userOp.signature[0:20])); + bytes calldata signature = userOp.signature[20:85]; // this may be problematic with stackup SessionData storage session = sessionData[sessionKey][msg.sender]; require(session.enabled, "SessionKeyValidator: session key not enabled"); if (session.merkleRoot == bytes32(0)) { // sessionKey allowed to execute any tx return _packValidationData(false, session.validUntil, session.validAfter); } + if (session.paymaster == address(1)) { + require(userOp.paymasterAndData.length != 0, "SessionKeyValidator: paymaster not set"); + } else if (session.paymaster != address(0)) { + require( + address(bytes20(userOp.paymasterAndData[0:20])) == session.paymaster, + "SessionKeyValidator: paymaster mismatch" + ); + } (Permission memory permission, bytes32[] memory merkleProof) = - abi.decode(userOp.signature[65:], (Permission, bytes32[])); - address target = address(bytes20(userOp.callData[16:36])); - require(target == permission.target, "SessionKeyValidator: target mismatch"); - uint256 value = uint256(bytes32(userOp.callData[36:68])); - require(value <= permission.valueLimit, "SessionKeyValidator: value limit exceeded"); - uint256 dataOffset = uint256(bytes32(userOp.callData[68:100])); + abi.decode(userOp.signature[85:], (Permission, bytes32[])); + require(address(bytes20(userOp.callData[16:36])) == permission.target, "SessionKeyValidator: target mismatch"); + require( + uint256(bytes32(userOp.callData[36:68])) <= permission.valueLimit, + "SessionKeyValidator: value limit exceeded" + ); + uint256 dataOffset = uint256(bytes32(userOp.callData[68:100])) + 4; // adding 4 for msg.sig uint256 dataLength = uint256(bytes32(userOp.callData[dataOffset:dataOffset + 32])); bytes calldata data = userOp.callData[dataOffset + 32:dataOffset + 32 + dataLength]; - bytes4 sig = bytes4(data[0:4]); - require(sig == permission.sig, "SessionKeyValidator: sig mismatch"); + require(bytes4(data[0:4]) == permission.sig, "SessionKeyValidator: sig mismatch"); for (uint256 i = 0; i < permission.rules.length; i++) { ParamRule memory rule = permission.rules[i]; bytes32 param = bytes32(data[4 + rule.index * 32:4 + rule.index * 32 + 32]); @@ -96,9 +106,9 @@ contract ExecuteSessionKeyValidator is IKernelValidator { require(param != rule.param, "SessionKeyValidator: param mismatch"); } } - bytes32 leaf = keccak256(abi.encodePacked(target, value, data)); - bool result = MerkleProofLib.verify(merkleProof, session.merkleRoot, leaf); - return _packValidationData(result, session.validUntil, session.validAfter); + bool result = MerkleProofLib.verify(merkleProof, session.merkleRoot, keccak256(abi.encode(permission))) + && (sessionKey == ECDSA.recover(ECDSA.toEthSignedMessageHash(userOpHash), signature)); + return _packValidationData(!result, session.validUntil, session.validAfter); } function validCaller(address caller, bytes calldata _data) external view returns (bool) { diff --git a/test/foundry/ERC4337Utils.sol b/test/foundry/ERC4337Utils.sol deleted file mode 100644 index 598b466f..00000000 --- a/test/foundry/ERC4337Utils.sol +++ /dev/null @@ -1,33 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -import "account-abstraction/core/EntryPoint.sol"; -import "forge-std/Test.sol"; -import "openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol"; - -library ERC4337Utils { - function fillUserOp(EntryPoint _entryPoint, address _sender, bytes memory _data) - internal - view - returns (UserOperation memory op) - { - op.sender = _sender; - op.nonce = _entryPoint.getNonce(_sender, 0); - op.callData = _data; - op.callGasLimit = 10000000; - op.verificationGasLimit = 10000000; - op.preVerificationGas = 50000; - op.maxFeePerGas = 50000; - op.maxPriorityFeePerGas = 1; - } - - function signUserOpHash(EntryPoint _entryPoint, Vm _vm, uint256 _key, UserOperation memory _op) - internal - view - returns (bytes memory signature) - { - bytes32 hash = _entryPoint.getUserOpHash(_op); - (uint8 v, bytes32 r, bytes32 s) = _vm.sign(_key, ECDSA.toEthSignedMessageHash(hash)); - signature = abi.encodePacked(r, s, v); - } -} diff --git a/test/foundry/Kernel.t.sol b/test/foundry/Kernel.t.sol index 8bbd3c79..26a73f1b 100644 --- a/test/foundry/Kernel.t.sol +++ b/test/foundry/Kernel.t.sol @@ -11,21 +11,11 @@ import "src/test/TestERC721.sol"; import "src/test/TestKernel.sol"; // test utils import "forge-std/Test.sol"; -import {ERC4337Utils} from "./ERC4337Utils.sol"; +import {ERC4337Utils, KernelTestBase} from "./utils/ERC4337Utils.sol"; using ERC4337Utils for EntryPoint; -contract KernelTest is Test { - Kernel kernel; - Kernel kernelImpl; - KernelFactory factory; - EntryPoint entryPoint; - ECDSAValidator validator; - address owner; - uint256 ownerKey; - address payable beneficiary; - address factoryOwner; - +contract KernelTest is KernelTestBase { function setUp() public { (owner, ownerKey) = makeAddrAndKey("owner"); (factoryOwner,) = makeAddrAndKey("factoryOwner"); diff --git a/test/foundry/KernelExecution.t.sol b/test/foundry/KernelExecution.t.sol index 8e469e28..b97e60eb 100644 --- a/test/foundry/KernelExecution.t.sol +++ b/test/foundry/KernelExecution.t.sol @@ -11,24 +11,14 @@ import "src/test/TestExecutor.sol"; import "src/test/TestERC721.sol"; // test utils import "forge-std/Test.sol"; -import {ERC4337Utils} from "./ERC4337Utils.sol"; +import "./utils/ERC4337Utils.sol"; // test actions/validators import "src/validator/ERC165SessionKeyValidator.sol"; import "src/executor/TokenActions.sol"; using ERC4337Utils for EntryPoint; -contract KernelExecutionTest is Test { - Kernel kernel; - Kernel kernelImpl; - KernelFactory factory; - EntryPoint entryPoint; - ECDSAValidator validator; - address owner; - uint256 ownerKey; - address payable beneficiary; - address factoryOwner; - +contract KernelExecutionTest is KernelTestBase { function setUp() public { (owner, ownerKey) = makeAddrAndKey("owner"); (factoryOwner,) = makeAddrAndKey("factoryOwner"); @@ -225,71 +215,4 @@ contract KernelExecutionTest is Test { assertEq(erc721.ownerOf(0), address(0xdead)); } - - function logGas(UserOperation memory op) internal returns (uint256 used) { - try this.consoleGasUsage(op) { - revert("should revert"); - } catch Error(string memory reason) { - used = abi.decode(bytes(reason), (uint256)); - console.log("validation gas usage :", used); - } - } - - function consoleGasUsage(UserOperation memory op) external { - uint256 gas = gasleft(); - vm.startPrank(address(entryPoint)); - kernel.validateUserOp(op, entryPoint.getUserOpHash(op), 0); - vm.stopPrank(); - revert(string(abi.encodePacked(gas - gasleft()))); - } -} - -// computes the hash of a permit -function getStructHash( - bytes4 sig, - uint48 validAfter, - uint48 validUntil, - address validator, - address executor, - bytes memory enableData -) pure returns (bytes32) { - return keccak256( - abi.encode( - keccak256("ValidatorApproved(bytes4 sig,uint256 validatorData,address executor,bytes enableData)"), - bytes4(sig), - uint256(uint256(uint160(validator)) | (uint256(validAfter) << 208) | (uint256(validUntil) << 160)), - executor, - keccak256(enableData) - ) - ); -} - -// computes the hash of the fully encoded EIP-712 message for the domain, which can be used to recover the signer -function getTypedDataHash( - address sender, - bytes4 sig, - uint48 validUntil, - uint48 validAfter, - address validator, - address executor, - bytes memory enableData -) view returns (bytes32) { - return keccak256( - abi.encodePacked( - "\x19\x01", - _buildDomainSeparator("Kernel", "0.2.1", sender), - getStructHash(sig, validAfter, validUntil, validator, executor, enableData) - ) - ); -} - -function _buildDomainSeparator(string memory name, string memory version, address verifyingContract) - view - returns (bytes32) -{ - bytes32 hashedName = keccak256(bytes(name)); - bytes32 hashedVersion = keccak256(bytes(version)); - bytes32 typeHash = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); - - return keccak256(abi.encode(typeHash, hashedName, hashedVersion, block.chainid, address(verifyingContract))); } diff --git a/test/foundry/KernelMultiOwned.t.sol b/test/foundry/KernelMultiOwned.t.sol deleted file mode 100644 index 8b137891..00000000 --- a/test/foundry/KernelMultiOwned.t.sol +++ /dev/null @@ -1 +0,0 @@ - diff --git a/test/foundry/utils/ERC4337Utils.sol b/test/foundry/utils/ERC4337Utils.sol new file mode 100644 index 00000000..50b6c1af --- /dev/null +++ b/test/foundry/utils/ERC4337Utils.sol @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import "account-abstraction/core/EntryPoint.sol"; +import "forge-std/Test.sol"; +import "solady/utils/ECDSA.sol"; +import "src/Kernel.sol"; +import "src/factory/KernelFactory.sol"; +import "src/validator/ECDSAValidator.sol"; + +abstract contract KernelTestBase is Test { + Kernel kernel; + Kernel kernelImpl; + KernelFactory factory; + EntryPoint entryPoint; + ECDSAValidator validator; + address owner; + uint256 ownerKey; + address payable beneficiary; + address factoryOwner; + + function logGas(UserOperation memory op) internal returns (uint256 used) { + try this.consoleGasUsage(op) { + revert("should revert"); + } catch Error(string memory reason) { + used = abi.decode(bytes(reason), (uint256)); + console.log("validation gas usage :", used); + } + } + + function consoleGasUsage(UserOperation memory op) external { + uint256 gas = gasleft(); + vm.startPrank(address(entryPoint)); + kernel.validateUserOp(op, entryPoint.getUserOpHash(op), 0); + vm.stopPrank(); + revert(string(abi.encodePacked(gas - gasleft()))); + } +} + +library ERC4337Utils { + function fillUserOp(EntryPoint _entryPoint, address _sender, bytes memory _data) + internal + view + returns (UserOperation memory op) + { + op.sender = _sender; + op.nonce = _entryPoint.getNonce(_sender, 0); + op.callData = _data; + op.callGasLimit = 10000000; + op.verificationGasLimit = 10000000; + op.preVerificationGas = 50000; + op.maxFeePerGas = 50000; + op.maxPriorityFeePerGas = 1; + } + + function signUserOpHash(EntryPoint _entryPoint, Vm _vm, uint256 _key, UserOperation memory _op) + internal + view + returns (bytes memory signature) + { + bytes32 hash = _entryPoint.getUserOpHash(_op); + (uint8 v, bytes32 r, bytes32 s) = _vm.sign(_key, ECDSA.toEthSignedMessageHash(hash)); + signature = abi.encodePacked(r, s, v); + } +} + +// computes the hash of a permit +function getStructHash( + bytes4 sig, + uint48 validUntil, + uint48 validAfter, + address validator, + address executor, + bytes memory enableData +) pure returns (bytes32) { + return keccak256( + abi.encode( + keccak256("ValidatorApproved(bytes4 sig,uint256 validatorData,address executor,bytes enableData)"), + bytes4(sig), + uint256(uint256(uint160(validator)) | (uint256(validAfter) << 160) | (uint256(validUntil) << (48 + 160))), + executor, + keccak256(enableData) + ) + ); +} + +// computes the hash of the fully encoded EIP-712 message for the domain, which can be used to recover the signer +function getTypedDataHash( + address sender, + bytes4 sig, + uint48 validUntil, + uint48 validAfter, + address validator, + address executor, + bytes memory enableData +) view returns (bytes32) { + return keccak256( + abi.encodePacked( + "\x19\x01", + _buildDomainSeparator("Kernel", "0.2.1", sender), + getStructHash(sig, validUntil, validAfter, validator, executor, enableData) + ) + ); +} + +function _buildDomainSeparator(string memory name, string memory version, address verifyingContract) + view + returns (bytes32) +{ + bytes32 hashedName = keccak256(bytes(name)); + bytes32 hashedVersion = keccak256(bytes(version)); + bytes32 typeHash = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + + return keccak256(abi.encode(typeHash, hashedName, hashedVersion, block.chainid, address(verifyingContract))); +} diff --git a/test/foundry/validator/SessionKeyValidator.t.sol b/test/foundry/validator/SessionKeyValidator.t.sol new file mode 100644 index 00000000..6990d30c --- /dev/null +++ b/test/foundry/validator/SessionKeyValidator.t.sol @@ -0,0 +1,216 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import "src/factory/AdminLessERC1967Factory.sol"; +import "src/Kernel.sol"; +import "src/validator/ECDSAValidator.sol"; +import "src/factory/KernelFactory.sol"; +// test artifacts +import "src/test/TestValidator.sol"; +import "src/test/TestExecutor.sol"; +import "src/test/TestERC721.sol"; +import "src/test/TestERC20.sol"; +// test utils +import "forge-std/Test.sol"; +import "test/foundry/utils/ERC4337Utils.sol"; +// test actions/validators +import "src/validator/SessionKeyValidator.sol"; + +using ERC4337Utils for EntryPoint; + +contract SessionKeyValidatorTest is KernelTestBase { + ExecuteSessionKeyValidator sessionKeyValidator; + TestERC20 testToken; + address sessionKey; + uint256 sessionKeyPriv; + + function setUp() public { + (owner, ownerKey) = makeAddrAndKey("owner"); + (factoryOwner,) = makeAddrAndKey("factoryOwner"); + (sessionKey, sessionKeyPriv) = makeAddrAndKey("sessionKey"); + entryPoint = new EntryPoint(); + kernelImpl = new Kernel(entryPoint); + factory = new KernelFactory(factoryOwner); + vm.startPrank(factoryOwner); + factory.setImplementation(address(kernelImpl), true); + vm.stopPrank(); + + validator = new ECDSAValidator(); + + kernel = Kernel( + payable( + address( + factory.createAccount( + address(kernelImpl), + abi.encodeWithSelector(KernelStorage.initialize.selector, validator, abi.encodePacked(owner)), + 0 + ) + ) + ) + ); + vm.deal(address(kernel), 1e30); + beneficiary = payable(address(makeAddr("beneficiary"))); + testToken = new TestERC20(); + sessionKeyValidator = new ExecuteSessionKeyValidator(); + } + + function test_mode_2_no_paymaster() external { + testToken.mint(address(kernel), 100e18); + TestERC20 testToken2 = new TestERC20(); + UserOperation memory op = entryPoint.fillUserOp( + address(kernel), + abi.encodeWithSelector( + Kernel.execute.selector, + address(testToken), + 0, + abi.encodeWithSelector(ERC20.transfer.selector, beneficiary, 100), + Operation.Call + ) + ); + + ExecuteSessionKeyValidator.ParamRule[] memory rules = new ExecuteSessionKeyValidator.ParamRule[](1); + rules[0] = ExecuteSessionKeyValidator.ParamRule({ + index: 1, + condition: ExecuteSessionKeyValidator.ParamCondition.LESS_THAN_OR_EQUAL, + param: bytes32(uint256(1e18)) + }); + + bytes32[] memory data = new bytes32[](2); + data[0] = keccak256( + abi.encode( + ExecuteSessionKeyValidator.Permission({ + valueLimit: 0, + target: address(testToken), + sig: ERC20.transfer.selector, + rules: rules + }) + ) + ); + + data[1] = keccak256( + abi.encode( + ExecuteSessionKeyValidator.Permission({ + valueLimit: 0, + target: address(testToken2), + sig: ERC20.transfer.selector, + rules: rules + }) + ) + ); + + bytes32 merkleRoot = _getRoot(data); + bytes memory enableData = abi.encodePacked(sessionKey, merkleRoot, uint48(0), uint48(0), address(0)); + bytes32 digest = getTypedDataHash( + address(kernel), Kernel.execute.selector, 0, 0, address(sessionKeyValidator), address(0), enableData + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, digest); + + op.signature = abi.encodePacked( + bytes4(0x00000002), + uint48(0), + uint48(0), + address(sessionKeyValidator), + address(0), + uint256(enableData.length), + enableData, + uint256(65), + r, + s, + v + ); + op.signature = bytes.concat( + op.signature, + abi.encodePacked( + sessionKey, + entryPoint.signUserOpHash(vm, sessionKeyPriv, op), + abi.encode( + ExecuteSessionKeyValidator.Permission({ + valueLimit: 0, + target: address(testToken), + sig: ERC20.transfer.selector, + rules: rules + }), + _getProof(data, 0) + ) + ) + ); + UserOperation[] memory ops = new UserOperation[](1); + ops[0] = op; + logGas(op); + + entryPoint.handleOps(ops, beneficiary); + } +} +// Following code is adapted from https://github.com/dmfxyz/murky/blob/main/src/common/MurkyBase.sol. + +function _getRoot(bytes32[] memory data) pure returns (bytes32) { + require(data.length > 1); + while (data.length > 1) { + data = _hashLevel(data); + } + return data[0]; +} + +function _getProof(bytes32[] memory data, uint256 nodeIndex) pure returns (bytes32[] memory) { + require(data.length > 1); + + bytes32[] memory result = new bytes32[](64); + uint256 pos; + + while (data.length > 1) { + unchecked { + if (nodeIndex & 0x1 == 1) { + result[pos] = data[nodeIndex - 1]; + } else if (nodeIndex + 1 == data.length) { + result[pos] = bytes32(0); + } else { + result[pos] = data[nodeIndex + 1]; + } + ++pos; + nodeIndex /= 2; + } + data = _hashLevel(data); + } + // Resize the length of the array to fit. + /// @solidity memory-safe-assembly + assembly { + mstore(result, pos) + } + + return result; +} + +function _hashLevel(bytes32[] memory data) pure returns (bytes32[] memory) { + bytes32[] memory result; + unchecked { + uint256 length = data.length; + if (length & 0x1 == 1) { + result = new bytes32[](length / 2 + 1); + result[result.length - 1] = _hashPair(data[length - 1], bytes32(0)); + } else { + result = new bytes32[](length / 2); + } + uint256 pos = 0; + for (uint256 i = 0; i < length - 1; i += 2) { + result[pos] = _hashPair(data[i], data[i + 1]); + ++pos; + } + } + return result; +} + +function _hashPair(bytes32 left, bytes32 right) pure returns (bytes32 result) { + /// @solidity memory-safe-assembly + assembly { + switch lt(left, right) + case 0 { + mstore(0x0, right) + mstore(0x20, left) + } + default { + mstore(0x0, left) + mstore(0x20, right) + } + result := keccak256(0x0, 0x40) + } +}