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
This commit is contained in:
cedoor
2024-03-21 15:51:06 +00:00
parent d147958c67
commit 255bccf2eb
6 changed files with 60 additions and 7 deletions

View File

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

View File

@@ -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,

View File

@@ -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 GroupAdminUpdated(groupId, msg.sender, 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, oldAdmin, msg.sender);
}
/// @dev Adds an identity commitment to an existing group.

View File

@@ -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.

View File

@@ -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.

View File

@@ -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])