From 376f7eacee9cece37be2daa27963172e90b9b885 Mon Sep 17 00:00:00 2001 From: leekt Date: Tue, 30 Jan 2024 02:04:33 +0900 Subject: [PATCH 1/5] updated dependencies --- lib/FreshCryptoLib | 2 +- lib/forge-std | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/FreshCryptoLib b/lib/FreshCryptoLib index 1a484cdf..d9bb3b0f 160000 --- a/lib/FreshCryptoLib +++ b/lib/FreshCryptoLib @@ -1 +1 @@ -Subproject commit 1a484cdfd810046203f37b3e3c794d05720fb99c +Subproject commit d9bb3b0fc6b737af2c70dab246cabbc7d05afc3c diff --git a/lib/forge-std b/lib/forge-std index 2f112697..4513bc20 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 2f112697506eab12d433a65fdc31a639548fe365 +Subproject commit 4513bc2063f23c57bee6558799584b518d387a39 From f093d4496f8af76359dce9d061f935934752984a Mon Sep 17 00:00:00 2001 From: leekt Date: Tue, 30 Jan 2024 02:05:03 +0900 Subject: [PATCH 2/5] updates solady to latest --- .gitmodules | 6 +++--- lib/solady | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.gitmodules b/.gitmodules index df6cfb84..ff8d8f4c 100644 --- a/.gitmodules +++ b/.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/FreshCryptoLib"] path = lib/FreshCryptoLib url = https://github.com/rdubois-crypto/FreshCryptoLib +[submodule "lib/solady"] + path = lib/solady + url = https://github.com/vectorized/solady diff --git a/lib/solady b/lib/solady index cb801a60..a6a95729 160000 --- a/lib/solady +++ b/lib/solady @@ -1 +1 @@ -Subproject commit cb801a60f8319a148697b09d19b748d04e3d65c4 +Subproject commit a6a95729f947bb2a24e05e862ba9522c10453a70 From dc57a11a2dfa2eb16fea6ecc12d9b3a17169a3a2 Mon Sep 17 00:00:00 2001 From: leekt Date: Tue, 30 Jan 2024 02:05:34 +0900 Subject: [PATCH 3/5] weighted ecdsa to valdiate the userOp for the last sig --- src/validator/WeightedECDSAValidator.sol | 34 ++-- .../validator/WeightedECDSAValidator.t.sol | 158 ++++++++++++++++++ 2 files changed, 180 insertions(+), 12 deletions(-) create mode 100644 test/foundry/validator/WeightedECDSAValidator.t.sol diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index 5c61d4a5..e0f58357 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -165,7 +165,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 +186,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 +206,28 @@ 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; + address signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(userOpHash), userOp.signature); + if (guardian[signer][msg.sender].weight != 0) { + proposal.status = ProposalStatus.Executed; + return packValidationData(ValidAfter.wrap(0), ValidUntil.wrap(0)); + } } + return SIG_VALIDATION_FAILED; } function getApproval(address kernel, bytes32 hash) public view returns (uint256 approvals, bool passed) { @@ -251,7 +261,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { uint256 totalWeight = 0; address signer; for (uint256 i = 0; i < sigCount; i++) { - signer = ECDSA.recover(hash, signature[i * 65:(i + 1) * 65]); + signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(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)); diff --git a/test/foundry/validator/WeightedECDSAValidator.t.sol b/test/foundry/validator/WeightedECDSAValidator.t.sol new file mode 100644 index 00000000..2a81a6f3 --- /dev/null +++ b/test/foundry/validator/WeightedECDSAValidator.t.sol @@ -0,0 +1,158 @@ +pragma solidity ^0.8.0; + +import {IEntryPoint} from "I4337/interfaces/IEntryPoint.sol"; +import "src/Kernel.sol"; +import "src/validator/WeightedECDSAValidator.sol"; +// test artifacts +// test utils +import "forge-std/Test.sol"; +import {ERC4337Utils} from "src/utils/ERC4337Utils.sol"; +import {KernelTestBase} from "src/utils/KernelTestBase.sol"; +import {TestExecutor} from "src/mock/TestExecutor.sol"; +import {TestValidator} from "src/mock/TestValidator.sol"; +import {IKernel} from "src/interfaces/IKernel.sol"; + +using ERC4337Utils for IEntryPoint; + +contract KernelWeightedECDSATest is KernelTestBase { + address[] public owners; + uint256[] public ownerKeys; + uint24[] public weights; + uint24 public threshold; + uint48 public delay; + + function setUp() public virtual { + _initialize(); + defaultValidator = new WeightedECDSAValidator(); + owners = new address[](3); + ownerKeys = new uint256[](3); + (owners[0], ownerKeys[0]) = makeAddrAndKey("owner0"); + (owners[1], ownerKeys[1]) = makeAddrAndKey("owner1"); + (owners[2], ownerKeys[2]) = makeAddrAndKey("owner2"); + weights = [uint24(1), uint24(2), uint24(3)]; + threshold = 3; + delay = 0; + _setAddress(); + _setExecutionDetail(); + } + + function test_ignore() external {} + + function _setExecutionDetail() internal virtual override { + executionDetail.executor = address(new TestExecutor()); + executionSig = TestExecutor.doNothing.selector; + executionDetail.validator = new TestValidator(); + } + + function getEnableData() internal view virtual override returns (bytes memory) { + return ""; + } + + function getValidatorSignature(UserOperation memory) internal view virtual override returns (bytes memory) { + return ""; + } + + function getOwners() internal view override returns (address[] memory) { + return owners; + } + + function getInitializeData() internal view override returns (bytes memory) { + bytes memory data = abi.encode(owners, weights, threshold, delay); + return abi.encodeWithSelector(KernelStorage.initialize.selector, defaultValidator, data); + } + + function test_external_call_execute_success() external override { + vm.skip(true); + } + + function test_external_call_default() external override { + vm.skip(true); + } + + function test_external_call_execute_delegatecall_success() external override { + vm.skip(true); + } + + function test_external_call_batch_execute_success() external override { + vm.skip(true); + } + + function signUserOp(UserOperation memory op) internal view override returns (bytes memory) { + bytes32 calldataAndNonceHash = keccak256(abi.encode(op.sender, op.callData, op.nonce)); + + bytes32 digest = keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256("WeightedECDSAValidator"), + keccak256("0.0.2"), + block.chainid, + address(defaultValidator) + ) + ); + + bytes32 structHash = + keccak256(abi.encode(keccak256("Approve(bytes32 callDataAndNonceHash)"), calldataAndNonceHash)); + assembly { + // Compute the digest. + mstore(0x00, 0x1901000000000000) // Store "\x19\x01". + mstore(0x1a, digest) // Store the domain separator. + mstore(0x3a, structHash) // Store the struct hash. + digest := keccak256(0x18, 0x42) + // Restore the part of the free memory slot that was overwritten. + mstore(0x3a, 0) + } + + (uint8 v0, bytes32 r0, bytes32 s0) = vm.sign(ownerKeys[0], digest); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(ownerKeys[1], digest); + bytes memory opSig = entryPoint.signUserOpHash(vm, ownerKeys[2], op); + return abi.encodePacked(bytes4(0x00000000), r0, s0, v0, r1, s1, v1, opSig); + } + + function getWrongSignature(UserOperation memory op) internal view override returns (bytes memory) { + return abi.encodePacked(bytes4(0x00000000), entryPoint.signUserOpHash(vm, ownerKey + 1, op)); + } + + function signHash(bytes32 hash) internal view override returns (bytes memory) { + (uint8 v0, bytes32 r0, bytes32 s0) = vm.sign(ownerKeys[0], ECDSA.toEthSignedMessageHash(hash)); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(ownerKeys[1], ECDSA.toEthSignedMessageHash(hash)); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(ownerKeys[2], ECDSA.toEthSignedMessageHash(hash)); + return abi.encodePacked(r0, s0, v0, r1, s1, v1, r2, s2, v2); + } + + function getWrongSignature(bytes32 hash) internal view override returns (bytes memory) { + (uint8 v0, bytes32 r0, bytes32 s0) = vm.sign(ownerKeys[0], ECDSA.toEthSignedMessageHash(hash)); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(ownerKeys[1] + 1, ECDSA.toEthSignedMessageHash(hash)); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(ownerKeys[2] + 1, ECDSA.toEthSignedMessageHash(hash)); + return abi.encodePacked(r0, s0, v0, r1, s1, v1, r2, s2, v2); + } + + function test_default_validator_enable() external override { + //UserOperation memory op = buildUserOperation( + // abi.encodeWithSelector( + // IKernel.execute.selector, + // address(defaultValidator), + // 0, + // abi.encodeWithSelector(ECDSAValidator.enable.selector, abi.encodePacked(address(0xdeadbeef))), + // Operation.Call + // ) + //); + //performUserOperationWithSig(op); + //(address owner) = ECDSAValidator(address(defaultValidator)).ecdsaValidatorStorage(address(kernel)); + //assertEq(owner, address(0xdeadbeef), "owner should be 0xdeadbeef"); + } + + function test_default_validator_disable() external override { + //UserOperation memory op = buildUserOperation( + // abi.encodeWithSelector( + // IKernel.execute.selector, + // address(defaultValidator), + // 0, + // abi.encodeWithSelector(ECDSAValidator.disable.selector, ""), + // Operation.Call + // ) + //); + //performUserOperationWithSig(op); + //(address owner) = ECDSAValidator(address(defaultValidator)).ecdsaValidatorStorage(address(kernel)); + //assertEq(owner, address(0), "owner should be 0"); + } +} From 78bbccd742a18a36f45c7e2eabcb1b699eace1a7 Mon Sep 17 00:00:00 2001 From: leekt Date: Tue, 30 Jan 2024 22:51:19 +0900 Subject: [PATCH 4/5] removed toEthSignedMessage --- src/validator/WeightedECDSAValidator.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index e0f58357..bd2888bd 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -261,7 +261,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { uint256 totalWeight = 0; address signer; for (uint256 i = 0; i < sigCount; i++) { - signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(hash), signature[i * 65:(i + 1) * 65]); + 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)); From 806085030b2fef14d35c0766cb4c5560ea7cdd86 Mon Sep 17 00:00:00 2001 From: leekt Date: Tue, 30 Jan 2024 22:54:23 +0900 Subject: [PATCH 5/5] test done --- .../foundry/validator/WeightedECDSAValidator.t.sol | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/foundry/validator/WeightedECDSAValidator.t.sol b/test/foundry/validator/WeightedECDSAValidator.t.sol index 2a81a6f3..46797c4d 100644 --- a/test/foundry/validator/WeightedECDSAValidator.t.sol +++ b/test/foundry/validator/WeightedECDSAValidator.t.sol @@ -109,20 +109,20 @@ contract KernelWeightedECDSATest is KernelTestBase { } function getWrongSignature(UserOperation memory op) internal view override returns (bytes memory) { - return abi.encodePacked(bytes4(0x00000000), entryPoint.signUserOpHash(vm, ownerKey + 1, op)); + return abi.encodePacked(bytes4(0x00000000), entryPoint.signUserOpHash(vm, ownerKeys[0], op)); } function signHash(bytes32 hash) internal view override returns (bytes memory) { - (uint8 v0, bytes32 r0, bytes32 s0) = vm.sign(ownerKeys[0], ECDSA.toEthSignedMessageHash(hash)); - (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(ownerKeys[1], ECDSA.toEthSignedMessageHash(hash)); - (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(ownerKeys[2], ECDSA.toEthSignedMessageHash(hash)); + (uint8 v0, bytes32 r0, bytes32 s0) = vm.sign(ownerKeys[0], hash); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(ownerKeys[1], hash); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(ownerKeys[2], hash); return abi.encodePacked(r0, s0, v0, r1, s1, v1, r2, s2, v2); } function getWrongSignature(bytes32 hash) internal view override returns (bytes memory) { - (uint8 v0, bytes32 r0, bytes32 s0) = vm.sign(ownerKeys[0], ECDSA.toEthSignedMessageHash(hash)); - (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(ownerKeys[1] + 1, ECDSA.toEthSignedMessageHash(hash)); - (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(ownerKeys[2] + 1, ECDSA.toEthSignedMessageHash(hash)); + (uint8 v0, bytes32 r0, bytes32 s0) = vm.sign(ownerKeys[0], hash); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(ownerKeys[1] + 1, hash); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(ownerKeys[2] + 1, hash); return abi.encodePacked(r0, s0, v0, r1, s1, v1, r2, s2, v2); }