From 0b643ee1e94a41ee6f5da3b001dd40768c4bb27a Mon Sep 17 00:00:00 2001 From: leekt Date: Fri, 8 Dec 2023 00:59:28 +0900 Subject: [PATCH 1/8] added threshold check to make sure threshold is not zero --- src/validator/WeightedECDSAValidator.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index faa4f3bc..42c2189e 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -84,6 +84,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { function approve(bytes32 _callDataAndNonceHash, address _kernel) external { require(guardian[msg.sender][_kernel].weight != 0, "Guardian not enabled"); + require(weightedStorage[_kernel].threoshold != 0, "Kernel not enabled"); ProposalStorage storage proposal = proposalStatus[_callDataAndNonceHash][_kernel]; require(proposal.status == ProposalStatus.Ongoing, "Proposal not ongoing"); VoteStorage storage vote = voteStatus[_callDataAndNonceHash][msg.sender][_kernel]; @@ -98,6 +99,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { function approveWithSig(bytes32 _callDataAndNonceHash, address _kernel, bytes calldata sigs) external { uint256 sigCount = sigs.length / 65; + require(weightedStorage[_kernel].threoshold != 0, "Kernel not enabled"); ProposalStorage storage proposal = proposalStatus[_callDataAndNonceHash][_kernel]; require(proposal.status == ProposalStatus.Ongoing, "Proposal not ongoing"); for (uint256 i = 0; i < sigCount; i++) { @@ -135,6 +137,9 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { bytes32 callDataAndNonceHash = keccak256(abi.encode(userOp.callData, userOp.nonce)); ProposalStorage storage proposal = proposalStatus[callDataAndNonceHash][msg.sender]; WeightedECDSAValidatorStorage storage strg = weightedStorage[msg.sender]; + if(strg.threshold == 0) { + return SIG_VALIDATION_FAILED; + } if (proposal.status == ProposalStatus.Ongoing) { if (strg.delay != 0) { // if delay > 0, only allow proposal to be approved before execution From baea262ff3f010def839321f40e78189f136cd50 Mon Sep 17 00:00:00 2001 From: leekt Date: Fri, 8 Dec 2023 01:02:20 +0900 Subject: [PATCH 2/8] added userOp.sender on callDataAndNonceHash --- 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 42c2189e..6053294d 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -134,7 +134,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { payable returns (ValidationData validationData) { - bytes32 callDataAndNonceHash = keccak256(abi.encode(userOp.callData, userOp.nonce)); + bytes32 callDataAndNonceHash = keccak256(abi.encode(userOp.sender, userOp.callData, userOp.nonce)); ProposalStorage storage proposal = proposalStatus[callDataAndNonceHash][msg.sender]; WeightedECDSAValidatorStorage storage strg = weightedStorage[msg.sender]; if(strg.threshold == 0) { From c2aa68f854723cb5fe123034f83e614656ddb0b6 Mon Sep 17 00:00:00 2001 From: leekt Date: Fri, 8 Dec 2023 01:04:14 +0900 Subject: [PATCH 3/8] added userOp.sender on callDataAndNonceHash --- 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 6053294d..62b02751 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -134,7 +134,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { payable returns (ValidationData validationData) { - bytes32 callDataAndNonceHash = keccak256(abi.encode(userOp.sender, userOp.callData, userOp.nonce)); + bytes32 callDataAndNonceHash = keccak256(abi.encode(userOp.sender, userOp.callData, userOp.nonce)); // TODO: what about block.chainid and address(this)?? ProposalStorage storage proposal = proposalStatus[callDataAndNonceHash][msg.sender]; WeightedECDSAValidatorStorage storage strg = weightedStorage[msg.sender]; if(strg.threshold == 0) { From 3a1fd34678f3a5bc495e4a65aa4257c3a2f6b37f Mon Sep 17 00:00:00 2001 From: leekt Date: Fri, 8 Dec 2023 01:07:50 +0900 Subject: [PATCH 4/8] added check if guardian is msg.sender --- src/validator/WeightedECDSAValidator.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index 62b02751..8df79a4c 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -59,6 +59,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { require(weightedStorage[msg.sender].totalWeight == 0, "Already enabled"); 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"); From fd5262cb51bf9366b5434cd22d7d2fc9f7e58c9f Mon Sep 17 00:00:00 2001 From: leekt Date: Mon, 11 Dec 2023 21:25:14 +0900 Subject: [PATCH 5/8] fmt --- lib/forge-std | 2 +- lib/p256-verifier | 2 +- lib/solady | 2 +- src/validator/WeightedECDSAValidator.sol | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/forge-std b/lib/forge-std index 37a37ab7..2f112697 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 37a37ab73364d6644bfe11edf88a07880f99bd56 +Subproject commit 2f112697506eab12d433a65fdc31a639548fe365 diff --git a/lib/p256-verifier b/lib/p256-verifier index fcaa173c..29475ae3 160000 --- a/lib/p256-verifier +++ b/lib/p256-verifier @@ -1 +1 @@ -Subproject commit fcaa173c36e6a0fd55370f74f949998356869b2c +Subproject commit 29475ae300ec95d98d5c7cc34c094846f0aa2dcd diff --git a/lib/solady b/lib/solady index cde0a5fb..cb801a60 160000 --- a/lib/solady +++ b/lib/solady @@ -1 +1 @@ -Subproject commit cde0a5fb594da8655ba6bfcdc2e40a7c870c0cc0 +Subproject commit cb801a60f8319a148697b09d19b748d04e3d65c4 diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index 8df79a4c..54f5bf75 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -135,10 +135,10 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { payable returns (ValidationData validationData) { - bytes32 callDataAndNonceHash = keccak256(abi.encode(userOp.sender, userOp.callData, userOp.nonce)); // TODO: what about block.chainid and address(this)?? + bytes32 callDataAndNonceHash = keccak256(abi.encode(userOp.sender, userOp.callData, userOp.nonce)); ProposalStorage storage proposal = proposalStatus[callDataAndNonceHash][msg.sender]; WeightedECDSAValidatorStorage storage strg = weightedStorage[msg.sender]; - if(strg.threshold == 0) { + if (strg.threshold == 0) { return SIG_VALIDATION_FAILED; } if (proposal.status == ProposalStatus.Ongoing) { From fb1ee791cdb2a142a0e816cb554cb6153ddff184 Mon Sep 17 00:00:00 2001 From: leekt Date: Mon, 11 Dec 2023 21:46:05 +0900 Subject: [PATCH 6/8] ZeroDev-RW-2 : using live weights --- src/validator/WeightedECDSAValidator.sol | 32 +++++++++++++----------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index 54f5bf75..e15783c2 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -29,8 +29,8 @@ enum ProposalStatus { struct ProposalStorage { ProposalStatus status; ValidAfter validAfter; - uint24 weightApproved; } +// TODO add validUntil enum VoteStatus { NA, @@ -85,22 +85,17 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { function approve(bytes32 _callDataAndNonceHash, address _kernel) external { require(guardian[msg.sender][_kernel].weight != 0, "Guardian not enabled"); - require(weightedStorage[_kernel].threoshold != 0, "Kernel not enabled"); + require(weightedStorage[_kernel].threshold != 0, "Kernel not enabled"); ProposalStorage storage proposal = proposalStatus[_callDataAndNonceHash][_kernel]; require(proposal.status == ProposalStatus.Ongoing, "Proposal not ongoing"); VoteStorage storage vote = voteStatus[_callDataAndNonceHash][msg.sender][_kernel]; require(vote.status == VoteStatus.NA, "Already voted"); vote.status = VoteStatus.Approved; - proposal.weightApproved += guardian[msg.sender][_kernel].weight; - if (proposal.weightApproved >= weightedStorage[_kernel].threshold) { - proposal.status = ProposalStatus.Approved; - proposal.validAfter = ValidAfter.wrap(uint48(block.timestamp + weightedStorage[_kernel].delay)); - } } function approveWithSig(bytes32 _callDataAndNonceHash, address _kernel, bytes calldata sigs) external { uint256 sigCount = sigs.length / 65; - require(weightedStorage[_kernel].threoshold != 0, "Kernel not enabled"); + require(weightedStorage[_kernel].threshold != 0, "Kernel not enabled"); ProposalStorage storage proposal = proposalStatus[_callDataAndNonceHash][_kernel]; require(proposal.status == ProposalStatus.Ongoing, "Proposal not ongoing"); for (uint256 i = 0; i < sigCount; i++) { @@ -113,11 +108,6 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { VoteStorage storage vote = voteStatus[_callDataAndNonceHash][signer][_kernel]; require(vote.status == VoteStatus.NA, "Already voted"); vote.status = VoteStatus.Approved; - proposal.weightApproved += guardian[signer][_kernel].weight; - } - if (proposal.weightApproved >= weightedStorage[_kernel].threshold) { - proposal.status = ProposalStatus.Approved; - proposal.validAfter = ValidAfter.wrap(uint48(block.timestamp + weightedStorage[_kernel].delay)); } } @@ -149,7 +139,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { bytes calldata sig = userOp.signature; // parse sig with 65 bytes uint256 sigCount = sig.length / 65; - uint256 totalWeight = proposal.weightApproved; + (uint256 totalWeight, bool passed) = getApproval(msg.sender, callDataAndNonceHash); address signer; VoteStorage storage vote; for (uint256 i = 0; i < sigCount; i++) { @@ -181,6 +171,20 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { } } + function getApproval(address kernel, bytes32 hash) public view returns (uint256 approvals, bool passed) { + WeightedECDSAValidatorStorage storage strg = weightedStorage[kernel]; + for ( + address currentGuardian = strg.firstGuardian; + currentGuardian != address(0); + currentGuardian = guardian[currentGuardian][kernel].nextGuardian + ) { + if (voteStatus[hash][currentGuardian][kernel].status == VoteStatus.Approved) { + approvals += guardian[currentGuardian][kernel].weight; + } + } + passed = approvals >= strg.threshold; + } + function validCaller(address, bytes calldata) external pure override returns (bool) { return false; } From fa3d33994da98d4c807f25b20f88cb2b2ea9987d Mon Sep 17 00:00:00 2001 From: leekt Date: Mon, 11 Dec 2023 21:56:15 +0900 Subject: [PATCH 7/8] ZeroDev-RW-4 and ZeroDev-RW-2 fix --- src/validator/WeightedECDSAValidator.sol | 52 +++++++++++++++++++++--- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index e15783c2..12bf5203 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -49,7 +49,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { voteStatus; function _domainNameAndVersion() internal pure override returns (string memory, string memory) { - return ("WeightedECDSAValidator", "1"); + return ("WeightedECDSAValidator", "0.0.1"); } function enable(bytes calldata _data) external payable override { @@ -83,6 +83,31 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { delete weightedStorage[msg.sender]; } + function renew(address[] calldata _guardians, uint24[] calldata _weights, uint24 _threshold, uint48 _delay) external payable { + require(weightedStorage[msg.sender].totalWeight != 0, "Not enabled"); + address currentGuardian = weightedStorage[msg.sender].firstGuardian; + while (currentGuardian != msg.sender) { + address nextGuardian = guardian[currentGuardian][msg.sender].nextGuardian; + delete guardian[currentGuardian][msg.sender]; + currentGuardian = nextGuardian; + } + 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]; + } + weightedStorage[msg.sender].delay = _delay; + weightedStorage[msg.sender].threshold = _threshold; + } + function approve(bytes32 _callDataAndNonceHash, address _kernel) external { require(guardian[msg.sender][_kernel].weight != 0, "Guardian not enabled"); require(weightedStorage[_kernel].threshold != 0, "Kernel not enabled"); @@ -91,6 +116,11 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { VoteStorage storage vote = voteStatus[_callDataAndNonceHash][msg.sender][_kernel]; require(vote.status == VoteStatus.NA, "Already voted"); vote.status = VoteStatus.Approved; + (, bool isApproved) = getApproval(_kernel, _callDataAndNonceHash); + if (isApproved) { + proposal.status = ProposalStatus.Approved; + proposal.validAfter = ValidAfter.wrap(uint48(block.timestamp + weightedStorage[_kernel].delay)); + } } function approveWithSig(bytes32 _callDataAndNonceHash, address _kernel, bytes calldata sigs) external { @@ -109,6 +139,12 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { require(vote.status == VoteStatus.NA, "Already voted"); vote.status = VoteStatus.Approved; } + + (, bool isApproved) = getApproval(_kernel, _callDataAndNonceHash); + if (isApproved) { + proposal.status = ProposalStatus.Approved; + proposal.validAfter = ValidAfter.wrap(uint48(block.timestamp + weightedStorage[_kernel].delay)); + } } function veto(bytes32 _callDataAndNonceHash) external { @@ -131,7 +167,9 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { if (strg.threshold == 0) { return SIG_VALIDATION_FAILED; } - if (proposal.status == ProposalStatus.Ongoing) { + (uint256 totalWeight, bool passed) = getApproval(msg.sender, callDataAndNonceHash); + uint256 threshold = strg.threshold; + if (proposal.status == ProposalStatus.Ongoing && !passed) { if (strg.delay != 0) { // if delay > 0, only allow proposal to be approved before execution return SIG_VALIDATION_FAILED; @@ -139,10 +177,9 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { bytes calldata sig = userOp.signature; // parse sig with 65 bytes uint256 sigCount = sig.length / 65; - (uint256 totalWeight, bool passed) = getApproval(msg.sender, callDataAndNonceHash); address signer; VoteStorage storage vote; - for (uint256 i = 0; i < sigCount; i++) { + for (uint256 i = 0; i < sigCount && !passed; i++) { // last sig is for userOpHash verification signer = ECDSA.recover( _hashTypedData( @@ -156,14 +193,17 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { } // skip if already voted vote.status = VoteStatus.Approved; totalWeight += guardian[signer][msg.sender].weight; + if (totalWeight >= threshold) { + passed = true; + } } - if (totalWeight >= strg.threshold) { + if (passed) { validationData = packValidationData(ValidAfter.wrap(0), ValidUntil.wrap(0)); proposal.status = ProposalStatus.Executed; } else { validationData = SIG_VALIDATION_FAILED; } - } else if (proposal.status == ProposalStatus.Approved) { + } else if (proposal.status == ProposalStatus.Approved || passed) { validationData = packValidationData(proposal.validAfter, ValidUntil.wrap(0)); proposal.status = ProposalStatus.Executed; } else { From acb70f0ab4647873f4d38a7021b0b5c7ab801f94 Mon Sep 17 00:00:00 2001 From: leekt Date: Wed, 13 Dec 2023 15:56:39 +0900 Subject: [PATCH 8/8] fmt --- src/validator/WeightedECDSAValidator.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index 12bf5203..d77b8807 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -83,7 +83,10 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { delete weightedStorage[msg.sender]; } - function renew(address[] calldata _guardians, uint24[] calldata _weights, uint24 _threshold, uint48 _delay) external payable { + function renew(address[] calldata _guardians, uint24[] calldata _weights, uint24 _threshold, uint48 _delay) + external + payable + { require(weightedStorage[msg.sender].totalWeight != 0, "Not enabled"); address currentGuardian = weightedStorage[msg.sender].firstGuardian; while (currentGuardian != msg.sender) {