-
Notifications
You must be signed in to change notification settings - Fork 36
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
Enables optional verification of Keccak tables #657
base: sai/conditionally_verify_in_root_circuit
Are you sure you want to change the base?
Changes from all commits
948d742
49bb96b
bbba820
8c4a362
5cdb37d
2f0129e
5b7f818
56deaa3
cfc6f24
4d288f8
e6bed69
0bd1448
daaabb6
ab2ec50
ec64af9
58f37e8
1540078
12c61a8
42afc90
e7ab9eb
09a1a4a
4a69e8a
81d3681
79aeb72
732020d
9f8ec9e
503085b
a36c5f7
640bae1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -486,15 +486,19 @@ fn get_all_memory_address_and_values(memory_before: &MemoryState) -> Vec<(Memory | |
res | ||
} | ||
|
||
type TablesWithPVsAndFinalMem<F> = ([Vec<PolynomialValues<F>>; NUM_TABLES], PublicValues<F>); | ||
pub struct TablesWithPVs<F: RichField> { | ||
pub tables: [Vec<PolynomialValues<F>>; NUM_TABLES], | ||
pub use_keccak_tables: bool, | ||
pub public_values: PublicValues<F>, | ||
} | ||
|
||
pub fn generate_traces<F: RichField + Extendable<D>, const D: usize>( | ||
all_stark: &AllStark<F, D>, | ||
inputs: &TrimmedGenerationInputs<F>, | ||
config: &StarkConfig, | ||
segment_data: &mut GenerationSegmentData, | ||
timing: &mut TimingTree, | ||
) -> anyhow::Result<TablesWithPVsAndFinalMem<F>> { | ||
) -> anyhow::Result<TablesWithPVs<F>> { | ||
let mut state = GenerationState::<F>::new_with_segment_data(inputs, segment_data) | ||
.map_err(|err| anyhow!("Failed to parse all the initial prover inputs: {:?}", err))?; | ||
|
||
|
@@ -581,6 +585,9 @@ pub fn generate_traces<F: RichField + Extendable<D>, const D: usize>( | |
mem_after: MemCap::default(), | ||
}; | ||
|
||
let use_keccak_tables = | ||
!state.traces.keccak_inputs.is_empty() || !state.traces.keccak_sponge_ops.is_empty(); | ||
Comment on lines
+588
to
+589
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: you can't have one empty and the other non-empty, so no need for the second evaluation |
||
|
||
let tables = timed!( | ||
timing, | ||
"convert trace data to tables", | ||
|
@@ -593,7 +600,12 @@ pub fn generate_traces<F: RichField + Extendable<D>, const D: usize>( | |
timing | ||
) | ||
); | ||
Ok((tables, public_values)) | ||
|
||
Ok(TablesWithPVs { | ||
tables, | ||
use_keccak_tables, | ||
public_values, | ||
}) | ||
} | ||
|
||
fn simulate_cpu<F: RichField>( | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -20,7 +20,7 @@ use starky::proof::{MultiProof, StarkProofWithMetadata}; | |||||
use starky::prover::prove_with_commitment; | ||||||
use starky::stark::Stark; | ||||||
|
||||||
use crate::all_stark::{AllStark, Table, NUM_TABLES}; | ||||||
use crate::all_stark::{AllStark, Table, KECCAK_TABLES_INDICES, NUM_TABLES}; | ||||||
use crate::cpu::kernel::aggregator::KERNEL; | ||||||
use crate::generation::segments::GenerationSegmentData; | ||||||
use crate::generation::{generate_traces, GenerationInputs, TrimmedGenerationInputs}; | ||||||
|
@@ -47,7 +47,7 @@ where | |||||
|
||||||
timed!(timing, "build kernel", Lazy::force(&KERNEL)); | ||||||
|
||||||
let (traces, mut public_values) = timed!( | ||||||
let mut tables_with_pvs = timed!( | ||||||
timing, | ||||||
"generate all traces", | ||||||
generate_traces(all_stark, &inputs, config, segment_data, timing)? | ||||||
|
@@ -58,8 +58,9 @@ where | |||||
let proof = prove_with_traces( | ||||||
all_stark, | ||||||
config, | ||||||
traces, | ||||||
&mut public_values, | ||||||
tables_with_pvs.tables, | ||||||
tables_with_pvs.use_keccak_tables, | ||||||
&mut tables_with_pvs.public_values, | ||||||
timing, | ||||||
abort_signal, | ||||||
)?; | ||||||
|
@@ -72,6 +73,7 @@ pub(crate) fn prove_with_traces<F, C, const D: usize>( | |||||
all_stark: &AllStark<F, D>, | ||||||
config: &StarkConfig, | ||||||
trace_poly_values: [Vec<PolynomialValues<F>>; NUM_TABLES], | ||||||
use_keccak_tables: bool, | ||||||
public_values: &mut PublicValues<F>, | ||||||
timing: &mut TimingTree, | ||||||
abort_signal: Option<Arc<AtomicBool>>, | ||||||
|
@@ -114,8 +116,14 @@ where | |||||
.map(|c| c.merkle_tree.cap.clone()) | ||||||
.collect::<Vec<_>>(); | ||||||
let mut challenger = Challenger::<F, C::Hasher>::new(); | ||||||
for cap in &trace_caps { | ||||||
challenger.observe_cap(cap); | ||||||
for (i, cap) in trace_caps.iter().enumerate() { | ||||||
if KECCAK_TABLES_INDICES.contains(&i) && !use_keccak_tables { | ||||||
// Observe zero merkle caps when skipping Keccak tables. | ||||||
let zero_merkle_cap = cap.flatten().iter().map(|_| F::ZERO).collect::<Vec<F>>(); | ||||||
challenger.observe_elements(&zero_merkle_cap); | ||||||
Comment on lines
+120
to
+123
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can they not just be ignored? (as in only observe if it's not a keccak table)? Also, has a similar change been applied on the verifier side (in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is difficult to implement conditionally observe in root circuits, so I choose observe zero caps instead. I was originally planning to implement it in the next PR (which requires changes on the Plonky2 side, and |
||||||
} else { | ||||||
challenger.observe_cap(cap); | ||||||
} | ||||||
} | ||||||
|
||||||
observe_public_values::<F, C, D>(&mut challenger, public_values) | ||||||
|
@@ -143,6 +151,7 @@ where | |||||
config, | ||||||
&trace_poly_values, | ||||||
trace_commitments, | ||||||
use_keccak_tables, | ||||||
ctl_data_per_table, | ||||||
&mut challenger, | ||||||
&ctl_challenges, | ||||||
|
@@ -206,6 +215,7 @@ where | |||||
ctl_challenges, | ||||||
}, | ||||||
public_values: public_values.clone(), | ||||||
use_keccak_tables, | ||||||
}) | ||||||
} | ||||||
|
||||||
|
@@ -229,6 +239,7 @@ fn prove_with_commitments<F, C, const D: usize>( | |||||
config: &StarkConfig, | ||||||
trace_poly_values: &[Vec<PolynomialValues<F>>; NUM_TABLES], | ||||||
trace_commitments: Vec<PolynomialBatch<F, C, D>>, | ||||||
use_keccak_tables: bool, | ||||||
ctl_data_per_table: [CtlData<F>; NUM_TABLES], | ||||||
challenger: &mut Challenger<F, C::Hasher>, | ||||||
ctl_challenges: &GrandProductChallengeSet<F>, | ||||||
|
@@ -262,8 +273,16 @@ where | |||||
let (arithmetic_proof, _) = prove_table!(arithmetic_stark, Table::Arithmetic); | ||||||
let (byte_packing_proof, _) = prove_table!(byte_packing_stark, Table::BytePacking); | ||||||
let (cpu_proof, _) = prove_table!(cpu_stark, Table::Cpu); | ||||||
let challenger_after_cpu = challenger.clone(); | ||||||
// TODO(sdeng): Keccak proofs are still required for CTLs, etc. Refactor the | ||||||
// code and remove the unnecessary parts. | ||||||
Nashtare marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
let (keccak_proof, _) = prove_table!(keccak_stark, Table::Keccak); | ||||||
let (keccak_sponge_proof, _) = prove_table!(keccak_sponge_stark, Table::KeccakSponge); | ||||||
if !use_keccak_tables { | ||||||
// We need to connect the challenger state of Logic and CPU tables when the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit (for consistency with actual ordering)
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also would need to probably rework the way we mark empty tables once we include BP / Logic / MemAfter, as we may do multiple hops before finding a non-empty table, and I find the use of multiple |
||||||
// Keccak tables are not in use. | ||||||
*challenger = challenger_after_cpu; | ||||||
} | ||||||
let (logic_proof, _) = prove_table!(logic_stark, Table::Logic); | ||||||
let (memory_proof, _) = prove_table!(memory_stark, Table::Memory); | ||||||
let (mem_before_proof, mem_before_cap) = prove_table!(mem_before_stark, Table::MemBefore); | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: it'd be best to return an error early than panicking in these calls