feat: add miscellaneous block to handle structure trace overflow #9733
Conversation
ledwards2225
requested review from
jeanmon,
IlyasRidhuan and
fcarreiro
as code owners
barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp
Show resolved
Hide resolved
Changes to public function bytecode sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
| @@ -88,6 +89,10 @@ template <IsHonkFlavor Flavor> class DeciderProvingKey_ { | |||
| if (IsGoblinFlavor<Flavor> && !is_structured) { | |||
| // Allocate full size polynomials | |||
| proving_key.polynomials = typename Flavor::ProverPolynomials(dyadic_circuit_size); | |||
| } | |||
There was a problem hiding this comment.
this branch and the one above lead to the same code being executed - did you separate them for readability? It seems like they canbe unified as well
There was a problem hiding this comment.
yeah I thought it was a bit more readable but seems both you and the linter disagree so I combined them into one :)
barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp
Show resolved
Hide resolved
| @@ -360,7 +360,7 @@ template <typename T> | |||
| concept IsUltraPlonkFlavor = IsAnyOf<T, plonk::flavor::Ultra, UltraKeccakFlavor>; | |||
|
|
|||
| template <typename T> | |||
| concept IsUltraPlonkOrHonk = IsAnyOf<T, plonk::flavor::Ultra, UltraFlavor, UltraKeccakFlavor, UltraFlavorWithZK, MegaFlavor>; | |||
| concept IsUltraPlonkOrHonk = IsAnyOf<T, plonk::flavor::Ultra, UltraFlavor, UltraKeccakFlavor, UltraFlavorWithZK, MegaFlavor, MegaZKFlavor>; | |||
There was a problem hiding this comment.
I presume this might have been a culprit for why a test was failing for the ZK flavor
There was a problem hiding this comment.
This concept should be called IsLiterallyAnything
| EXPECT_EQ(proving_key->proving_key.log_circuit_size, 19); | ||
| } | ||
|
|
||
| TEST_F(MegaMockCircuitsPinning, E2EStructuredCircuitSize) | ||
| { | ||
| GoblinProver goblin; | ||
| MegaCircuitBuilder app_circuit{ goblin.op_queue }; | ||
| auto proving_key = std::make_shared<DeciderProvingKey>(app_circuit, TraceStructure::E2E_FULL_TEST); | ||
| TraceSettings trace_settings{ TraceStructure::E2E_FULL_TEST }; | ||
| auto proving_key = std::make_shared<DeciderProvingKey>(app_circuit, trace_settings); |
There was a problem hiding this comment.
Can this be:
| auto proving_key = std::make_shared<DeciderProvingKey>(app_circuit, trace_settings); | |
| auto proving_key = std::make_shared<DeciderProvingKey>(app_circuit, TraceSettings{TraceStructure::E2E_FULL_TEST}); |
?
There was a problem hiding this comment.
it could yeah, just seemed like an unreasonably long single line
| uint32_t fixed_block_size = block.get_fixed_size(); | ||
| if (block_size > fixed_block_size && block != overflow_block) { | ||
| // We dont handle this case | ||
| ASSERT(!block.is_pub_inputs); |
There was a problem hiding this comment.
updated this to also include ecc_op block and added comment explaining that blocks not expected to be used by the App circuits arent allowed to overflow and will not be handled correctly
| * to accommodate circuits which cannot fit into a prescribed trace, gates which overflow their corresponding block are | ||
| * placed into an overflow block which can contain arbitrary gate types. | ||
| * @note One sublety is that gates at row i may in general utilize the values at row i+1 via shifts. If the last row in | ||
| * a full-capacity block is such a gate, then moving the overflow out of sequence will cause that gate not to be |
There was a problem hiding this comment.
maybe explain here what you mean by a dummy gate (i.e. that selectors are turn off) and also that it needs duplication so the second to last gate can function appropriately if it needs to check shifts
There was a problem hiding this comment.
clarified, thanks
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
Adds an
overflowblock to the set of execution trace blocks which can store arbitrary gate types that have "overflowed" the capacity of their corresponding designated gate type block. This allows for accumulation of app circuits which do not fit into a given structured trace configuration.It works as follows: when constructing a circuit, there is no bound on the size of each gate block. When a proving key is constructed, we check the capacity of each block and move any overflow into the overflow block. In all cases, the polynomials have the prescribed trace structure at the beginning plus a possible overflow block at the end.