Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impl Absorb for String #138

Merged
merged 5 commits into from
Feb 10, 2024
Merged

impl Absorb for String #138

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
53004bd
impl Absorb for String
mmagician Feb 6, 2024
4b59220
add String from ark_std
mmagician Feb 6, 2024
7b4e10b
absorb length when converting String to sponge bytes
mmagician Feb 9, 2024
9ddd53c
add test to ensure that absorbing two strings is different from concat
mmagician Feb 9, 2024
9bd0338
Merge branch 'main' into absorb-string
mmagician Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/sponge/absorb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use ark_ec::{
use ark_ff::models::{Fp, FpConfig};
use ark_ff::{BigInteger, Field, PrimeField, ToConstraintField};
use ark_serialize::CanonicalSerialize;
use ark_std::string::String;
use ark_std::vec::Vec;

/// An interface for objects that can be absorbed by a `CryptographicSponge`.
Expand Down Expand Up @@ -227,6 +228,16 @@ impl Absorb for isize {
}
}

impl Absorb for String {
fn to_sponge_bytes(&self, dest: &mut Vec<u8>) {
dest.extend_from_slice(self.as_bytes())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably this absorbs the length too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also implement AbsorbWithLength for String and leave this as is, wdyt?
Then it's just the question of whether the caller knows about the trait AbsorbWithLength, which arguably is not so obvious since most types only implement pure Absorb.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think we should absorb the length by default, because I don't want it to be the case that absorb two strings is the same as the absorbing the concatenation of the strings

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the length by default to to_sponge_bytes.

I left to_sponge_field_elements as-is. It already prevents the scenario you mentioned: each substring gets mapped to a new field element, whereas a concat string gets mapped to a single element, so the resulting vectors even have different lengths.

}

fn to_sponge_field_elements<F: PrimeField>(&self, dest: &mut Vec<F>) {
self.as_bytes().to_sponge_field_elements(dest)
}
}

impl<P: TEModelParameters> Absorb for TEAffine<P>
where
P::BaseField: ToConstraintField<<P::BaseField as Field>::BasePrimeField>,
Expand Down
Loading