Skip to content

refactor: ServerCircuitProver return values#9391

Merged
alexghr merged 4 commits intomasterfrom
ag/refactor-avm-proof-fields
Nov 12, 2024
Merged

refactor: ServerCircuitProver return values#9391
alexghr merged 4 commits intomasterfrom
ag/refactor-avm-proof-fields

Conversation

@alexghr
Copy link
Contributor

@alexghr alexghr commented Oct 24, 2024

This PR changes some of the types returned by functions on the ServerCircuitProver interface to make them more friendly for serialisation/deserialisation

@alexghr alexghr changed the title refactor: change AVM proof from binary to fields refactor: ServerCircuitProver return values Oct 24, 2024
@alexghr alexghr force-pushed the ag/refactor-avm-proof-fields branch from 527a4c9 to 6ec88c6 Compare October 24, 2024 12:52
@alexghr alexghr marked this pull request as draft October 24, 2024 14:15
@alexghr alexghr force-pushed the ag/refactor-avm-proof-fields branch from 5351c4b to 6ec88c6 Compare October 24, 2024 14:16
@alexghr alexghr marked this pull request as ready for review October 24, 2024 14:16
signal?: AbortSignal,
epochNumber?: number,
): Promise<ProofAndVerificationKey<Proof>> {
): Promise<ProofAndVerificationKey<number>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wonder if type Unbounded = number is too hacky :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good shout! I think it would it clean and I can leave a comment at the definition point about it

return Promise.resolve(
makeProofAndVerificationKey(
makeEmptyProof(),
makeEmptyRecursiveProof<number>(0),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not really sure why the type is even called recursive proof, why separates it structurally from a non-recursive proof?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a leftover from when we were proving with plonk

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed, all the (non)recursive proof terminology comes from the days of plonk

@alexghr alexghr force-pushed the ag/refactor-avm-proof-fields branch from 6ec88c6 to ca65f95 Compare November 12, 2024 09:34
@alexghr alexghr merged commit ec717b5 into master Nov 12, 2024
@alexghr alexghr deleted the ag/refactor-avm-proof-fields branch November 12, 2024 10:55
alexghr added a commit that referenced this pull request Nov 15, 2024
Reopening of #8609, which was closed/merged by mistake. This PR is
stacked on top of #9391

This PR adds ProvingBroker which implements a new interface for
distributing proving jobs to workers as specified in
#8495
just-mitch pushed a commit that referenced this pull request Nov 16, 2024
Reopening of #8609, which was closed/merged by mistake. This PR is
stacked on top of #9391

This PR adds ProvingBroker which implements a new interface for
distributing proving jobs to workers as specified in
#8495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants