Merge pull request #376 from quadratic-funding/fix/verifycontribution

Fix: add race condition to make verifycontribution cf exit when hang
This commit is contained in:
Giacomo
2023-04-06 15:37:10 +02:00
committed by GitHub
7 changed files with 216 additions and 164 deletions

View File

@@ -1,5 +1,12 @@
import { Functions, httpsCallable, httpsCallableFromURL } from "firebase/functions"
import { CeremonyInputData, CircuitDocument, ContributionVerificationData, ETagWithPartNumber } from "../types"
import { DocumentSnapshot, onSnapshot } from "firebase/firestore"
import {
CeremonyInputData,
CircuitDocument,
ContributionVerificationData,
ETagWithPartNumber,
FirebaseDocumentInfo
} from "../types"
import { commonTerms } from "./constants"
/**
@@ -294,7 +301,7 @@ export const checkIfObjectExist = async (
* Request to verify the newest contribution for the circuit.
* @param functions <Functions> - the Firebase cloud functions object instance.
* @param ceremonyId <string> - the unique identifier of the ceremony.
* @param circuitId <string> - the unique identifier of the circuit.
* @param circuit <FirebaseDocumentInfo> - the document info about the circuit.
* @param bucketName <string> - the name of the ceremony bucket.
* @param contributorOrCoordinatorIdentifier <string> - the identifier of the contributor or coordinator (only when finalizing).
* @param verifyContributionCloudFunctionEndpoint <string> - the endpoint (direct url) necessary to call the V2 Cloud Function.
@@ -303,7 +310,7 @@ export const checkIfObjectExist = async (
export const verifyContribution = async (
functions: Functions,
ceremonyId: string,
circuitId: string,
circuit: FirebaseDocumentInfo, // any just to avoid breaking the tests.
bucketName: string,
contributorOrCoordinatorIdentifier: string,
verifyContributionCloudFunctionEndpoint: string
@@ -312,12 +319,76 @@ export const verifyContribution = async (
timeout: 3600000 // max timeout 60 minutes.
})
const { data: contributionVerificationData }: any = await cf({
ceremonyId,
circuitId,
contributorOrCoordinatorIdentifier,
bucketName
})
/**
* @dev Force a race condition to fix #57.
* TL;DR if the cloud function does not return despite having finished its execution, we use
* a listener on the circuit, we check and retrieve the info about the correct execution and
* return it manually. In other cases, it will be the function that returns either a timeout in case it
* remains in execution for too long.
*/
const { data: contributionVerificationData }: any = await Promise.race([
cf({
ceremonyId,
circuitId: circuit.id,
contributorOrCoordinatorIdentifier,
bucketName
}),
new Promise((resolve): any => {
setTimeout(() => {
const unsubscribeToCeremonyCircuitListener = onSnapshot(
circuit.ref,
async (changedCircuit: DocumentSnapshot) => {
// Check data.
if (!circuit.data || !changedCircuit.data())
throw Error(`Unable to retrieve circuit data from the ceremony.`)
// Extract data.
const { avgTimings: changedAvgTimings, waitingQueue: changedWaitingQueue } =
changedCircuit.data()!
const {
contributionComputation: changedContributionComputation,
fullContribution: changedFullContribution,
verifyCloudFunction: changedVerifyCloudFunction
} = changedAvgTimings
const {
failedContributions: changedFailedContributions,
completedContributions: changedCompletedContributions
} = changedWaitingQueue
const { avgTimings: prevAvgTimings, waitingQueue: prevWaitingQueue } = changedCircuit.data()!
const {
contributionComputation: prevContributionComputation,
fullContribution: prevFullContribution,
verifyCloudFunction: prevVerifyCloudFunction
} = prevAvgTimings
const {
failedContributions: prevFailedContributions,
completedContributions: prevCompletedContributions
} = prevWaitingQueue
// Pre-conditions.
const invalidContribution = prevFailedContributions === changedFailedContributions - 1
const validContribution = prevCompletedContributions === changedCompletedContributions - 1
const avgTimeUpdates =
prevContributionComputation !== changedContributionComputation &&
prevFullContribution !== changedFullContribution &&
prevVerifyCloudFunction !== changedVerifyCloudFunction
if ((invalidContribution || validContribution) && avgTimeUpdates) {
resolve({
data: {
valid: prevFailedContributions !== changedFailedContributions - 1
}
})
}
}
)
// Unsubscribe from listener.
unsubscribeToCeremonyCircuitListener()
}, 3600000 - 1000) // 59:59 throws 1s before max time for CF execution.
})
])
return contributionVerificationData
}

View File

@@ -361,13 +361,9 @@ export type CircuitDocument = CircuitInputData & {
* Necessary data to define contribution verification output.
* @typedef {Object} ContributionVerificationData
* @property {boolean} valid - true if and only if the contribution was verified as correct; otherwise false..
* @property {number} verifyCloudFunctionTime - the amount of time necessary for the cloud function execution.
* @property {number} fullContributionTime - the amount of time necessary for the full contribution execution.
*/
export type ContributionVerificationData = {
valid: boolean
verifyCloudFunctionTime: number
fullContributionTime: number
}
/**

View File

@@ -26,7 +26,8 @@ import {
verifyContribution,
progressToNextCircuitForContribution,
getPotStorageFilePath,
getTranscriptStorageFilePath
getTranscriptStorageFilePath,
getCircuitsCollectionPath
} from "../../src"
import { fakeCeremoniesData, fakeCircuitsData, fakeUsersData } from "../data/samples"
import {
@@ -141,10 +142,10 @@ describe("Contribution", () => {
// create mock ceremony with circuit data
await createMockCeremony(adminFirestore, ceremony, tmpCircuit)
await sleep(1000)
})
// @note figure out how to clean up transcripts
if (envType === TestingEnvironment.PRODUCTION) {
it.skip("should allow an authenticated user to contribute to a ceremony", async () => {
it("should allow an authenticated user to contribute to a ceremony", async () => {
// 1. login as user 2
await signInWithEmailAndPassword(userAuth, users[2].data.email, passwords[2])
await sleep(500)
@@ -235,10 +236,16 @@ describe("Contribution", () => {
objectsToDelete.push(nextZkeyStoragePath)
// Execute contribution verification.
const tempCircuit = await getDocumentById(
userFirestore,
getCircuitsCollectionPath(ceremonyId),
tmpCircuit.uid
)
const { valid } = await verifyContribution(
userFunctions,
ceremonyId,
tmpCircuit.uid,
tempCircuit,
bucketName,
users[2].uid,
String(process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION)

View File

@@ -1,6 +1,7 @@
import chai, { expect } from "chai"
import chaiAsPromised from "chai-as-promised"
import { getAuth, signInWithEmailAndPassword, signOut } from "firebase/auth"
import { DocumentData, DocumentSnapshot } from "firebase/firestore"
import { ETagWithPartNumber } from "../../src/types"
import {
fakeCeremoniesData,
@@ -23,7 +24,9 @@ import {
getParticipantsCollectionPath,
convertBytesOrKbToGb,
getPublicAttestationPreambleForContributor,
getContributionsValidityForContributor
getContributionsValidityForContributor,
getDocumentById,
getCircuitsCollectionPath
} from "../../src"
import {
cleanUpMockUsers,
@@ -448,21 +451,7 @@ describe("Contribute", () => {
resumeContributionAfterTimeoutExpiration(userFunctions, fakeCeremoniesData.fakeCeremonyOpenedFixed.uid)
).to.be.rejectedWith("Unable to progress to next circuit for contribution")
})
// @todo check this test causes the following error that makes CI tests fail:
/*
⚠ functions: Error: 5 NOT_FOUND: no entity to update: app: "dev~demo-zkmpc"
path <
Element {
type: "ceremonies"
name: "0000000000000000000C"
}
Element {
type: "participants"
name: "G7q1AT7bUADB6OgFzEylfTQpfeWG"
}
>
*/
it.skip("should succesfully resume the contribution", async () => {
it("should succesfully resume the contribution", async () => {
await signInWithEmailAndPassword(userAuth, users[0].data.email, passwords[0])
await expect(
resumeContributionAfterTimeoutExpiration(userFunctions, fakeCeremoniesData.fakeCeremonyOpenedFixed.uid)
@@ -621,7 +610,7 @@ describe("Contribute", () => {
})
// if we have the url for the cloud function, we can test it
if (process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION) {
if (envType === TestingEnvironment.PRODUCTION) {
/// @todo update error messages after refactoring
describe("verifyContribution", () => {
const bucketName = "test-bucket"
@@ -633,56 +622,82 @@ describe("Contribute", () => {
)
})
it("should revert when the user is not authenticated", async () => {
const circuitDocument = await getDocumentById(
userFirestore,
getCircuitsCollectionPath(fakeCeremoniesData.fakeCeremonyContributeTest.uid),
fakeCircuitsData.fakeCircuitSmallContributors.uid
)
await signOut(userAuth)
await expect(
verifyContribution(
userFunctions,
process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION!,
fakeCeremoniesData.fakeCeremonyOpenedFixed.uid,
fakeCircuitsData.fakeCircuitSmallContributors.uid,
fakeCeremoniesData.fakeCeremonyContributeTest.uid,
circuitDocument,
bucketName,
"contributor",
bucketName
process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION!
)
).to.be.rejectedWith("internal")
).to.be.rejectedWith("Unable to retrieve the authenticated user.")
})
it("should revert when given a non existent ceremony id", async () => {
await signInWithEmailAndPassword(userAuth, users[0].data.email, passwords[0])
const circuitDocument = await getDocumentById(
userFirestore,
getCircuitsCollectionPath(fakeCeremoniesData.fakeCeremonyContributeTest.uid),
fakeCircuitsData.fakeCircuitSmallContributors.uid
)
await expect(
verifyContribution(
userFunctions,
process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION!,
"notExistentId",
fakeCircuitsData.fakeCircuitSmallContributors.uid,
circuitDocument,
bucketName,
"contributor",
bucketName
process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION!
)
).to.be.rejectedWith("internal")
).to.be.rejectedWith(
"Unable to find a document with the given identifier for the provided collection path."
)
})
it("should revert when given a non existent circuit id", async () => {
await signInWithEmailAndPassword(userAuth, users[0].data.email, passwords[0])
await expect(
verifyContribution(
userFunctions,
process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION!,
fakeCeremoniesData.fakeCeremonyOpenedFixed.uid,
"notExistentId",
{} as DocumentSnapshot<DocumentData>,
bucketName,
"contributor",
bucketName
process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION!
)
).to.be.rejectedWith("internal")
).to.be.rejectedWith("Unable to perform the operation due to incomplete or incorrect data.")
})
it("should revert when called by a user which did not contribute to this ceremony", async () => {
await signInWithEmailAndPassword(userAuth, users[1].data.email, passwords[1])
const circuitDocument = await getDocumentById(
userFirestore,
getCircuitsCollectionPath(fakeCeremoniesData.fakeCeremonyContributeTest.uid),
fakeCircuitsData.fakeCircuitSmallContributors.uid
)
await expect(
verifyContribution(
userFunctions,
process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION!,
fakeCeremoniesData.fakeCeremonyOpenedFixed.uid,
fakeCircuitsData.fakeCircuitSmallContributors.uid,
circuitDocument,
bucketName,
"contributor",
bucketName
process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION!
)
).to.be.rejectedWith("internal")
).to.be.rejectedWith(
"Unable to find a document with the given identifier for the provided collection path."
)
})
it("should store the contribution verification result", async () => {})
afterAll(async () => {

View File

@@ -24,12 +24,9 @@
"pubsub": {
"port": 8085
},
"storage": {
"port": 9199
},
"ui": {
"enabled": false,
"port": 4000
}
}
}
}

View File

@@ -28,7 +28,7 @@ import {
computeSHA256ToHex
} from "@zkmpc/actions/src"
import { ParticipantStatus, ParticipantContributionStep, CeremonyState } from "@zkmpc/actions/src/types/enums"
import { FinalizeCircuitData, VerifyContributionData } from "../../types"
import { FinalizeCircuitData, VerifyContributionData } from "types"
import { Contribution } from "@zkmpc/actions/src/types"
import { LogLevel } from "../../types/enums"
import { COMMON_ERRORS, logAndThrowError, printLog, SPECIFIC_ERRORS } from "../lib/errors"
@@ -401,11 +401,10 @@ export const verifycontribution = functionsV2.https.onCall(
// Derive necessary data.
const lastZkeyIndex = formatZkeyIndex(completedContributions + 1)
const verificationTranscriptCompleteFilename = `${prefix}_${
isFinalizing
? `${contributorOrCoordinatorIdentifier}_${finalContributionIndex}_verification_transcript.log`
: `${lastZkeyIndex}_${contributorOrCoordinatorIdentifier}_verification_transcript.log`
}`
const verificationTranscriptCompleteFilename = `${prefix}_${isFinalizing
? `${contributorOrCoordinatorIdentifier}_${finalContributionIndex}_verification_transcript.log`
: `${lastZkeyIndex}_${contributorOrCoordinatorIdentifier}_verification_transcript.log`
}`
const firstZkeyFilename = `${prefix}_${genesisZkeyIndex}.zkey`
const lastZkeyFilename = `${prefix}_${isFinalizing ? finalContributionIndex : lastZkeyIndex}.zkey`
@@ -436,10 +435,8 @@ export const verifycontribution = functionsV2.https.onCall(
// Create and populate transcript.
const transcriptLogger = createCustomLoggerForFile(verificationTranscriptTemporaryLocalPath)
transcriptLogger.info(
`${
isFinalizing ? `Final verification` : `Verification`
} transcript for ${prefix} circuit Phase 2 contribution.\n${
isFinalizing ? `Coordinator ` : `Contributor # ${Number(lastZkeyIndex)}`
`${isFinalizing ? `Final verification` : `Verification`
} transcript for ${prefix} circuit Phase 2 contribution.\n${isFinalizing ? `Coordinator ` : `Contributor # ${Number(lastZkeyIndex)}`
} (${contributorOrCoordinatorIdentifier})\n`
)
@@ -611,17 +608,14 @@ export const verifycontribution = functionsV2.https.onCall(
await batch.commit()
printLog(
`The contribution #${lastZkeyIndex} of circuit ${circuitId} (ceremony ${ceremonyId}) has been verified as ${
isContributionValid ? "valid" : "invalid"
`The contribution #${lastZkeyIndex} of circuit ${circuitId} (ceremony ${ceremonyId}) has been verified as ${isContributionValid ? "valid" : "invalid"
} for the participant ${participantDoc.id}`,
LogLevel.DEBUG
)
// Step (3).
return {
valid: isContributionValid,
fullContributionTime,
verifyCloudFunctionTime
valid: isContributionValid
}
}
)
@@ -717,77 +711,79 @@ export const finalizeCircuit = functionsV1
.runWith({
memory: "512MB"
})
.https.onCall(async (data: FinalizeCircuitData, context: functionsV1.https.CallableContext) => {
if (!context.auth || !context.auth.token.coordinator) logAndThrowError(COMMON_ERRORS.CM_NOT_COORDINATOR_ROLE)
.https.onCall(
async (data: FinalizeCircuitData, context: functionsV1.https.CallableContext) => {
if (!context.auth || !context.auth.token.coordinator) logAndThrowError(COMMON_ERRORS.CM_NOT_COORDINATOR_ROLE)
if (!data.ceremonyId || !data.circuitId || !data.bucketName || !data.beacon)
logAndThrowError(COMMON_ERRORS.CM_MISSING_OR_WRONG_INPUT_DATA)
if (!data.ceremonyId || !data.circuitId || !data.bucketName || !data.beacon)
logAndThrowError(COMMON_ERRORS.CM_MISSING_OR_WRONG_INPUT_DATA)
// Get data.
const { ceremonyId, circuitId, bucketName, beacon } = data
const userId = context.auth?.uid
// Get data.
const { ceremonyId, circuitId, bucketName, beacon } = data
const userId = context.auth?.uid
// Look for documents.
const ceremonyDoc = await getDocumentById(commonTerms.collections.ceremonies.name, ceremonyId)
const participantDoc = await getDocumentById(getParticipantsCollectionPath(ceremonyId), userId!)
const circuitDoc = await getDocumentById(getCircuitsCollectionPath(ceremonyId), circuitId)
const contributionDoc = await getFinalContribution(ceremonyId, circuitId)
// Look for documents.
const ceremonyDoc = await getDocumentById(commonTerms.collections.ceremonies.name, ceremonyId)
const participantDoc = await getDocumentById(getParticipantsCollectionPath(ceremonyId), userId!)
const circuitDoc = await getDocumentById(getCircuitsCollectionPath(ceremonyId), circuitId)
const contributionDoc = await getFinalContribution(ceremonyId, circuitId)
if (!ceremonyDoc.data() || !circuitDoc.data() || !participantDoc.data() || !contributionDoc.data())
logAndThrowError(COMMON_ERRORS.CM_INEXISTENT_DOCUMENT_DATA)
if (!ceremonyDoc.data() || !circuitDoc.data() || !participantDoc.data() || !contributionDoc.data())
logAndThrowError(COMMON_ERRORS.CM_INEXISTENT_DOCUMENT_DATA)
// Extract data.
const { prefix: circuitPrefix } = circuitDoc.data()!
const { files } = contributionDoc.data()!
// Extract data.
const { prefix: circuitPrefix } = circuitDoc.data()!
const { files } = contributionDoc.data()!
// Prepare filenames and storage paths.
const verificationKeyFilename = `${circuitPrefix}_${verificationKeyAcronym}.json`
const verifierContractFilename = `${circuitPrefix}_${verifierSmartContractAcronym}.sol`
const verificationKeyStorageFilePath = getVerificationKeyStorageFilePath(circuitPrefix, verificationKeyFilename)
const verifierContractStorageFilePath = getVerifierContractStorageFilePath(
circuitPrefix,
verifierContractFilename
)
// Prepare filenames and storage paths.
const verificationKeyFilename = `${circuitPrefix}_${verificationKeyAcronym}.json`
const verifierContractFilename = `${circuitPrefix}_${verifierSmartContractAcronym}.sol`
const verificationKeyStorageFilePath = getVerificationKeyStorageFilePath(circuitPrefix, verificationKeyFilename)
const verifierContractStorageFilePath = getVerifierContractStorageFilePath(
circuitPrefix,
verifierContractFilename
)
// Prepare temporary paths.
const verificationKeyTemporaryFilePath = createTemporaryLocalPath(verificationKeyFilename)
const verifierContractTemporaryFilePath = createTemporaryLocalPath(verifierContractFilename)
// Prepare temporary paths.
const verificationKeyTemporaryFilePath = createTemporaryLocalPath(verificationKeyFilename)
const verifierContractTemporaryFilePath = createTemporaryLocalPath(verifierContractFilename)
// Download artifact from ceremony bucket.
await downloadArtifactFromS3Bucket(bucketName, verificationKeyStorageFilePath, verificationKeyTemporaryFilePath)
await downloadArtifactFromS3Bucket(
bucketName,
verifierContractStorageFilePath,
verifierContractTemporaryFilePath
)
// Download artifact from ceremony bucket.
await downloadArtifactFromS3Bucket(bucketName, verificationKeyStorageFilePath, verificationKeyTemporaryFilePath)
await downloadArtifactFromS3Bucket(
bucketName,
verifierContractStorageFilePath,
verifierContractTemporaryFilePath
)
// Compute hash before unlink.
const verificationKeyBlake2bHash = await blake512FromPath(verificationKeyTemporaryFilePath)
const verifierContractBlake2bHash = await blake512FromPath(verifierContractTemporaryFilePath)
// Compute hash before unlink.
const verificationKeyBlake2bHash = await blake512FromPath(verificationKeyTemporaryFilePath)
const verifierContractBlake2bHash = await blake512FromPath(verifierContractTemporaryFilePath)
// Free resources by unlinking temporary folders.
fs.unlinkSync(verificationKeyTemporaryFilePath)
fs.unlinkSync(verifierContractTemporaryFilePath)
// Free resources by unlinking temporary folders.
fs.unlinkSync(verificationKeyTemporaryFilePath)
fs.unlinkSync(verifierContractTemporaryFilePath)
// Add references and hashes of the final contribution artifacts.
await contributionDoc.ref.update({
files: {
...files,
verificationKeyBlake2bHash,
verificationKeyFilename,
verificationKeyStoragePath: verificationKeyStorageFilePath,
verifierContractBlake2bHash,
verifierContractFilename,
verifierContractStoragePath: verifierContractStorageFilePath
},
beacon: {
value: beacon,
hash: computeSHA256ToHex(beacon)
}
})
// Add references and hashes of the final contribution artifacts.
await contributionDoc.ref.update({
files: {
...files,
verificationKeyBlake2bHash,
verificationKeyFilename,
verificationKeyStoragePath: verificationKeyStorageFilePath,
verifierContractBlake2bHash,
verifierContractFilename,
verifierContractStoragePath: verifierContractStorageFilePath
},
beacon: {
value: beacon,
hash: computeSHA256ToHex(beacon)
}
})
printLog(
`Circuit ${circuitId} finalization completed - Ceremony ${ceremonyDoc.id} - Coordinator ${participantDoc.id}`,
LogLevel.DEBUG
)
})
printLog(
`Circuit ${circuitId} finalization completed - Ceremony ${ceremonyDoc.id} - Coordinator ${participantDoc.id}`,
LogLevel.DEBUG
)
}
)

View File

@@ -740,10 +740,10 @@ export const handleStartOrResumeContribution = async (
spinner.start()
// Execute contribution verification.
const { valid, verifyCloudFunctionTime, fullContributionTime } = await verifyContribution(
const { valid } = await verifyContribution(
cloudFunctions,
ceremony.id,
circuit.id,
circuit,
bucketName,
contributorOrCoordinatorIdentifier,
String(process.env.FIREBASE_CF_URL_VERIFY_CONTRIBUTION)
@@ -751,18 +751,6 @@ export const handleStartOrResumeContribution = async (
await sleep(3000) // workaround cf termination.
// Format time.
const {
seconds: verificationSeconds,
minutes: verificationMinutes,
hours: verificationHours
} = getSecondsMinutesHoursFromMillis(verifyCloudFunctionTime)
const {
seconds: contributionSeconds,
minutes: contributionMinutes,
hours: contributionHours
} = getSecondsMinutesHoursFromMillis(fullContributionTime + verifyCloudFunctionTime)
// Display verification output.
if (valid)
spinner.succeed(
@@ -780,23 +768,5 @@ export const handleStartOrResumeContribution = async (
: `Contribution ${theme.text.bold(`#${nextZkeyIndex}`)} has been evaluated as`
} ${theme.text.bold("invalid")}`
)
console.log(
`${theme.symbols.success} ${
isFinalizing ? `Contribution` : `Contribution ${theme.text.bold(`#${nextZkeyIndex}`)}`
} verification took ${theme.text.bold(
`${convertToDoubleDigits(verificationHours)}:${convertToDoubleDigits(
verificationMinutes
)}:${convertToDoubleDigits(verificationSeconds)}`
)}`
)
console.log(
`${theme.symbols.info} Your contribution has lasted for ${theme.text.bold(
`${convertToDoubleDigits(contributionHours)}:${convertToDoubleDigits(
contributionMinutes
)}:${convertToDoubleDigits(contributionSeconds)}`
)}`
)
}
}