feat(contracts)!: create a two-step mechanism to update group admins

The best practice to update a group admin is through a two-step update. The existing admin assigns
the new potential admin, and the new admin accepts in a separate transaction. This prevents existing
admins from making mistakes and assigning wrong addresses.

re #690

packages/contracts/README.md

@@ -1 +1 @@
-contracts/README.md
+contracts/README.md

packages/contracts/contracts/Semaphore.sol

@@ -56,6 +56,11 @@ contract Semaphore is ISemaphore, SemaphoreGroups {
         _updateGroupAdmin(groupId, newAdmin);
     }
 
+    /// @dev See {SemaphoreGroups- acceptGroupAdmin}.
+    function acceptGroupAdmin(uint256 groupId) external override {
+        _acceptGroupAdmin(groupId);
+    }
+
     /// @dev See {ISemaphore-updateGroupMerkleTreeDuration}.
     function updateGroupMerkleTreeDuration(
         uint256 groupId,

packages/contracts/contracts/base/SemaphoreGroups.sol

@@ -19,6 +19,10 @@ abstract contract SemaphoreGroups is ISemaphoreGroups {
     /// The admin can be an Ethereum account or a smart contract.
     mapping(uint256 => address) internal admins;
 
+    /// @dev Gets a group id and returns any pending admin.
+    /// The pending admin can be an Ethereum account or a smart contract.
+    mapping(uint256 => address) internal pendingAdmins;
+
     /// @dev Checks if the group admin is the transaction sender.
     /// @param groupId: Id of the group.
     modifier onlyGroupAdmin(uint256 groupId) {
@@ -48,13 +52,30 @@ abstract contract SemaphoreGroups is ISemaphoreGroups {
         emit GroupAdminUpdated(groupId, address(0), admin);
     }
 
-    /// @dev Updates the group admin.
+    /// @dev Updates the group admin. In order for the new admin to actually be updated,
+    /// they must explicitly accept by calling `_acceptGroupAdmin`.
     /// @param groupId: Id of the group.
     /// @param newAdmin: New admin of the group.
     function _updateGroupAdmin(uint256 groupId, address newAdmin) internal virtual onlyGroupAdmin(groupId) {
-        admins[groupId] = newAdmin;
+        pendingAdmins[groupId] = newAdmin;
+
+        emit GroupAdminPending(groupId, msg.sender, newAdmin);
+    }
+
+    /// @dev Allows the new admin to accept to update the group admin with their address.
+    /// @param groupId: Id of the group.
+    function _acceptGroupAdmin(uint256 groupId) internal virtual {
+        if (pendingAdmins[groupId] != msg.sender) {
+            revert Semaphore__CallerIsNotThePendingGroupAdmin();
+        }
+
+        address oldAdmin = admins[groupId];
+
+        admins[groupId] = msg.sender;
+
+        delete pendingAdmins[groupId];
 
-        emit GroupAdminUpdated(groupId, msg.sender, newAdmin);
+        emit GroupAdminUpdated(groupId, oldAdmin, msg.sender);
     }
 
     /// @dev Adds an identity commitment to an existing group.

packages/contracts/contracts/interfaces/ISemaphore.sol

@@ -70,6 +70,9 @@ interface ISemaphore {
     /// @dev See {SemaphoreGroups-_updateGroupAdmin}.
     function updateGroupAdmin(uint256 groupId, address newAdmin) external;
 
+    /// @dev See {SemaphoreGroups-_acceptGroupAdmin}.
+    function acceptGroupAdmin(uint256 groupId) external;
+
     /// @dev Updates the group Merkle tree duration.
     /// @param groupId: Id of the group.
     /// @param newMerkleTreeDuration: New Merkle tree duration.

packages/contracts/contracts/interfaces/ISemaphoreGroups.sol

@@ -5,17 +5,24 @@ pragma solidity 0.8.23;
 interface ISemaphoreGroups {
     error Semaphore__GroupDoesNotExist();
     error Semaphore__CallerIsNotTheGroupAdmin();
+    error Semaphore__CallerIsNotThePendingGroupAdmin();
 
     /// @dev Event emitted when a new group is created.
     /// @param groupId: Id of the group.
     event GroupCreated(uint256 indexed groupId);
 
-    /// @dev Event emitted when an admin is assigned to a group.
+    /// @dev Event emitted when a new admin is assigned to a group.
     /// @param groupId: Id of the group.
     /// @param oldAdmin: Old admin of the group.
     /// @param newAdmin: New admin of the group.
     event GroupAdminUpdated(uint256 indexed groupId, address indexed oldAdmin, address indexed newAdmin);
 
+    /// @dev Event emitted when a group admin is being updated.
+    /// @param groupId: Id of the group.
+    /// @param oldAdmin: Old admin of the group.
+    /// @param newAdmin: New admin of the group.
+    event GroupAdminPending(uint256 indexed groupId, address indexed oldAdmin, address indexed newAdmin);
+
     /// @dev Event emitted when a new identity commitment is added.
     /// @param groupId: Group id of the group.
     /// @param index: Merkle tree leaf index.

packages/contracts/test/Semaphore.ts

@@ -87,7 +87,7 @@ describe("Semaphore", () => {
     })
 
     describe("# updateGroupAdmin", () => {
-        it("Should not update a group admin if the caller is not the group admin", async () => {
+        it("Should not update an admin if the caller is not the admin", async () => {
             const transaction = semaphoreContract.updateGroupAdmin(groupId, accountAddresses[0])
 
             await expect(transaction).to.be.revertedWithCustomError(
@@ -96,9 +96,26 @@ describe("Semaphore", () => {
             )
         })
 
-        it("Should update the group admin", async () => {
+        it("Should update the admin", async () => {
             const transaction = semaphoreContract.connect(accounts[1]).updateGroupAdmin(groupId, accountAddresses[0])
 
+            await expect(transaction)
+                .to.emit(semaphoreContract, "GroupAdminPending")
+                .withArgs(groupId, accountAddresses[1], accountAddresses[0])
+        })
+
+        it("Should not accept accept the new admin if the caller is not the new admin", async () => {
+            const transaction = semaphoreContract.connect(accounts[2]).acceptGroupAdmin(groupId)
+
+            await expect(transaction).to.be.revertedWithCustomError(
+                semaphoreContract,
+                "Semaphore__CallerIsNotThePendingGroupAdmin"
+            )
+        })
+
+        it("Should accept the new admin", async () => {
+            const transaction = semaphoreContract.acceptGroupAdmin(groupId)
+
             await expect(transaction)
                 .to.emit(semaphoreContract, "GroupAdminUpdated")
                 .withArgs(groupId, accountAddresses[1], accountAddresses[0])