From a1fe87feae92cdfc4661704405605655469427c4 Mon Sep 17 00:00:00 2001 From: Ian Joiner <14581281+iajoiner@users.noreply.github.com> Date: Thu, 14 Nov 2024 07:46:35 -0500 Subject: [PATCH] fix: fix issues --- .../src/base/database/table_utility.rs | 18 +++++- .../src/sql/proof/query_proof_test.rs | 57 ++++++------------- .../sql/proof/verifiable_query_result_test.rs | 22 +++---- .../verifiable_query_result_test_utility.rs | 17 ++++-- .../src/sql/proof_plans/empty_exec.rs | 12 +++- .../src/sql/proof_plans/filter_exec.rs | 10 +++- .../filter_exec_test_dishonest_prover.rs | 10 +++- .../src/sql/proof_plans/group_by_exec.rs | 4 +- .../src/sql/proof_plans/projection_exec.rs | 47 ++++++++------- 9 files changed, 105 insertions(+), 92 deletions(-) diff --git a/crates/proof-of-sql/src/base/database/table_utility.rs b/crates/proof-of-sql/src/base/database/table_utility.rs index 5273f04a8..799c75cb0 100644 --- a/crates/proof-of-sql/src/base/database/table_utility.rs +++ b/crates/proof-of-sql/src/base/database/table_utility.rs @@ -15,7 +15,7 @@ //! borrowed_decimal75("f", 12, 1, [1, 2, 3], &alloc), //! ]); //! ``` -use super::{Column, Table}; +use super::{Column, Table, TableOptions}; use crate::base::scalar::Scalar; use alloc::{string::String, vec::Vec}; use bumpalo::Bump; @@ -27,7 +27,8 @@ use proof_of_sql_parser::{ /// Creates an [`Table`] from a list of `(Identifier, Column)` pairs. /// This is a convenience wrapper around [`Table::try_from_iter`] primarily for use in tests and -/// intended to be used along with the other methods in this module (e.g. [bigint], [boolean], etc). +/// intended to be used along with the other methods in this module (e.g. [`borrowed_bigint`], +/// [`borrowed_boolean`], etc). /// The function will panic under a variety of conditions. See [`Table::try_from_iter`] for more details. /// /// # Example @@ -53,6 +54,19 @@ pub fn table<'a, S: Scalar>( Table::try_from_iter(iter).unwrap() } +/// Creates an [`Table`] from a list of `(Identifier, Column)` pairs with a specified row count. +/// The main reason for this function is to allow for creating tables that may potentially have +/// no columns, but still have a specified row count. +/// +/// # Panics +/// - Panics if the given row count doesn't match the number of rows in any of the columns. +pub fn table_with_row_count<'a, S: Scalar>( + iter: impl IntoIterator)>, + row_count: usize, +) -> Table<'a, S> { + Table::try_from_iter_with_options(iter, TableOptions::new(Some(row_count))).unwrap() +} + /// Creates a (Identifier, `Column`) pair for a tinyint column. /// This is primarily intended for use in conjunction with [`table`]. /// # Example diff --git a/crates/proof-of-sql/src/sql/proof/query_proof_test.rs b/crates/proof-of-sql/src/sql/proof/query_proof_test.rs index 3f231b831..15ffba158 100644 --- a/crates/proof-of-sql/src/sql/proof/query_proof_test.rs +++ b/crates/proof-of-sql/src/sql/proof/query_proof_test.rs @@ -6,10 +6,11 @@ use crate::{ commitment::InnerProductProof, database::{ owned_table_utility::{bigint, owned_table}, - Column, ColumnField, ColumnRef, ColumnType, DataAccessor, OwnedTable, - OwnedTableTestAccessor, Table, TableRef, + table_utility::*, + ColumnField, ColumnRef, ColumnType, DataAccessor, OwnedTable, OwnedTableTestAccessor, + Table, TableRef, }, - map::{indexmap, indexset, IndexMap, IndexSet}, + map::{indexset, IndexMap, IndexSet}, proof::ProofError, scalar::{Curve25519Scalar, Scalar}, }, @@ -45,11 +46,8 @@ impl ProverEvaluate for TrivialTestProofPlan { alloc: &'a Bump, _accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { - let col = alloc.alloc_slice_fill_copy(self.length, self.column_fill_value); - Table::<'a, S>::try_new(indexmap! { - "a1".parse().unwrap() => Column::BigInt(col), - }) - .unwrap() + let col = vec![self.column_fill_value; self.length]; + table([borrowed_bigint("a1", col, alloc)]) } fn first_round_evaluate(&self, _builder: &mut FirstRoundBuilder) {} @@ -66,10 +64,11 @@ impl ProverEvaluate for TrivialTestProofPlan { SumcheckSubpolynomialType::Identity, vec![(S::ONE, vec![Box::new(col as &[_])])], ); - Table::<'a, S>::try_new(indexmap! { - "a1".parse().unwrap() => Column::BigInt(col), - }) - .unwrap() + table([borrowed_bigint( + "a1", + vec![self.column_fill_value; self.length], + alloc, + )]) } } impl ProofPlan for TrivialTestProofPlan { @@ -220,11 +219,7 @@ impl ProverEvaluate for SquareTestProofPlan { alloc: &'a Bump, _accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { - let res: &[_] = alloc.alloc_slice_copy(&self.res); - Table::<'a, S>::try_new(indexmap! { - "a1".parse().unwrap() => Column::BigInt(res), - }) - .unwrap() + table([borrowed_bigint("a1", self.res, alloc)]) } fn first_round_evaluate(&self, _builder: &mut FirstRoundBuilder) {} @@ -249,10 +244,7 @@ impl ProverEvaluate for SquareTestProofPlan { (-S::ONE, vec![Box::new(x), Box::new(x)]), ], ); - Table::<'a, S>::try_new(indexmap! { - "a1".parse().unwrap() => Column::BigInt(&[9, 25]), - }) - .unwrap() + table([borrowed_bigint("a1", self.res, alloc)]) } } impl ProofPlan for SquareTestProofPlan { @@ -403,11 +395,7 @@ impl ProverEvaluate for DoubleSquareTestProofPlan { alloc: &'a Bump, _accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { - let res: &[_] = alloc.alloc_slice_copy(&self.res); - Table::<'a, S>::try_new(indexmap! { - "a1".parse().unwrap() => Column::BigInt(res), - }) - .unwrap() + table([borrowed_bigint("a1", self.res, alloc)]) } fn first_round_evaluate(&self, _builder: &mut FirstRoundBuilder) {} @@ -445,10 +433,7 @@ impl ProverEvaluate for DoubleSquareTestProofPlan { ], ); builder.produce_intermediate_mle(res); - Table::<'a, S>::try_new(indexmap! { - "a1".parse().unwrap() => Column::BigInt(res), - }) - .unwrap() + table([borrowed_bigint("a1", self.res, alloc)]) } } impl ProofPlan for DoubleSquareTestProofPlan { @@ -613,13 +598,10 @@ struct ChallengeTestProofPlan {} impl ProverEvaluate for ChallengeTestProofPlan { fn result_evaluate<'a, S: Scalar>( &self, - _alloc: &'a Bump, + alloc: &'a Bump, _accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { - Table::<'a, S>::try_new(indexmap! { - "a1".parse().unwrap() => Column::BigInt(&[9, 25]), - }) - .unwrap() + table([borrowed_bigint("a1", [9, 25], alloc)]) } fn first_round_evaluate(&self, builder: &mut FirstRoundBuilder) { @@ -648,10 +630,7 @@ impl ProverEvaluate for ChallengeTestProofPlan { (-alpha, vec![Box::new(x), Box::new(x)]), ], ); - Table::<'a, S>::try_new(indexmap! { - "a1".parse().unwrap() => Column::BigInt(&[9, 25]), - }) - .unwrap() + table([borrowed_bigint("a1", [9, 25], alloc)]) } } impl ProofPlan for ChallengeTestProofPlan { diff --git a/crates/proof-of-sql/src/sql/proof/verifiable_query_result_test.rs b/crates/proof-of-sql/src/sql/proof/verifiable_query_result_test.rs index 12a594129..194da80e9 100644 --- a/crates/proof-of-sql/src/sql/proof/verifiable_query_result_test.rs +++ b/crates/proof-of-sql/src/sql/proof/verifiable_query_result_test.rs @@ -7,8 +7,9 @@ use crate::{ commitment::InnerProductProof, database::{ owned_table_utility::{bigint, owned_table}, - Column, ColumnField, ColumnRef, ColumnType, DataAccessor, OwnedTable, - OwnedTableTestAccessor, Table, TableRef, + table_utility::*, + ColumnField, ColumnRef, ColumnType, DataAccessor, OwnedTable, OwnedTableTestAccessor, + Table, TableRef, }, map::{indexset, IndexMap, IndexSet}, proof::ProofError, @@ -30,12 +31,11 @@ impl ProverEvaluate for EmptyTestQueryExpr { alloc: &'a Bump, _accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { - let zeros = vec![0; self.length]; - let res: &[_] = alloc.alloc_slice_copy(&zeros); - Table::<'a, S>::try_from_iter( - (1..=self.columns).map(|i| (format!("a{i}").parse().unwrap(), Column::BigInt(res))), + let zeros = vec![0_i64; self.length]; + table_with_row_count( + (1..=self.columns).map(|i| borrowed_bigint(format!("a{i}"), zeros.clone(), alloc)), + self.length, ) - .unwrap() } fn first_round_evaluate(&self, _builder: &mut FirstRoundBuilder) {} fn final_round_evaluate<'a, S: Scalar>( @@ -44,15 +44,15 @@ impl ProverEvaluate for EmptyTestQueryExpr { alloc: &'a Bump, _accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { - let zeros = vec![0; self.length]; + let zeros = vec![0_i64; self.length]; let res: &[_] = alloc.alloc_slice_copy(&zeros); let _ = std::iter::repeat_with(|| builder.produce_intermediate_mle(res)) .take(self.columns) .collect::>(); - Table::<'a, S>::try_from_iter( - (1..=self.columns).map(|i| (format!("a{i}").parse().unwrap(), Column::BigInt(res))), + table_with_row_count( + (1..=self.columns).map(|i| borrowed_bigint(format!("a{i}"), zeros.clone(), alloc)), + self.length, ) - .unwrap() } } impl ProofPlan for EmptyTestQueryExpr { diff --git a/crates/proof-of-sql/src/sql/proof/verifiable_query_result_test_utility.rs b/crates/proof-of-sql/src/sql/proof/verifiable_query_result_test_utility.rs index 19ec95691..2bc13b836 100644 --- a/crates/proof-of-sql/src/sql/proof/verifiable_query_result_test_utility.rs +++ b/crates/proof-of-sql/src/sql/proof/verifiable_query_result_test_utility.rs @@ -39,10 +39,10 @@ pub fn exercise_verification( "Verification failed: {:?}", verification_result.err() ); - + dbg!("1"); // try changing the result tamper_result(res, expr, accessor); - + dbg!("2a"); if res.proof.is_none() { return; } @@ -54,7 +54,7 @@ pub fn exercise_verification( res_p.proof.as_mut().unwrap().pcs_proof_evaluations[i] += Curve25519Scalar::one(); assert!(res_p.verify(expr, accessor, &()).is_err()); } - + dbg!("2"); // try changing intermediate commitments let commit_p = RistrettoPoint::compute_commitments( &[CommittableColumn::BigInt(&[ @@ -70,7 +70,7 @@ pub fn exercise_verification( res_p.proof.as_mut().unwrap().commitments[i] = commit_p; assert!(res_p.verify(expr, accessor, &()).is_err()); } - + dbg!("3"); // try changing the offset // // Note: in the n = 1 case with proof.commmitments all the identity element, @@ -84,6 +84,7 @@ pub fn exercise_verification( fake_accessor.update_offset(table_ref, offset_generators); res.verify(expr, &fake_accessor, &()).unwrap(); fake_accessor.update_offset(table_ref, offset_generators + 1); + dbg!("4"); assert!(res.verify(expr, &fake_accessor, &()).is_err()); } } @@ -94,11 +95,12 @@ fn tamper_no_result( accessor: &impl CommitmentAccessor, ) { // add a result + dbg!("tamper_no_result11"); let mut res_p = res.clone(); let cols: [Column<'_, Curve25519Scalar>; 1] = [Column::BigInt(&[0_i64; 0])]; res_p.provable_result = Some(ProvableQueryResult::new(0, &cols)); assert!(res_p.verify(expr, accessor, &()).is_err()); - + dbg!("tamper_no_result22"); // add a proof let mut res_p = res.clone(); let expr_p = EmptyTestQueryExpr { @@ -114,6 +116,7 @@ fn tamper_no_result( ); let (proof, _result) = QueryProof::new(&expr_p, &accessor_p, &()); res_p.proof = Some(proof); + dbg!("tamper_no_result33"); assert!(res_p.verify(expr, accessor, &()).is_err()); } @@ -141,13 +144,16 @@ fn tamper_result( expr: &(impl ProofPlan + Serialize), accessor: &impl CommitmentAccessor, ) { + dbg!("tamper_result"); if res.provable_result.is_none() { + dbg!("tamper_no_result"); tamper_no_result(res, expr, accessor); return; } let provable_res = res.provable_result.as_ref().unwrap(); if provable_res.table_length() == 0 { + dbg!("tamper_empty_result"); tamper_empty_result(res, expr, accessor); return; } @@ -157,5 +163,6 @@ fn tamper_result( let mut provable_res_p = provable_res.clone(); provable_res_p.data_mut()[0] += 1; res_p.provable_result = Some(provable_res_p); + dbg!("tamper_result 1"); assert!(res_p.verify(expr, accessor, &()).is_err()); } diff --git a/crates/proof-of-sql/src/sql/proof_plans/empty_exec.rs b/crates/proof-of-sql/src/sql/proof_plans/empty_exec.rs index b6673cab7..a06420c7b 100644 --- a/crates/proof-of-sql/src/sql/proof_plans/empty_exec.rs +++ b/crates/proof-of-sql/src/sql/proof_plans/empty_exec.rs @@ -1,6 +1,8 @@ use crate::{ base::{ - database::{ColumnField, ColumnRef, DataAccessor, OwnedTable, Table, TableRef}, + database::{ + ColumnField, ColumnRef, DataAccessor, OwnedTable, Table, TableOptions, TableRef, + }, map::{IndexMap, IndexSet}, proof::ProofError, scalar::Scalar, @@ -68,7 +70,9 @@ impl ProverEvaluate for EmptyExec { _alloc: &'a Bump, _accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { - Table::<'a, S>::try_new(IndexMap::default()).unwrap() + // Create an empty table with one row + Table::<'a, S>::try_new_with_options(IndexMap::default(), TableOptions::new(Some(1))) + .unwrap() } fn first_round_evaluate(&self, _builder: &mut FirstRoundBuilder) {} @@ -81,6 +85,8 @@ impl ProverEvaluate for EmptyExec { _alloc: &'a Bump, _accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { - Table::<'a, S>::try_new(IndexMap::default()).unwrap() + // Create an empty table with one row + Table::<'a, S>::try_new_with_options(IndexMap::default(), TableOptions::new(Some(1))) + .unwrap() } } diff --git a/crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs b/crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs index 884613e41..5fadee4e0 100644 --- a/crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs +++ b/crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs @@ -3,7 +3,7 @@ use crate::{ base::{ database::{ filter_util::filter_columns, Column, ColumnField, ColumnRef, DataAccessor, OwnedTable, - TableRef, + Table, TableOptions, TableRef, }, map::{IndexMap, IndexSet}, proof::ProofError, @@ -149,6 +149,7 @@ impl ProverEvaluate for FilterExec { let selection = selection_column .as_boolean() .expect("selection is not boolean"); + let output_length = selection.into_iter().filter(|b| **b).count(); // 2. columns let columns: Vec<_> = self @@ -159,11 +160,12 @@ impl ProverEvaluate for FilterExec { // Compute filtered_columns and indexes let (filtered_columns, _) = filter_columns(alloc, &columns, selection); - Table::<'a, S>::try_from_iter( + Table::<'a, S>::try_from_iter_with_options( self.aliased_results .iter() .map(|expr| expr.alias) .zip(filtered_columns), + TableOptions::new(Some(output_length)), ) .expect("Failed to create table from iterator") } @@ -189,6 +191,7 @@ impl ProverEvaluate for FilterExec { let selection = selection_column .as_boolean() .expect("selection is not boolean"); + let output_length = selection.into_iter().filter(|b| **b).count(); // 2. columns let columns: Vec<_> = self @@ -220,11 +223,12 @@ impl ProverEvaluate for FilterExec { &filtered_columns, result_len, ); - Table::<'a, S>::try_from_iter( + Table::<'a, S>::try_from_iter_with_options( self.aliased_results .iter() .map(|expr| expr.alias) .zip(filtered_columns), + TableOptions::new(Some(output_length)), ) .expect("Failed to create table from iterator") } diff --git a/crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs b/crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs index 176eb6fd5..966680bf9 100644 --- a/crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs +++ b/crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs @@ -3,7 +3,7 @@ use crate::{ base::{ database::{ filter_util::*, owned_table_utility::*, Column, DataAccessor, OwnedTableTestAccessor, - TestAccessor, + Table, TableOptions, TestAccessor, }, proof::ProofError, scalar::Scalar, @@ -45,6 +45,7 @@ impl ProverEvaluate for DishonestFilterExec { let selection = selection_column .as_boolean() .expect("selection is not boolean"); + let output_length = selection.into_iter().filter(|b| **b).count(); // 2. columns let columns: Vec<_> = self .aliased_results @@ -54,11 +55,12 @@ impl ProverEvaluate for DishonestFilterExec { // Compute filtered_columns let (filtered_columns, _) = filter_columns(alloc, &columns, selection); let filtered_columns = tamper_column(alloc, filtered_columns); - Table::<'a, S>::try_from_iter( + Table::<'a, S>::try_from_iter_with_options( self.aliased_results .iter() .map(|expr| expr.alias) .zip(filtered_columns), + TableOptions::new(Some(output_length)), ) .expect("Failed to create table from iterator") } @@ -88,6 +90,7 @@ impl ProverEvaluate for DishonestFilterExec { let selection = selection_column .as_boolean() .expect("selection is not boolean"); + let output_length = selection.into_iter().filter(|b| **b).count(); // 2. columns let columns: Vec<_> = self .aliased_results @@ -119,11 +122,12 @@ impl ProverEvaluate for DishonestFilterExec { &filtered_columns, result_len, ); - Table::<'a, S>::try_from_iter( + Table::<'a, S>::try_from_iter_with_options( self.aliased_results .iter() .map(|expr| expr.alias) .zip(filtered_columns), + TableOptions::new(Some(output_length)), ) .expect("Failed to create table from iterator") } diff --git a/crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs b/crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs index b5251fada..41f6db0b6 100644 --- a/crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs +++ b/crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs @@ -5,7 +5,7 @@ use crate::{ group_by_util::{ aggregate_columns, compare_indexes_by_owned_columns, AggregatedColumns, }, - Column, ColumnField, ColumnRef, ColumnType, DataAccessor, OwnedTable, TableRef, + Column, ColumnField, ColumnRef, ColumnType, DataAccessor, OwnedTable, Table, TableRef, }, map::{IndexMap, IndexSet}, proof::ProofError, @@ -309,7 +309,7 @@ impl ProverEvaluate for GroupByExec { ) .expect("Failed to create table from column references"); // 5. Produce MLEs - for column in columns.into_iter() { + for column in columns { builder.produce_intermediate_mle(column); } // 6. Prove group by diff --git a/crates/proof-of-sql/src/sql/proof_plans/projection_exec.rs b/crates/proof-of-sql/src/sql/proof_plans/projection_exec.rs index bbb96a86b..464307be1 100644 --- a/crates/proof-of-sql/src/sql/proof_plans/projection_exec.rs +++ b/crates/proof-of-sql/src/sql/proof_plans/projection_exec.rs @@ -1,6 +1,8 @@ use crate::{ base::{ - database::{ColumnField, ColumnRef, DataAccessor, OwnedTable, Table, TableRef}, + database::{ + ColumnField, ColumnRef, DataAccessor, OwnedTable, Table, TableOptions, TableRef, + }, map::{IndexMap, IndexSet}, proof::ProofError, scalar::Scalar, @@ -91,25 +93,17 @@ impl ProverEvaluate for ProjectionExec { accessor: &'a dyn DataAccessor, ) -> Table<'a, S> { let column_refs = self.get_column_references(); -<<<<<<< HEAD let used_table = accessor.get_table(self.table.table_ref, &column_refs); - let columns: Vec<_> = self - .aliased_results - .iter() - .map(|aliased_expr| aliased_expr.expr.result_evaluate(alloc, &used_table)) - .collect(); - columns -======= - let used_table = - Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc); - Table::<'a, S>::try_from_iter(self.aliased_results.iter().map(|aliased_expr| { - ( - aliased_expr.alias, - aliased_expr.expr.result_evaluate(alloc, &used_table), - ) - })) + Table::<'a, S>::try_from_iter_with_options( + self.aliased_results.iter().map(|aliased_expr| { + ( + aliased_expr.alias, + aliased_expr.expr.result_evaluate(alloc, &used_table), + ) + }), + TableOptions::new(Some(accessor.get_length(self.table.table_ref))), + ) .expect("Failed to create table from iterator") ->>>>>>> 531ba7cc (refactor!: let `ProofPlan::result_evaluate` and `final_round_evaluate` return `Table`) } fn first_round_evaluate(&self, _builder: &mut FirstRoundBuilder) {} @@ -129,12 +123,17 @@ impl ProverEvaluate for ProjectionExec { let column_refs = self.get_column_references(); let used_table = accessor.get_table(self.table.table_ref, &column_refs); // 1. Evaluate result expressions - let res = Table::<'a, S>::try_from_iter(self.aliased_results.iter().map(|aliased_expr| { - ( - aliased_expr.alias, - aliased_expr.expr.result_evaluate(alloc, &used_table), - ) - })) + let res = Table::<'a, S>::try_from_iter_with_options( + self.aliased_results.iter().map(|aliased_expr| { + ( + aliased_expr.alias, + aliased_expr + .expr + .prover_evaluate(builder, alloc, &used_table), + ) + }), + TableOptions::new(Some(accessor.get_length(self.table.table_ref))), + ) .expect("Failed to create table from iterator"); // 2. Produce MLEs res.inner_table().values().for_each(|column| {