⚡️ Optimise gas usage when enabling p256 validator, add a few comments

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;
 

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) {

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);

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);
 

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);

test/foundry/validator/P256Validator.t.sol

@@ -16,17 +16,14 @@ 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 {
     P256Verifier p256Verifier;
     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