Skip to content

Commit 26d4f3f

Browse files
jotabulaciosdiegokingstonMauroToscano
authored
Fix/soundness underconstrained chips (#652)
* register missing in-chip soundness constraints * add underconstrained-chip soundness guard * add underconstrained-chip soundness regression tests * harden in_chip_constraint_count against unsigned-subtraction underflow * Revert beyond-spec SHIFT/DVRM constraints * Update stale comments and capacity estimate * Fix stale bus-interaction comments left by soundness fixes The IS_HALF/IS_HALFWORD senders added in this PR made several doc comments and one capacity hint inaccurate; update them to match. - dvrm.rs: module doc IS_HALF count ×16 -> ×20 (matches the function doc, which already said ×20 after n/d were added) - mul.rs: module + bus_interactions docs IS_HALF ×8 -> ×16 and note the lhs/rhs input range checks, not just lo/hi outputs - shift.rs: module doc "11 total" -> "15 total", add IS_HALFWORD (×4) to the sender list, and bump Vec::with_capacity(11) -> 15 - test_utils.rs: create_lt_air / create_mul_air now wire transition constraints, so their doc comments say "constraints and bus interactions" (matching create_shift_air / create_dvrm_air) --------- Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com>
1 parent 5cfac85 commit 26d4f3f

10 files changed

Lines changed: 440 additions & 32 deletions

File tree

prover/src/tables/dvrm.rs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
//! - `sign_n`, `sign_d`, `sign_q`, `sign_r`: Bit - sign bits
2323
//!
2424
//! ## Bus Interactions
25-
//! - Sender: IS_HALF (×16: n, d, r, n_sub_r, q)
25+
//! - Sender: IS_HALF (×20: n, d, r, n_sub_r, q)
2626
//! - Sender: MSB16 (×3 for sign extraction: n, d, r)
2727
//! - Sender: LT (×1 for abs_r < abs_d)
2828
//! - Sender: MUL (×2 for n_sub_r = d * q verification)
@@ -384,9 +384,34 @@ pub fn generate_dvrm_trace(
384384
pub fn bus_interactions() -> Vec<BusInteraction> {
385385
let mut interactions = Vec::new();
386386

387-
// DVRM-A1.i (IS_HALF[n[i]]) and DVRM-A2.i (IS_HALF[d[i]]) are assumptions:
388-
// the CPU (sender) is responsible for range-checking n and d before sending
389-
// to DVRM. The DVRM table does NOT send these IS_HALF lookups.
387+
// -------------------------------------------------------------------------
388+
// DVRM-A1.i: IS_HALF[n[i]] (×4) and DVRM-A2.i: IS_HALF[d[i]] (×4),
389+
// multiplicity: μ_q + μ_r.
390+
// The bus binds only the packed 32-bit words (DWordHL/DWordBL emit two
391+
// words, not the four halves), so without these the input halves are free:
392+
// a prover could supply non-canonical halves that re-pack to the same word
393+
// yet sum to 0 in the field, forging div_by_zero (DVRM-C17 keys on the
394+
// half-sum) for a nonzero denominator. Range-checking each half closes that.
395+
// -------------------------------------------------------------------------
396+
for col in [
397+
cols::N_0,
398+
cols::N_1,
399+
cols::N_2,
400+
cols::N_3,
401+
cols::D_0,
402+
cols::D_1,
403+
cols::D_2,
404+
cols::D_3,
405+
] {
406+
interactions.push(BusInteraction::sender(
407+
BusId::IsHalfword,
408+
Multiplicity::Sum(cols::MU_Q, cols::MU_R),
409+
vec![BusValue::Packed {
410+
start_column: col,
411+
packing: Packing::Direct,
412+
}],
413+
));
414+
}
390415

391416
// -------------------------------------------------------------------------
392417
// DVRM-C13.i: IS_HALF[r[i]] (×4), multiplicity: μ_q + μ_r

prover/src/tables/mul.rs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
//!
2626
//! ## Bus Interactions
2727
//! - Sender: MSB16 (×2 for sign extraction)
28-
//! - Sender: IS_HALF (×8 for lo/hi range checks)
28+
//! - Sender: IS_HALF (×16 for lhs/rhs input and lo/hi output range checks)
2929
//! - Sender: IS_B20 (×4 for carry range checks)
3030
//! - Receiver: MUL (×2 for lo and hi results)
3131
@@ -358,7 +358,7 @@ pub fn generate_mul_trace(
358358
///
359359
/// The MUL table:
360360
/// - **Sends** MSB16 lookups for sign bit extraction (×2)
361-
/// - **Sends** IS_HALF lookups for lo/hi range checks (×8)
361+
/// - **Sends** IS_HALF lookups for lhs/rhs input and lo/hi output range checks (×16)
362362
/// - **Sends** IS_B20 lookups for carry range checks (×4)
363363
/// - **Receives** MUL lookups from CPU table (×2: lo and hi)
364364
pub fn bus_interactions() -> Vec<BusInteraction> {
@@ -399,6 +399,31 @@ pub fn bus_interactions() -> Vec<BusInteraction> {
399399
],
400400
));
401401

402+
// -------------------------------------------------------------------------
403+
// IS_HALF lookups for lhs/rhs INPUT range checks (multiplicity: mu_lo + mu_hi).
404+
// The bus binds only the packed 32-bit words, so without these the input
405+
// half-limbs are free (non-canonical halves re-packing to the same word).
406+
// -------------------------------------------------------------------------
407+
for col in [
408+
cols::LHS_0,
409+
cols::LHS_1,
410+
cols::LHS_2,
411+
cols::LHS_3,
412+
cols::RHS_0,
413+
cols::RHS_1,
414+
cols::RHS_2,
415+
cols::RHS_3,
416+
] {
417+
interactions.push(BusInteraction::sender(
418+
BusId::IsHalfword,
419+
Multiplicity::Sum(cols::MU_LO, cols::MU_HI),
420+
vec![BusValue::Packed {
421+
start_column: col,
422+
packing: Packing::Direct,
423+
}],
424+
));
425+
}
426+
402427
// -------------------------------------------------------------------------
403428
// IS_HALF lookups for lo range checks (multiplicity: mu_lo + mu_hi)
404429
// -------------------------------------------------------------------------

prover/src/tables/shift.rs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
//! - Virtual: `limb_shift[3] = 1 - limb_shift_raw[0] - limb_shift_raw[1] - limb_shift_raw[2]`
1414
//! - Multiplicity: `μ`
1515
//!
16-
//! ## Bus Interactions (11 total)
17-
//! - Senders: MSB16, AND_BYTE (×3), ZERO, HWSL (×5)
16+
//! ## Bus Interactions (15 total)
17+
//! - Senders: MSB16, AND_BYTE (×3), ZERO, HWSL (×5), IS_HALFWORD (×4)
1818
//! - Receiver: SHIFT (from CPU)
1919
2020
use math::field::element::FieldElement;
@@ -377,7 +377,7 @@ pub fn generate_shift_trace(
377377

378378
/// Creates all bus interactions for the SHIFT table.
379379
pub fn bus_interactions() -> Vec<BusInteraction> {
380-
let mut interactions = Vec::with_capacity(11);
380+
let mut interactions = Vec::with_capacity(15);
381381

382382
// SHIFT-C14: MSB16[in[3]] → is_negative | signed
383383
interactions.push(BusInteraction::sender(
@@ -599,6 +599,22 @@ pub fn bus_interactions() -> Vec<BusInteraction> {
599599
],
600600
));
601601

602+
// VM-3: range-check every input half `in[i]` as a 16-bit value, unconditionally
603+
// on every active row. The SHIFT bus carries only the *packed* operand, so
604+
// without these a non-canonical half-decomposition that wraps in the field
605+
// (keeping the packed word constant) would be invisible to the caller while
606+
// still changing the shifted output.
607+
for input_col in cols::IN {
608+
interactions.push(BusInteraction::sender(
609+
BusId::IsHalfword,
610+
Multiplicity::Column(cols::MU),
611+
vec![BusValue::Packed {
612+
start_column: input_col,
613+
packing: Packing::Direct,
614+
}],
615+
));
616+
}
617+
602618
interactions
603619
}
604620

@@ -932,6 +948,16 @@ pub fn collect_bitwise_from_shift(operations: &[ShiftOperation]) -> Vec<BitwiseO
932948
op.shift,
933949
mask,
934950
));
951+
952+
// VM-3: IS_HALF[in[i]] for the four input halves, unconditional on every
953+
// active row — matches the four IS_HALF senders added in `bus_interactions`.
954+
for i in 0..4 {
955+
bitwise_ops.push(BitwiseOperation::halfword(
956+
BitwiseOperationType::IsHalf,
957+
(op.in_halves[i] & 0xFF) as u8,
958+
(op.in_halves[i] >> 8) as u8,
959+
));
960+
}
935961
}
936962

937963
bitwise_ops

prover/src/tables/trace_builder.rs

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,16 +1141,30 @@ fn collect_bitwise_from_lt(lt_ops: &[LtOperation]) -> Vec<BitwiseOperation> {
11411141
/// Collects bitwise lookups from MUL operations (MSB16 for sign bits).
11421142
///
11431143
/// MUL sends MSB16 lookups when signed=1 to extract sign bits,
1144-
/// IS_HALF lookups for lo/hi range checks, and IS_B20 lookups for carry range checks.
1144+
/// IS_HALF lookups for lhs/rhs input and lo/hi output range checks,
1145+
/// and IS_B20 lookups for carry range checks.
11451146
///
11461147
/// Returns: Vec of bitwise lookups
11471148
fn collect_bitwise_from_mul(mul_ops: &[(MulOperation, bool)]) -> Vec<BitwiseOperation> {
1148-
let mut bitwise_ops = Vec::with_capacity(mul_ops.len() * 14);
1149+
let mut bitwise_ops = Vec::with_capacity(mul_ops.len() * 20);
11491150

11501151
// IS_HALF and IS_B20: one set per raw op (multiplicity Sum(MU_LO, MU_HI))
11511152
for (op, _wants_hi) in mul_ops {
11521153
let (lo, hi) = op.compute_product();
11531154

1155+
// IS_HALF for lhs/rhs INPUT halfwords (matches the lhs/rhs IS_HALF senders
1156+
// in mul::bus_interactions).
1157+
for word in [op.lhs, op.rhs] {
1158+
for shift in [0, 16, 32, 48] {
1159+
let half = ((word >> shift) & 0xFFFF) as u16;
1160+
bitwise_ops.push(BitwiseOperation::halfword(
1161+
BitwiseOperationType::IsHalf,
1162+
(half & 0xFF) as u8,
1163+
(half >> 8) as u8,
1164+
));
1165+
}
1166+
}
1167+
11541168
// IS_HALF for lo halfwords
11551169
for shift in [0, 16, 32, 48] {
11561170
let half = ((lo >> shift) & 0xFFFF) as u16;
@@ -1213,16 +1227,32 @@ fn collect_bitwise_from_mul(mul_ops: &[(MulOperation, bool)]) -> Vec<BitwiseOper
12131227

12141228
/// Collects bitwise lookups from DVRM operations.
12151229
///
1216-
/// Generates: IS_HALF (×12), MSB16 (×3), ZERO (×2 per raw op + up to ×4 per unique signed op).
1230+
/// Generates: IS_HALF (×20: n, d, r, n_sub_r, q) and ZERO (×2) per raw op, plus
1231+
/// MSB16 (up to ×3) and NEG ZERO (up to ×4) per unique signed op.
12171232
///
1218-
/// DVRM-A1 (IS_HALF[n]) and DVRM-A2 (IS_HALF[d]) are assumptions enforced by the CPU sender,
1219-
/// not by the DVRM table. Only constraint-level IS_HALF lookups are generated here.
1233+
/// DVRM-A1 (IS_HALF[n]) and DVRM-A2 (IS_HALF[d]) are range-checked by the DVRM
1234+
/// table itself (n/d IS_HALF senders in dvrm::bus_interactions), so their lookups
1235+
/// are collected here alongside the constraint-level ones.
12201236
///
12211237
/// Returns: Vec of bitwise lookups
12221238
fn collect_bitwise_from_dvrm(dvrm_ops: &[(DvrmOperation, bool)]) -> Vec<BitwiseOperation> {
1223-
let mut bitwise_ops = Vec::with_capacity(dvrm_ops.len() * 16);
1239+
let mut bitwise_ops = Vec::with_capacity(dvrm_ops.len() * 24);
12241240

12251241
for (op, _wants_remainder) in dvrm_ops {
1242+
// IS_HALF for n[0..4] and d[0..4] (DVRM-A1/A2): range-check the input
1243+
// half-limbs so a prover cannot supply non-canonical halves (matches the
1244+
// n/d IS_HALF senders in dvrm::bus_interactions).
1245+
for word in [op.n, op.d] {
1246+
for shift in [0, 16, 32, 48] {
1247+
let half = ((word >> shift) & 0xFFFF) as u16;
1248+
bitwise_ops.push(BitwiseOperation::halfword(
1249+
BitwiseOperationType::IsHalf,
1250+
(half & 0xFF) as u8,
1251+
(half >> 8) as u8,
1252+
));
1253+
}
1254+
}
1255+
12261256
// IS_HALF for r[0..4] (DVRM-C13)
12271257
let r = op.compute_remainder();
12281258
for shift in [0, 16, 32, 48] {

prover/src/test_utils.rs

Lines changed: 93 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ use executor::vm::logs::Log;
2020
use executor::vm::memory::U64HashMap;
2121
use math::field::element::FieldElement;
2222
use stark::constraints::transition::{TransitionConstraint, TransitionConstraintEvaluator};
23-
use stark::lookup::{AirWithBuses, AuxiliaryTraceBuildData, NullBoundaryConstraintBuilder};
23+
use stark::debug::validate_trace;
24+
use stark::domain::Domain;
25+
use stark::lookup::{
26+
AirWithBuses, AuxiliaryTraceBuildData, BusInteraction, BusValue, NullBoundaryConstraintBuilder,
27+
};
2428
use stark::proof::options::ProofOptions;
2529
use stark::proof::stark::MultiProof;
2630
use stark::prover::{IsStarkProver, Prover, ProvingError};
@@ -59,7 +63,9 @@ use crate::tables::keccak_rnd::{
5963
use crate::tables::load::{
6064
bus_interactions as load_bus_interactions, cols as load_cols, constraints as load_constraints,
6165
};
62-
use crate::tables::lt::{LtOperation, bus_interactions as lt_bus_interactions, cols as lt_cols};
66+
use crate::tables::lt::{
67+
LtOperation, bus_interactions as lt_bus_interactions, cols as lt_cols, lt_constraints,
68+
};
6369
use crate::tables::memw::{
6470
bus_interactions as memw_bus_interactions, cols as memw_cols, constraints as memw_constraints,
6571
};
@@ -71,15 +77,17 @@ use crate::tables::memw_register::{
7177
bus_interactions as memw_register_bus_interactions, cols as memw_register_cols,
7278
constraints as memw_register_constraints,
7379
};
74-
use crate::tables::mul::{bus_interactions as mul_bus_interactions, cols as mul_cols};
80+
use crate::tables::mul::{
81+
bus_interactions as mul_bus_interactions, cols as mul_cols, mul_constraints,
82+
};
7583
use crate::tables::page::{bus_interactions as page_bus_interactions, cols as page_cols};
7684
use crate::tables::register::{
7785
bus_interactions as register_bus_interactions, cols as register_cols,
7886
};
7987
use crate::tables::shift::{
8088
bus_interactions as shift_bus_interactions, cols as shift_cols, shift_constraints,
8189
};
82-
use crate::tables::types::{GoldilocksExtension, GoldilocksField};
90+
use crate::tables::types::{BusId, GoldilocksExtension, GoldilocksField};
8391

8492
pub type F = GoldilocksField;
8593
pub type E = GoldilocksExtension;
@@ -108,6 +116,79 @@ where
108116
)
109117
}
110118

119+
// =============================================================================
120+
// Soundness regression helpers (negative AIR tests)
121+
// =============================================================================
122+
123+
/// Build a bus-less AIR carrying only the given in-chip transition constraints.
124+
/// With zero bus interactions, `AirWithBuses::new` appends no LogUp constraints
125+
/// and allocates no aux columns, so `validate_trace` evaluates exactly the chip's
126+
/// transition constraints over a main-only trace.
127+
pub fn busless_air<C: TransitionConstraint<F, E> + 'static>(
128+
num_columns: usize,
129+
constraints: Vec<C>,
130+
) -> VmAir {
131+
let transition_constraints = constraints.into_iter().map(|c| c.boxed()).collect();
132+
AirWithBuses::new(
133+
num_columns,
134+
AuxiliaryTraceBuildData {
135+
interactions: vec![],
136+
},
137+
&ProofOptions::default_test_options(),
138+
1,
139+
transition_constraints,
140+
)
141+
}
142+
143+
/// Run `validate_trace` for a bus-less chip AIR over a main-only trace.
144+
/// Returns `true` iff every transition constraint holds on every row.
145+
pub fn validate_busless(air: &VmAir, trace: &TraceTable<F, E>) -> bool {
146+
let domain = Domain::new(air, trace.num_rows());
147+
validate_trace(air, &(), trace, &domain, &[], None)
148+
}
149+
150+
/// Number of transition constraints a production builder registers on top of its
151+
/// bus constraints, as a delta against a bus-only AIR with the same interactions
152+
/// but no in-chip constraints. Isolates the in-chip count even though
153+
/// `AirWithBuses::new` also appends LogUp constraints, so a plain count cannot.
154+
pub fn in_chip_constraint_count(
155+
wired: usize,
156+
num_columns: usize,
157+
buses: Vec<BusInteraction>,
158+
) -> usize {
159+
let bus_only = AirWithBuses::<F, E, NullBoundaryConstraintBuilder, ()>::new(
160+
num_columns,
161+
AuxiliaryTraceBuildData {
162+
interactions: buses,
163+
},
164+
&ProofOptions::default_test_options(),
165+
1,
166+
vec![],
167+
)
168+
.num_transition_constraints();
169+
wired
170+
.checked_sub(bus_only)
171+
.expect("wired (in-chip + bus constraints) must be >= bus-only constraint count")
172+
}
173+
174+
/// Collect the `start_column`s of every `IS_HALFWORD` sender in `interactions`.
175+
/// Used to assert input/operand half-limbs are range-checked. Scope: only
176+
/// single-column `Packed` senders (which is how every current IS_HALFWORD sender is
177+
/// declared); it does not inspect `Linear` senders or sender multiplicities.
178+
pub fn is_halfword_sender_columns(interactions: &[BusInteraction]) -> Vec<usize> {
179+
let id: u64 = BusId::IsHalfword.into();
180+
interactions
181+
.iter()
182+
.filter(|i| i.is_sender && i.bus_id == id)
183+
.flat_map(|i| {
184+
i.values.iter().filter_map(|v| match v {
185+
BusValue::Packed { start_column, .. } => Some(*start_column),
186+
BusValue::Linear(_) => None,
187+
})
188+
})
189+
.collect()
190+
}
191+
111192
// =============================================================================
112193
// ELF Execution Helpers
113194
// =============================================================================
@@ -540,9 +621,11 @@ pub fn create_bitwise_air(proof_options: &ProofOptions) -> VmAir {
540621
.with_name("BITWISE")
541622
}
542623

543-
/// Create LT AIR with bus interactions.
624+
/// Create LT AIR with constraints and bus interactions.
544625
pub fn create_lt_air(proof_options: &ProofOptions) -> VmAir {
545-
let transition_constraints: Vec<Box<dyn TransitionConstraintEvaluator<F, E>>> = vec![];
626+
let (constraints, _) = lt_constraints(0);
627+
let transition_constraints: Vec<Box<dyn TransitionConstraintEvaluator<F, E>>> =
628+
constraints.into_iter().map(|c| c.boxed()).collect();
546629

547630
let auxiliary_trace_build_data = AuxiliaryTraceBuildData {
548631
interactions: lt_bus_interactions(),
@@ -680,9 +763,11 @@ pub fn create_decode_air(proof_options: &ProofOptions) -> VmAir {
680763
.with_name("DECODE")
681764
}
682765

683-
/// Create MUL AIR with bus interactions.
766+
/// Create MUL AIR with constraints and bus interactions.
684767
pub fn create_mul_air(proof_options: &ProofOptions) -> VmAir {
685-
let transition_constraints: Vec<Box<dyn TransitionConstraintEvaluator<F, E>>> = vec![];
768+
let (constraints, _) = mul_constraints(0);
769+
let transition_constraints: Vec<Box<dyn TransitionConstraintEvaluator<F, E>>> =
770+
constraints.into_iter().map(|c| c.boxed()).collect();
686771

687772
let auxiliary_trace_build_data = AuxiliaryTraceBuildData {
688773
interactions: mul_bus_interactions(),

0 commit comments

Comments
 (0)