search

Consensys / gnark-crypto

gnark-crypto provides elliptic curve and pairing-based cryptography on BN, BLS12, BLS24 and BW6 curves. It also provides various algorithms (algebra, crypto) of particular interest to zero knowledge proof systems.
Apache License 2.0
495 stars 160 forks source link

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

Closed ivokub closed 9 months ago

ivokub commented 10 months ago

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 https://github.com/Consensys/gnark/pull/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 9 months ago

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