arkworks-rs / arkworks-rs/poly-commit

Add `Absorb` trait bound on `PCCommitment`

Open
#143 3 comments 1 reaction 0 assignees View on GitHub
Dominant language
Rust
Stars
440
Forks
160
PR merge metrics
No merged PRs in 30d

Description

## Summary

Using the PCS in a wider context will require absorbing the commitment for Fiat Shamir.

The current challenge is that `Absorb` is not implemented for `AffineRepr`, and the current pairing-based schemes in this repo have commitments of the form:
```
pub struct Commitment(
/// The commitment is a group element.
pub E::G1Affine,
);
```

We unfortunately can't add a blanket impl like:
```
impl Absorb for T
where
T: AffineRepr,
{
fn to_sponge_bytes(&self, dest: &mut Vec) {
todo!()
}

fn to_sponge_field_elements(&self, dest: &mut Vec) {
todo!()
}
}
```
Rust complains "conflicting implementations of trait `absorb::Absorb` for type `u8`".

### Option 1
We could add `Absorb` on `AffineRepr` in the ec crate. However, aside from creating a cyclic dependency from `ec` to `crypto-primitives`, I think this is not the best design.

### Option 2
One solution is to further restrict `G: AffineRepr + Absorb`. This propagates into a lot of trait bounds in the poly-commit repo, though. The upside is that it doesn't require any upstream changes, and the two current implementors of `AffineRepr` which are the structs `short_weierstrass::Affine` and `twisted_edwards::Affine` already have `Absorb` implemented.

### Option 3
Larger breaking refactor that makes use of the fact that most pairing computations (and certainly our implementations in `ec::models`) only ever support SW. The proposal is then to rip out the associated type `G1Affine` from `Pairing`, and restrict the class of pairings that can be done to SW-curves, so that we can have:

```
pub struct Commitment(
/// The commitment is a group element.
pub Affine

,
);
```
This **almost** works, as we still have IPA `Commitment` struct here in poly-commit which uses the trait `AffineRepr` (i.e. not an associated type from `Pairing`):
```
pub struct Commitment {
/// A Pedersen commitment to the polynomial.
pub comm: G,
...
}
```
And so mixing in Option 2 would need to happen anyway, but limited to one PCS.

### Option 4
Any other ideas?

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.