feat: integrate bytecode and program image reductions#19
Conversation
|
Warning This PR has more than 500 changed lines and does not include a spec. Large features and architectural changes benefit from a spec-driven workflow. If this PR is a bug fix, refactor, or doesn't warrant a spec, feel free to ignore this message. |
e4cdbb5 to
744ada7
Compare
01ce43e to
8d14d29
Compare
744ada7 to
d8d5384
Compare
8d14d29 to
b2cab0e
Compare
d8d5384 to
0b06125
Compare
b2cab0e to
5d0aa55
Compare
0b06125 to
f33facb
Compare
5d0aa55 to
4741cdb
Compare
f33facb to
77801f2
Compare
4741cdb to
6150eee
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6150eee. Configure here.
| ), | ||
| padded_len, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Program image words not padded for precommitted polynomial
Medium Severity
When constructing PrecommittedPolynomial::ProgramImage for Stage 8, the words field is set to the raw bytecode_words from RAM preprocessing (unpadded), but padded_len is set to the padded power-of-two length. The vmp_precommitted_contribution function iterates over words and uses padded_len for matrix dimensions (log_2, balanced sigma/nu), creating a mismatch — the words vector is shorter than padded_len, so indexing row_idx = coeff_idx / precommitted_cols may skip contributions that would exist in a properly padded polynomial, and the matrix shape doesn't match the committed polynomial.
Reviewed by Cursor Bugbot for commit 6150eee. Configure here.
|
|
||
| let booleanity_cycle_params = | ||
| BooleanityCyclePhaseParams::new(booleanity.into_params(), &self.opening_accumulator); | ||
| drop(instances); |
There was a problem hiding this comment.
ZK sumcheck proof removed from Stage 6a prover
High Severity
The prove_stage6a method previously had a #[cfg(feature = "zk")] block that used BatchedSumcheck::prove_zk with a local BlindFoldAccumulator, because Stage 6a input claims depend on hidden prior-stage outputs in ZK mode. The new code unconditionally uses prove_batched_sumcheck (the non-ZK path), which means Stage 6a no longer produces a ZK sumcheck proof when the zk feature is enabled. This breaks zero-knowledge soundness. The blindfold stage count assertion also changed from 7 to 8, which seems inconsistent with removing a ZK stage.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6150eee. Configure here.
| let entry_bytecode_index = self | ||
| .preprocessing | ||
| .shared | ||
| .program_meta | ||
| .entry_address | ||
| .saturating_sub(self.preprocessing.shared.program_meta.min_bytecode_address) | ||
| as usize | ||
| / BYTES_PER_INSTRUCTION | ||
| + 1; |
There was a problem hiding this comment.
This duplicates the logic from BytecodePreprocessing::entry_bytecode_index(). We can move entry_bytecode_index onto ProgramMetadata instead so it can be used by both prover and verifier
| bytecode_read_raf_params: BytecodeReadRafSumcheckParams<F>, | ||
| booleanity_params: BooleanitySumcheckParams<F>, | ||
| ) -> Result<StageVerifyResult<F>, ProofVerifyError> { | ||
| let bytecode_reduction_seed_params = bytecode_read_raf_params.clone(); |
There was a problem hiding this comment.
this is a potentially heavy clone, when BytecodeClaimReductionParams really only needs the gamma values
| @@ -1293,46 +1343,25 @@ | |||
| write_instance_flamegraph_svg(&instances, "stage6a_start_flamechart.svg"); | |||
| tracing::info!("Stage 6a proving"); | |||
|
|
|||
| #[cfg(feature = "zk")] | |||
| let (sumcheck_proof, _r_stage6a, _initial_claim) = { | |||
| // Stage 6a input claims depend on hidden prior-stage outputs in ZK mode, | |||
| // so we prove it with a ZK sumcheck proof. We keep a local blindfold | |||
| // accumulator so this split-internal phase does not add a new global | |||
| // BlindFold stage. | |||
| let mut rng = rand::thread_rng(); | |||
| let mut local_blindfold = | |||
| crate::subprotocols::blindfold::BlindFoldAccumulator::<F, C>::new(); | |||
| BatchedSumcheck::prove_zk::<F, C, _, _>( | |||
| instances.iter_mut().map(|v| &mut **v as _).collect(), | |||
| &mut self.opening_accumulator, | |||
| &mut local_blindfold, | |||
| &mut self.transcript, | |||
| &self.pedersen_generators, | |||
| &mut rng, | |||
| ) | |||
| }; | |||
| #[cfg(not(feature = "zk"))] | |||
| let (sumcheck_proof, _r_stage6a, _initial_claim) = | |||
| self.prove_batched_sumcheck(instances.iter_mut().map(|v| &mut **v as _).collect()); | |||
|
|
|||
| #[cfg(feature = "allocative")] | |||
| write_instance_flamegraph_svg(&instances, "stage6a_end_flamechart.svg"); | |||
|
|
|||
| let booleanity_cycle_params = | |||
There was a problem hiding this comment.
this sort of Arc::new(bytecode.clone()) appears a couple of times in this PR; we should avoid cloning the bytecode as much as possible. Can we return an Arc from a preprocessing method?
77801f2 to
d1eb7e0
Compare
6150eee to
42d671b
Compare
d1eb7e0 to
78c82ba
Compare
42d671b to
5c76a6b
Compare
78c82ba to
9d5b82e
Compare
5c76a6b to
546aa02
Compare
9d5b82e to
e587e65
Compare
546aa02 to
30d2a9b
Compare
e587e65 to
36378dd
Compare
48e80c2 to
f307a64
Compare
850790c to
268b434
Compare
3e65aa6 to
4637de5
Compare
Co-authored-by: Amirhossein Khajehpour <khajepour.amirhossein@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Amirhossein Khajehpour <khajepour.amirhossein@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Update bytecode and program-image verifier integration to use the shared precommitted phase transition helper after cycle-phase openings are cached. Made-with: Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Thread committed bytecode metadata consistently through prover and verifier paths so bytecode reduction state stays synchronized. Made-with: Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
268b434 to
bf067dd
Compare
4637de5 to
881b8fc
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>


Generated stack PR from
amir/bytecode-commitment-merged.Depends on spec PR: a16z#1565
Stack position:
04Base branch:
amir/bytecode-stack/03-program-foundationOwned paths:
This PR is expected to be updated manually when
amir/bytecode-commitment-mergedis resliced.