Skip to content

Commit

Permalink
Merge pull request #47 from aragon/f/clearer-names
Browse files Browse the repository at this point in the history
Clearer names for encryption methods and events
  • Loading branch information
brickpop authored Dec 5, 2024
2 parents f882d6b + 7ca725f commit 3c39fc3
Show file tree
Hide file tree
Showing 15 changed files with 323 additions and 320 deletions.
2 changes: 1 addition & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ PRODNET_RPC_URL="https://eth.drpc.org"
# ETHERSCAN_API_KEY="..."

# MULTISIG PARAMETERS
# define a list of multisig members - said multisig will be assigned administrator roles of the ve contracts
MULTISIG_MEMBERS_JSON_FILE_NAME="/script/multisig-members.json"

# GOVERNANCE PARAMETERS
Expand All @@ -37,6 +36,7 @@ TAIKO_L1_ADDRESS="0x79C9109b764609df928d16fC4a91e9081F7e87DB" # Address of the T
TAIKO_BRIDGE_ADDRESS="0xA098b76a3Dd499D3F6D58D8AcCaFC8efBFd06807" # Address of the Taiko Bridge

# OSx BASE CONTRACT ADDRESSES (network dependent, see active_contracts.json on lib/osx)
# HOLESKY
DAO_FACTORY="0xE640Da5AD169630555A86D9b6b9C145B4961b1EB"
PLUGIN_SETUP_PROCESSOR="0xCe0B4124dea6105bfB85fB4461c4D39f360E9ef3"
PLUGIN_REPO_FACTORY="0x95D563382BeD5AcB458759EE05b27DF2CB019Cc7"
Expand Down
5 changes: 2 additions & 3 deletions TEST_TREE.md
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,8 @@ SignerListTest
│ │ └── It votingWallet should be the given address
│ └── Given the resolved owner was not listed on resolveEncryptionAccountAtBlock
│ ├── It should return a zero owner
│ └── It should return a zero appointedWallet
└── When calling getEncryptionRecipients
│ └── It should return a zero appointedAgent
└── When calling getEncryptionAgents
├── Given the encryption registry has no accounts
│ ├── It returns an empty list, even with signers
│ └── It returns an empty list, without signers
Expand All @@ -560,4 +560,3 @@ SignerListTest
├── It result does not contain unlisted addresses
└── It result does not contain non appointed addresses
```

10 changes: 5 additions & 5 deletions src/EmergencyMultisig.sol
Original file line number Diff line number Diff line change
Expand Up @@ -357,17 +357,17 @@ contract EmergencyMultisig is IEmergencyMultisig, PluginUUPSUpgradeable, Proposa

// This internally calls `isListedAtBlock`.
// If not listed or resolved, it returns address(0)
(address _resolvedOwner, address _resolvedVoter) =
(address _owner, address _agent) =
multisigSettings.signerList.resolveEncryptionAccountAtBlock(_approver, proposal_.parameters.snapshotBlock);
if (_resolvedOwner == address(0) || _resolvedVoter == address(0)) {
if (_owner == address(0) || _agent == address(0)) {
// Not listedAtBlock() nor appointed by a listed owner
return false;
} else if (_approver != _resolvedVoter) {
// Only the voter account can vote (owners who appointed, can't)
} else if (_approver != _agent) {
// Only the agent can vote (owners who appointed, can't)
return false;
}

if (proposal_.approvers[_resolvedOwner]) {
if (proposal_.approvers[_owner]) {
// The account already approved
return false;
}
Expand Down
69 changes: 34 additions & 35 deletions src/EncryptionRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ import {IEncryptionRegistry} from "./interfaces/IEncryptionRegistry.sol";
/// @author Aragon Association - 2024
/// @notice A smart contract where accounts can register their libsodium public key for encryption purposes, as well as appointing an EOA
contract EncryptionRegistry is IEncryptionRegistry, ERC165 {
struct AccountEntry {
address appointedWallet;
struct RegisteredAccount {
address appointedAgent;
bytes32 publicKey;
}

/// @notice Allows to enumerate the addresses on the registry
address[] public registeredAccounts;
address[] public accountList;

/// @notice The database of appointed wallets and their public key
mapping(address => AccountEntry) public accounts;
/// @notice The public key and (optional) appointed agent or each registered account
mapping(address => RegisteredAccount) public accounts;

/// @notice A reference to the account that appointed each wallet
/// @notice A reference to the account that appointed each agent
mapping(address => address) public appointerOf;

/// @dev The contract to check whether the caller is a multisig member
Expand All @@ -38,26 +38,26 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {
}

/// @inheritdoc IEncryptionRegistry
function appointWallet(address _newWallet) public {
function appointAgent(address _newAgent) public {
// Appointing ourselves is the same as unappointing
if (_newWallet == msg.sender) _newWallet = address(0);
if (_newAgent == msg.sender) _newAgent = address(0);

if (!addresslist.isListed(msg.sender)) {
revert MustBeListed();
} else if (Address.isContract(_newWallet)) {
} else if (Address.isContract(_newAgent)) {
revert CannotAppointContracts();
} else if (addresslist.isListed(_newWallet)) {
} else if (addresslist.isListed(_newAgent)) {
// Appointing an already listed signer is not allowed, as votes would be locked
revert AlreadyListed();
} else if (_newWallet == accounts[msg.sender].appointedWallet) {
} else if (_newAgent == accounts[msg.sender].appointedAgent) {
return; // done
} else if (appointerOf[_newWallet] != address(0)) {
} else if (appointerOf[_newAgent] != address(0)) {
revert AlreadyAppointed();
}

bool exists;
for (uint256 i = 0; i < registeredAccounts.length;) {
if (registeredAccounts[i] == msg.sender) {
for (uint256 i = 0; i < accountList.length;) {
if (accountList[i] == msg.sender) {
exists = true;
break;
}
Expand All @@ -68,26 +68,26 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {

// New account?
if (!exists) {
registeredAccounts.push(msg.sender);
accountList.push(msg.sender);
}
// Existing account
else {
// Clear the current appointerOf[], if needed
if (accounts[msg.sender].appointedWallet != address(0)) {
appointerOf[accounts[msg.sender].appointedWallet] = address(0);
if (accounts[msg.sender].appointedAgent != address(0)) {
appointerOf[accounts[msg.sender].appointedAgent] = address(0);
}
// Clear the current public key, if needed
if (accounts[msg.sender].publicKey != bytes32(0)) {
// The old appointed wallet should no longer be able to see new content
// The old appointed agent should no longer be able to see new content
accounts[msg.sender].publicKey = bytes32(0);
}
}

accounts[msg.sender].appointedWallet = _newWallet;
if (_newWallet != address(0)) {
appointerOf[_newWallet] = msg.sender;
accounts[msg.sender].appointedAgent = _newAgent;
if (_newAgent != address(0)) {
appointerOf[_newAgent] = msg.sender;
}
emit WalletAppointed(msg.sender, _newWallet);
emit AgentAppointed(msg.sender, _newAgent);
}

/// @inheritdoc IEncryptionRegistry
Expand All @@ -97,10 +97,9 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {
}
// If someone else if appointed, the public key cannot be overriden.
// The appointed value should be set to address(0) or msg.sender first.
else if (
accounts[msg.sender].appointedWallet != address(0) && accounts[msg.sender].appointedWallet != msg.sender
) {
revert MustResetAppointment();
else if (accounts[msg.sender].appointedAgent != address(0) && accounts[msg.sender].appointedAgent != msg.sender)
{
revert MustResetAppointedAgent();
}

_setPublicKey(msg.sender, _publicKey);
Expand All @@ -111,7 +110,7 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {
function setPublicKey(address _accountOwner, bytes32 _publicKey) public {
if (!addresslist.isListed(_accountOwner)) {
revert MustBeListed();
} else if (accounts[_accountOwner].appointedWallet != msg.sender) {
} else if (accounts[_accountOwner].appointedAgent != msg.sender) {
revert MustBeAppointed();
}

Expand All @@ -121,15 +120,15 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {

/// @inheritdoc IEncryptionRegistry
function getRegisteredAccounts() public view returns (address[] memory) {
return registeredAccounts;
return accountList;
}

/// @inheritdoc IEncryptionRegistry
function getAppointedWallet(address _member) public view returns (address) {
if (accounts[_member].appointedWallet != address(0)) {
return accounts[_member].appointedWallet;
function getAppointedAgent(address _account) public view returns (address) {
if (accounts[_account].appointedAgent != address(0)) {
return accounts[_account].appointedAgent;
}
return _member;
return _account;
}

/// @notice Checks if this or the parent contract supports an interface by its ID.
Expand All @@ -143,8 +142,8 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {

function _setPublicKey(address _account, bytes32 _publicKey) internal {
bool exists;
for (uint256 i = 0; i < registeredAccounts.length;) {
if (registeredAccounts[i] == _account) {
for (uint256 i = 0; i < accountList.length;) {
if (accountList[i] == _account) {
exists = true;
break;
}
Expand All @@ -154,7 +153,7 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {
}
if (!exists) {
// New account
registeredAccounts.push(_account);
accountList.push(_account);
}

accounts[_account].publicKey = _publicKey;
Expand Down
10 changes: 5 additions & 5 deletions src/Multisig.sol
Original file line number Diff line number Diff line change
Expand Up @@ -342,17 +342,17 @@ contract Multisig is IMultisig, PluginUUPSUpgradeable, ProposalUpgradeable {

// This internally calls `isListedAtBlock`.
// If not listed or resolved, it returns address(0)
(address _resolvedOwner, address _resolvedVoter) =
(address _owner, address _agent) =
multisigSettings.signerList.resolveEncryptionAccountAtBlock(_approver, proposal_.parameters.snapshotBlock);
if (_resolvedOwner == address(0) || _resolvedVoter == address(0)) {
if (_owner == address(0) || _agent == address(0)) {
// Not listedAtBlock() nor appointed by a listed owner
return false;
} else if (_approver != _resolvedVoter) {
// Only the voter account can vote (owners who appointed, can't)
} else if (_approver != _agent) {
// Only the agent can vote (owners who appointed, can't)
return false;
}

if (proposal_.approvers[_resolvedOwner]) {
if (proposal_.approvers[_owner]) {
// The account already approved
return false;
}
Expand Down
33 changes: 17 additions & 16 deletions src/SignerList.sol
Original file line number Diff line number Diff line change
Expand Up @@ -134,58 +134,59 @@ contract SignerList is ISignerList, Addresslist, ERC165Upgradeable, DaoAuthoriza
function resolveEncryptionAccountAtBlock(address _address, uint256 _blockNumber)
public
view
returns (address _owner, address _voter)
returns (address _owner, address _agent)
{
if (isListedAtBlock(_address, _blockNumber)) {
// The owner + the voter
return (_address, settings.encryptionRegistry.getAppointedWallet(_address));
// The owner + the agent
return (_address, settings.encryptionRegistry.getAppointedAgent(_address));
}

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

// Not found, returning empty addresses
}

/// @inheritdoc ISignerList
function getEncryptionRecipients() external view returns (address[] memory result) {
function getEncryptionAgents() external view returns (address[] memory result) {
address[] memory _encryptionAccounts = settings.encryptionRegistry.getRegisteredAccounts();

// Allocating the full length.
// If any member is no longer listed, the size will be decreased.
result = new address[](_encryptionAccounts.length);

uint256 rIdx; // Result iterator. Will never be greater than erIdx.
uint256 erIdx; // EncryptionRegistry iterator
uint256 resIdx; // Result iterator. Will never be greater than accIdx.
uint256 accIdx; // Encryption accounts iterator
address appointed;
for (erIdx = 0; erIdx < _encryptionAccounts.length;) {
if (isListed(_encryptionAccounts[erIdx])) {
for (accIdx = 0; accIdx < _encryptionAccounts.length;) {
if (isListed(_encryptionAccounts[accIdx])) {
// Add it to the result array if listed
appointed = settings.encryptionRegistry.getAppointedWallet(_encryptionAccounts[erIdx]);
appointed = settings.encryptionRegistry.getAppointedAgent(_encryptionAccounts[accIdx]);
// Use the appointed address if non-zero
if (appointed != address(0)) {
result[rIdx] = appointed;
result[resIdx] = appointed;
} else {
result[rIdx] = _encryptionAccounts[erIdx];
result[resIdx] = _encryptionAccounts[accIdx];
}

unchecked {
rIdx++;
resIdx++;
}
}
// Skip non-listed accounts othersise

unchecked {
erIdx++;
accIdx++;
}
}

if (rIdx < erIdx) {
// Is the resulting list smaller than the list of registered accounts?
if (resIdx < accIdx) {
// Decrease the array size to return listed accounts without blank entries
uint256 diff = erIdx - rIdx;
uint256 diff = accIdx - resIdx;
assembly {
mstore(result, sub(mload(result), diff))
}
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IEmergencyMultisig.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface IEmergencyMultisig {

/// @notice Checks if an account can participate on a proposal vote. This can be because the vote
/// - was executed, or
/// - the voter is not listed.
/// - the approver is not listed or appointed.
/// @param _proposalId The proposal Id.
/// @param _account The address of the user to check.
/// @return Returns true if the account is allowed to vote.
Expand Down
40 changes: 21 additions & 19 deletions src/interfaces/IEncryptionRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ interface IEncryptionRegistry {
/// @notice Emitted when a public key is defined
event PublicKeySet(address account, bytes32 publicKey);

/// @notice Emitted when an externally owned wallet is appointed
event WalletAppointed(address account, address appointedWallet);
/// @notice Emitted when an externally owned wallet is appointed as the encryption agent
event AgentAppointed(address account, address agent);

/// @notice Raised when the caller is not an addresslist member
error MustBeListed();
Expand All @@ -21,39 +21,41 @@ interface IEncryptionRegistry {
/// @notice Raised when attempting to appoint an already appointed address
error AlreadyAppointed();

/// @notice Raised when a non appointed wallet tries to define the public key
/// @notice Raised when a wallet not appointed as an agent tries to define a public key
error MustBeAppointed();

/// @notice Raised when someone else is appointed and the account owner tries to override the public key of the appointed wallet. The appointed value should be set to address(0) or msg.sender first.
error MustResetAppointment();
/// @notice Raised when an agent is appointed and the account owner tries to override the account's public key. The appointed value should be set to address(0) or msg.sender first.
error MustResetAppointedAgent();

/// @notice Raised when the caller is not an addresslist compatible contract
/// @notice Raised when the caller is not an AddressList compatible contract
error InvalidAddressList();

/// @notice Registers the externally owned wallet's address to use for encryption. This allows smart contracts to appoint an EOA that can decrypt data.
/// @dev NOTE: calling this function will wipe any existing public key previously registered.
function appointWallet(address newWallet) external;
/// @notice Registers the given address as the account's encryption agent. This allows smart contract accounts to appoint an EOA that can decrypt data on their behalf.
/// @dev NOTE: calling this function will wipe any previously registered public key.
/// @param newAgent The address of an EOA to define as the new agent.
function appointAgent(address newAgent) external;

/// @notice Registers the given public key as the account's target for decrypting messages.
/// @dev NOTE: Calling this function from a smart contracts will revert.
/// @param publicKey The libsodium public key to register
function setOwnPublicKey(bytes32 publicKey) external;

/// @notice Registers the given public key as the member's target for decrypting messages. Only if the sender is appointed.
/// @param account The address of the account to set the public key for. The sender must be appointed or the transaction will revert.
/// @notice Registers the given public key as the agent's target for decrypting messages. Only if the sender is an appointed agent.
/// @param accountOwner The address of the account to set the public key for. The sender must be appointed or the transaction will revert.
/// @param publicKey The libsodium public key to register
function setPublicKey(address account, bytes32 publicKey) external;
function setPublicKey(address accountOwner, bytes32 publicKey) external;

/// @notice Returns the address of the account that appointed the given wallet, if any.
/// @return appointerAddress The address of the appointer account or zero.
function appointerOf(address wallet) external returns (address appointerAddress);
/// @notice Returns the address of the account that appointed the given agent, if any.
/// @return owner The address of the owner who appointed the given agent, or zero.
function appointerOf(address agent) external returns (address owner);

/// @notice Returns the address of the account registered at the given index
function registeredAccounts(uint256) external view returns (address);
function accountList(uint256) external view returns (address);

/// @notice Returns the list of addresses on the registry
/// @dev Use this function to get all addresses in a single call. You can still call registeredAccounts[idx] to resolve them one by one.
/// @dev Use this function to get all addresses in a single call. You can still call accountList[idx] to resolve them one by one.
function getRegisteredAccounts() external view returns (address[] memory);

/// @notice Returns the address of the wallet appointed for encryption purposes
function getAppointedWallet(address member) external view returns (address);
/// @notice Returns the address of the account's encryption agent, or the account itself if no agent is appointed.
function getAppointedAgent(address account) external view returns (address agent);
}
Loading

0 comments on commit 3c39fc3

Please sign in to comment.