change kernel sig order for validUntil and validAfter (#21)

* forge fmt

* fix: parsing typo and mode(1)

Author: leekt

gas/ecdsa/report.txt

@@ -3,17 +3,17 @@ 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: 453376)
+[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: 411711)
+[PASS] test_set_execution() (gas: 411708)
 [PASS] test_validate_signature() (gas: 163680)
-[PASS] test_validate_userOp() (gas: 1729466)
-Test result: ok. 8 passed; 0 failed; 0 skipped; finished in 2.82ms
+[PASS] test_validate_userOp() (gas: 1727059)
+Test result: ok. 8 passed; 0 failed; 0 skipped; finished in 2.90ms
 | src/Kernel.sol:Kernel contract |                 |       |        |       |         |
 |--------------------------------|-----------------|-------|--------|-------|---------|
 | Deployment Cost                | Deployment Size |       |        |       |         |
-| 1586591                        | 8333            |       |        |       |         |
+| 1584184                        | 8321            |       |        |       |         |
 | Function Name                  | min             | avg   | median | max   | # calls |
 | disableMode                    | 3765            | 3765  | 3765   | 3765  | 1       |
 | getDefaultValidator            | 341             | 341   | 341    | 341   | 1       |
@@ -22,7 +22,7 @@ Test result: ok. 8 passed; 0 failed; 0 skipped; finished in 2.82ms
 | initialize                     | 3046            | 43982 | 48253  | 50753 | 10      |
 | isValidSignature               | 6047            | 6047  | 6047   | 6047  | 1       |
 | setDefaultValidator            | 7870            | 7870  | 7870   | 7870  | 1       |
-| setExecution                   | 49877           | 49877 | 49877  | 49877 | 2       |
+| setExecution                   | 49874           | 49874 | 49874  | 49874 | 2       |
 | validateUserOp                 | 46040           | 46234 | 46256  | 46386 | 4       |
 
 

src/Kernel.sol

@@ -114,7 +114,7 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
             assembly {
                 validator := shr(80, storage_slot_1)
             }
-        } else if(mode & (storage_slot_1 << 224) != 0x00000000) {
+        } else if (mode & (storage_slot_1 << 224) != 0x00000000) {
             revert DisabledMode();
         } else if (mode == 0x00000001) {
             bytes4 sig = bytes4(userOp.callData[0:4]);
@@ -126,12 +126,12 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
                 }
             }
             op.signature = userOp.signature[4:];
-            validationData = (uint256(detail.validAfter) << 160) | (uint256(detail.validUntil) << 208);
+            validationData = (uint256(detail.validAfter) << 208) | (uint256(detail.validUntil) << 160);
         } else if (mode == 0x00000002) {
             bytes4 sig = bytes4(userOp.callData[0:4]);
             // use given validator
-            // userOp.signature[4:10] = validUntil,
-            // userOp.signature[10:16] = validAfter,
+            // userOp.signature[4:10] = validAfter,
+            // userOp.signature[10:16] = validUntil,
             // userOp.signature[16:36] = validator address,
             validator = IKernelValidator(address(bytes20(userOp.signature[16:36])));
             bytes calldata enableData;
@@ -159,7 +159,7 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
     {
         unchecked {
             uint256 cursor = 88;
-            uint256 length  = uint256(bytes32(signature[56:88])); // this is enableDataLength
+            uint256 length = uint256(bytes32(signature[56:88])); // this is enableDataLength
             assembly {
                 enableData.offset := add(signature.offset, cursor)
                 enableData.length := length
@@ -181,9 +181,7 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
                 )
             );
             validationData = _intersectValidationData(
-                getKernelStorage().defaultValidator.validateSignature(
-                    enableDigest, signature[cursor:cursor+length]
-                ),
+                getKernelStorage().defaultValidator.validateSignature(enableDigest, signature[cursor:cursor + length]),
                 uint256(bytes32(signature[4:36])) & 0xffffffffffffffffffffffff0000000000000000000000000000000000000000
             );
             assembly {
@@ -192,9 +190,9 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
                 validationSig.length := sub(signature.length, cursor)
             }
             getKernelStorage().execution[sig] = ExecutionDetail({
+                validAfter: uint48(bytes6(signature[4:10])),
+                validUntil: uint48(bytes6(signature[10:16])),
                 executor: address(bytes20(signature[36:56])),
-                validUntil: uint48(bytes6(signature[4:10])),
-                validAfter: uint48(bytes6(signature[10:16])),
                 validator: IKernelValidator(address(bytes20(signature[16:36])))
             });
         }

src/abstract/KernelStorage.sol

@@ -7,8 +7,8 @@ import "src/interfaces/IValidator.sol";
 
 // Defining a struct for execution details
 struct ExecutionDetail {
-    uint48 validUntil; // Until what time is this execution valid
-    uint48 validAfter; // After what time is this execution valid
+    uint48 validAfter; // Until what time is this execution valid
+    uint48 validUntil; // After what time is this execution valid
     address executor; // Who is the executor of this execution
     IKernelValidator validator; // The validator for this execution
 }
@@ -95,9 +95,9 @@ contract KernelStorage {
     }
 
     function getDisabledMode() public view returns (bytes4 disabled) {
-       assembly {
-          disabled := shl(224, sload(KERNEL_STORAGE_SLOT_1))
-       }
+        assembly {
+            disabled := shl(224, sload(KERNEL_STORAGE_SLOT_1))
+        }
     }
 
     function getLastDisabledTime() public view returns (uint48) {

src/factory/AdminLessERC1967Factory.sol

@@ -28,7 +28,7 @@ contract AdminLessERC1967Factory {
 
     /// @dev A proxy has been deployed.
     event Deployed(address indexed proxy, address indexed implementation);
-    
+
     /// @dev `keccak256(bytes("Deployed(address,address)"))`.
     uint256 internal constant _DEPLOYED_EVENT_SIGNATURE =
         0x09e48df7857bd0c1e0d31bb8a85d42cf1874817895f171c917f6ee2cea73ec20;
@@ -39,8 +39,7 @@ contract AdminLessERC1967Factory {
 
     /// @dev The ERC-1967 storage slot for the implementation in the proxy.
     /// `uint256(keccak256("eip1967.proxy.implementation")) - 1`.
-    uint256 internal constant _IMPLEMENTATION_SLOT =
-        0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
+    uint256 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
 
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
     /*                      DEPLOY FUNCTIONS                      */
@@ -57,32 +56,25 @@ contract AdminLessERC1967Factory {
     /// and returns its address.
     /// The value passed into this function will be forwarded to the proxy.
     /// Then, calls the proxy with abi encoded `data`.
-    function deployAndCall(address implementation, bytes calldata data)
-        internal
-        returns (address proxy)
-    {
+    function deployAndCall(address implementation, bytes calldata data) internal returns (address proxy) {
         proxy = _deploy(implementation, bytes32(0), false, data);
     }
 
     /// @dev Deploys a proxy for `implementation`, with `salt`,
     /// and returns its deterministic address.
     /// The value passed into this function will be forwarded to the proxy.
-    function deployDeterministic(address implementation, bytes32 salt)
-        internal
-        returns (address proxy)
-    {
+    function deployDeterministic(address implementation, bytes32 salt) internal returns (address proxy) {
         proxy = deployDeterministicAndCall(implementation, salt, _emptyData());
     }
 
     /// @dev Deploys a proxy for `implementation`, with `salt`,
     /// and returns its deterministic address.
     /// The value passed into this function will be forwarded to the proxy.
     /// Then, calls the proxy with abi encoded `data`.
-    function deployDeterministicAndCall(
-        address implementation,
-        bytes32 salt,
-        bytes calldata data
-    ) internal returns (address proxy) {
+    function deployDeterministicAndCall(address implementation, bytes32 salt, bytes calldata data)
+        internal
+        returns (address proxy)
+    {
         /// @solidity memory-safe-assembly
         assembly {
             // If the salt does not start with the zero address or the caller.
@@ -95,12 +87,10 @@ contract AdminLessERC1967Factory {
     }
 
     /// @dev Deploys the proxy, with optionality to deploy deterministically with a `salt`.
-    function _deploy(
-        address implementation,
-        bytes32 salt,
-        bool useSalt,
-        bytes calldata data
-    ) internal returns (address proxy) {
+    function _deploy(address implementation, bytes32 salt, bool useSalt, bytes calldata data)
+        internal
+        returns (address proxy)
+    {
         bytes memory m = _initCode();
         /// @solidity memory-safe-assembly
         assembly {

src/factory/KernelFactory.sol

@@ -8,8 +8,7 @@ import "src/Kernel.sol";
 import "src/validator/ECDSAValidator.sol";
 import "solady/auth/Ownable.sol";
 
-contract KernelFactory is AdminLessERC1967Factory, Ownable{
-
+contract KernelFactory is AdminLessERC1967Factory, Ownable {
     mapping(address => bool) public isAllowedImplementation;
 
     constructor(address _owner) {
@@ -30,11 +29,7 @@ contract KernelFactory is AdminLessERC1967Factory, Ownable{
         proxy = deployDeterministicAndCall(_implementation, salt, _data);
     }
 
-    function getAccountAddress(bytes calldata _data, uint256 _index)
-        public
-        view
-        returns (address)
-    {
+    function getAccountAddress(bytes calldata _data, uint256 _index) public view returns (address) {
         bytes32 salt = bytes32(uint256(keccak256(abi.encodePacked(_data, _index))) & type(uint96).max);
         return predictDeterministicAddress(salt);
     }

src/interfaces/IValidator.sol

@@ -9,7 +9,8 @@ interface IKernelValidator {
     function disable(bytes calldata _data) external payable;
 
     function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 missingFunds)
-        external payable
+        external
+        payable
         returns (uint256);
 
     function validateSignature(bytes32 hash, bytes calldata signature) external view returns (uint256);

src/test/TestKernel.sol

@@ -4,7 +4,7 @@ import "src/Kernel.sol";
 
 contract TestKernel is Kernel {
     constructor(IEntryPoint _entryPoint) Kernel(_entryPoint) {}
-    
+
     function sudoInitialize(IKernelValidator _defaultValidator, bytes calldata _data) external payable {
         WalletKernelStorage storage ws = getKernelStorage();
         ws.defaultValidator = _defaultValidator;

src/test/TestValidator.sol

@@ -18,7 +18,12 @@ contract TestValidator is IKernelValidator {
         return 0;
     }
 
-    function validateUserOp(UserOperation calldata, bytes32 userOpHash, uint256) external payable override returns (uint256) {
+    function validateUserOp(UserOperation calldata, bytes32 userOpHash, uint256)
+        external
+        payable
+        override
+        returns (uint256)
+    {
         emit TestValidateUserOp(userOpHash);
         return 0;
     }

src/utils/KernelHelper.sol

@@ -8,23 +8,19 @@ function _intersectValidationData(uint256 a, uint256 b) pure returns (uint256 va
         // xor(a,b) == shows only matching bits
         // and(xor(a,b), 0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff) == filters out the validAfter and validUntil bits
         // if the result is not zero, then aggregator part is not matching
-        switch iszero(and(xor(a,b), 0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff))
+        switch iszero(and(xor(a, b), 0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff))
         case 1 {
             // validAfter
-            let a_vd :=         and(0xffffffffffff000000000000ffffffffffffffffffffffffffffffffffffffff, a)
-            let b_vd :=         and(0xffffffffffff000000000000ffffffffffffffffffffffffffffffffffffffff, b)
+            let a_vd := and(0xffffffffffff000000000000ffffffffffffffffffffffffffffffffffffffff, a)
+            let b_vd := and(0xffffffffffff000000000000ffffffffffffffffffffffffffffffffffffffff, b)
             validationData := xor(a_vd, mul(xor(a_vd, b_vd), gt(b_vd, a_vd)))
             // validUntil
-            a_vd     :=         and(0x000000000000ffffffffffff0000000000000000000000000000000000000000, a)
-            b_vd     :=         and(0x000000000000ffffffffffff0000000000000000000000000000000000000000, b)
+            a_vd := and(0x000000000000ffffffffffff0000000000000000000000000000000000000000, a)
+            b_vd := and(0x000000000000ffffffffffff0000000000000000000000000000000000000000, b)
             let until := xor(a_vd, mul(xor(a_vd, b_vd), lt(b_vd, a_vd)))
-            if iszero(until) {
-                until :=            0x000000000000ffffffffffff0000000000000000000000000000000000000000
-            }
+            if iszero(until) { until := 0x000000000000ffffffffffff0000000000000000000000000000000000000000 }
             validationData := or(validationData, until)
         }
-        default {
-            validationData := SIG_VALIDATION_FAILED
-        }
+        default { validationData := SIG_VALIDATION_FAILED }
     }
 }

test/foundry/Kernel.t.sol

@@ -28,7 +28,7 @@ contract KernelTest is Test {
 
     function setUp() public {
         (owner, ownerKey) = makeAddrAndKey("owner");
-        (factoryOwner, ) = makeAddrAndKey("factoryOwner");
+        (factoryOwner,) = makeAddrAndKey("factoryOwner");
         entryPoint = new EntryPoint();
         kernelImpl = new Kernel(entryPoint);
         factory = new KernelFactory(factoryOwner);
@@ -38,7 +38,17 @@ contract KernelTest is Test {
 
         validator = new ECDSAValidator();
 
-        kernel = Kernel(payable(address(factory.createAccount(address(kernelImpl), abi.encodeWithSelector(KernelStorage.initialize.selector, validator, abi.encodePacked(owner)), 0))));
+        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")));
     }
@@ -55,7 +65,17 @@ contract KernelTest is Test {
     }
 
     function test_validate_signature() external {
-        Kernel kernel2 = Kernel(payable(address(factory.createAccount(address(kernelImpl), abi.encodeWithSelector(KernelStorage.initialize.selector, validator, abi.encodePacked(owner)), 1))));
+        Kernel kernel2 = Kernel(
+            payable(
+                address(
+                    factory.createAccount(
+                        address(kernelImpl),
+                        abi.encodeWithSelector(KernelStorage.initialize.selector, validator, abi.encodePacked(owner)),
+                        1
+                    )
+                )
+            )
+        );
         bytes32 hash = keccak256(abi.encodePacked("hello world"));
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, hash);
         assertEq(kernel2.isValidSignature(hash, abi.encodePacked(r, s, v)), Kernel.isValidSignature.selector);
@@ -66,8 +86,7 @@ contract KernelTest is Test {
         kernel2.sudoInitialize(validator, abi.encodePacked(owner));
 
         UserOperation memory op = entryPoint.fillUserOp(
-            address(kernel),
-            abi.encodeWithSelector(Kernel.execute.selector, address(0), 0, bytes(""))
+            address(kernel), abi.encodeWithSelector(Kernel.execute.selector, address(0), 0, bytes(""))
         );
         op.signature = abi.encodePacked(bytes4(0x00000000), entryPoint.signUserOpHash(vm, ownerKey, op));
         bytes32 hash = entryPoint.getUserOpHash(op);