From 4fd942f4adbc018ab2015e2a45e75d3ad1bf020f Mon Sep 17 00:00:00 2001 From: sripwoud Date: Mon, 30 Oct 2023 20:33:46 +0100 Subject: [PATCH 1/3] fix(contribute command): listen to user doc changes instead of fetching doc to check if exists There can be some lag after running the auth command before the user can be found in firebase. In which case trying to fetch the user will fail with `FirebaseError: Unable to retrieve the authenticated user.`. Instead we can listen and wait for user doc changes. #220 --- packages/actions/src/helpers/database.ts | 21 +++++++++++++++++++ packages/actions/src/index.ts | 3 ++- packages/phase2cli/src/commands/contribute.ts | 13 +++--------- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/actions/src/helpers/database.ts b/packages/actions/src/helpers/database.ts index 01490282..d8860690 100644 --- a/packages/actions/src/helpers/database.ts +++ b/packages/actions/src/helpers/database.ts @@ -6,6 +6,7 @@ import { Firestore, getDoc, getDocs, + onSnapshot, query, QueryConstraint, QueryDocumentSnapshot, @@ -123,6 +124,26 @@ export const getDocumentById = async ( return getDoc(docRef) } +export const waitForUserDocumentToExist = (firestoreDatabase: Firestore, collection: string, documentId: string) => { + return new Promise((resolve, reject) => { + const docRef = doc(firestoreDatabase, collection, documentId) + + const unsubscribe = onSnapshot( + docRef, + (docSnapshot) => { + if (docSnapshot.exists()) { + unsubscribe() + resolve() + } + }, + (error) => { + unsubscribe() + reject(error) + } + ) + }) +} + /** * Query for opened ceremonies. * @param firestoreDatabase - the Firestore service instance associated to the current Firebase application. diff --git a/packages/actions/src/index.ts b/packages/actions/src/index.ts index 1f4a960b..c4e64336 100644 --- a/packages/actions/src/index.ts +++ b/packages/actions/src/index.ts @@ -23,7 +23,8 @@ export { getContributionsCollectionPath, getTimeoutsCollectionPath, getOpenedCeremonies, - getCeremonyCircuits + getCeremonyCircuits, + waitForUserDocumentToExist } from "./helpers/database" export { compareCeremonyArtifacts, diff --git a/packages/phase2cli/src/commands/contribute.ts b/packages/phase2cli/src/commands/contribute.ts index 9ff99dec..c004e288 100644 --- a/packages/phase2cli/src/commands/contribute.ts +++ b/packages/phase2cli/src/commands/contribute.ts @@ -21,7 +21,8 @@ import { FirebaseDocumentInfo, generateValidContributionsAttestation, commonTerms, - convertToDoubleDigits + convertToDoubleDigits, + waitForUserDocumentToExist } from "@p0tion/actions" import { DocumentSnapshot, DocumentData, Firestore, onSnapshot, Timestamp } from "firebase/firestore" import { Functions } from "firebase/functions" @@ -947,15 +948,7 @@ const contribute = async (opt: any) => { const spinner = customSpinner(`Verifying your participant status...`, `clock`) spinner.start() - // Check that the user's document is created - const userDoc = await getDocumentById(firestoreDatabase, commonTerms.collections.users.name, user.uid) - const userData = userDoc.data() - if (!userData) { - spinner.fail( - `Unfortunately we could not find a user document with your information. This likely means that you did not pass the GitHub reputation checks and therefore are not elegible to contribute to any ceremony. If you believe you pass the requirements, it might be possible that your profile is private and we were not able to fetch your real statistics, in this case please consider making your profile public for the duration of the contribution. Please contact the coordinator if you believe this to be an error.` - ) - process.exit(0) - } + await waitForUserDocumentToExist(firestoreDatabase, commonTerms.collections.users.name, user.uid) // Check the user's current participant readiness for contribution status (eligible, already contributed, timed out). const canParticipantContributeToCeremony = await checkParticipantForCeremony(firebaseFunctions, selectedCeremony.id) From a87f7b58f9cea3ca53f6fc30e24a60c92cf52334 Mon Sep 17 00:00:00 2001 From: sripwoud Date: Mon, 30 Oct 2023 20:36:17 +0100 Subject: [PATCH 2/3] test(waitforuserdocumenttoexist): test new `waitForUserDocumentToExist` function --- packages/actions/test/unit/contribute.test.ts | 4 +-- packages/actions/test/unit/database.test.ts | 35 +++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/packages/actions/test/unit/contribute.test.ts b/packages/actions/test/unit/contribute.test.ts index 36345434..b11effdd 100644 --- a/packages/actions/test/unit/contribute.test.ts +++ b/packages/actions/test/unit/contribute.test.ts @@ -911,7 +911,7 @@ describe("Contribute", () => { false ) expect(preamble).to.eq( - `Hey, I'm ${users[0].uid} and I have contributed to the ${fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix} MPC Phase2 Trusted Setup ceremony.\nThe following are my contribution signatures:` + `Hey, I'm ${users[0].uid} and I have contributed to the ${fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix}.\nThe following are my contribution signatures:` ) }) it("should return the correct preamble for the final contribution", () => { @@ -921,7 +921,7 @@ describe("Contribute", () => { true ) expect(preamble).to.eq( - `Hey, I'm ${users[0].uid} and I have finalized the ${fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix} MPC Phase2 Trusted Setup ceremony.\nThe following are my contribution signatures:` + `Hey, I'm ${users[0].uid} and I have finalized the ${fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix}.\nThe following are my contribution signatures:` ) }) }) diff --git a/packages/actions/test/unit/database.test.ts b/packages/actions/test/unit/database.test.ts index 0b0133eb..54d5dc35 100644 --- a/packages/actions/test/unit/database.test.ts +++ b/packages/actions/test/unit/database.test.ts @@ -1,7 +1,9 @@ +import { expect as jestExpect } from "@jest/globals" import chai, { expect } from "chai" import chaiAsPromised from "chai-as-promised" import { getAuth, signInWithEmailAndPassword, signOut } from "firebase/auth" -import { where } from "firebase/firestore" +import { onSnapshot, where } from "firebase/firestore" +import * as firestore from "firebase/firestore" import { fakeCeremoniesData, fakeCircuitsData, fakeParticipantsData, fakeUsersData } from "../data/samples" import { getCurrentFirebaseAuthUser, @@ -16,7 +18,8 @@ import { getCircuitsCollectionPath, getContributionsCollectionPath, getTimeoutsCollectionPath, - commonTerms + commonTerms, + waitForUserDocumentToExist } from "../../src/index" import { deleteAdminApp, @@ -32,6 +35,11 @@ import { } from "../utils/index" import { CeremonyState } from "../../src/types/enums" +jest.mock("firebase/firestore", () => ({ + __esModule: true, + ...jest.requireActual("firebase/firestore") +})) + chai.use(chaiAsPromised) /** @@ -150,6 +158,29 @@ describe("Database", () => { }) }) + describe("waitForUserDocumentToExist", () => { + it("should wait for a user document to exist", async () => { + await signInWithEmailAndPassword(userAuth, users[0].data.email, passwords[0]) + + let handleSnapshot: any + jest.spyOn(firestore, "onSnapshot").mockImplementationOnce((...args) => { + ;[, handleSnapshot] = args + return jest.fn() + }) + + const promise = waitForUserDocumentToExist( + userFirestore, + commonTerms.collections.ceremonies.name, + fakeCeremoniesData.fakeCeremonyOpenedFixed.uid + ) + + handleSnapshot({ exists: () => true }) // Simulate document creation + await expect(promise).to.be.fulfilled + jestExpect(onSnapshot).toHaveBeenCalledTimes(1) + }) + jest.clearAllMocks() + }) + describe("getCircuitContributionsFromContributor", () => { it("should return an empty array when a ceremony has not participants", async () => { const contributions = await getCircuitContributionsFromContributor( From 8e238bd174af11c0a9c85ed3abcbfed29a1c9f05 Mon Sep 17 00:00:00 2001 From: sripwoud Date: Mon, 30 Oct 2023 22:16:49 +0100 Subject: [PATCH 3/3] revert changes in contribute.test.ts --- packages/actions/test/unit/contribute.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/actions/test/unit/contribute.test.ts b/packages/actions/test/unit/contribute.test.ts index b11effdd..36345434 100644 --- a/packages/actions/test/unit/contribute.test.ts +++ b/packages/actions/test/unit/contribute.test.ts @@ -911,7 +911,7 @@ describe("Contribute", () => { false ) expect(preamble).to.eq( - `Hey, I'm ${users[0].uid} and I have contributed to the ${fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix}.\nThe following are my contribution signatures:` + `Hey, I'm ${users[0].uid} and I have contributed to the ${fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix} MPC Phase2 Trusted Setup ceremony.\nThe following are my contribution signatures:` ) }) it("should return the correct preamble for the final contribution", () => { @@ -921,7 +921,7 @@ describe("Contribute", () => { true ) expect(preamble).to.eq( - `Hey, I'm ${users[0].uid} and I have finalized the ${fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix}.\nThe following are my contribution signatures:` + `Hey, I'm ${users[0].uid} and I have finalized the ${fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix} MPC Phase2 Trusted Setup ceremony.\nThe following are my contribution signatures:` ) }) })