Skip to content

Commit 5726c98

Browse files
cedoorcedoor
authored
Merge pull request #197 from semaphore-protocol/fix/zero-values
Generate zeroValue based on groupId
2 parents f7367b6 + 2188677 commit 5726c98

File tree

15 files changed

+80
-63
lines changed

15 files changed

+80
-63
lines changed

‎packages/contracts/contracts/Semaphore.sol

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,9 @@ contract Semaphore is ISemaphore, SemaphoreGroups {
4040
function createGroup(
4141
uint256 groupId,
4242
uint256 merkleTreeDepth,
43-
uint256 zeroValue,
4443
address admin
4544
) external override onlySupportedMerkleTreeDepth(merkleTreeDepth) {
46-
_createGroup(groupId, merkleTreeDepth, zeroValue);
45+
_createGroup(groupId, merkleTreeDepth);
4746

4847
groups[groupId].admin = admin;
4948
groups[groupId].merkleTreeDuration = 1 hours;
@@ -55,11 +54,10 @@ contract Semaphore is ISemaphore, SemaphoreGroups {
5554
function createGroup(
5655
uint256 groupId,
5756
uint256 merkleTreeDepth,
58-
uint256 zeroValue,
5957
address admin,
6058
uint256 merkleTreeDuration
6159
) external override onlySupportedMerkleTreeDepth(merkleTreeDepth) {
62-
_createGroup(groupId, merkleTreeDepth, zeroValue);
60+
_createGroup(groupId, merkleTreeDepth);
6361

6462
groups[groupId].admin = admin;
6563
groups[groupId].merkleTreeDuration = merkleTreeDuration;

‎packages/contracts/contracts/base/SemaphoreGroups.sol

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,16 @@ abstract contract SemaphoreGroups is Context, ISemaphoreGroups {
1717
/// @dev Creates a new group by initializing the associated tree.
1818
/// @param groupId: Id of the group.
1919
/// @param merkleTreeDepth: Depth of the tree.
20-
/// @param zeroValue: Zero value of the tree.
21-
function _createGroup(
22-
uint256 groupId,
23-
uint256 merkleTreeDepth,
24-
uint256 zeroValue
25-
) internal virtual {
20+
function _createGroup(uint256 groupId, uint256 merkleTreeDepth) internal virtual {
2621
if (getMerkleTreeDepth(groupId) != 0) {
2722
revert Semaphore__GroupAlreadyExists();
2823
}
2924

25+
// The zeroValue is in fact an implicit member of the group.
26+
// Although there is a remote possibility that the preimage of
27+
// the hash may be calculated, using this value minimizes the risk.
28+
uint256 zeroValue = uint256(keccak256(abi.encodePacked(groupId))) >> 8;
29+
3030
merkleTree[groupId].init(merkleTreeDepth, zeroValue);
3131

3232
emit GroupCreated(groupId, merkleTreeDepth, zeroValue);

‎packages/contracts/contracts/extensions/SemaphoreVoting.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ contract SemaphoreVoting is ISemaphoreVoting, SemaphoreGroups {
4343
revert Semaphore__MerkleTreeDepthIsNotSupported();
4444
}
4545

46-
_createGroup(pollId, merkleTreeDepth, 0);
46+
_createGroup(pollId, merkleTreeDepth);
4747

4848
Poll memory poll;
4949

‎packages/contracts/contracts/extensions/SemaphoreWhistleblowing.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ contract SemaphoreWhistleblowing is ISemaphoreWhistleblowing, SemaphoreGroups {
4141
revert Semaphore__MerkleTreeDepthIsNotSupported();
4242
}
4343

44-
_createGroup(entityId, merkleTreeDepth, 0);
44+
_createGroup(entityId, merkleTreeDepth);
4545

4646
entities[entityId] = editor;
4747

‎packages/contracts/contracts/interfaces/ISemaphore.sol

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,25 +68,21 @@ interface ISemaphore {
6868
/// @dev Creates a new group. Only the admin will be able to add or remove members.
6969
/// @param groupId: Id of the group.
7070
/// @param depth: Depth of the tree.
71-
/// @param zeroValue: Zero value of the tree.
7271
/// @param admin: Admin of the group.
7372
function createGroup(
7473
uint256 groupId,
7574
uint256 depth,
76-
uint256 zeroValue,
7775
address admin
7876
) external;
7977

8078
/// @dev Creates a new group. Only the admin will be able to add or remove members.
8179
/// @param groupId: Id of the group.
8280
/// @param depth: Depth of the tree.
83-
/// @param zeroValue: Zero value of the tree.
8481
/// @param admin: Admin of the group.
8582
/// @param merkleTreeRootDuration: Time before the validity of a root expires.
8683
function createGroup(
8784
uint256 groupId,
8885
uint256 depth,
89-
uint256 zeroValue,
9086
address admin,
9187
uint256 merkleTreeRootDuration
9288
) external;

‎packages/contracts/test/Semaphore.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ describe("Semaphore", () => {
1717

1818
const treeDepth = Number(process.env.TREE_DEPTH) || 20
1919
const groupId = 1
20+
const group = new Group(groupId, treeDepth)
2021
const members = createIdentityCommitments(3)
2122

2223
const wasmFilePath = `../../snark-artifacts/${treeDepth}/semaphore.wasm`
@@ -36,12 +37,7 @@ describe("Semaphore", () => {
3637

3738
describe("# createGroup", () => {
3839
it("Should not create a group if the tree depth is not supported", async () => {
39-
const transaction = semaphoreContract["createGroup(uint256,uint256,uint256,address)"](
40-
groupId,
41-
10,
42-
0,
43-
accounts[0]
44-
)
40+
const transaction = semaphoreContract["createGroup(uint256,uint256,address)"](groupId, 10, accounts[0])
4541

4642
await expect(transaction).to.be.revertedWithCustomError(
4743
semaphoreContract,
@@ -52,30 +48,34 @@ describe("Semaphore", () => {
5248
it("Should create a group", async () => {
5349
const transaction = semaphoreContract
5450
.connect(signers[1])
55-
["createGroup(uint256,uint256,uint256,address)"](groupId, treeDepth, 0, accounts[1])
51+
["createGroup(uint256,uint256,address)"](groupId, treeDepth, accounts[1])
5652

57-
await expect(transaction).to.emit(semaphoreContract, "GroupCreated").withArgs(groupId, treeDepth, 0)
53+
await expect(transaction)
54+
.to.emit(semaphoreContract, "GroupCreated")
55+
.withArgs(groupId, treeDepth, group.zeroValue)
5856
await expect(transaction)
5957
.to.emit(semaphoreContract, "GroupAdminUpdated")
6058
.withArgs(groupId, constants.AddressZero, accounts[1])
6159
})
6260

6361
it("Should create a group with a custom Merkle tree root expiration", async () => {
6462
const groupId = 2
63+
const group = new Group(2)
6564
const transaction = await semaphoreContract
6665
.connect(signers[1])
67-
["createGroup(uint256,uint256,uint256,address,uint256)"](
66+
["createGroup(uint256,uint256,address,uint256)"](
6867
groupId,
6968
treeDepth,
70-
0,
7169
accounts[0],
7270
5 // 5 seconds.
7371
)
7472
await semaphoreContract.addMember(groupId, members[0])
7573
await semaphoreContract.addMember(groupId, members[1])
7674
await semaphoreContract.addMember(groupId, members[2])
7775

78-
await expect(transaction).to.emit(semaphoreContract, "GroupCreated").withArgs(groupId, treeDepth, 0)
76+
await expect(transaction)
77+
.to.emit(semaphoreContract, "GroupCreated")
78+
.withArgs(groupId, treeDepth, group.zeroValue)
7979
await expect(transaction)
8080
.to.emit(semaphoreContract, "GroupAdminUpdated")
8181
.withArgs(groupId, constants.AddressZero, accounts[0])
@@ -133,7 +133,7 @@ describe("Semaphore", () => {
133133
})
134134

135135
it("Should add a new member in an existing group", async () => {
136-
const group = new Group(treeDepth)
136+
const group = new Group(groupId, treeDepth)
137137

138138
group.addMember(members[0])
139139

@@ -149,11 +149,11 @@ describe("Semaphore", () => {
149149
it("Should add new members to an existing group", async () => {
150150
const groupId = 3
151151
const members = [BigInt(1), BigInt(2), BigInt(3)]
152-
const group = new Group(treeDepth)
152+
const group = new Group(groupId, treeDepth)
153153

154154
group.addMembers(members)
155155

156-
await semaphoreContract["createGroup(uint256,uint256,uint256,address)"](groupId, treeDepth, 0, accounts[0])
156+
await semaphoreContract["createGroup(uint256,uint256,address)"](groupId, treeDepth, accounts[0])
157157

158158
const transaction = semaphoreContract.addMembers(groupId, members)
159159

@@ -178,13 +178,13 @@ describe("Semaphore", () => {
178178
it("Should update a member from an existing group", async () => {
179179
const groupId = 4
180180
const members = [BigInt(1), BigInt(2), BigInt(3)]
181-
const group = new Group(treeDepth)
181+
const group = new Group(groupId, treeDepth)
182182

183183
group.addMembers(members)
184184

185185
group.updateMember(0, BigInt(4))
186186

187-
await semaphoreContract["createGroup(uint256,uint256,uint256,address)"](groupId, treeDepth, 0, accounts[0])
187+
await semaphoreContract["createGroup(uint256,uint256,address)"](groupId, treeDepth, accounts[0])
188188
await semaphoreContract.addMembers(groupId, members)
189189

190190
const { siblings, pathIndices, root } = group.generateMerkleProof(0)
@@ -212,13 +212,13 @@ describe("Semaphore", () => {
212212
it("Should remove a member from an existing group", async () => {
213213
const groupId = 5
214214
const members = [BigInt(1), BigInt(2), BigInt(3)]
215-
const group = new Group(treeDepth)
215+
const group = new Group(groupId, treeDepth)
216216

217217
group.addMembers(members)
218218

219219
group.removeMember(2)
220220

221-
await semaphoreContract["createGroup(uint256,uint256,uint256,address)"](groupId, treeDepth, 0, accounts[0])
221+
await semaphoreContract["createGroup(uint256,uint256,address)"](groupId, treeDepth, accounts[0])
222222
await semaphoreContract.addMembers(groupId, members)
223223

224224
const { siblings, pathIndices, root } = group.generateMerkleProof(2)
@@ -233,7 +233,7 @@ describe("Semaphore", () => {
233233
const signal = 2
234234
const identity = new Identity("0")
235235

236-
const group = new Group(treeDepth)
236+
const group = new Group(groupId, treeDepth)
237237

238238
group.addMembers(members)
239239

@@ -317,7 +317,7 @@ describe("Semaphore", () => {
317317

318318
it("Should not verify a proof if the Merkle tree root is expired", async () => {
319319
const groupId = 2
320-
const group = new Group(treeDepth)
320+
const group = new Group(groupId, treeDepth)
321321

322322
group.addMembers([members[0], members[1]])
323323

‎packages/contracts/test/SemaphoreVoting.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ describe("SemaphoreVoting", () => {
116116

117117
it("Should add a voter to an existing poll", async () => {
118118
const { commitment } = new Identity("test")
119-
const group = new Group(treeDepth)
119+
const group = new Group(pollIds[1], treeDepth)
120120

121121
group.addMember(commitment)
122122

@@ -138,7 +138,7 @@ describe("SemaphoreVoting", () => {
138138
const identity = new Identity("test")
139139
const vote = 1
140140

141-
const group = new Group(treeDepth)
141+
const group = new Group(pollIds[1], treeDepth)
142142

143143
group.addMembers([identity.commitment, BigInt(1)])
144144

‎packages/contracts/test/SemaphoreWhistleblowing.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ describe("SemaphoreWhistleblowing", () => {
7373

7474
it("Should add a whistleblower to an existing entity", async () => {
7575
const { commitment } = new Identity("test")
76-
const group = new Group(treeDepth)
76+
const group = new Group(entityIds[0], treeDepth)
7777

7878
group.addMember(commitment)
7979

@@ -96,7 +96,7 @@ describe("SemaphoreWhistleblowing", () => {
9696
describe("# removeWhistleblower", () => {
9797
it("Should not remove a whistleblower if the caller is not the editor", async () => {
9898
const { commitment } = new Identity()
99-
const group = new Group(treeDepth)
99+
const group = new Group(entityIds[0], treeDepth)
100100

101101
group.addMember(commitment)
102102

@@ -117,7 +117,7 @@ describe("SemaphoreWhistleblowing", () => {
117117

118118
it("Should remove a whistleblower from an existing entity", async () => {
119119
const { commitment } = new Identity("test")
120-
const group = new Group(treeDepth)
120+
const group = new Group(entityIds[0], treeDepth)
121121

122122
group.addMember(commitment)
123123

@@ -139,7 +139,7 @@ describe("SemaphoreWhistleblowing", () => {
139139
const identity = new Identity("test")
140140
const leak = utils.formatBytes32String("This is a leak")
141141

142-
const group = new Group(treeDepth)
142+
const group = new Group(entityIds[1], treeDepth)
143143

144144
group.addMembers([identity.commitment, BigInt(1)])
145145

‎packages/group/README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,19 @@ yarn add @semaphore-protocol/group
6767

6868
## 📜 Usage
6969

70-
\# **new Group**(treeDepth = 20, zeroValue = BigInt(0)): _Group_
70+
\# **new Group**(groupId: _Member_, treeDepth = 20): _Group_
7171

7272
```typescript
7373
import { Group } from "@semaphore-protocol/group"
7474

7575
// Group with max 1048576 members (20^²).
76-
const group1 = new Group()
76+
const group1 = new Group(1)
7777

7878
// Group with max 65536 members (16^²).
79-
const group2 = new Group(16)
79+
const group2 = new Group(1, 16)
8080

8181
// Group with max 16777216 members (24^²).
82-
const group3 = new Group(24)
82+
const group3 = new Group(1, 24)
8383
```
8484

8585
\# **addMember**(identityCommitment: _Member_)

‎packages/group/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
"typedoc": "^0.22.11"
3636
},
3737
"dependencies": {
38+
"@ethersproject/bignumber": "^5.7.0",
39+
"@ethersproject/bytes": "^5.7.0",
40+
"@ethersproject/keccak256": "^5.7.0",
3841
"@zk-kit/incremental-merkle-tree": "1.0.0",
3942
"poseidon-lite": "^0.0.2"
4043
}

0 commit comments

Comments
 (0)