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.

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

fix: listen to user doc changes instead of fetching doc to check if exists #226

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 21 additions & 0 deletions packages/actions/src/helpers/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
Firestore,
getDoc,
getDocs,
onSnapshot,
query,
QueryConstraint,
QueryDocumentSnapshot,
Expand Down Expand Up @@ -123,6 +124,26 @@ export const getDocumentById = async (
return getDoc(docRef)
}

export const waitForUserDocumentToExist = (firestoreDatabase: Firestore, collection: string, documentId: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this would be a great solution, the code might hang forever if a user tried to auth and they did not pass the sybil checks. As discussed, a fetch and retry mechanism might work better for now until we find a more elegant solution

return new Promise<void>((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 <Firestore> - the Firestore service instance associated to the current Firebase application.
Expand Down
3 changes: 2 additions & 1 deletion packages/actions/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export {
getContributionsCollectionPath,
getTimeoutsCollectionPath,
getOpenedCeremonies,
getCeremonyCircuits
getCeremonyCircuits,
waitForUserDocumentToExist
} from "./helpers/database"
export {
compareCeremonyArtifacts,
Expand Down
35 changes: 33 additions & 2 deletions packages/actions/test/unit/database.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -16,7 +18,8 @@ import {
getCircuitsCollectionPath,
getContributionsCollectionPath,
getTimeoutsCollectionPath,
commonTerms
commonTerms,
waitForUserDocumentToExist
} from "../../src/index"
import {
deleteAdminApp,
Expand All @@ -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)

/**
Expand Down Expand Up @@ -150,6 +158,29 @@ describe("Database", () => {
})
})

describe("waitForUserDocumentToExist", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great stuff adding a test for it

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(
Expand Down
13 changes: 3 additions & 10 deletions packages/phase2cli/src/commands/contribute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
Loading