Consensys/gnark-crypto

refactor: make applying domain separation optional in Fiat-Shamir Transcript

Closed this issue · 1 comments

ivokub commented

Is your feature request related to a problem? Please describe.

Currently when the hashing function implements WriteString, then fiatshamir.Transcript uses this instead of Write. For MiMC implementation the WriteString prepends a domain separation string which is hardcoded. This implementation makes it difficult to be compatible with other implementations as this domain separation is not documented nor configurable.

Describe the solution you'd like

Instead on switching on interface {WriteString(input string) (int, error)} allow an option when initializing fiatshamir.Transcript. The syntax should be similar as in gnark. See Consensys/gnark#900.

Describe alternatives you've considered
If user wants to have different domain separation processing, then they should wrap native hash functions to modify WriteString method. But in that case we still need to document the behaviour better.

cc @Tabaie - this would impact GKR and sumcheck, but it is already mitigated on gnark side.
cc @ThomasPiellard for visibility.

ivokub commented

Upgraded priority - it breaks zkevm now after upgrading to gnark master.