impl Absorb for String

[mmagician]

Description

Needed for deriving Absorb for LabeledCommitment in poly-commit.

closes: #XXXX

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (main)
• Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
• Wrote unit tests
• Updated relevant documentation in the code
• Added a relevant changelog entry to the Pending section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer

[Pratyush]

Presumably this absorbs the length too?

[mmagician]

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.

[Pratyush]

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

[mmagician]

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.

[Pratyush]

Also, I don't think we should derive Absorb for LabelledCommitment. Really the label only exists in the code for debugging purposes; the commitment should already be a unique representative of its polynomial. Absorbing the string would only add inefficiency.

[mmagician]

Also, I don't think we should derive Absorb for LabelledCommitment. Really the label only exists in the code for debugging purposes; the commitment should already be a unique representative of its polynomial. Absorbing the string would only add inefficiency.

Sure, makes sense. We can impl Absorb for String regardless.

[Pratyush]

Looks good modulo the nit about the length!

[burdges]

If one wants String then one typically wants str instead (or perhaps in addition).

As an aside, it's bizarre LabeledCommitment::label returns &String instead of &str.

I think folks have typically chosen &[u8]instead of the utf8String/str` for labels, but actually the unicode lookalikes are not a problem here, so probably not a real concern.