Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve signature replication issue in weighted ecdsa validator #101

Merged
merged 2 commits into from
Apr 23, 2024
Merged

fix: resolve signature replication issue in weighted ecdsa validator #101

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c59691
fix weighted ecdsa validator signature replication issue
adnpark Apr 22, 2024
d7adae4
feat: port weighted ecdsa validator v2.4
adnpark Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 46 additions & 35 deletions src/validator/WeightedECDSAValidator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,34 +59,45 @@ contract WeightedECDSAValidator is EIP712, IValidator {
event GuardianRemoved(address indexed guardian, address indexed kernel);

function _domainNameAndVersion() internal pure override returns (string memory, string memory) {
return ("WeightedECDSAValidator", "0.0.2");
return ("WeightedECDSAValidator", "0.0.3");
}

function onInstall(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");
if (_isInitialized(msg.sender)) revert AlreadyInitialized(msg.sender);
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 onInstall(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");
if (_isInitialized(msg.sender)) revert AlreadyInitialized(msg.sender);
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 onUninstall(bytes calldata) external payable override {
if (!_isInitialized(msg.sender)) revert NotInitialized(msg.sender);
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];
Expand All @@ -113,31 +124,21 @@ contract WeightedECDSAValidator is EIP712, IValidator {
{
if (!_isInitialized(msg.sender)) revert NotInitialized(msg.sender);
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];
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];
emit GuardianAdded(_guardians[i], msg.sender, _weights[i]);
}
weightedStorage[msg.sender].firstGuardian = address(uint160(type(uint160).max));
_addGuardians(_guardians, _weights, msg.sender);
weightedStorage[msg.sender].delay = _delay;
weightedStorage[msg.sender].threshold = _threshold;
}

function approve(bytes32 _callDataAndNonceHash, address _kernel) external payable {
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");
ProposalStorage storage proposal = proposalStatus[_callDataAndNonceHash][_kernel];
Expand All @@ -152,7 +153,7 @@ contract WeightedECDSAValidator is EIP712, IValidator {
}
}

function approveWithSig(bytes32 _callDataAndNonceHash, address _kernel, bytes calldata sigs) external payable {
function approveWithSig(bytes32 _callDataAndNonceHash, address _kernel, bytes calldata sigs) external {
uint256 sigCount = sigs.length / 65;
require(weightedStorage[_kernel].threshold != 0, "Kernel not enabled");
ProposalStorage storage proposal = proposalStatus[_callDataAndNonceHash][_kernel];
Expand All @@ -176,7 +177,7 @@ contract WeightedECDSAValidator is EIP712, IValidator {
}
}

function veto(bytes32 _callDataAndNonceHash) external payable {
function veto(bytes32 _callDataAndNonceHash) external {
ProposalStorage storage proposal = proposalStatus[_callDataAndNonceHash][msg.sender];
require(
proposal.status == ProposalStatus.Ongoing || proposal.status == ProposalStatus.Approved,
Expand Down Expand Up @@ -237,13 +238,19 @@ contract WeightedECDSAValidator is EIP712, IValidator {
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_UINT;
} else if (proposal.status == ProposalStatus.Approved || passed) {
address signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(userOpHash), userOp.signature);
if (guardian[signer][msg.sender].weight != 0) {
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));
}
Expand Down Expand Up @@ -281,13 +288,17 @@ contract WeightedECDSAValidator is EIP712, IValidator {
return ERC1271_INVALID;
}
uint256 totalWeight = 0;
address signer;
address prevSigner = address(uint160(type(uint160).max));
for (uint256 i = 0; i < sigCount; i++) {
signer = ECDSA.recover(hash, data[i * 65:(i + 1) * 65]);
address signer = ECDSA.recover(hash, data[i * 65:(i + 1) * 65]);
totalWeight += guardian[signer][msg.sender].weight;
if (totalWeight >= strg.threshold) {
return ERC1271_MAGICVALUE;
}
if (signer >= prevSigner) {
return ERC1271_INVALID;
}
prevSigner = signer;
}
return ERC1271_INVALID;
}
Expand Down
Loading