diff --git a/src/Kernel.sol b/src/Kernel.sol index 74f2425b..25a94b18 100644 --- a/src/Kernel.sol +++ b/src/Kernel.sol @@ -29,7 +29,7 @@ contract Kernel is EIP712, Compatibility, KernelStorage { /// @dev Selector of the `DisabledMode()` error, to be used in assembly, 'bytes4(keccak256(bytes("DisabledMode()")))', same as DisabledMode.selector() uint256 private constant _DISABLED_MODE_SELECTOR = 0xfc2f51c5; - /// @dev Current kernel name and version, todo: Need to expose getter for this variables? + /// @dev Current kernel name and version, todo: Need to expose getter for this variables? string public constant name = KERNEL_NAME; string public constant version = KERNEL_VERSION; diff --git a/src/validator/P256Validator.sol b/src/validator/P256Validator.sol index 02f122fd..fbbe7c1a 100644 --- a/src/validator/P256Validator.sol +++ b/src/validator/P256Validator.sol @@ -9,55 +9,72 @@ import {ValidationData} from "../common/Types.sol"; import {SIG_VALIDATION_FAILED} from "../common/Constants.sol"; import {P256} from "p256-verifier/P256.sol"; +/// @title P256Validator +/// @notice This validator uses the P256 curve to validate signatures. contract P256Validator is IKernelValidator { - event P256PublicKeysChanged(address indexed kernel, PublicKey indexed oldKeys, PublicKey indexed newKeys); + /// @notice Emitted when a bad key is provided. + error BadKey(); + + /// @notice Emitted when the public key of a kernel is changed. + event P256PublicKeysChanged(address indexed kernel, PublicKey newKeys); + /// @notice The P256 public key of a kernel. struct PublicKey { uint256 x; uint256 y; } - error BadKey(); - - mapping(address => PublicKey) public p256PublicKey; + /// @notice The P256 public keys of a kernel. + mapping(address kernel => PublicKey PublicKey) public p256PublicKey; + /// @notice Enable this validator for a kernel account. + /// @dev The kernel account need to be the `msg.sender`. + /// @dev The public key is encoded as `abi.encode(PublicKey)` inside the data, so (uint256,uint256). function enable(bytes calldata _data) external payable override { PublicKey memory key = abi.decode(_data, (PublicKey)); if (key.x == 0 || key.y == 0) { revert BadKey(); } - PublicKey memory oldKey = p256PublicKey[msg.sender]; + // Update the key (so a sstore) p256PublicKey[msg.sender] = key; - emit P256PublicKeysChanged(msg.sender, oldKey, key); + // And emit the event + emit P256PublicKeysChanged(msg.sender, key); } + /// @notice Disable this validator for a kernel account. + /// @dev The kernel account need to be the `msg.sender`. function disable(bytes calldata) external payable override { delete p256PublicKey[msg.sender]; } - function validateUserOp(UserOperation calldata _userOp, bytes32 _userOpHash, uint256) external payable override returns (ValidationData validationData) { + /// @notice Validate a user operation. + function validateUserOp(UserOperation calldata _userOp, bytes32 _userOpHash, uint256) + external + payable + override + returns (ValidationData validationData) + { (uint256 r, uint256 s) = abi.decode(_userOp.signature, (uint256, uint256)); PublicKey memory key = p256PublicKey[_userOp.sender]; - bytes32 hash = ECDSA.toEthSignedMessageHash(_userOpHash); - if (P256.verifySignature(hash, r, s, key.x, key.y)) { + if (P256.verifySignature(_userOpHash, r, s, key.x, key.y)) { return ValidationData.wrap(0); - } - if (!P256.verifySignature(_userOpHash, r, s, key.x, key.y)) { - return SIG_VALIDATION_FAILED; } + return SIG_VALIDATION_FAILED; } - function validateSignature(bytes32 hash, bytes calldata signature) external view override returns (ValidationData) { + /// @notice Validate a signature. + function validateSignature(bytes32 hash, bytes calldata signature) + external + view + override + returns (ValidationData) + { (uint256 r, uint256 s) = abi.decode(signature, (uint256, uint256)); PublicKey memory key = p256PublicKey[msg.sender]; if (P256.verifySignature(hash, r, s, key.x, key.y)) { return ValidationData.wrap(0); } - bytes32 ethHash = ECDSA.toEthSignedMessageHash(hash); - if (!P256.verifySignature(ethHash, r, s, key.x, key.y)) { - return SIG_VALIDATION_FAILED; - } - return ValidationData.wrap(0); + return SIG_VALIDATION_FAILED; } function validCaller(address _caller, bytes calldata) external view override returns (bool) { diff --git a/test/foundry/KernelLiteECDSA.t.sol b/test/foundry/KernelLiteECDSA.t.sol index 3496986c..192555c3 100644 --- a/test/foundry/KernelLiteECDSA.t.sol +++ b/test/foundry/KernelLiteECDSA.t.sol @@ -97,7 +97,8 @@ contract KernelECDSATest is KernelTestBase { function test_transfer_ownership() external { address newOwner = makeAddr("new owner"); - UserOperation memory op = buildUserOperation(abi.encodeWithSelector(KernelLiteECDSA.transferOwnership.selector, newOwner)); + UserOperation memory op = + buildUserOperation(abi.encodeWithSelector(KernelLiteECDSA.transferOwnership.selector, newOwner)); vm.expectEmit(true, true, true, false, address(entryPoint)); emit UserOperationEvent(entryPoint.getUserOpHash(op), address(kernel), address(0), op.nonce, false, 0, 0); performUserOperationWithSig(op); diff --git a/test/foundry/KernelTestBase.sol b/test/foundry/KernelTestBase.sol index 1951cba4..366761c8 100644 --- a/test/foundry/KernelTestBase.sol +++ b/test/foundry/KernelTestBase.sol @@ -272,7 +272,9 @@ abstract contract KernelTestBase is Test { function test_disable_mode() external { vm.warp(1000); bytes memory empty; - UserOperation memory op = buildUserOperation(abi.encodeWithSelector(IKernel.disableMode.selector, bytes4(0x00000001), address(0), empty)); + UserOperation memory op = buildUserOperation( + abi.encodeWithSelector(IKernel.disableMode.selector, bytes4(0x00000001), address(0), empty) + ); performUserOperationWithSig(op); assertEq(uint256(bytes32(IKernel(address(kernel)).getDisabledMode())), 1 << 224); assertEq(uint256(IKernel(address(kernel)).getLastDisabledTime()), block.timestamp); @@ -336,7 +338,8 @@ abstract contract KernelTestBase is Test { function test_revert_when_mode_disabled() external { vm.warp(1000); bytes memory empty; - UserOperation memory op = buildUserOperation(abi.encodeWithSelector(IKernel.disableMode.selector, bytes4(0x00000001), address(0), empty) + UserOperation memory op = buildUserOperation( + abi.encodeWithSelector(IKernel.disableMode.selector, bytes4(0x00000001), address(0), empty) ); performUserOperationWithSig(op); diff --git a/test/foundry/validator/KillSwitchValidator.t.sol b/test/foundry/validator/KillSwitchValidator.t.sol index 6b080d64..8a3861fb 100644 --- a/test/foundry/validator/KillSwitchValidator.t.sol +++ b/test/foundry/validator/KillSwitchValidator.t.sol @@ -68,7 +68,8 @@ contract KillSwitchValidatorTest is KernelECDSATest { } function test_force_unblock() external { - UserOperation memory op = buildUserOperation(abi.encodeWithSelector(Kernel.execute.selector, owner, 0, "", Operation.Call)); + UserOperation memory op = + buildUserOperation(abi.encodeWithSelector(Kernel.execute.selector, owner, 0, "", Operation.Call)); op.signature = bytes.concat(bytes4(0), entryPoint.signUserOpHash(vm, ownerKey, op)); performUserOperation(op); diff --git a/test/foundry/validator/P256Validator.t.sol b/test/foundry/validator/P256Validator.t.sol index 25336449..6ffc012f 100644 --- a/test/foundry/validator/P256Validator.t.sol +++ b/test/foundry/validator/P256Validator.t.sol @@ -16,7 +16,6 @@ import {P256} from "p256-verifier/P256.sol"; import {FCL_ecdsa_utils} from "FreshCryptoLib/FCL_ecdsa_utils.sol"; import {IKernel} from "src/interfaces/IKernel.sol"; - using ERC4337Utils for IEntryPoint; contract P256ValidatorTest is KernelTestBase { @@ -24,9 +23,7 @@ contract P256ValidatorTest is KernelTestBase { P256Validator p256Validator; // Curve order (number of points) - uint256 constant n = - 0xFFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551; - + uint256 constant n = 0xFFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551; uint256 x; uint256 y; @@ -55,7 +52,7 @@ contract P256ValidatorTest is KernelTestBase { return abi.encodePacked(bytes4(0x00000000), abi.encode(r, s)); } - function getOwners() internal virtual override returns (address[] memory _owners){ + function getOwners() internal virtual override returns (address[] memory _owners) { _owners = new address[](1); _owners[0] = address(0); return _owners; @@ -69,7 +66,7 @@ contract P256ValidatorTest is KernelTestBase { return abi.encodeWithSelector(KernelStorage.initialize.selector, p256Validator, abi.encode(x, y)); } - function test_default_validator_enable() external override{ + function test_default_validator_enable() external override { UserOperation memory op = buildUserOperation( abi.encodeWithSelector( IKernel.execute.selector, @@ -106,7 +103,7 @@ contract P256ValidatorTest is KernelTestBase { function test_external_call_execute_success() external override { vm.skip(true); } - + function test_external_call_execute_delegatecall_success() external override { vm.skip(true); } @@ -150,7 +147,7 @@ contract P256ValidatorTest is KernelTestBase { vm.assume(privateKey != 0); (uint256 x1, uint256 y1) = generatePublicKey(privateKey); (uint256 r, uint256 s) = generateSignature(privateKey, hash); - + vm.assume(x1 != 0); vm.assume(y1 != 0); vm.assume(r != 0); @@ -161,7 +158,7 @@ contract P256ValidatorTest is KernelTestBase { function test_validate_signature() external override { Kernel kernel2 = Kernel(payable(factory.createAccount(address(kernelImpl), getInitializeData(), 3))); bytes32 hash = keccak256(abi.encodePacked("hello world")); - + bytes32 digest = keccak256( abi.encodePacked( "\x19\x01", ERC4337Utils._buildDomainSeparator(KERNEL_NAME, KERNEL_VERSION, address(kernel)), hash