From bf56c86cfcdb21ab72e92cda87903c6bccb16db4 Mon Sep 17 00:00:00 2001 From: Rob Knight Date: Fri, 13 Feb 2026 11:32:40 +0100 Subject: [PATCH] MultiPodBuilder fixes (#480) * Dedupe statements during POD-building * Fix failure to assume existence of Contains statement * Remove possible source of non-determinism * Faster ILP backend * Formatting --- Cargo.toml | 9 +++- src/frontend/multi_pod/deps.rs | 40 ++++++++++++++++- src/frontend/multi_pod/mod.rs | 80 +++++++++++++++++++++++++++------- 3 files changed, 109 insertions(+), 20 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ac0295d..a1f7511 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,9 @@ num = { version = "0.4.3", features = ["num-bigint"] } num-bigint = { version = "0.4.6", features = ["rand"] } # num-bigint 0.4 requires rand 0.8 rand = "0.8.5" -hashbrown = { version = "0.14.3", default-features = false, features = ["serde"] } +hashbrown = { version = "0.14.3", default-features = false, features = [ + "serde", +] } pest = "2.8.0" pest_derive = "2.8.0" petgraph = "0.6" @@ -41,7 +43,10 @@ serde_bytes = "0.11" serde_arrays = "0.2.0" sha2 = { version = "0.10.9" } rand_chacha = "0.3.1" -good_lp = { version = "1.8", default-features = false, features = ["microlp"] } +good_lp = { version = "1.8", default-features = false, features = [ + "scip", + "scip_bundled", +] } annotate-snippets = "0.11" # Uncomment for debugging with https://github.com/ed255/plonky2/ at branch `feat/debug`. The repo directory needs to be checked out next to the pod2 repo directory. diff --git a/src/frontend/multi_pod/deps.rs b/src/frontend/multi_pod/deps.rs index 328dd5b..0c411bd 100644 --- a/src/frontend/multi_pod/deps.rs +++ b/src/frontend/multi_pod/deps.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; +use super::cost::AnchoredKeyId; use crate::{ frontend::{Operation, OperationArg}, middleware::{Hash, Statement}, @@ -87,9 +88,15 @@ impl DependencyGraph { // Check if this is from an external POD if let Some(&pod_hash) = external_pod_statements.get(dep_stmt) { deps.push(StatementSource::External(pod_hash)); + } else if AnchoredKeyId::from_contains_statement(dep_stmt).is_some() { + // Anchored-key Contains args may be implicit requirements that are + // auto-materialized by MainPodBuilder. They are handled by anchored-key + // resource accounting, not by statement dependency edges. + continue; } else { // Statement arguments should either be internal (created earlier) - // or from external PODs. If neither, something is wrong. + // or from external PODs (except anchored-key implicit Contains). + // If neither, something is wrong. unreachable!( "Statement argument not found in internal statements or external PODs: {:?}", dep_stmt @@ -109,8 +116,9 @@ impl DependencyGraph { mod tests { use super::*; use crate::{ + dict, frontend::Operation as FrontendOp, - middleware::{NativeOperation, OperationAux, OperationType, Value, ValueRef}, + middleware::{AnchoredKey, NativeOperation, OperationAux, OperationType, Value, ValueRef}, }; fn equal_stmt(n: i64) -> Statement { @@ -175,4 +183,32 @@ mod tests { assert_eq!(graph.statement_deps[1], vec![StatementSource::Internal(0)]); assert_eq!(graph.statement_deps[2], vec![StatementSource::Internal(0)]); } + + #[test] + fn test_anchored_key_contains_arg_is_treated_as_implicit_requirement() { + // A literal Contains statement can be used as an anchored-key argument even when + // no explicit producer statement exists in internal/external statements, because + // MainPodBuilder auto-inserts Contains statements for anchored keys. + let dict = dict!({ + "k" => 7_i64 + }); + + let anchored_contains = Statement::Contains( + ValueRef::Literal(Value::from(dict.clone())), + ValueRef::Literal(Value::from("k")), + ValueRef::Literal(Value::from(7_i64)), + ); + let ak = AnchoredKey::from((&dict, "k")); + let produced_statement = Statement::Equal(ValueRef::Key(ak.clone()), ValueRef::Key(ak)); + + // Use a typical frontend operation that consumes entry-like args. + // We're only testing the dependency graph, not the actual proof, so the operation + // just needs to have the right arguments to test what we're looking for. + let statements = vec![produced_statement]; + let operations = vec![FrontendOp::eq(anchored_contains.clone(), anchored_contains)]; + + let graph = DependencyGraph::build(&statements, &operations, &HashMap::new()); + + assert!(graph.statement_deps[0].is_empty()); + } } diff --git a/src/frontend/multi_pod/mod.rs b/src/frontend/multi_pod/mod.rs index 70d14c3..7937d39 100644 --- a/src/frontend/multi_pod/mod.rs +++ b/src/frontend/multi_pod/mod.rs @@ -48,7 +48,7 @@ //! [`MainPodBuilder`]: crate::frontend::MainPodBuilder use std::{ - collections::{BTreeSet, HashMap}, + collections::{BTreeMap, BTreeSet, HashMap}, fmt, }; @@ -260,13 +260,24 @@ impl SolvedMultiPod { let public_set = &solution.pod_public_statements[pod_idx]; // Track statements proved locally in this POD for argument remapping. - // When an operation references a statement proved earlier in this same POD, - // we need to use the Statement object that MainPodBuilder created (not the - // original from MultiPodBuilder) so that find_op_arg can locate it. - let mut added_statements: HashMap = HashMap::new(); + // We index by statement content so duplicate statements can reuse a single + // built statement slot in MainPodBuilder. + let mut added_statements_by_content: HashMap = HashMap::new(); for &stmt_idx in &statements_sorted { let is_public = public_set.contains(&stmt_idx); + let original_stmt = self.statements[stmt_idx].clone(); + + // If this statement content was already built in this POD, reuse it instead + // of replaying the operation. If any duplicate is public, reveal the + // already-built statement. + if let Some(existing_stmt) = added_statements_by_content.get(&original_stmt) { + if is_public { + builder.reveal(existing_stmt); + } + continue; + } + let mut op = self.operations[stmt_idx].clone(); let wildcard_values = self.operations_wildcard_values[stmt_idx].clone(); @@ -276,19 +287,15 @@ impl SolvedMultiPod { // the input POD's public statements via find_op_arg. for arg in &mut op.1 { if let OperationArg::Statement(ref orig_stmt) = arg { - // Find the original statement's index - if let Some(orig_idx) = self.statements.iter().position(|s| s == orig_stmt) { - // Get the remapped statement if it was proved locally in this POD - if let Some(remapped_stmt) = added_statements.get(&orig_idx) { - *arg = OperationArg::Statement(remapped_stmt.clone()); - } + if let Some(remapped_stmt) = added_statements_by_content.get(orig_stmt) { + *arg = OperationArg::Statement(remapped_stmt.clone()); } } } let stmt = builder.op(is_public, wildcard_values, op)?; - added_statements.insert(stmt_idx, stmt); + added_statements_by_content.insert(original_stmt, stmt); } // Step 4: Prove the POD @@ -575,12 +582,14 @@ impl MultiPodBuilder { // Build statement content groups for deduplication. // Statements with identical content share a single slot in the POD. - // Group statement indices by their content. - let mut content_to_indices: HashMap<&Statement, Vec> = HashMap::new(); + // Keep groups ordered by first occurrence index for deterministic solver input. + let mut first_idx_by_stmt: HashMap<&Statement, usize> = HashMap::new(); + let mut groups_by_first_idx: BTreeMap> = BTreeMap::new(); for (idx, stmt) in self.statements.iter().enumerate() { - content_to_indices.entry(stmt).or_default().push(idx); + let first_idx = *first_idx_by_stmt.entry(stmt).or_insert(idx); + groups_by_first_idx.entry(first_idx).or_default().push(idx); } - let statement_content_groups: Vec> = content_to_indices.into_values().collect(); + let statement_content_groups: Vec> = groups_by_first_idx.into_values().collect(); // Run solver let input = solver::SolverInput { @@ -720,6 +729,45 @@ mod tests { Ok(()) } + #[test] + fn test_duplicate_statement_content_reused_within_pod() -> Result<()> { + // This test verifies that duplicate statement content is reused within a POD. + // We run three operations, but each produces the same statement. This allows us to + // deuplicate the private statement, matching the solver's deduplication logic. + // Since there is only space for 2 private statements with these parameters, the + // test can only succeed if deduplication is working correctly. + // Public statements/reveals of private statements are not deduplicated, so we can + // have 3 of them. + let params = Params { + max_statements: 5, + max_public_statements: 3, + // Derived: max_priv_statements = 2 + max_input_pods: 2, + max_input_pods_public_statements: 4, + ..Params::default() + }; + let vd_set = &*MOCK_VD_SET; + + let mut builder = MultiPodBuilder::new(¶ms, vd_set); + builder.pub_op(FrontendOp::eq(7, 7))?; + builder.pub_op(FrontendOp::eq(7, 7))?; + builder.pub_op(FrontendOp::eq(7, 7))?; + + let solved = builder.solve()?; + let pod_count = solved.solution().pod_count; + + let prover = MockProver {}; + let result = solved.prove(&prover)?; + assert_eq!(result.pods.len(), pod_count); + for (i, pod) in result.pods.iter().enumerate() { + pod.pod + .verify() + .unwrap_or_else(|_| panic!("POD {} verification failed", i)); + } + + Ok(()) + } + #[test] fn test_cross_pod_dependencies() -> Result<()> { // Verifies that a dependency chain can be split across PODs.