fix(data): use member index from event instead of array position in S… (#1081)

fix(data): use member index from event instead of array position in SemaphoreEthers.getGroupMembers

Author: johnson86tw

packages/data/src/ethers.ts

@@ -219,21 +219,32 @@ export default class SemaphoreEthers {
 
         const memberAddedEvents = await getEvents(this._contract, "MemberAdded", [groupId], this._options.startBlock)
 
+        // Create a map from member index to identity commitment for single additions
+        const memberAddedEventsMap = new Map<string, string>()
+
+        for (const [, index, identityCommitment] of memberAddedEvents) {
+            memberAddedEventsMap.set(index.toString(), identityCommitment.toString())
+        }
+
         const members: string[] = []
 
         const merkleTreeSize = await this._contract.getMerkleTreeSize(groupId)
 
         let i = 0
 
         while (i < Number(merkleTreeSize)) {
-            const identityCommitments = membersAddedEventsMap.get(i.toString())
+            const batchIdentityCommitments = membersAddedEventsMap.get(i.toString())
 
-            if (identityCommitments) {
-                members.push(...identityCommitments)
+            if (batchIdentityCommitments) {
+                members.push(...batchIdentityCommitments)
 
-                i += identityCommitments.length
+                i += batchIdentityCommitments.length
             } else {
-                members.push(memberAddedEvents[i][2])
+                const singleIdentityCommitment = memberAddedEventsMap.get(i.toString())
+
+                if (singleIdentityCommitment) {
+                    members.push(singleIdentityCommitment)
+                }
 
                 i += 1
             }

packages/data/tests/ethers.test.ts

@@ -171,10 +171,10 @@ describe("SemaphoreEthers", () => {
             )
             getEventsMocked.mockReturnValueOnce(
                 Promise.resolve([
-                    [, , "110"],
-                    [, , "111"],
-                    [, , "112"],
-                    [, , "113"]
+                    [, "0", "110"],
+                    [, "1", "111"],
+                    [, "2", "112"],
+                    [, "3", "113"]
                 ])
             )
 
@@ -199,6 +199,33 @@ describe("SemaphoreEthers", () => {
 
             await expect(fun).rejects.toThrow("Group '666' not found")
         })
+
+        it("Should correctly handle mixed batch and single additions where single additions don't align with array index", async () => {
+            // Scenario: Batch add members at index 0-1, then single add at index 2
+            // The MemberAdded events array will have only 1 element (for index 2),
+            ContractMocked.mockReturnValueOnce({
+                getGroupAdmin: () => "0xA9C2B639a28cDa8b59C4377e980F75A93dD8605F",
+                getMerkleTreeSize: () => BigInt(3)
+            } as any)
+
+            const semaphore = new SemaphoreEthers()
+
+            // MemberRemoved events - none
+            getEventsMocked.mockReturnValueOnce(Promise.resolve([]))
+            // MemberUpdated events - none
+            getEventsMocked.mockReturnValueOnce(Promise.resolve([]))
+            // MembersAdded events - batch add at index 0
+            getEventsMocked.mockReturnValueOnce(Promise.resolve([[, "0", ["100", "101"]]]))
+            // MemberAdded events - single add at index 2 (only 1 event, but index is 2)
+            getEventsMocked.mockReturnValueOnce(Promise.resolve([[, "2", "102"]]))
+
+            const members = await semaphore.getGroupMembers("42")
+
+            expect(members).toHaveLength(3)
+            expect(members[0]).toBe("100")
+            expect(members[1]).toBe("101")
+            expect(members[2]).toBe("102")
+        })
     })
 
     describe("# getGroupVerifiedProofs", () => {