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

feat: Epoch orchestrator #8614

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

spalladino
Copy link
Collaborator

@spalladino spalladino commented Sep 17, 2024

Paused in favor of #8675

@spalladino spalladino force-pushed the palla/epoch-orchestrator branch 2 times, most recently from 25dedf6 to 0a4558f Compare September 17, 2024 22:00
const previousKernelProof = await this.previousKernelProof.promise;
const avmProof = await this.nestedAvmProof.promise;

// TODO: The kernel does not require the AVM VK, we're just getting the proof. Is that correct?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it will require the VK. Maybe it doesn't now because we don't recursively verify the AVM proof, but we will do.

const avmProof = await this.nestedAvmProof.promise;

// TODO: The kernel does not require the AVM VK, we're just getting the proof. Is that correct?
// TODO(#7369): How should we properly set up vkPath, vkIndex, and clientIvc for previous kernel?
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 sure I follow. All of the public kernels are recursively verified at the moment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, but public kernels require not just the recursive proof and vk, but also the vkPath, vkIndex, and clientIvc:

/**
* Proof of the previous kernel.
*/
public proof: RecursiveProof<typeof NESTED_RECURSIVE_PROOF_LENGTH>,
/**
* Verification key of the previous kernel.
*/
public vk: VerificationKeyData,
/**
* Index of the previous kernel's vk in a tree of vks.
*/
public vkIndex: UInt32,
/**
* Sibling path of the previous kernel's vk in a tree of vks.
*/
public vkPath: Tuple<Fr, typeof VK_TREE_HEIGHT>,
/**
* TODO(https://github.com/AztecProtocol/aztec-packages/issues/7369) this should be tube-proved for the first iteration and replace proof above
*/
public clientIvcProof: ClientIvcProof = ClientIvcProof.empty(),

However, in the current orchestrator, only proof and vk are set:

// pass both the proof and verification key forward to the next circuit
nextFunction.publicKernelRequest.inputs.previousKernel.proof = proof;
nextFunction.publicKernelRequest.inputs.previousKernel.vk = verificationKey;

if (!this.context.options.simulationOnly) {
void job
.prove()
.then(proof => rootParityJob.setNestedProof(proof, index))
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 having difficulty understanding the general flow. I can see here we update the nested proof for each base parity, what kicks off the root parity once the final base parity is completed?

@spalladino spalladino force-pushed the palla/epoch-orchestrator branch 4 times, most recently from eeef089 to 35a0494 Compare September 18, 2024 23:13
@spalladino spalladino marked this pull request as ready for review September 18, 2024 23:14
@spalladino spalladino force-pushed the palla/epoch-orchestrator branch 2 times, most recently from 59cd9bb to 3773e64 Compare September 18, 2024 23:15
@spalladino spalladino force-pushed the palla/epoch-orchestrator branch from 3773e64 to 54ece9e Compare September 18, 2024 23:32
spalladino added a commit that referenced this pull request Oct 3, 2024
Adds a new decorator for memoizing results. Extracted from #8614.
@spalladino spalladino marked this pull request as draft December 11, 2024 17:16
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.

2 participants