From ba8b92d7323910648e3a4a5df9acd6d503daa74d Mon Sep 17 00:00:00 2001 From: adnpark Date: Tue, 12 Mar 2024 09:37:45 +0900 Subject: [PATCH 1/3] fix: avoid infinite loop on renew --- src/validator/WeightedECDSAValidator.sol | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index 47921c7c..e6924476 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -63,7 +63,11 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { require(_guardians[i] != address(0), "Guardian cannot be 0"); require(_weights[i] != 0, "Weight cannot be 0"); require(guardian[_guardians[i]][_kernel].weight == 0, "Guardian already enabled"); - require(uint160(_guardians[i]) < prevGuardian, "Guardians not sorted"); + // when index is 0 meaning that adding the first guardian, since prevGuardian is type(uint160).max, we skip the check + if (i != 0) { + // check if guardians are sorted + 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]; @@ -104,7 +108,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { { 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]; From 9689fb5dedbd7aaac99e6232189c98765923dd6e Mon Sep 17 00:00:00 2001 From: adnpark Date: Tue, 12 Mar 2024 10:45:29 +0900 Subject: [PATCH 2/3] fix: set max address as first guardian when renew --- src/validator/WeightedECDSAValidator.sol | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index e6924476..6f2170b5 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -63,11 +63,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { require(_guardians[i] != address(0), "Guardian cannot be 0"); require(_weights[i] != 0, "Weight cannot be 0"); require(guardian[_guardians[i]][_kernel].weight == 0, "Guardian already enabled"); - // when index is 0 meaning that adding the first guardian, since prevGuardian is type(uint160).max, we skip the check - if (i != 0) { - // check if guardians are sorted - require(uint160(_guardians[i]) < prevGuardian, "Guardians not sorted"); - } + 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]; @@ -116,7 +112,7 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { } delete weightedStorage[msg.sender]; require(_guardians.length == _weights.length, "Length mismatch"); - weightedStorage[msg.sender].firstGuardian = _guardians[0]; + weightedStorage[msg.sender].firstGuardian = address(uint160(type(uint160).max)); _addGuardians(_guardians, _weights, msg.sender); weightedStorage[msg.sender].delay = _delay; weightedStorage[msg.sender].threshold = _threshold; From b5b1fe1f88b396501ae79b26b7b05eb3f6c01d39 Mon Sep 17 00:00:00 2001 From: adnpark Date: Thu, 21 Mar 2024 17:40:40 +0900 Subject: [PATCH 3/3] fix: update code for dummy signature issue --- src/validator/WeightedECDSAValidator.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/validator/WeightedECDSAValidator.sol b/src/validator/WeightedECDSAValidator.sol index 6f2170b5..c82299c3 100644 --- a/src/validator/WeightedECDSAValidator.sol +++ b/src/validator/WeightedECDSAValidator.sol @@ -217,10 +217,11 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator { passed = true; } } + proposal.status = ProposalStatus.Executed; if (passed && guardian[signer][msg.sender].weight != 0) { - proposal.status = ProposalStatus.Executed; return packValidationData(ValidAfter.wrap(0), ValidUntil.wrap(0)); } + return SIG_VALIDATION_FAILED; } else if (proposal.status == ProposalStatus.Approved || passed) { if (userOp.paymasterAndData.length == 0 || address(bytes20(userOp.paymasterAndData[0:20])) == address(0)) { address signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(userOpHash), userOp.signature);