Skip to content

Commit

Permalink
refactor!: remove DecompressedCommitment from VecCommitmentExt (#80)
Browse files Browse the repository at this point in the history
# Rationale for this change

In the process of allowing for single table commitments, there is some
refactoring needed of the commitment API. This PR simplifies it a bit to
ease future PR complexity.

# What changes are included in this PR?

`DecompressedCommitment` was made redundant because every commitment is
"decompressed" in the sense that there is no concept of compression in
the traits.

This commit simply removes it, which consequently allows for a decent
simplification of many of the commitment structures.

# Are these changes tested?

By existing tests
  • Loading branch information
JayWhite2357 authored Aug 6, 2024
1 parent 6f3207b commit 0cdd666
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 141 deletions.
56 changes: 15 additions & 41 deletions crates/proof-of-sql/src/base/commitment/column_commitments.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{
committable_column::CommittableColumn, ColumnCommitmentMetadata, ColumnCommitmentMetadataMap,
ColumnCommitmentMetadataMapExt, ColumnCommitmentsMismatch, VecCommitmentExt,
ColumnCommitmentMetadataMapExt, ColumnCommitmentsMismatch, Commitment, VecCommitmentExt,
};
use crate::base::database::{ColumnField, ColumnRef, CommitmentAccessor, TableRef};
use proof_of_sql_parser::Identifier;
Expand Down Expand Up @@ -28,40 +28,23 @@ pub enum AppendColumnCommitmentsError {
///
/// These columns do not need to belong to the same table, and can have differing lengths.
#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct ColumnCommitments<C>
where
Vec<C>: VecCommitmentExt,
{
pub struct ColumnCommitments<C> {
commitments: Vec<C>,
column_metadata: ColumnCommitmentMetadataMap,
}

/// Private convenience aliases.
type Decompressed<C> = <Vec<C> as VecCommitmentExt>::DecompressedCommitment;
type Setup<'a, C> = <Vec<C> as VecCommitmentExt>::CommitmentPublicSetup<'a>;

impl<C> ColumnCommitments<C>
where
Vec<C>: VecCommitmentExt,
{
impl<C: Commitment> ColumnCommitments<C> {
/// Create a new [`ColumnCommitments`] for a table from a commitment accessor.
pub fn from_accessor_with_max_bounds(
table: TableRef,
columns: &[ColumnField],
accessor: &impl CommitmentAccessor<Decompressed<C>>,
) -> Self
where
Decompressed<C>: Into<C>,
{
accessor: &impl CommitmentAccessor<C>,
) -> Self {
let column_metadata =
ColumnCommitmentMetadataMap::from_column_fields_with_max_bounds(columns);
let commitments = columns
.iter()
.map(|c| {
accessor
.get_commitment(ColumnRef::new(table, c.name(), c.data_type()))
.into()
})
.map(|c| accessor.get_commitment(ColumnRef::new(table, c.name(), c.data_type())))
.collect();
ColumnCommitments {
commitments,
Expand Down Expand Up @@ -95,10 +78,10 @@ where
}

/// Returns the commitment with the given identifier.
pub fn get_commitment(&self, identifier: &Identifier) -> Option<Decompressed<C>> {
pub fn get_commitment(&self, identifier: &Identifier) -> Option<C> {
self.column_metadata
.get_index_of(identifier)
.map(|index| self.commitments.get_decompressed_commitment(index).unwrap())
.map(|index| self.commitments[index])
}

/// Returns the metadata for the commitment with the given identifier.
Expand All @@ -115,7 +98,7 @@ where
pub fn try_from_columns_with_offset<'a, COL>(
columns: impl IntoIterator<Item = (&'a Identifier, COL)>,
offset: usize,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<ColumnCommitments<C>, DuplicateIdentifiers>
where
COL: Into<CommittableColumn<'a>>,
Expand Down Expand Up @@ -165,7 +148,7 @@ where
&mut self,
columns: impl IntoIterator<Item = (&'a Identifier, COL)>,
offset: usize,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<(), AppendColumnCommitmentsError>
where
COL: Into<CommittableColumn<'a>>,
Expand Down Expand Up @@ -210,7 +193,7 @@ where
&mut self,
columns: impl IntoIterator<Item = (&'a Identifier, COL)>,
offset: usize,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<(), DuplicateIdentifiers>
where
COL: Into<CommittableColumn<'a>>,
Expand All @@ -235,7 +218,7 @@ where

// this constructor will check for duplicates among the new columns
let new_column_commitments =
ColumnCommitments::try_from_columns_with_offset(unique_columns, offset, setup)?;
ColumnCommitments::<C>::try_from_columns_with_offset(unique_columns, offset, setup)?;

self.commitments.extend(new_column_commitments.commitments);
self.column_metadata
Expand Down Expand Up @@ -291,10 +274,7 @@ pub type IntoIter<C> = std::iter::Map<
fn(((Identifier, ColumnCommitmentMetadata), C)) -> (Identifier, ColumnCommitmentMetadata, C),
>;

impl<C> IntoIterator for ColumnCommitments<C>
where
Vec<C>: VecCommitmentExt,
{
impl<C> IntoIterator for ColumnCommitments<C> {
type Item = (Identifier, ColumnCommitmentMetadata, C);
type IntoIter = IntoIter<C>;
fn into_iter(self) -> Self::IntoIter {
Expand All @@ -316,10 +296,7 @@ pub type Iter<'a, C> = std::iter::Map<
) -> (&'a Identifier, &'a ColumnCommitmentMetadata, &'a C),
>;

impl<'a, C> IntoIterator for &'a ColumnCommitments<C>
where
Vec<C>: VecCommitmentExt,
{
impl<'a, C> IntoIterator for &'a ColumnCommitments<C> {
type Item = (&'a Identifier, &'a ColumnCommitmentMetadata, &'a C);
type IntoIter = Iter<'a, C>;
fn into_iter(self) -> Self::IntoIter {
Expand All @@ -330,10 +307,7 @@ where
}
}

impl<C> FromIterator<(Identifier, ColumnCommitmentMetadata, C)> for ColumnCommitments<C>
where
Vec<C>: VecCommitmentExt,
{
impl<C> FromIterator<(Identifier, ColumnCommitmentMetadata, C)> for ColumnCommitments<C> {
fn from_iter<T: IntoIterator<Item = (Identifier, ColumnCommitmentMetadata, C)>>(
iter: T,
) -> Self {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ pub fn test_simple_commitment_evaluation_proof<CP: CommitmentEvaluationProof>(
])],
0,
prover_setup,
)
.to_decompressed()
.unwrap();
);

let mut transcript = Transcript::new(b"evaluation_proof");
let r = proof.verify_proof(
Expand All @@ -52,9 +50,7 @@ pub fn test_commitment_evaluation_proof_with_length_1<CP: CommitmentEvaluationPr
let mut transcript = Transcript::new(b"evaluation_proof");
let proof = CP::new(&mut transcript, &[r], &[], 0, prover_setup);

let commits = Vec::from_columns_with_offset(&[Column::Scalar(&[r])], 0, prover_setup)
.to_decompressed()
.unwrap();
let commits = Vec::from_columns_with_offset(&[Column::Scalar(&[r])], 0, prover_setup);

let mut transcript = Transcript::new(b"evaluation_proof");
let r = proof.verify_proof(&mut transcript, &commits[0], &r, &[], 0, 1, verifier_setup);
Expand Down Expand Up @@ -82,9 +78,7 @@ pub fn test_random_commitment_evaluation_proof<CP: CommitmentEvaluationProof>(
let mut transcript = Transcript::new(b"evaluation_proof");
let proof = CP::new(&mut transcript, &a, &b_point, offset as u64, prover_setup);

let commits = Vec::from_columns_with_offset(&[Column::Scalar(&a)], offset, prover_setup)
.to_decompressed()
.unwrap();
let commits = Vec::from_columns_with_offset(&[Column::Scalar(&a)], offset, prover_setup);

let mut b = vec![CP::Scalar::zero(); a.len()];
crate::base::polynomial::compute_evaluation_vector(&mut b, &b_point);
Expand Down
35 changes: 9 additions & 26 deletions crates/proof-of-sql/src/base/commitment/query_commitments.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{TableCommitment, VecCommitmentExt};
use super::{Commitment, TableCommitment};
use crate::base::database::{
ColumnField, ColumnRef, ColumnType, CommitmentAccessor, MetadataAccessor, SchemaAccessor,
TableRef,
Expand All @@ -17,24 +17,19 @@ pub type QueryCommitments<C> = HashMap<TableRef, TableCommitment<C>>;
/// A trait for extending the functionality of the [`QueryCommitments`] alias.
pub trait QueryCommitmentsExt<C>
where
Vec<C>: VecCommitmentExt,
Decompressed<C>: Into<C>,
C: Commitment,
{
/// Create a new `QueryCommitments` from a collection of columns and an accessor.
fn from_accessor_with_max_bounds(
columns: impl IntoIterator<Item = ColumnRef>,
accessor: &(impl CommitmentAccessor<Decompressed<C>> + SchemaAccessor),
accessor: &(impl CommitmentAccessor<C> + SchemaAccessor),
) -> Self;
}

impl<C> QueryCommitmentsExt<C> for QueryCommitments<C>
where
Vec<C>: VecCommitmentExt,
Decompressed<C>: Into<C>,
{
impl<C: Commitment> QueryCommitmentsExt<C> for QueryCommitments<C> {
fn from_accessor_with_max_bounds(
columns: impl IntoIterator<Item = ColumnRef>,
accessor: &(impl CommitmentAccessor<Decompressed<C>> + SchemaAccessor),
accessor: &(impl CommitmentAccessor<C> + SchemaAccessor),
) -> Self {
columns
.into_iter()
Expand Down Expand Up @@ -65,10 +60,7 @@ where
}
}

impl<C> MetadataAccessor for QueryCommitments<C>
where
Vec<C>: VecCommitmentExt,
{
impl<C: Commitment> MetadataAccessor for QueryCommitments<C> {
fn get_length(&self, table_ref: crate::base::database::TableRef) -> usize {
let table_commitment = self.get(&table_ref).unwrap();

Expand All @@ -82,14 +74,8 @@ where
}
}

/// Private convenience alias.
type Decompressed<C> = <Vec<C> as VecCommitmentExt>::DecompressedCommitment;

impl<C> CommitmentAccessor<Decompressed<C>> for QueryCommitments<C>
where
Vec<C>: VecCommitmentExt,
{
fn get_commitment(&self, column: ColumnRef) -> Decompressed<C> {
impl<C: Commitment> CommitmentAccessor<C> for QueryCommitments<C> {
fn get_commitment(&self, column: ColumnRef) -> C {
let table_commitment = self.get(&column.table_ref()).unwrap();

table_commitment
Expand All @@ -99,10 +85,7 @@ where
}
}

impl<C> SchemaAccessor for QueryCommitments<C>
where
Vec<C>: VecCommitmentExt,
{
impl<C: Commitment> SchemaAccessor for QueryCommitments<C> {
fn lookup_column(
&self,
table_ref: crate::base::database::TableRef,
Expand Down
42 changes: 16 additions & 26 deletions crates/proof-of-sql/src/base/commitment/table_commitment.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{
committable_column::CommittableColumn, AppendColumnCommitmentsError, ColumnCommitments,
ColumnCommitmentsMismatch, Commitment, DuplicateIdentifiers, VecCommitmentExt,
ColumnCommitmentsMismatch, Commitment, DuplicateIdentifiers,
};
use crate::base::{
database::{
Expand Down Expand Up @@ -90,29 +90,19 @@ pub enum AppendRecordBatchTableCommitmentError {
#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct TableCommitment<C>
where
Vec<C>: VecCommitmentExt,
C: Commitment,
{
column_commitments: ColumnCommitments<C>,
range: Range<usize>,
}

/// Private convenience alias.
type Setup<'a, C> = <Vec<C> as VecCommitmentExt>::CommitmentPublicSetup<'a>;
type Decompressed<C> = <Vec<C> as VecCommitmentExt>::DecompressedCommitment;

impl<C> TableCommitment<C>
where
Vec<C>: VecCommitmentExt,
{
impl<C: Commitment> TableCommitment<C> {
/// Create a new [`TableCommitment`] for a table from a commitment accessor.
pub fn from_accessor_with_max_bounds<A: CommitmentAccessor<Decompressed<C>>>(
pub fn from_accessor_with_max_bounds(
table_ref: TableRef,
columns: &[ColumnField],
accessor: &A,
) -> Self
where
Decompressed<C>: Into<C>,
{
accessor: &impl CommitmentAccessor<C>,
) -> Self {
let length = accessor.get_length(table_ref);
let offset = accessor.get_offset(table_ref);
Self::try_new(
Expand Down Expand Up @@ -170,7 +160,7 @@ where
pub fn try_from_columns_with_offset<'a, COL>(
columns: impl IntoIterator<Item = (&'a Identifier, COL)>,
offset: usize,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<TableCommitment<C>, TableCommitmentFromColumnsError>
where
COL: Into<CommittableColumn<'a>>,
Expand Down Expand Up @@ -199,7 +189,7 @@ where
pub fn from_owned_table_with_offset<S>(
owned_table: &OwnedTable<S>,
offset: usize,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> TableCommitment<C>
where
S: Scalar,
Expand All @@ -216,7 +206,7 @@ where
pub fn try_append_rows<'a, COL>(
&mut self,
columns: impl IntoIterator<Item = (&'a Identifier, COL)>,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<(), AppendTableCommitmentError>
where
COL: Into<CommittableColumn<'a>>,
Expand Down Expand Up @@ -246,7 +236,7 @@ where
pub fn append_owned_table<S>(
&mut self,
owned_table: &OwnedTable<S>,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<(), ColumnCommitmentsMismatch>
where
S: Scalar,
Expand All @@ -271,7 +261,7 @@ where
pub fn try_extend_columns<'a, COL>(
&mut self,
columns: impl IntoIterator<Item = (&'a Identifier, COL)>,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<(), TableCommitmentFromColumnsError>
where
COL: Into<CommittableColumn<'a>>,
Expand Down Expand Up @@ -367,10 +357,10 @@ where
pub fn try_append_record_batch(
&mut self,
batch: &RecordBatch,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<(), AppendRecordBatchTableCommitmentError> {
match self.try_append_rows(
batch_to_columns::<<Decompressed<C> as Commitment>::Scalar>(batch, &Bump::new())?
batch_to_columns::<C::Scalar>(batch, &Bump::new())?
.iter()
.map(|(a, b)| (a, b)),
setup,
Expand All @@ -392,7 +382,7 @@ where
/// Returns a [`TableCommitment`] to the provided arrow [`RecordBatch`].
pub fn try_from_record_batch(
batch: &RecordBatch,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<TableCommitment<C>, RecordBatchToColumnsError> {
Self::try_from_record_batch_with_offset(batch, 0, setup)
}
Expand All @@ -401,10 +391,10 @@ where
pub fn try_from_record_batch_with_offset(
batch: &RecordBatch,
offset: usize,
setup: &Setup<C>,
setup: &C::PublicSetup<'_>,
) -> Result<TableCommitment<C>, RecordBatchToColumnsError> {
match Self::try_from_columns_with_offset(
batch_to_columns::<<Decompressed<C> as Commitment>::Scalar>(batch, &Bump::new())?
batch_to_columns::<C::Scalar>(batch, &Bump::new())?
.iter()
.map(|(a, b)| (a, b)),
offset,
Expand Down
Loading

0 comments on commit 0cdd666

Please sign in to comment.