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

refactor: switch to consume_intermediate_mle in ProofPlan::verifier_evaluate #155

Merged
merged 4 commits into from
Oct 5, 2024

Conversation

iajoiner
Copy link
Contributor

@iajoiner iajoiner commented Sep 12, 2024

Rationale for this change

In order to accommodate composition of ProofPlans we need to allow either result or intermediate MLEs to be used depending on whether the plan is at top level.

What changes are included in this PR?

  • switch to consume_intermediate_mle in ProofPlan::verifier_evaluate

Are these changes tested?

Existing tests should pass

@iajoiner iajoiner changed the title refactor: add is_top_level to prover_evaluate and verifier_evaluate for ProofPlans refactor!: add is_top_level to prover_evaluate and verifier_evaluate for ProofPlans Sep 12, 2024
@iajoiner iajoiner force-pushed the refactor/mles branch 2 times, most recently from 28c2e74 to 35de9cc Compare September 16, 2024 16:02
Copy link
Contributor

@JayWhite2357 JayWhite2357 left a comment

Choose a reason for hiding this comment

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

My biggest concert here is that a node could produce a different number of intermediate mles and result mles, depending on whether it is top level or not. This would mean that consume_result_or_intermediate_mle doesn't really cover all cases.

I don't have a great solution to this problem, but I don't think this is the correct one.

@iajoiner iajoiner force-pushed the refactor/mles branch 3 times, most recently from cb20a4e to ced1b34 Compare September 16, 2024 19:22
@iajoiner iajoiner changed the title refactor!: add is_top_level to prover_evaluate and verifier_evaluate for ProofPlans refactor: switch to consume_intermediate_mle in ProofPlan::verifier_evaluate Sep 16, 2024
@iajoiner iajoiner force-pushed the refactor/mles branch 4 times, most recently from 9353ad4 to 51e87eb Compare September 17, 2024 16:18
@JayWhite2357 JayWhite2357 removed their request for review September 17, 2024 16:32
@iajoiner iajoiner force-pushed the refactor/mles branch 5 times, most recently from 33b5144 to 994fbe1 Compare September 18, 2024 19:00
@iajoiner iajoiner force-pushed the refactor/mles branch 3 times, most recently from e4cb1ec to 8d653b9 Compare September 19, 2024 12:27
@iajoiner iajoiner force-pushed the refactor/mles branch 4 times, most recently from 56b7695 to 8e8acf9 Compare September 19, 2024 21:51
JayWhite2357
JayWhite2357 previously approved these changes Oct 4, 2024
Copy link
Contributor

@JayWhite2357 JayWhite2357 left a comment

Choose a reason for hiding this comment

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

Conflicts need to be resolved. But, otherwise, approved.

…r_evaluate`

- `ProofPlan` change above
- remove `consume_result_mle` from `VerificationBuilder`
@iajoiner iajoiner added this pull request to the merge queue Oct 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 4, 2024
@JayWhite2357 JayWhite2357 added this pull request to the merge queue Oct 5, 2024
Merged via the queue into main with commit 5238673 Oct 5, 2024
11 checks passed
@JayWhite2357 JayWhite2357 deleted the refactor/mles branch October 5, 2024 01:19
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