Merge branch 'dev' into modular_permission

.github/workflows/test.yml

@@ -1,6 +1,12 @@
 name: test
 
-on: workflow_dispatch
+on:
+  workflow_dispatch:
+  pull_request:
+  push:
+    branches:
+      - "main"
+      - "dev"
 
 env:
   FOUNDRY_PROFILE: ci
@@ -32,3 +38,10 @@ jobs:
         run: |
           forge test -vvv
         id: test
+
+      - name: Run snapshot
+        run: NO_COLOR=1 forge snapshot --via-ir >> $GITHUB_STEP_SUMMARY
+        id: snapshot
+
+      - name: Run coverage
+        run: NO_COLOR=1 forge coverage >> $GITHUB_STEP_SUMMARY

.gitmodules

@@ -1,12 +1,12 @@
 [submodule "lib/forge-std"]
 	path = lib/forge-std
 	url = https://github.com/foundry-rs/forge-std
-[submodule "lib/solady"]
-	path = lib/solady
-	url = https://github.com/vectorized/solady
 [submodule "lib/I4337"]
 	path = lib/I4337
 	url = https://github.com/leekt/I4337
-[submodule "lib/p256-verifier"]
-	path = lib/p256-verifier
-	url = https://github.com/daimo-eth/p256-verifier
+[submodule "lib/FreshCryptoLib"]
+	path = lib/FreshCryptoLib
+	url = https://github.com/rdubois-crypto/FreshCryptoLib
+[submodule "lib/solady"]
+	path = lib/solady
+	url = https://github.com/vectorized/solady

README.md

@@ -17,7 +17,7 @@ Kernel is supported by all major AA SDKs, including:
 
 ## Resources
 
-- [Developing plugins](https://docs.zerodev.app/extend-wallets/overview)
+- [Learn more about plugins](https://docs.zerodev.app/sdk/plugins/intro)
 - [Read the source code](https://github.com/zerodevapp/kernel)
 
 ## Build

remappings.txt

@@ -1,6 +1,5 @@
 ds-test/=lib/forge-std/lib/ds-test/src/
 forge-std/=lib/forge-std/src/
 solady/=lib/solady/src/
-p256-verifier/=lib/p256-verifier/src/
-FreshCryptoLib/=lib/FreshCryptoLib/solidity/src/
 I4337/=lib/I4337/src/
+FreshCryptoLib/=lib/FreshCryptoLib/solidity/src/

script/DeployWebAuthnFclValidator.sol

@@ -0,0 +1,22 @@
+pragma solidity ^0.8.0;
+
+import "src/factory/KernelFactory.sol";
+import "src/utils/P256VerifierWrapper.sol";
+import "src/validator/webauthn//WebAuthnFclValidator.sol";
+import "forge-std/Script.sol";
+import "forge-std/console.sol";
+
+contract DeployWebAuthnFclValidator is Script {
+    function run() public {
+        uint256 key = vm.envUint("DEPLOYER_PRIVATE_KEY");
+        vm.startBroadcast(key);
+
+        P256VerifierWrapper p256VerifierWrapper = new P256VerifierWrapper{salt: 0}();
+        console.log("p256 wrapper address: %s", address(p256VerifierWrapper));
+
+        WebAuthnFclValidator validator = new WebAuthnFclValidator{salt: 0}(address(p256VerifierWrapper));
+        console.log("validator address: %s", address(validator));
+
+        vm.stopBroadcast();
+    }
+}

src/Kernel.sol

@@ -28,6 +28,8 @@ import {ValidationData, ValidAfter, ValidUntil, parseValidationData, packValidat
 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;
+    bytes32 internal constant EIP712_DOMAIN_TYPEHASH =
+        0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f;
 
     /// @dev Current kernel name and version
     string public constant name = KERNEL_NAME;
@@ -288,8 +290,7 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
         address proxyAddress = address(this);
 
         // Construct the domain separator with name, version, chainId, and proxy address.
-        bytes32 typeHash =
-            keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
+        bytes32 typeHash = EIP712_DOMAIN_TYPEHASH;
         return keccak256(abi.encode(typeHash, nameHash, versionHash, block.chainid, proxyAddress));
     }
 

src/utils/KernelTestBase.sol

@@ -15,10 +15,11 @@ import {IKernelValidator} from "../interfaces/IKernelValidator.sol";
 import {Call, ExecutionDetail} from "../common/Structs.sol";
 import {ValidationData, ValidUntil, ValidAfter} from "../common/Types.sol";
 import {KERNEL_VERSION, KERNEL_NAME} from "../common/Constants.sol";
+import {ECDSA} from "solady/utils/ECDSA.sol";
 
 import {ERC4337Utils} from "./ERC4337Utils.sol";
 import {Test} from "forge-std/Test.sol";
-import {console} from "forge-std/Console.sol";
+import {console} from "forge-std/console.sol";
 import {TestValidator} from "../mock/TestValidator.sol";
 import {TestExecutor} from "../mock/TestExecutor.sol";
 import {TestERC721} from "../mock/TestERC721.sol";
@@ -196,7 +197,8 @@ abstract contract KernelTestBase is Test {
 
     function test_validate_signature() external virtual {
         Kernel kernel2 = Kernel(payable(factory.createAccount(address(kernelImpl), getInitializeData(), 3)));
-        bytes32 hash = keccak256(abi.encodePacked("hello world"));
+        string memory message = "hello world";
+        bytes32 hash = ECDSA.toEthSignedMessageHash(bytes(message));
         bytes32 digest = keccak256(
             abi.encodePacked(
                 "\x19\x01", ERC4337Utils._buildDomainSeparator(KERNEL_NAME, KERNEL_VERSION, address(kernel)), hash

src/utils/P256VerifierWrapper.sol

@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import {FCL_ecdsa} from "FreshCryptoLib/FCL_ecdsa.sol";
+
+/// @title P256VerifierWrapper
+/// @author rdubois-crypto
+/// @author KONFeature
+/// @notice Wrapper arround the P256Verifier contract of @rdubois-crypto, using it to accept EIP-7212 compliant verification (p256 pre-compiled curve)
+/// @dev This lib is only a wrapper around the P256Verifier contract.
+///     It will call the verifySignature function of the P256Verifier contract.
+///     Once the RIP-7212 will be deployed and effective, this contract will be useless.
+///     Tracker on polygon: PR: https://github.com/maticnetwork/bor/pull/1069
+///                         Now waiting on the Napoli hardfork to be deployed
+contract P256VerifierWrapper {
+    /**
+     * Precompiles don't use a function signature. The first byte of callldata
+     * is the first byte of an input argument. In this case:
+     *
+     * input[  0: 32] = signed data hash
+     * input[ 32: 64] = signature r
+     * input[ 64: 96] = signature s
+     * input[ 96:128] = public key x
+     * input[128:160] = public key y
+     *
+     * result[ 0: 32] = 0x00..00 (invalid) or 0x00..01 (valid)
+     *
+     * For details, see https://eips.ethereum.org/EIPS/eip-7212
+     */
+    fallback(bytes calldata input) external returns (bytes memory) {
+        if (input.length != 160) {
+            return abi.encodePacked(uint256(0));
+        }
+
+        bytes32 hash = bytes32(input[0:32]);
+        uint256 r = uint256(bytes32(input[32:64]));
+        uint256 s = uint256(bytes32(input[64:96]));
+        uint256 x = uint256(bytes32(input[96:128]));
+        uint256 y = uint256(bytes32(input[128:160]));
+
+        uint256 ret = FCL_ecdsa.ecdsa_verify(hash, r, s, x, y) ? 1 : 0;
+
+        return abi.encodePacked(ret);
+    }
+}

src/validator/WeightedECDSAValidator.sol

@@ -51,34 +51,45 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator {
     event GuardianRemoved(address indexed guardian, address indexed kernel);
 
     function _domainNameAndVersion() internal pure override returns (string memory, string memory) {
-        return ("WeightedECDSAValidator", "0.0.1");
+        return ("WeightedECDSAValidator", "0.0.3");
     }
 
-    function enable(bytes calldata _data) external payable override {
-        (address[] memory _guardians, uint24[] memory _weights, uint24 _threshold, uint48 _delay) =
-            abi.decode(_data, (address[], uint24[], uint24, uint48));
+    function _addGuardians(address[] memory _guardians, uint24[] memory _weights, address _kernel) internal {
+        uint24 totalWeight = weightedStorage[_kernel].totalWeight;
         require(_guardians.length == _weights.length, "Length mismatch");
-        require(weightedStorage[msg.sender].totalWeight == 0, "Already enabled");
-        weightedStorage[msg.sender].firstGuardian = msg.sender;
+        uint160 prevGuardian = uint160(weightedStorage[_kernel].firstGuardian);
         for (uint256 i = 0; i < _guardians.length; i++) {
-            require(_guardians[i] != msg.sender, "Guardian cannot be self");
+            require(_guardians[i] != _kernel, "Guardian cannot be self");
             require(_guardians[i] != address(0), "Guardian cannot be 0");
             require(_weights[i] != 0, "Weight cannot be 0");
-            require(guardian[_guardians[i]][msg.sender].weight == 0, "Guardian already enabled");
-            guardian[_guardians[i]][msg.sender] =
-                GuardianStorage({weight: _weights[i], nextGuardian: weightedStorage[msg.sender].firstGuardian});
-            weightedStorage[msg.sender].firstGuardian = _guardians[i];
-            weightedStorage[msg.sender].totalWeight += _weights[i];
-            emit GuardianAdded(_guardians[i], msg.sender, _weights[i]);
+            require(guardian[_guardians[i]][_kernel].weight == 0, "Guardian already enabled");
+            require(uint160(_guardians[i]) < prevGuardian, "Guardians not sorted");
+            guardian[_guardians[i]][_kernel] =
+                GuardianStorage({weight: _weights[i], nextGuardian: weightedStorage[_kernel].firstGuardian});
+            weightedStorage[_kernel].firstGuardian = _guardians[i];
+            totalWeight += _weights[i];
+            prevGuardian = uint160(_guardians[i]);
+            emit GuardianAdded(_guardians[i], _kernel, _weights[i]);
         }
+        weightedStorage[_kernel].totalWeight = totalWeight;
+    }
+
+    function enable(bytes calldata _data) external payable override {
+        (address[] memory _guardians, uint24[] memory _weights, uint24 _threshold, uint48 _delay) =
+            abi.decode(_data, (address[], uint24[], uint24, uint48));
+        require(_guardians.length == _weights.length, "Length mismatch");
+        require(weightedStorage[msg.sender].totalWeight == 0, "Already enabled");
+        weightedStorage[msg.sender].firstGuardian = address(uint160(type(uint160).max));
+        _addGuardians(_guardians, _weights, msg.sender);
         weightedStorage[msg.sender].delay = _delay;
         weightedStorage[msg.sender].threshold = _threshold;
+        require(_threshold <= weightedStorage[msg.sender].totalWeight, "Threshold too high");
     }
 
     function disable(bytes calldata) external payable override {
         require(weightedStorage[msg.sender].totalWeight != 0, "Not enabled");
         address currentGuardian = weightedStorage[msg.sender].firstGuardian;
-        while (currentGuardian != msg.sender) {
+        while (currentGuardian != address(uint160(type(uint160).max))) {
             address nextGuardian = guardian[currentGuardian][msg.sender].nextGuardian;
             emit GuardianRemoved(currentGuardian, msg.sender);
             delete guardian[currentGuardian][msg.sender];
@@ -101,18 +112,8 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator {
         }
         delete weightedStorage[msg.sender];
         require(_guardians.length == _weights.length, "Length mismatch");
-        weightedStorage[msg.sender].firstGuardian = msg.sender;
-        for (uint256 i = 0; i < _guardians.length; i++) {
-            require(_guardians[i] != msg.sender, "Guardian cannot be self");
-            require(_guardians[i] != address(0), "Guardian cannot be 0");
-            require(_weights[i] != 0, "Weight cannot be 0");
-            require(guardian[_guardians[i]][msg.sender].weight == 0, "Guardian already enabled");
-            guardian[_guardians[i]][msg.sender] =
-                GuardianStorage({weight: _weights[i], nextGuardian: weightedStorage[msg.sender].firstGuardian});
-            weightedStorage[msg.sender].firstGuardian = _guardians[i];
-            weightedStorage[msg.sender].totalWeight += _weights[i];
-            emit GuardianAdded(_guardians[i], msg.sender, _weights[i]);
-        }
+        weightedStorage[msg.sender].firstGuardian = _guardians[0];
+        _addGuardians(_guardians, _weights, msg.sender);
         weightedStorage[msg.sender].delay = _delay;
         weightedStorage[msg.sender].threshold = _threshold;
     }
@@ -165,7 +166,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator {
         proposal.status = ProposalStatus.Rejected;
     }
 
-    function validateUserOp(UserOperation calldata userOp, bytes32, uint256)
+    function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256)
         external
         payable
         returns (ValidationData validationData)
@@ -186,10 +187,10 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator {
             bytes calldata sig = userOp.signature;
             // parse sig with 65 bytes
             uint256 sigCount = sig.length / 65;
+            require(sigCount > 0, "No sig");
             address signer;
             VoteStorage storage vote;
-            for (uint256 i = 0; i < sigCount && !passed; i++) {
-                // last sig is for userOpHash verification
+            for (uint256 i = 0; i < sigCount - 1 && !passed; i++) {
                 signer = ECDSA.recover(
                     _hashTypedData(
                         keccak256(abi.encode(keccak256("Approve(bytes32 callDataAndNonceHash)"), callDataAndNonceHash))
@@ -206,18 +207,33 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator {
                     passed = true;
                 }
             }
-            if (passed) {
-                validationData = packValidationData(ValidAfter.wrap(0), ValidUntil.wrap(0));
+            // userOpHash verification for the last sig
+            signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(userOpHash), sig[sig.length - 65:]);
+            vote = voteStatus[callDataAndNonceHash][signer][msg.sender];
+            if (vote.status == VoteStatus.NA) {
+                vote.status = VoteStatus.Approved;
+                totalWeight += guardian[signer][msg.sender].weight;
+                if (totalWeight >= threshold) {
+                    passed = true;
+                }
+            }
+            if (passed && guardian[signer][msg.sender].weight != 0) {
                 proposal.status = ProposalStatus.Executed;
-            } else {
-                validationData = SIG_VALIDATION_FAILED;
+                return packValidationData(ValidAfter.wrap(0), ValidUntil.wrap(0));
             }
         } else if (proposal.status == ProposalStatus.Approved || passed) {
-            validationData = packValidationData(proposal.validAfter, ValidUntil.wrap(0));
-            proposal.status = ProposalStatus.Executed;
-        } else {
-            return SIG_VALIDATION_FAILED;
+            if (userOp.paymasterAndData.length == 0 || address(bytes20(userOp.paymasterAndData[0:20])) == address(0)) {
+                address signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(userOpHash), userOp.signature);
+                if (guardian[signer][msg.sender].weight != 0) {
+                    proposal.status = ProposalStatus.Executed;
+                    return packValidationData(proposal.validAfter, ValidUntil.wrap(0));
+                }
+            } else {
+                proposal.status = ProposalStatus.Executed;
+                return packValidationData(proposal.validAfter, ValidUntil.wrap(0));
+            }
         }
+        return SIG_VALIDATION_FAILED;
     }
 
     function getApproval(address kernel, bytes32 hash) public view returns (uint256 approvals, bool passed) {
@@ -238,7 +254,29 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator {
         return false;
     }
 
-    function validateSignature(bytes32, bytes calldata) external pure returns (ValidationData) {
+    function validateSignature(bytes32 hash, bytes calldata signature) external view returns (ValidationData) {
+        WeightedECDSAValidatorStorage storage strg = weightedStorage[msg.sender];
+        if (strg.threshold == 0) {
+            return SIG_VALIDATION_FAILED;
+        }
+
+        uint256 sigCount = signature.length / 65;
+        if (sigCount == 0) {
+            return SIG_VALIDATION_FAILED;
+        }
+        uint256 totalWeight = 0;
+        address prevSigner = address(uint160(type(uint160).max));
+        for (uint256 i = 0; i < sigCount; i++) {
+            address signer = ECDSA.recover(hash, signature[i * 65:(i + 1) * 65]);
+            totalWeight += guardian[signer][msg.sender].weight;
+            if (totalWeight >= strg.threshold) {
+                return packValidationData(ValidAfter.wrap(0), ValidUntil.wrap(0));
+            }
+            if (signer >= prevSigner) {
+                return SIG_VALIDATION_FAILED;
+            }
+            prevSigner = signer;
+        }
         return SIG_VALIDATION_FAILED;
     }
 }