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/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 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 diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index 5c61d4a5..bd2888bd 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) { diff --git a/test/foundry/validator/WeightedECDSAValidator.t.sol b/test/foundry/validator/WeightedECDSAValidator.t.sol new file mode 100644 index 00000000..46797c4d --- /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, ownerKeys[0], op)); + } + + function signHash(bytes32 hash) internal view override returns (bytes memory) { + (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], 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); + } + + 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"); + } +}