Merge pull request #732 from semaphore-protocol/fix/group-checks

Add checks for removed members

Former-commit-id: 2faac4e30790a049a8377807ab507d980fa21b31

.yarn/releases/yarn-4.1.0.cjs.REMOVED.git-id

@@ -1 +0,0 @@
-738adce5914a0e193f2e1255e4dcf7042256a1c1

packages/cli-template-contracts-hardhat/.yarn/releases/yarn-4.1.0.cjs.REMOVED.git-id

@@ -1 +0,0 @@
-738adce5914a0e193f2e1255e4dcf7042256a1c1

packages/cli-template-monorepo-ethers/.yarn/releases/yarn-4.1.0.cjs.REMOVED.git-id

@@ -1 +0,0 @@
-738adce5914a0e193f2e1255e4dcf7042256a1c1

packages/cli-template-monorepo-subgraph/.yarn/releases/yarn-4.1.0.cjs.REMOVED.git-id

@@ -1 +0,0 @@
-738adce5914a0e193f2e1255e4dcf7042256a1c1

packages/group/src/index.ts

@@ -72,6 +72,10 @@ export class Group {
      * @param member The new member to be added.
      */
     public addMember(member: BigNumber) {
+        if (member === 0n || member === "0") {
+            throw new Error("Failed to add member: value cannot be 0")
+        }
+
         this.leanIMT.insert(BigInt(member))
     }
 
@@ -80,6 +84,12 @@ export class Group {
      * @param members New members.
      */
     public addMembers(members: BigNumber[]) {
+        for (const member of members) {
+            if (member === 0n || member === "0") {
+                throw new Error("Failed to add member: value cannot be 0")
+            }
+        }
+
         this.leanIMT.insertMany(members.map(BigInt))
     }
 
@@ -89,6 +99,10 @@ export class Group {
      * @param member New member value.
      */
     public updateMember(index: number, member: BigNumber) {
+        if (this.members[index] === 0n) {
+            throw new Error("Failed to update member: it has been removed")
+        }
+
         this.leanIMT.update(index, BigInt(member))
     }
 
@@ -97,7 +111,11 @@ export class Group {
      * @param index The index of the member to be removed.
      */
     public removeMember(index: number) {
-        this.leanIMT.update(index, BigInt(0))
+        if (this.members[index] === 0n) {
+            throw new Error("Failed to remove member: it has already been removed")
+        }
+
+        this.leanIMT.update(index, 0n)
     }
 
     /**
@@ -105,8 +123,8 @@ export class Group {
      * @param index The index of the member.
      * @returns The {@link MerkleProof} object.
      */
-    public generateMerkleProof(_index: number): LeanIMTMerkleProof {
-        return this.leanIMT.generateProof(_index)
+    public generateMerkleProof(index: number): LeanIMTMerkleProof {
+        return this.leanIMT.generateProof(index)
     }
 
     /**

packages/group/tests/index.test.ts

@@ -33,6 +33,14 @@ describe("Group", () => {
 
             expect(group.size).toBe(1)
         })
+
+        it("Should not add a member to a group if its value is 0", () => {
+            const group = new Group()
+
+            const fun = () => group.addMember(0n)
+
+            expect(fun).toThrow("Failed to add member: value cannot be 0")
+        })
     })
 
     describe("# addMembers", () => {
@@ -43,6 +51,14 @@ describe("Group", () => {
 
             expect(group.size).toBe(2)
         })
+
+        it("Should not add many members to a group if any value is 0", () => {
+            const group = new Group()
+
+            const fun = () => group.addMembers([1n, 0n])
+
+            expect(fun).toThrow("Failed to add member: value cannot be 0")
+        })
     })
 
     describe("# indexOf", () => {
@@ -66,6 +82,16 @@ describe("Group", () => {
             expect(group.size).toBe(2)
             expect(group.members[0]).toBe(1n)
         })
+
+        it("Should not update a member in a group if it has previously been removed", () => {
+            const group = new Group()
+            group.addMembers([1n, 3n])
+            group.removeMember(0)
+
+            const fun = () => group.updateMember(0, 1n)
+
+            expect(fun).toThrow("Failed to update member: it has been removed")
+        })
     })
 
     describe("# removeMember", () => {
@@ -78,12 +104,21 @@ describe("Group", () => {
             expect(group.size).toBe(2)
             expect(group.members[0]).toBe(0n)
         })
+
+        it("Should not remove a member from a group if it has already been removed", () => {
+            const group = new Group()
+            group.addMembers([1n, 3n])
+            group.removeMember(0)
+
+            const fun = () => group.removeMember(0)
+
+            expect(fun).toThrow("Failed to remove member: it has already been removed")
+        })
     })
 
     describe("# generateMerkleProof", () => {
         it("Should generate a proof of membership", () => {
             const group = new Group()
-
             group.addMembers([1n, 3n])
 
             const proof = group.generateMerkleProof(0)

yarn.lock.REMOVED.git-id

@@ -1 +0,0 @@
-63dcf3bdb3a777690dd40e2f08ed264dcba3a4db