Skip to content

Commit

Permalink
refactor!: simplify VerifiableQueryResult::verify (#398)
Browse files Browse the repository at this point in the history
# Rationale for this change

`VerifiableQueryResult::verify` is more complex than it needs to be

# What changes are included in this PR?

See individual commit messages.

# Are these changes tested?

Yes, by existing tests.
  • Loading branch information
JayWhite2357 authored Dec 3, 2024
1 parent a10ff3b commit ba1ebbd
Show file tree
Hide file tree
Showing 25 changed files with 98 additions and 101 deletions.
8 changes: 6 additions & 2 deletions crates/proof-of-sql/benches/scaffold/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub fn jaeger_scaffold<CP: CommitmentEvaluationProof>(
}

#[allow(dead_code, clippy::module_name_repetitions)]
pub fn criterion_scaffold<CP: CommitmentEvaluationProof>(
pub fn criterion_scaffold<CP: CommitmentEvaluationProof + Clone>(
c: &mut Criterion,
title: &str,
query: &str,
Expand Down Expand Up @@ -107,7 +107,11 @@ pub fn criterion_scaffold<CP: CommitmentEvaluationProof>(
});
});
group.bench_function("Verify Proof", |b| {
b.iter(|| result.verify(query.proof_expr(), &accessor, verifier_setup));
b.iter(|| {
result
.clone()
.verify(query.proof_expr(), &accessor, verifier_setup)
});
});
}
}
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/albums/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/avocado-prices/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/books/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/brands/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/census/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/countries/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/dinosaurs/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/dog_breeds/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/hello_world/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn main() {
let result = proof.verify(
query.proof_expr(),
&accessor,
&serialized_result,
serialized_result,
&&verifier_setup,
);
end_timer(timer);
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/plastics/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/programming_books/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/rockets/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/space/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/stocks/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/sushi/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/tech_gadget_prices/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn prove_and_verify_query(
let result = proof.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)?;
println!("Verified in {} ms.", now.elapsed().as_secs_f64() * 1000.);
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/vehicles/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/examples/wood_types/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn prove_and_verify_query(
.verify(
query_plan.proof_expr(),
accessor,
&provable_result,
provable_result,
&verifier_setup,
)
.unwrap();
Expand Down
6 changes: 3 additions & 3 deletions crates/proof-of-sql/src/sql/proof/query_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ impl<CP: CommitmentEvaluationProof> QueryProof<CP> {
#[tracing::instrument(name = "QueryProof::verify", level = "debug", skip_all, err)]
/// Verify a `QueryProof`. Note: This does NOT transform the result!
pub fn verify(
&self,
self,
expr: &(impl ProofPlan + Serialize),
accessor: &impl CommitmentAccessor<CP::Commitment>,
result: &ProvableQueryResult,
result: ProvableQueryResult,
setup: &CP::VerifierPublicSetup<'_>,
) -> QueryResult<CP::Scalar> {
let owned_table_result = result.to_owned_table(&expr.get_column_result_fields())?;
Expand Down Expand Up @@ -244,7 +244,7 @@ impl<CP: CommitmentEvaluationProof> QueryProof<CP> {
// construct a transcript for the proof
let mut transcript: Keccak256Transcript = make_transcript(
expr,
result,
&result,
self.range_length,
min_row_num,
&self.one_evaluation_lengths,
Expand Down
42 changes: 27 additions & 15 deletions crates/proof-of-sql/src/sql/proof/query_proof_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ fn verify_a_trivial_query_proof_with_given_offset(n: usize, offset_generators: u
let QueryData {
verification_hash,
table,
} = proof.verify(&expr, &accessor, &result, &()).unwrap();
} = proof
.clone()
.verify(&expr, &accessor, result.clone(), &())
.unwrap();
assert_ne!(verification_hash, [0; 32]);
let expected_result = owned_table([bigint("a1", column)]);
assert_eq!(table, expected_result);
Expand Down Expand Up @@ -166,7 +169,7 @@ fn verify_fails_if_the_summation_in_sumcheck_isnt_zero() {
(),
);
let (proof, result) = QueryProof::<InnerProductProof>::new(&expr, &accessor, &());
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[test]
Expand All @@ -184,7 +187,7 @@ fn verify_fails_if_the_sumcheck_evaluation_isnt_correct() {
(),
);
let (proof, result) = QueryProof::<InnerProductProof>::new(&expr, &accessor, &());
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[test]
Expand All @@ -202,7 +205,7 @@ fn verify_fails_if_counts_dont_match() {
(),
);
let (proof, result) = QueryProof::<InnerProductProof>::new(&expr, &accessor, &());
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

/// prove and verify an artificial query where
Expand Down Expand Up @@ -320,7 +323,10 @@ fn verify_a_proof_with_an_anchored_commitment_and_given_offset(offset_generators
let QueryData {
verification_hash,
table,
} = proof.verify(&expr, &accessor, &result, &()).unwrap();
} = proof
.clone()
.verify(&expr, &accessor, result.clone(), &())
.unwrap();
assert_ne!(verification_hash, [0; 32]);
let expected_result = owned_table([bigint("a1", [9, 25])]);
assert_eq!(table, expected_result);
Expand All @@ -332,7 +338,7 @@ fn verify_a_proof_with_an_anchored_commitment_and_given_offset(offset_generators
offset_generators + 1,
(),
);
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[test]
Expand Down Expand Up @@ -363,7 +369,7 @@ fn verify_fails_if_the_result_doesnt_satisfy_an_anchored_equation() {
(),
);
let (proof, result) = QueryProof::<InnerProductProof>::new(&expr, &accessor, &());
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[test]
Expand All @@ -382,7 +388,7 @@ fn verify_fails_if_the_anchored_commitment_doesnt_match() {
(),
);
let (proof, result) = QueryProof::<InnerProductProof>::new(&expr, &accessor, &());
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

// prove and verify an artificial query where
Expand Down Expand Up @@ -523,7 +529,10 @@ fn verify_a_proof_with_an_intermediate_commitment_and_given_offset(offset_genera
let QueryData {
verification_hash,
table,
} = proof.verify(&expr, &accessor, &result, &()).unwrap();
} = proof
.clone()
.verify(&expr, &accessor, result.clone(), &())
.unwrap();
assert_ne!(verification_hash, [0; 32]);
let expected_result = owned_table([bigint("a1", [81, 625])]);
assert_eq!(table, expected_result);
Expand All @@ -535,7 +544,7 @@ fn verify_a_proof_with_an_intermediate_commitment_and_given_offset(offset_genera
offset_generators + 1,
(),
);
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[test]
Expand Down Expand Up @@ -565,7 +574,7 @@ fn verify_fails_if_an_intermediate_commitment_doesnt_match() {
);
let (mut proof, result) = QueryProof::<InnerProductProof>::new(&expr, &accessor, &());
proof.commitments[0] = proof.commitments[0] * Curve25519Scalar::from(2u64);
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[test]
Expand All @@ -586,7 +595,7 @@ fn verify_fails_if_an_intermediate_equation_isnt_satified() {
(),
);
let (proof, result) = QueryProof::<InnerProductProof>::new(&expr, &accessor, &());
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[test]
Expand All @@ -608,7 +617,7 @@ fn verify_fails_the_result_doesnt_satisfy_an_intermediate_equation() {
(),
);
let (proof, result) = QueryProof::<InnerProductProof>::new(&expr, &accessor, &());
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[derive(Debug, Serialize)]
Expand Down Expand Up @@ -717,7 +726,10 @@ fn verify_a_proof_with_a_post_result_challenge_and_given_offset(offset_generator
let QueryData {
verification_hash,
table,
} = proof.verify(&expr, &accessor, &result, &()).unwrap();
} = proof
.clone()
.verify(&expr, &accessor, result.clone(), &())
.unwrap();
assert_ne!(verification_hash, [0; 32]);
let expected_result = owned_table([bigint("a1", [9, 25])]);
assert_eq!(table, expected_result);
Expand All @@ -729,7 +741,7 @@ fn verify_a_proof_with_a_post_result_challenge_and_given_offset(offset_generator
offset_generators + 1,
(),
);
assert!(proof.verify(&expr, &accessor, &result, &()).is_err());
assert!(proof.verify(&expr, &accessor, result, &()).is_err());
}

#[test]
Expand Down
Loading

0 comments on commit ba1ebbd

Please sign in to comment.