Fix for recursion when max_proofs_verified = 1 #11698
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
August 23, 2022 09:42
…th the Wrap_hack. previously this was working because we had only tested it with 2 or 0 max_proofs_verified, in which cases padding at the front or at the back are identical
mrmr1993
added
the
ci-build-me
mrmr1993
approved these changes
Aug 24, 2022
|
Merged develop here to (hopefully) fix the nix CI failures. |
added 2 commits
October 12, 2022 11:39
added 7 commits
October 31, 2022 11:07
…hange some extends/trims to be on front
|
!ci-build-me |
|
!ci-build-me |
added 6 commits
November 9, 2022 09:41
…hould be fixed so branch_data is already reversed.
|
!ci-build-me |
|
!ci-build-me |
1 similar comment
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
mrmr1993
reviewed
Nov 10, 2022
| @@ -619,8 +619,16 @@ module Make (Inputs : Intf.Test.Inputs_intf) = struct | |||
| ~f:Fn.id | |||
| in | |||
| let snark_work_event_subscription = | |||
| Event_router.on (event_router t) Snark_work_gossip ~f:(fun _ _ -> | |||
| [%log info] "Received new snark work" ; | |||
| Event_router.on (event_router t) Snark_work_gossip | |||
There was a problem hiding this comment.
We should revert this, since it's fairly heavy, and only for debugging.
| @@ -194,6 +194,14 @@ module Make_str (_ : Wire_types.Concrete) = struct | |||
| local_max_proofs_verifieds | |||
| H1.T(Proof_.Messages_for_next_proof_over_same_field.Wrap).t ) = | |||
| let dummy_chals = Dummy.Ipa.Wrap.challenges in | |||
| let rev_magic : | |||
There was a problem hiding this comment.
We should fix the iterator below and remove this. Best not to lie to the typechecker!
| go M.maxes messages_for_next_wrap_proofs | ||
| rev_magic | ||
| (go | ||
| (Obj.magic (List.rev (Obj.magic M.maxes))) |
| @@ -333,7 +344,9 @@ module Make_str (_ : Wire_types.Concrete) = struct | |||
| = | |||
| fun rule -> | |||
| let (T (_, l)) = HT.length rule.prevs in | |||
| Vector.extend_exn (V.f l (M.f rule.prevs)) Max_proofs_verified.n 0 | |||
| Vector.extend_front_exn | |||
There was a problem hiding this comment.
It's a pity that we had to reverse all of these; probably there was just one place that was wrong before. (No need to change now though, since this works as written.)
| exists | ||
| (Vector.typ' | ||
| (Vector.map | ||
| ~f:(fun uses_lookup -> | ||
| Unfinalized.typ ~wrap_rounds:Backend.Tock.Rounds.n | ||
| ~uses_lookup ) | ||
| (Vector.extend lookup_usage lte Max_proofs_verified.n No) ) ) | ||
| lookup_usage | ||
| (* Vector.extend lookup_usage lte Max_proofs_verified.n No *) ) ) |
| @@ -106,3 +106,10 @@ let typ ~wrap_rounds ~uses_lookup : (t, Constant.t) Typ.t = | |||
| (Shifted_value.typ Other_field.typ) | |||
| ~assert_16_bits:(Step_verifier.assert_n_bits ~n:16) | |||
| ~zero:Common.Lookup_parameters.tick_zero ~uses_lookup | |||
|
|
|||
| let dummy () : t = | |||
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a long-standing issue related to verifying proofs that have max_proofs_verified=1 via
Pickles.Proof.of_side_loaded.The issue was that the accumulators of the proofs were being padded at the back of the list of commitments/challenges in certain parts of the code, but padded at the front of the list in others. This changes all padding to happen at the front of the list for compatibility with a sponge-state pre-computation optimization.
Previously, this bug was not uncovered because it had only been tested with max_proofs_verified= 0 or 2, in which case padding at the front and the back are identical.