Bump plonky2 (serialization fix) #729
Conversation
Nashtare
requested review from
wborgeaud,
muursh and
LindaGuiga
as code owners
github-actions
bot
added
the
crate: evm_arithmetization
| fn check_num_ctls() { | ||
| assert_eq!(all_cross_table_lookups::<F>().len(), NUM_CTLS); | ||
| } |
There was a problem hiding this comment.
I can't add such check for the MEMORY_CTL_IDX constant, because the looked_table field within CrossTableLookup is crate-private, though I could change it in the sibling PR if wanted.
There was a problem hiding this comment.
I think we can add a comment before ctl_memory(), to indicate we cannot change its position.
| @@ -249,12 +250,16 @@ pub mod testing { | |||
|
|
|||
| // Extra sums to add to the looked last value. | |||
| // Only necessary for the Memory values. | |||
| let mut extra_looking_sums = vec![vec![F::ZERO; config.num_challenges]; NUM_TABLES]; | |||
| let mut extra_looking_sums = | |||
There was a problem hiding this comment.
I think we can use let mut extra_looking_sums = HashMap::new()
There was a problem hiding this comment.
This causes the proving jobs to fail (two artifacts blocks) if I don't initialize each individual sum with vec![F::ZERO; config.num_challenges]
There was a problem hiding this comment.
Just curious, the unit tests passed, so why did the proving blocks job fail?
There was a problem hiding this comment.
There is only 1 recursive unit test (apart from consistency of constraints) that is ran in CI, it's empty_tables that is not complete (my comment on one of your PRs actually) as it only generates segment proofs.
Initializing the HashMap without specifying routable wires causes the recursive verification (not native one) to fail, which is done at the layer above (segment_aggregation). You can test it on two_to_one test for instance (though it is ignored in the CI).
There was a problem hiding this comment.
Do you know why it fails at the layer above?
There was a problem hiding this comment.
Because during verification of the CTLs, we add the extra sum values, which default to virtual wires if we don't specify builder.zero(). These are not part of the permutation argument and hence can have any value.
This reverts commit 50553fd.
We are currently having serialization issues due to the addition of
DummyProofGeneratoron theRootCircuitData, which is causing the prover_state to not be deserializable properly, ruining caching mechanism with persistence mode activated.This is currently pinned to
07c20447bfcdada6bd479bd8a13af26082923520on the plonky2 side (i.e. related branch fix), and updates the codebase with all changes related to plonky2 bump, in addition to fixing the range intwo_to_onetest and adding a serialization check consistency on theempty_tablestest.sibling PR: 0xPolygonZero/plonky2#1634