Skip to content

Sync ERC7579Multisig and ERC7579MultisigWeighted with vanilla's signers #193

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Sync ERC7579Multisig and ERC7579MultisigWeighted with vanilla's signers #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 42 additions & 31 deletions contracts/account/modules/ERC7579Multisig.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ abstract contract ERC7579Multisig is ERC7579Validator {
event ERC7913SignerRemoved(address indexed account, bytes signer);

/// @dev Emitted when the threshold is updated.
event ERC7913ThresholdSet(address indexed account, uint256 threshold);
event ERC7913ThresholdSet(address indexed account, uint64 threshold);

/// @dev The `signer` already exists.
error ERC7579MultisigAlreadyExists(bytes signer);
Expand All @@ -42,19 +42,22 @@ abstract contract ERC7579Multisig is ERC7579Validator {
/// @dev The `signer` is less than 20 bytes long.
error ERC7579MultisigInvalidSigner(bytes signer);

/// @dev The `threshold` is zero.
error ERC7579MultisigZeroThreshold();

/// @dev The `threshold` is unreachable given the number of `signers`.
error ERC7579MultisigUnreachableThreshold(uint256 signers, uint256 threshold);
error ERC7579MultisigUnreachableThreshold(uint64 signers, uint64 threshold);

mapping(address account => EnumerableSet.BytesSet) private _signersSetByAccount;
mapping(address account => uint256) private _thresholdByAccount;
mapping(address account => uint64) private _thresholdByAccount;

/**
* @dev Sets up the module's initial configuration when installed by an account.
* See {ERC7579DelayedExecutor-onInstall}. Besides the delay setup, the `initdata` can
* include `signers` and `threshold`.
*
* The initData should be encoded as:
* `abi.encode(bytes[] signers, uint256 threshold)`
* `abi.encode(bytes[] signers, uint64 threshold)`
*
* If no signers or threshold are provided, the multisignature functionality will be
* disabled until they are added later.
Expand All @@ -63,9 +66,9 @@ abstract contract ERC7579Multisig is ERC7579Validator {
* the signer will be set to the provided data. Future installations will behave as a no-op.
*/
function onInstall(bytes calldata initData) public virtual {
if (initData.length > 32 && _signers(msg.sender).length() == 0) {
if (initData.length > 32 && getSignerCount(msg.sender) == 0) {
// More than just delay parameter
(bytes[] memory signers_, uint256 threshold_) = abi.decode(initData, (bytes[], uint256));
(bytes[] memory signers_, uint64 threshold_) = abi.decode(initData, (bytes[], uint64));
_addSigners(msg.sender, signers_);
_setThreshold(msg.sender, threshold_);
}
Expand All @@ -86,30 +89,33 @@ abstract contract ERC7579Multisig is ERC7579Validator {
}

/**
* @dev Returns the set of authorized signers for the specified account.
* @dev Returns a slice of the set of authorized signers for the specified account.
*
* Using `start = 0` and `end = type(uint64).max` will return the entire set of signers.
*
* WARNING: This operation copies the entire signers set to memory, which
* can be expensive or may result in unbounded computation.
* WARNING: Depending on the `start` and `end`, this operation can copy a large amount of data to memory, which
* can be expensive. This is designed for view accessors queried without gas fees. Using it in state-changing
* functions may become uncallable if the slice grows too large.
*/
function signers(address account) public view virtual returns (bytes[] memory) {
return _signers(account).values();
function getSigners(address account, uint64 start, uint64 end) public view virtual returns (bytes[] memory) {
return _signersSetByAccount[account].values(start, end);
}

/// @dev Returns whether the `signer` is an authorized signer for the specified account.
function isSigner(address account, bytes memory signer) public view virtual returns (bool) {
return _signers(account).contains(signer);
/// @dev Returns the number of authorized signers for the specified account.
function getSignerCount(address account) public view virtual returns (uint256) {
return _signersSetByAccount[account].length();
}

/// @dev Returns the set of authorized signers for the specified account.
function _signers(address account) internal view virtual returns (EnumerableSet.BytesSet storage) {
return _signersSetByAccount[account];
/// @dev Returns whether the `signer` is an authorized signer for the specified account.
function isSigner(address account, bytes memory signer) public view virtual returns (bool) {
return _signersSetByAccount[account].contains(signer);
}

/**
* @dev Returns the minimum number of signers required to approve a multisignature operation
* for the specified account.
*/
function threshold(address account) public view virtual returns (uint256) {
function threshold(address account) public view virtual returns (uint64) {
return _thresholdByAccount[account];
}

Expand Down Expand Up @@ -147,7 +153,7 @@ abstract contract ERC7579Multisig is ERC7579Validator {
*
* * The threshold must be reachable with the current number of signers.
*/
function setThreshold(uint256 newThreshold) public virtual {
function setThreshold(uint64 newThreshold) public virtual {
_setThreshold(msg.sender, newThreshold);
}

Expand Down Expand Up @@ -181,11 +187,10 @@ abstract contract ERC7579Multisig is ERC7579Validator {
* * Each of `newSigners` must not be authorized. Reverts with {ERC7579MultisigAlreadyExists} if it already exists.
*/
function _addSigners(address account, bytes[] memory newSigners) internal virtual {
uint256 newSignersLength = newSigners.length;
for (uint256 i = 0; i < newSignersLength; i++) {
for (uint256 i = 0; i < newSigners.length; ++i) {
bytes memory signer = newSigners[i];
require(signer.length >= 20, ERC7579MultisigInvalidSigner(signer));
require(_signers(account).add(signer), ERC7579MultisigAlreadyExists(signer));
require(_signersSetByAccount[account].add(signer), ERC7579MultisigAlreadyExists(signer));
emit ERC7913SignerAdded(account, signer);
}
}
Expand All @@ -199,10 +204,9 @@ abstract contract ERC7579Multisig is ERC7579Validator {
* * The threshold must remain reachable after removal. See {_validateReachableThreshold} for details.
*/
function _removeSigners(address account, bytes[] memory oldSigners) internal virtual {
uint256 oldSignersLength = oldSigners.length;
for (uint256 i = 0; i < oldSignersLength; i++) {
for (uint256 i = 0; i < oldSigners.length; ++i) {
bytes memory signer = oldSigners[i];
require(_signers(account).remove(signer), ERC7579MultisigNonexistentSigner(signer));
require(_signersSetByAccount[account].remove(signer), ERC7579MultisigNonexistentSigner(signer));
emit ERC7913SignerRemoved(account, signer);
}
_validateReachableThreshold(account);
Expand All @@ -213,9 +217,11 @@ abstract contract ERC7579Multisig is ERC7579Validator {
*
* Requirements:
*
* * The threshold must be greater than 0. Reverts with {ERC7579MultisigZeroThreshold} if not.
* * The threshold must be reachable with the current number of signers. See {_validateReachableThreshold} for details.
*/
function _setThreshold(address account, uint256 newThreshold) internal virtual {
function _setThreshold(address account, uint64 newThreshold) internal virtual {
require(newThreshold > 0, ERC7579MultisigZeroThreshold());
_thresholdByAccount[account] = newThreshold;
_validateReachableThreshold(account);
emit ERC7913ThresholdSet(account, newThreshold);
Expand All @@ -229,9 +235,15 @@ abstract contract ERC7579Multisig is ERC7579Validator {
* * The number of signers must be >= the threshold. Reverts with {ERC7579MultisigUnreachableThreshold} if not.
*/
function _validateReachableThreshold(address account) internal view virtual {
uint256 totalSigners = _signers(account).length();
uint256 currentThreshold = threshold(account);
require(totalSigners >= currentThreshold, ERC7579MultisigUnreachableThreshold(totalSigners, currentThreshold));
uint256 totalSigners = getSignerCount(account);
uint64 currentThreshold = threshold(account);
require(
totalSigners >= currentThreshold,
ERC7579MultisigUnreachableThreshold(
uint64(totalSigners), // Safe cast. Economically impossible to overflow.
currentThreshold
)
);
}

/**
Expand All @@ -252,8 +264,7 @@ abstract contract ERC7579Multisig is ERC7579Validator {
bytes[] memory signingSigners,
bytes[] memory signatures
) internal view virtual returns (bool valid) {
uint256 signersLength = signingSigners.length;
for (uint256 i = 0; i < signersLength; i++) {
for (uint256 i = 0; i < signingSigners.length; ++i) {
if (!isSigner(account, signingSigners[i])) {
return false;
}
Expand Down
Loading