Skip to content

Commit

Permalink
fix!: address NITs in PR #361
Browse files Browse the repository at this point in the history
  • Loading branch information
iajoiner committed Nov 12, 2024
1 parent a0ffccb commit c135661
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 82 deletions.
3 changes: 0 additions & 3 deletions crates/proof-of-sql/src/base/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ pub use expression_evaluation_error::{ExpressionEvaluationError, ExpressionEvalu

mod test_accessor;
pub use test_accessor::TestAccessor;
#[cfg(test)]
#[allow(unused_imports)]
pub(crate) use test_accessor::UnimplementedTestAccessor;

#[cfg(test)]
mod test_schema_accessor;
Expand Down
60 changes: 2 additions & 58 deletions crates/proof-of-sql/src/base/database/test_accessor.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
use super::{
Column, ColumnRef, ColumnType, CommitmentAccessor, DataAccessor, MetadataAccessor,
SchemaAccessor, TableRef,
};
use crate::base::{commitment::Commitment, scalar::Curve25519Scalar};
use super::{CommitmentAccessor, DataAccessor, MetadataAccessor, SchemaAccessor, TableRef};
use crate::base::commitment::Commitment;
use alloc::vec::Vec;
use curve25519_dalek::ristretto::RistrettoPoint;
use proof_of_sql_parser::Identifier;

/// A trait that defines the interface for a combined metadata, schema, commitment, and data accessor for unit testing or example purposes.
pub trait TestAccessor<C: Commitment>:
Expand All @@ -31,54 +26,3 @@ pub trait TestAccessor<C: Commitment>:
/// Update the table offset alongside its column commitments
fn update_offset(&mut self, table_ref: TableRef, new_offset: usize);
}

#[derive(Clone, Default)]
/// A test accessor that leaves all of the required methods except `new` `unimplemented!()`.
pub struct UnimplementedTestAccessor;
impl TestAccessor<RistrettoPoint> for UnimplementedTestAccessor {
type Table = ();

fn new_empty() -> Self {
UnimplementedTestAccessor
}

fn add_table(&mut self, _table_ref: TableRef, _data: (), _table_offset: usize) {
unimplemented!()
}

fn get_column_names(&self, _table_ref: TableRef) -> Vec<&str> {
unimplemented!()
}

fn update_offset(&mut self, _table_ref: TableRef, _new_offset: usize) {
unimplemented!()
}
}
impl DataAccessor<Curve25519Scalar> for UnimplementedTestAccessor {
fn get_column(&self, _column: ColumnRef) -> Column<Curve25519Scalar> {
unimplemented!()
}
}
impl CommitmentAccessor<RistrettoPoint> for UnimplementedTestAccessor {
fn get_commitment(&self, _column: ColumnRef) -> RistrettoPoint {
unimplemented!()
}
}
impl MetadataAccessor for UnimplementedTestAccessor {
fn get_length(&self, _table_ref: TableRef) -> usize {
unimplemented!()
}

fn get_offset(&self, _table_ref: TableRef) -> usize {
unimplemented!()
}
}
impl SchemaAccessor for UnimplementedTestAccessor {
fn lookup_column(&self, _table_ref: TableRef, _column_id: Identifier) -> Option<ColumnType> {
unimplemented!()
}

fn lookup_schema(&self, _table_ref: TableRef) -> Vec<(Identifier, ColumnType)> {
unimplemented!()
}
}
11 changes: 1 addition & 10 deletions crates/proof-of-sql/src/sql/proof/proof_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use super::{CountBuilder, FinalRoundBuilder, FirstRoundBuilder, VerificationBuil
use crate::base::{
commitment::Commitment,
database::{
Column, ColumnField, ColumnRef, CommitmentAccessor, DataAccessor, MetadataAccessor,
OwnedTable, TableRef,
Column, ColumnField, ColumnRef, CommitmentAccessor, DataAccessor, OwnedTable, TableRef,
},
map::IndexSet,
proof::ProofError,
Expand All @@ -19,14 +18,6 @@ pub trait ProofPlan: Debug + Send + Sync + ProverEvaluate {
/// Count terms used within the Query's proof
fn count(&self, builder: &mut CountBuilder) -> Result<(), ProofError>;

/// Check if the input table is empty
fn is_empty(&self, accessor: &dyn MetadataAccessor) -> bool {
let table_refs = self.get_table_references();
table_refs
.iter()
.all(|table_ref| accessor.get_length(*table_ref) == 0)
}

/// Form components needed to verify and proof store into `VerificationBuilder`
fn verifier_evaluate<C: Commitment>(
&self,
Expand Down
5 changes: 2 additions & 3 deletions crates/proof-of-sql/src/sql/proof/query_proof_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ fn verify_a_trivial_query_proof_with_given_offset(n: usize, offset_generators: u
let column: Vec<i64> = vec![0_i64; n];
let accessor = OwnedTableTestAccessor::<InnerProductProof>::new_from_table(
"sxt.test".parse().unwrap(),
owned_table([bigint("a1", column)]),
owned_table([bigint("a1", column.clone())]),
offset_generators,
(),
);
Expand All @@ -122,8 +122,7 @@ fn verify_a_trivial_query_proof_with_given_offset(n: usize, offset_generators: u
table,
} = proof.verify(&expr, &accessor, &result, &()).unwrap();
assert_ne!(verification_hash, [0; 32]);
let expected_col = vec![0_i64; n];
let expected_result = owned_table([bigint("a1", expected_col)]);
let expected_result = owned_table([bigint("a1", column)]);
assert_eq!(table, expected_result);
}

Expand Down
12 changes: 10 additions & 2 deletions crates/proof-of-sql/src/sql/proof/verifiable_query_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ impl<CP: CommitmentEvaluationProof> VerifiableQueryResult<CP> {
// have been rejected at the parsing stage.

// handle the empty case
if expr.is_empty(accessor) {
let table_refs = expr.get_table_references();
if table_refs
.iter()
.all(|table_ref| accessor.get_length(*table_ref) == 0)
{
return VerifiableQueryResult {
provable_result: None,
proof: None,
Expand Down Expand Up @@ -124,7 +128,11 @@ impl<CP: CommitmentEvaluationProof> VerifiableQueryResult<CP> {
// have been rejected at the parsing stage.

// handle the empty case
if expr.is_empty(accessor) {
let table_refs = expr.get_table_references();
if table_refs
.iter()
.all(|table_ref| accessor.get_length(*table_ref) == 0)
{
if self.provable_result.is_some() || self.proof.is_some() {
return Err(ProofError::VerificationError {
error: "zero sumcheck variables but non-empty result",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,9 @@ fn we_can_verify_queries_on_an_empty_table() {
columns: 1,
..Default::default()
};
let column: Vec<i64> = vec![0_i64; 0];
let accessor = OwnedTableTestAccessor::<InnerProductProof>::new_from_table(
"sxt.test".parse().unwrap(),
owned_table([bigint("a1", column)]),
owned_table([bigint("a1", [0_i64; 0])]),
0,
(),
);
Expand All @@ -113,10 +112,9 @@ fn empty_verification_fails_if_the_result_contains_non_null_members() {
columns: 1,
..Default::default()
};
let column: Vec<i64> = vec![0_i64; 0];
let accessor = OwnedTableTestAccessor::<InnerProductProof>::new_from_table(
"sxt.test".parse().unwrap(),
owned_table([bigint("a1", column)]),
owned_table([bigint("a1", [0_i64; 0])]),
0,
(),
);
Expand Down
3 changes: 1 addition & 2 deletions crates/proof-of-sql/src/sql/proof_plans/dyn_proof_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ use crate::{
base::{
commitment::Commitment,
database::{
Column, ColumnField, ColumnRef, CommitmentAccessor, DataAccessor, MetadataAccessor,
OwnedTable, TableRef,
Column, ColumnField, ColumnRef, CommitmentAccessor, DataAccessor, OwnedTable, TableRef,
},
map::IndexSet,
proof::ProofError,
Expand Down

0 comments on commit c135661

Please sign in to comment.