Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom nullifier hashes #138

Merged
merged 1 commit into from
Sep 19, 2022
Merged

Custom nullifier hashes #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 19 additions & 18 deletions packages/contracts/contracts/Semaphore.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
_;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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}.
Expand All @@ -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}.
Expand Down Expand Up @@ -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);
}
Expand Down
16 changes: 0 additions & 16 deletions packages/contracts/contracts/base/SemaphoreCore.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(
Expand All @@ -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.
Expand Down
18 changes: 9 additions & 9 deletions packages/contracts/contracts/base/SemaphoreGroups.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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.
Expand Down
11 changes: 9 additions & 2 deletions packages/contracts/contracts/extensions/SemaphoreVoting.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -90,15 +94,18 @@ contract SemaphoreVoting is ISemaphoreVoting, SemaphoreCore, SemaphoreGroups {
revert Semaphore__PollIsNotOngoing();
}

if (nullifierHashes[nullifierHash]) {
revert Semaphore__YouAreUsingTheSameNillifierTwice();
}

uint256 merkleTreeDepth = getMerkleTreeDepth(pollId);
uint256 merkleTreeRoot = getMerkleTreeRoot(pollId);

IVerifier verifier = verifiers[merkleTreeDepth];

_verifyProof(vote, merkleTreeRoot, nullifierHash, pollId, proof, verifier);

// Prevent double-voting (nullifierHash = hash(pollId + identityNullifier)).
_saveNullifierHash(nullifierHash);
nullifierHashes[nullifierHash] = true;

emit VoteAdded(pollId, vote);
}
Expand Down
12 changes: 7 additions & 5 deletions packages/contracts/contracts/interfaces/ISemaphore.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 0 additions & 2 deletions packages/contracts/contracts/interfaces/ISemaphoreCore.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/contracts/contracts/interfaces/ISemaphoreVoting.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ interface ISemaphoreVoting {
error Semaphore__MerkleTreeDepthIsNotSupported();
error Semaphore__PollHasAlreadyBeenStarted();
error Semaphore__PollIsNotOngoing();
error Semaphore__YouAreUsingTheSameNillifierTwice();

enum PollState {
Created,
Expand Down
13 changes: 13 additions & 0 deletions packages/contracts/test/Semaphore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down