Ensuring that self appointing resets the appointed address

aragon · Nov 22, 2024 · 324ecc8 · 324ecc8
1 parent 702c38e
commit 324ecc8
Show file tree

Hide file tree

Showing 4 changed files with 246 additions and 75 deletions.
diff --git a/src/EncryptionRegistry.sol b/src/EncryptionRegistry.sol
@@ -24,7 +24,7 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {
     mapping(address => AccountEntry) public accounts;
 
     /// @notice A reference to the account that appointed each wallet
-    mapping(address => address) public appointedBy;
+    mapping(address => address) public appointerOf;
 
     /// @dev The contract to check whether the caller is a multisig member
     Addresslist addresslist;
@@ -39,33 +39,54 @@ contract EncryptionRegistry is IEncryptionRegistry, ERC165 {
 
     /// @inheritdoc IEncryptionRegistry
     function appointWallet(address _newWallet) public {
+        // Appointing ourselves is the same as unappointing
+        if (_newWallet == msg.sender) _newWallet = address(0);
+
         if (!addresslist.isListed(msg.sender)) {
             revert MustBeListed();
         } else if (Address.isContract(_newWallet)) {
             revert CannotAppointContracts();
-        } else if (appointedBy[_newWallet] != address(0)) {
+        } else if (addresslist.isListed(_newWallet)) {
+            // Appointing an already listed signer is not allowed, as votes would be locked
+            revert AlreadyListed();
+        } else if (_newWallet == accounts[msg.sender].appointedWallet) {
+            return; // done
+        } else if (appointerOf[_newWallet] != address(0)) {
             revert AlreadyAppointed();
         }
 
+        bool exists;
+        for (uint256 i = 0; i < registeredAccounts.length;) {
+            if (registeredAccounts[i] == msg.sender) {
+                exists = true;
+                break;
+            }
+            unchecked {
+                i++;
+            }
+        }
+
         // New account?
-        if (accounts[msg.sender].appointedWallet == address(0) && accounts[msg.sender].publicKey == bytes32(0)) {
+        if (!exists) {
             registeredAccounts.push(msg.sender);
         }
         // Existing account
         else {
-            // Clear the old appointedBy[], if needed
+            // Clear the current appointerOf[], if needed
             if (accounts[msg.sender].appointedWallet != address(0)) {
-                appointedBy[accounts[msg.sender].appointedWallet] = address(0);
+                appointerOf[accounts[msg.sender].appointedWallet] = address(0);
             }
-            // Clear the old public key, if needed
+            // 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
                 accounts[msg.sender].publicKey = bytes32(0);
             }
         }
 
         accounts[msg.sender].appointedWallet = _newWallet;
-        appointedBy[_newWallet] = msg.sender;
+        if (_newWallet != address(0)) {
+            appointerOf[_newWallet] = msg.sender;
+        }
         emit WalletAppointed(msg.sender, _newWallet);
     }
 

diff --git a/src/SignerList.sol b/src/SignerList.sol
@@ -106,7 +106,7 @@ contract SignerList is ISignerList, Addresslist, ERC165Upgradeable, DaoAuthoriza
     function isListedOrAppointedByListed(address _address) public view returns (bool listedOrAppointedByListed) {
         if (isListed(_address)) {
             return true;
-        } else if (isListed(settings.encryptionRegistry.appointedBy(_address))) {
+        } else if (isListed(settings.encryptionRegistry.appointerOf(_address))) {
             return true;
         }
 
@@ -122,7 +122,7 @@ contract SignerList is ISignerList, Addresslist, ERC165Upgradeable, DaoAuthoriza
         if (isListedAtBlock(_address, _blockNumber)) {
             return _address;
         }
-        address _appointer = settings.encryptionRegistry.appointedBy(_address);
+        address _appointer = settings.encryptionRegistry.appointerOf(_address);
         if (isListedAtBlock(_appointer, _blockNumber)) {
             return _appointer;
         }
@@ -141,7 +141,7 @@ contract SignerList is ISignerList, Addresslist, ERC165Upgradeable, DaoAuthoriza
             return (_address, settings.encryptionRegistry.getAppointedWallet(_address));
         }
 
-        address _appointer = settings.encryptionRegistry.appointedBy(_address);
+        address _appointer = settings.encryptionRegistry.appointerOf(_address);
         if (this.isListedAtBlock(_appointer, _blockNumber)) {
             // The appointed wallet votes
             return (_appointer, _address);

diff --git a/src/interfaces/IEncryptionRegistry.sol b/src/interfaces/IEncryptionRegistry.sol
@@ -15,6 +15,9 @@ interface IEncryptionRegistry {
     /// @notice Raised when attempting to register a contract instead of a wallet
     error CannotAppointContracts();
 
+    /// @notice Raised when attempting to appoint an address which is already a listed signer
+    error AlreadyListed();
+
     /// @notice Raised when attempting to appoint an already appointed address
     error AlreadyAppointed();
 
@@ -42,7 +45,7 @@ interface IEncryptionRegistry {
 
     /// @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 appointedBy(address wallet) external returns (address appointerAddress);
+    function appointerOf(address wallet) external returns (address appointerAddress);
 
     /// @notice Returns the address of the account registered at the given index
     function registeredAccounts(uint256) external view returns (address);