Skip to content

Commit

Permalink
Optimizing the signer list getters
Browse files Browse the repository at this point in the history
  • Loading branch information
@brickpop
brickpop committed Nov 12, 2024
1 parent 6adf49e commit a0e3ae2
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 127 deletions.
34 changes: 15 additions & 19 deletions src/EmergencyMultisig.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,10 @@ contract EmergencyMultisig is IEmergencyMultisig, PluginUUPSUpgradeable, Proposa
bool _approveProposal
) external returns (uint256 proposalId) {
if (multisigSettings.onlyListed) {
(bool ownerIsListed,) = multisigSettings.signerList.resolveEncryptionAccountStatus(msg.sender);
bool _listedOrAppointedByListed = multisigSettings.signerList.isListedOrAppointedByListed(msg.sender);

// Only the account or its appointed address may create proposals
if (!ownerIsListed) {
if (!_listedOrAppointedByListed) {
revert ProposalCreationForbidden(msg.sender);
}
}
Expand Down Expand Up @@ -237,10 +237,11 @@ contract EmergencyMultisig is IEmergencyMultisig, PluginUUPSUpgradeable, Proposa
proposal_.approvals += 1;
}

// Register the approval as being made by the owner, the one who isListed() relates to
address _owner = multisigSettings.signerList.resolveEncryptionOwner(_sender);
// Register the approval as being made by the owner
address _owner = multisigSettings.signerList.getOwnerAtBlock(_sender, proposal_.parameters.snapshotBlock);
proposal_.approvers[_owner] = true;

// We emit the event as the owner's approval
emit Approved({proposalId: _proposalId, approver: _owner});

// Automatic execution is intentionally omitted in order to prevent
Expand Down Expand Up @@ -292,7 +293,7 @@ contract EmergencyMultisig is IEmergencyMultisig, PluginUUPSUpgradeable, Proposa

/// @inheritdoc IEmergencyMultisig
function hasApproved(uint256 _proposalId, address _account) public view returns (bool) {
address _owner = multisigSettings.signerList.resolveEncryptionOwner(_account);
address _owner = multisigSettings.signerList.getCurrentOwner(_account);

return proposals[_proposalId].approvers[_owner];
}
Expand Down Expand Up @@ -351,24 +352,19 @@ contract EmergencyMultisig is IEmergencyMultisig, PluginUUPSUpgradeable, Proposa
return false;
}

(address _owner, address _appointedWallet) = multisigSettings.signerList.resolveEncryptionAccount(_approver);

if (_owner == address(0)) {
// Not resolved
// This internally calls `isListedAtBlock`.
// If not listed or resolved, it returns address(0)
(address _resolvedOwner, address _resolvedVoter) =
multisigSettings.signerList.resolveAccountAtBlock(_approver, proposal_.parameters.snapshotBlock);
if (_resolvedOwner == address(0) || _resolvedVoter == address(0)) {
// Not listedAtBlock() nor appointed by a listed owner
return false;
} else if (!multisigSettings.signerList.isListedAtBlock(_owner, proposal_.parameters.snapshotBlock)) {
// The owner account had no voting power
} else if (_approver != _resolvedVoter) {
// Only the voter account can vote (owners who appointed, can't)
return false;
}
// If there is an appointed wallet, only that wallet can approve
else if (_appointedWallet != address(0)) {
// Someone else is appointed
if (_approver != _appointedWallet) return false;
}

// If _appointedWallet == address(0), then _owner == _approver. No need to check.

if (proposal_.approvers[_owner]) {
if (proposal_.approvers[_resolvedOwner]) {
// The account already approved
return false;
}
Expand Down
34 changes: 15 additions & 19 deletions src/Multisig.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,10 @@ contract Multisig is IMultisig, PluginUUPSUpgradeable, ProposalUpgradeable {
bool _approveProposal
) external returns (uint256 proposalId) {
if (multisigSettings.onlyListed) {
(bool ownerIsListed,) = multisigSettings.signerList.resolveEncryptionAccountStatus(msg.sender);
bool _listedOrAppointedByListed = multisigSettings.signerList.isListedOrAppointedByListed(msg.sender);

// Only the account or its appointed address may create proposals
if (!ownerIsListed) {
if (!_listedOrAppointedByListed) {
revert ProposalCreationForbidden(msg.sender);
}
}
Expand Down Expand Up @@ -241,10 +241,11 @@ contract Multisig is IMultisig, PluginUUPSUpgradeable, ProposalUpgradeable {
proposal_.approvals += 1;
}

// Register the approval as being made by the owner, the one who isListed() relates to
address _owner = multisigSettings.signerList.resolveEncryptionOwner(_sender);
// Register the approval as being made by the owner. isListedAtBlock() relates to it
address _owner = multisigSettings.signerList.getOwnerAtBlock(_sender, proposal_.parameters.snapshotBlock);
proposal_.approvers[_owner] = true;

// We emit the event as the owner's approval
emit Approved({proposalId: _proposalId, approver: _owner});

if (_tryExecution && _canExecute(_proposalId)) {
Expand Down Expand Up @@ -294,7 +295,7 @@ contract Multisig is IMultisig, PluginUUPSUpgradeable, ProposalUpgradeable {

/// @inheritdoc IMultisig
function hasApproved(uint256 _proposalId, address _account) public view returns (bool) {
address _owner = multisigSettings.signerList.resolveEncryptionOwner(_account);
address _owner = multisigSettings.signerList.getCurrentOwner(_account);

return proposals[_proposalId].approvers[_owner];
}
Expand Down Expand Up @@ -336,24 +337,19 @@ contract Multisig is IMultisig, PluginUUPSUpgradeable, ProposalUpgradeable {
return false;
}

(address _owner, address _appointedWallet) = multisigSettings.signerList.resolveEncryptionAccount(_approver);

if (_owner == address(0)) {
// Not resolved
// This internally calls `isListedAtBlock`.
// If not listed or resolved, it returns address(0)
(address _resolvedOwner, address _resolvedVoter) =
multisigSettings.signerList.resolveAccountAtBlock(_approver, proposal_.parameters.snapshotBlock);
if (_resolvedOwner == address(0) || _resolvedVoter == address(0)) {
// Not listedAtBlock() nor appointed by a listed owner
return false;
} else if (!multisigSettings.signerList.isListedAtBlock(_owner, proposal_.parameters.snapshotBlock)) {
// The owner account had no voting power
} else if (_approver != _resolvedVoter) {
// Only the voter account can vote (owners who appointed, can't)
return false;
}
// If there is an appointed wallet, only that wallet can approve
else if (_appointedWallet != address(0)) {
// Someone else is appointed
if (_approver != _appointedWallet) return false;
}

// If _appointedWallet == address(0), then _owner == _approver. No need to check.

if (proposal_.approvers[_owner]) {
if (proposal_.approvers[_resolvedOwner]) {
// The account already approved
return false;
}
Expand Down
72 changes: 42 additions & 30 deletions src/SignerList.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,48 +103,60 @@ contract SignerList is ISignerList, Addresslist, ERC165Upgradeable, DaoAuthoriza
}

/// @inheritdoc ISignerList
function resolveEncryptionAccountStatus(address _address)
public
view
returns (bool ownerIsListed, bool isAppointed)
{
if (this.isListed(_address)) {
ownerIsListed = true;
} else if (this.isListed(settings.encryptionRegistry.appointedBy(_address))) {
ownerIsListed = true;
isAppointed = true;
function isListedOrAppointedByListed(address _address) public view returns (bool listedOrAppointedByListed) {
if (isListed(_address)) {
return true;
} else if (isListed(settings.encryptionRegistry.appointedBy(_address))) {
return true;
}

// Not found, return blank values
// Not found, return blank (false)
}

/// @inheritdoc ISignerList
function resolveEncryptionOwner(address _address) public view returns (address owner) {
(bool ownerIsListed, bool isAppointed) = resolveEncryptionAccountStatus(_address);
function getCurrentOwner(address _address) public view returns (address _owner) {
if (isListed(_address)) {
return _address;
}
address _appointer = settings.encryptionRegistry.appointedBy(_address);
if (isListed(_appointer)) {
return _appointer;
}

// Not found, return a blank address
}

if (!ownerIsListed) {
return address(0);
} else if (isAppointed) {
return settings.encryptionRegistry.appointedBy(_address);
/// @inheritdoc ISignerList
function getOwnerAtBlock(address _address, uint256 _blockNumber) public view returns (address _owner) {
if (isListedAtBlock(_address, _blockNumber)) {
return _address;
}
return _address;
address _appointer = settings.encryptionRegistry.appointedBy(_address);
if (isListedAtBlock(_appointer, _blockNumber)) {
return _appointer;
}

// Not found, return a blank address
}

/// @inheritdoc ISignerList
function resolveEncryptionAccount(address _address) public view returns (address owner, address appointedWallet) {
(bool ownerIsListed, bool isAppointed) = resolveEncryptionAccountStatus(_address);

if (ownerIsListed) {
if (isAppointed) {
owner = settings.encryptionRegistry.appointedBy(_address);
appointedWallet = _address;
} else {
owner = _address;
appointedWallet = settings.encryptionRegistry.getAppointedWallet(_address);
}
function resolveAccountAtBlock(address _address, uint256 _blockNumber)
public
view
returns (address _owner, address _voter)
{
if (isListedAtBlock(_address, _blockNumber)) {
// The owner + the voter
return (_address, settings.encryptionRegistry.getAppointedWallet(_address));
}

address _appointer = settings.encryptionRegistry.appointedBy(_address);
if (this.isListedAtBlock(_appointer, _blockNumber)) {
// The appointed wallet votes
return (_appointer, _address);
}

// Not found, return blank values
// Not found, returning empty addresses
}

/// @inheritdoc ISignerList
Expand Down
26 changes: 15 additions & 11 deletions src/interfaces/ISignerList.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,29 @@ interface ISignerList {
/// @param signers The addresses of the signers to be removed.
function removeSigners(address[] calldata signers) external;

/// @notice Given an address, determines the corresponding (listed) owner account and the appointed wallet, if any.
/// @notice Given an address, determines whether it is a listed signer or a wallet appointed by a listed owner.
/// @dev NOTE: This function will only resolve based on the current state. Do not use it as an alias of `isListedAtBock()`.
/// @return ownerIsListed If resolved, whether the given address is currently listed as a member. False otherwise.
/// @return isAppointed If resolved, whether the given address is appointed by the owner. False otherwise.
function resolveEncryptionAccountStatus(address _sender)
external
view
returns (bool ownerIsListed, bool isAppointed);
/// @return listedOrAppointedByListed If resolved, whether the given address is currently listed as a member. False otherwise.
function isListedOrAppointedByListed(address _address) external returns (bool listedOrAppointedByListed);

/// @notice Given an address, determines the corresponding (listed) owner account and the appointed wallet, if any.
/// @dev NOTE: This function will only resolve based on the current state. Do not use it as an alias of `isListedAtBock()`.
/// @param sender The address to check within the list of signers or the appointed accounts.
/// @return owner If resolved to an account, it contains the encryption owner's address. Returns address(0) otherwise.
function resolveEncryptionOwner(address _sender) external view returns (address owner);
function getCurrentOwner(address sender) external returns (address owner);

/// @notice Given an address, determines the corresponding (listed) owner account and the appointed wallet, if any.
/// @dev NOTE: This function will only resolve based on the current state. Do not use it as an alias of `isListedAtBock()`.
/// @param sender The address to check within the list of signers or the appointed accounts.
/// @param blockNumber The block at which the list should be checked
/// @return owner If resolved to an account, it contains the encryption owner's address. Returns address(0) otherwise.
/// @return appointedWallet If resolved, it contains the wallet address appointed for decryption, if any. Returns address(0) otherwise.
function resolveEncryptionAccount(address sender) external view returns (address owner, address appointedWallet);
function getOwnerAtBlock(address sender, uint256 blockNumber) external returns (address owner);

/// @notice Given an address, determines the corresponding (listed) owner account and the appointed wallet, if any.
/// @return owner If listed and resolved to an account, it contains the encryption owner's address. Returns address(0) otherwise.
/// @return voter If listed and resolved, it contains the wallet address appointed for decryption, if any. Returns address(0) otherwise.
function resolveAccountAtBlock(address sender, uint256 _blockNumber)
external
returns (address owner, address voter);

/// @notice Among the SignerList's members registered on the EncryptionRegistry, return the effective address they use for encryption
function getEncryptionRecipients() external view returns (address[] memory);
Expand Down
Loading

0 comments on commit a0e3ae2

Please sign in to comment.