feat: move nullifier hashes out of SemaphoreCore

Former-commit-id: f57e21c77d
This commit is contained in:
cedoor
2022-09-19 16:36:30 +02:00
parent 0c9c0c9791
commit 7329ca6a48
8 changed files with 58 additions and 52 deletions

View File

@@ -11,16 +11,13 @@ contract Semaphore is ISemaphore, SemaphoreCore, SemaphoreGroups {
/// @dev Gets a tree depth and returns its verifier address.
mapping(uint256 => IVerifier) public verifiers;
/// @dev Gets a group id and returns the group admin address.
mapping(uint256 => address) public groupAdmins;
/// @dev Gets a group id and returns data to check if a Merkle root is expired.
mapping(uint256 => MerkleTreeExpiry) public merkleTreeExpiries;
/// @dev Gets a group id and returns the group parameters.
mapping(uint256 => Group) public groups;
/// @dev Checks if the group admin is the transaction sender.
/// @param groupId: Id of the group.
modifier onlyGroupAdmin(uint256 groupId) {
if (groupAdmins[groupId] != _msgSender()) {
if (groups[groupId].admin != _msgSender()) {
revert Semaphore__CallerIsNotTheGroupAdmin();
}
_;
@@ -56,8 +53,8 @@ contract Semaphore is ISemaphore, SemaphoreCore, SemaphoreGroups {
) external override onlySupportedMerkleTreeDepth(merkleTreeDepth) {
_createGroup(groupId, merkleTreeDepth, zeroValue);
groupAdmins[groupId] = admin;
merkleTreeExpiries[groupId].rootDuration = 1 hours;
groups[groupId].admin = admin;
groups[groupId].merkleRootDuration = 1 hours;
emit GroupAdminUpdated(groupId, address(0), admin);
}
@@ -72,15 +69,15 @@ contract Semaphore is ISemaphore, SemaphoreCore, SemaphoreGroups {
) external override onlySupportedMerkleTreeDepth(merkleTreeDepth) {
_createGroup(groupId, merkleTreeDepth, zeroValue);
groupAdmins[groupId] = admin;
merkleTreeExpiries[groupId].rootDuration = merkleTreeRootDuration;
groups[groupId].admin = admin;
groups[groupId].merkleRootDuration = merkleTreeRootDuration;
emit GroupAdminUpdated(groupId, address(0), admin);
}
/// @dev See {ISemaphore-updateGroupAdmin}.
function updateGroupAdmin(uint256 groupId, address newAdmin) external override onlyGroupAdmin(groupId) {
groupAdmins[groupId] = newAdmin;
groups[groupId].admin = newAdmin;
emit GroupAdminUpdated(groupId, _msgSender(), newAdmin);
}
@@ -91,7 +88,7 @@ contract Semaphore is ISemaphore, SemaphoreCore, SemaphoreGroups {
uint256 merkleTreeRoot = getMerkleTreeRoot(groupId);
merkleTreeExpiries[groupId].rootCreationDates[merkleTreeRoot] = block.timestamp;
groups[groupId].merkleRootCreationDates[merkleTreeRoot] = block.timestamp;
}
/// @dev See {ISemaphore-addMembers}.
@@ -110,7 +107,7 @@ contract Semaphore is ISemaphore, SemaphoreCore, SemaphoreGroups {
uint256 merkleTreeRoot = getMerkleTreeRoot(groupId);
merkleTreeExpiries[groupId].rootCreationDates[merkleTreeRoot] = block.timestamp;
groups[groupId].merkleRootCreationDates[merkleTreeRoot] = block.timestamp;
}
/// @dev See {ISemaphore-updateMember}.
@@ -150,25 +147,29 @@ contract Semaphore is ISemaphore, SemaphoreCore, SemaphoreGroups {
}
if (merkleTreeRoot != currentMerkleTreeRoot) {
uint256 rootCreationDate = merkleTreeExpiries[groupId].rootCreationDates[merkleTreeRoot];
uint256 rootDuration = merkleTreeExpiries[groupId].rootDuration;
uint256 merkleRootCreationDate = groups[groupId].merkleRootCreationDates[merkleTreeRoot];
uint256 merkleRootDuration = groups[groupId].merkleRootDuration;
if (rootCreationDate == 0) {
if (merkleRootCreationDate == 0) {
revert Semaphore__MerkleTreeRootIsNotPartOfTheGroup();
}
if (block.timestamp > rootCreationDate + rootDuration) {
if (block.timestamp > merkleRootCreationDate + merkleRootDuration) {
revert Semaphore__MerkleTreeRootIsExpired();
}
}
if (groups[groupId].nullifierHashes[nullifierHash]) {
revert Semaphore__YouAreUsingTheSameNillifierTwice();
}
uint256 merkleTreeDepth = getMerkleTreeDepth(groupId);
IVerifier verifier = verifiers[merkleTreeDepth];
_verifyProof(signal, merkleTreeRoot, nullifierHash, externalNullifier, proof, verifier);
_saveNullifierHash(uint256(keccak256(abi.encodePacked(groupId, nullifierHash))));
groups[groupId].nullifierHashes[nullifierHash] = true;
emit ProofVerified(groupId, merkleTreeRoot, nullifierHash, externalNullifier, signal);
}

View File

@@ -10,10 +10,6 @@ import "../interfaces/IVerifier.sol";
/// nullifier to prevent double-signaling. External nullifier and Merkle trees (i.e. groups) must be
/// managed externally.
contract SemaphoreCore is ISemaphoreCore {
/// @dev Gets a nullifier hash and returns true or false.
/// It is used to prevent double-signaling.
mapping(uint256 => bool) internal nullifierHashes;
/// @dev Asserts that no nullifier already exists and if the zero-knowledge proof is valid.
/// Otherwise it reverts.
/// @param signal: Semaphore signal.
@@ -30,10 +26,6 @@ contract SemaphoreCore is ISemaphoreCore {
uint256[8] calldata proof,
IVerifier verifier
) internal view {
if (nullifierHashes[nullifierHash]) {
revert Semaphore__YouAreUsingTheSameNillifierTwice();
}
uint256 signalHash = _hashSignal(signal);
verifier.verifyProof(
@@ -44,14 +36,6 @@ contract SemaphoreCore is ISemaphoreCore {
);
}
/// @dev Stores the nullifier hash to prevent double-signaling.
/// Attention! Remember to call it when you verify a proof if you
/// need to prevent double-signaling.
/// @param nullifierHash: Semaphore nullifier hash.
function _saveNullifierHash(uint256 nullifierHash) internal {
nullifierHashes[nullifierHash] = true;
}
/// @dev Creates a keccak256 hash of the signal.
/// @param signal: Semaphore signal.
/// @return Hash of the signal.

View File

@@ -12,8 +12,8 @@ import "@openzeppelin/contracts/utils/Context.sol";
abstract contract SemaphoreGroups is Context, ISemaphoreGroups {
using IncrementalBinaryTree for IncrementalTreeData;
/// @dev Gets a group id and returns the group/tree data.
mapping(uint256 => IncrementalTreeData) internal groups;
/// @dev Gets a group id and returns the tree data.
mapping(uint256 => IncrementalTreeData) internal merkleTree;
/// @dev Creates a new group by initializing the associated tree.
/// @param groupId: Id of the group.
@@ -32,7 +32,7 @@ abstract contract SemaphoreGroups is Context, ISemaphoreGroups {
revert Semaphore__GroupAlreadyExists();
}
groups[groupId].init(merkleTreeDepth, zeroValue);
merkleTree[groupId].init(merkleTreeDepth, zeroValue);
emit GroupCreated(groupId, merkleTreeDepth, zeroValue);
}
@@ -45,7 +45,7 @@ abstract contract SemaphoreGroups is Context, ISemaphoreGroups {
revert Semaphore__GroupDoesNotExist();
}
groups[groupId].insert(identityCommitment);
merkleTree[groupId].insert(identityCommitment);
uint256 merkleTreeRoot = getMerkleTreeRoot(groupId);
uint256 index = getNumberOfMerkleTreeLeaves(groupId) - 1;
@@ -71,7 +71,7 @@ abstract contract SemaphoreGroups is Context, ISemaphoreGroups {
revert Semaphore__GroupDoesNotExist();
}
groups[groupId].update(identityCommitment, newIdentityCommitment, proofSiblings, proofPathIndices);
merkleTree[groupId].update(identityCommitment, newIdentityCommitment, proofSiblings, proofPathIndices);
uint256 merkleTreeRoot = getMerkleTreeRoot(groupId);
uint256 index = proofPathIndicesToMemberIndex(proofPathIndices);
@@ -95,7 +95,7 @@ abstract contract SemaphoreGroups is Context, ISemaphoreGroups {
revert Semaphore__GroupDoesNotExist();
}
groups[groupId].remove(identityCommitment, proofSiblings, proofPathIndices);
merkleTree[groupId].remove(identityCommitment, proofSiblings, proofPathIndices);
uint256 merkleTreeRoot = getMerkleTreeRoot(groupId);
uint256 index = proofPathIndicesToMemberIndex(proofPathIndices);
@@ -105,17 +105,17 @@ abstract contract SemaphoreGroups is Context, ISemaphoreGroups {
/// @dev See {ISemaphoreGroups-getMerkleTreeRoot}.
function getMerkleTreeRoot(uint256 groupId) public view virtual override returns (uint256) {
return groups[groupId].root;
return merkleTree[groupId].root;
}
/// @dev See {ISemaphoreGroups-getMerkleTreeDepth}.
function getMerkleTreeDepth(uint256 groupId) public view virtual override returns (uint256) {
return groups[groupId].depth;
return merkleTree[groupId].depth;
}
/// @dev See {ISemaphoreGroups-getNumberOfMerkleTreeLeaves}.
function getNumberOfMerkleTreeLeaves(uint256 groupId) public view virtual override returns (uint256) {
return groups[groupId].numberOfLeaves;
return merkleTree[groupId].numberOfLeaves;
}
/// @dev Converts the path indices of a Merkle proof to the identity commitment index in the tree.

View File

@@ -14,6 +14,10 @@ contract SemaphoreVoting is ISemaphoreVoting, SemaphoreCore, SemaphoreGroups {
/// @dev Gets a poll id and returns the poll data.
mapping(uint256 => Poll) internal polls;
/// @dev Gets a nullifier hash and returns true or false.
/// It is used to prevent double-voting.
mapping(uint256 => bool) internal nullifierHashes;
/// @dev Initializes the Semaphore verifiers used to verify the user's ZK proofs.
/// @param _verifiers: List of Semaphore verifiers (address and related Merkle tree depth).
constructor(Verifier[] memory _verifiers) {
@@ -90,6 +94,10 @@ contract SemaphoreVoting is ISemaphoreVoting, SemaphoreCore, SemaphoreGroups {
revert Semaphore__PollIsNotOngoing();
}
if (nullifierHashes[nullifierHash]) {
revert Semaphore__YouAreUsingTheSameNillifierTwice();
}
uint256 merkleTreeDepth = getMerkleTreeDepth(pollId);
uint256 merkleTreeRoot = getMerkleTreeRoot(pollId);
@@ -97,8 +105,7 @@ contract SemaphoreVoting is ISemaphoreVoting, SemaphoreCore, SemaphoreGroups {
_verifyProof(vote, merkleTreeRoot, nullifierHash, pollId, proof, verifier);
// Prevent double-voting (nullifierHash = hash(pollId + identityNullifier)).
_saveNullifierHash(nullifierHash);
nullifierHashes[nullifierHash] = true;
emit VoteAdded(pollId, vote);
}

View File

@@ -8,17 +8,19 @@ interface ISemaphore {
error Semaphore__MerkleTreeDepthIsNotSupported();
error Semaphore__MerkleTreeRootIsExpired();
error Semaphore__MerkleTreeRootIsNotPartOfTheGroup();
error Semaphore__YouAreUsingTheSameNillifierTwice();
struct Verifier {
address contractAddress;
uint256 merkleTreeDepth;
}
/// It defines all the parameters needed to check whether a
/// zero-knowledge proof generated with a certain Merkle tree is still valid.
struct MerkleTreeExpiry {
uint256 rootDuration;
mapping(uint256 => uint256) rootCreationDates;
/// It defines all the group parameters, in addition to those in the Merkle tree.
struct Group {
address admin;
uint256 merkleRootDuration;
mapping(uint256 => uint256) merkleRootCreationDates;
mapping(uint256 => bool) nullifierHashes;
}
/// @dev Emitted when an admin is assigned to a group.

View File

@@ -4,8 +4,6 @@ pragma solidity 0.8.4;
/// @title SemaphoreCore interface.
/// @dev Interface of SemaphoreCore contract.
interface ISemaphoreCore {
error Semaphore__YouAreUsingTheSameNillifierTwice();
/// @notice Emitted when a proof is verified correctly and a new nullifier hash is added.
/// @param nullifierHash: Hash of external and identity nullifiers.
event NullifierHashAdded(uint256 nullifierHash);

View File

@@ -8,6 +8,7 @@ interface ISemaphoreVoting {
error Semaphore__MerkleTreeDepthIsNotSupported();
error Semaphore__PollHasAlreadyBeenStarted();
error Semaphore__PollIsNotOngoing();
error Semaphore__YouAreUsingTheSameNillifierTwice();
enum PollState {
Created,

View File

@@ -252,6 +252,19 @@ describe("Semaphore", () => {
)
})
it("Should not verify the same proof for an onchain group twice", async () => {
const transaction = contract.verifyProof(
groupId,
group.root,
signal,
fullProof.publicSignals.nullifierHash,
fullProof.publicSignals.merkleRoot,
solidityProof
)
await expect(transaction).to.be.revertedWith("Semaphore__YouAreUsingTheSameNillifierTwice()")
})
it("Should not verify a proof if the Merkle tree root is expired", async () => {
const groupId = 2
const group = new Group(treeDepth)