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.

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

Ethereum Verkle IPA refactoring part 2 #397

Merged
merged 21 commits into from
Jun 23, 2024
Merged

Ethereum Verkle IPA refactoring part 2 #397

merged 21 commits into from
Jun 23, 2024

Conversation

mratsim
Copy link
Owner

@mratsim mratsim commented Jun 23, 2024

This PR is a followup to #392. Unfortunately this PR has become the biggest PR ever and a part 3 is needed.

An important background to this PR is #396.

The previous IPA implementation:

  1. Has a bug in proof generation which made proof invalid
  2. Has a bug in proof verification which accepted those proof
  3. Was lacking test coverage to highlight those bugs
  4. Multiproofs were incorrect due to 1 and 2

This refactoring may fix 1 and 2, however at the moment only if run in AddressSanitizer. If not for some reason verification fails.
Hence this refactoring improves structure and lay out groundwork for addressing the bugs once and for all but does not improve or degrade compared to past implementation.

The files constantine/ethereum_verkle_primitives.nim and constantine/ethereum_verkle_trees.nim have been deleted and replaced by constantine/ethereum_verkle_ipa.nim

The final public API for Eth Verkle IPA still needs to be implemented. It will be similar to the KZG one and will expose only what's necessary and not Constantine's internals.

However before that #396 must be fixed


Highlight of changes:

  • reimplemented eth-verkle-ipa:
    • Structures like array[N, Field] which are 256x32 bytes = 8kB are now on heap.
    • No more seq usage or Nim allocs to ensure no exceptions and no need to call NimMain when using Constantine as a library
    • exported short names like Bytes/EC_P / EC_P_Aff cause confusion if library is used for many purposes, for example BN254, BLS12-381 and Banderwagon curves and should be private to a module or left to the caller.
    • Use opening challenges sparsity in multiproof to minimize memory allocation.
    • Thorough documentation of the protocol
    • ipa_verify should be significantly faster than before as a MSM is used instead of ~10 individual scalar mul. The change of basis computation is also linear in domain size instead of being ½ n log₂ n.
    • Use of affine coordinates where it make senses and avoiding redundant affine transformation (for example ingesting in a transcript requires affine)
    • Avoid several copies and intermediate values.
  • Transcripts now have a cryptographic sponge with duplex construction API:
  • Specified the order of arguments for Constantine and change KZG to follow spec. This only affects internal KZG backend. The Ethereum-flavored KZG follows c-kzg-4844 order of arguments, hence no impact on C/Go/Rust.
  • common getQuotientPoly between KZG and IPA
  • MSM benchmarks now compare to vartime scalar mul as well
  • Views: splitMiddle renamed to splitHalf

@mratsim mratsim merged commit efff14b into master Jun 23, 2024
12 checks passed
@mratsim mratsim deleted the ipa-refactoring-2 branch June 23, 2024 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant