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

Feat/roman/tree builder #525

Merged
merged 40 commits into from
Jul 11, 2024
Merged

Feat/roman/tree builder #525

merged 40 commits into from
Jul 11, 2024

Conversation

ChickenLover
Copy link
Contributor

@ChickenLover ChickenLover commented May 23, 2024

Updates:

Hashing

  • Added SpongeHasher class
  • Can be used to accept any hash function as an argument
  • Absorb and squeeze are now separated
  • Memory management is now mostly done by SpongeHasher class, each hash function only describes permutation kernels

Tree builder

  • Tree builder is now hash-agnostic.
  • Tree builder now supports 2D input (matrices)
  • Tree builder can now use two different hash functions for layer 0 and compression layers

Poseidon1

  • Interface changed to classes
  • Now allows for any alpha
  • Now allows passing constants not in a single vector
  • Now allows for any domain tag
  • Constants are now released upon going out of scope
  • Rust wrappers changed to Poseidon struct

Poseidon2

  • Interface changed to classes
  • Constants are now released upon going out of scope
  • Rust wrappers changed to Poseidon2 struct

Keccak

  • Added Keccak class which inherits SpongeHasher
  • Now doesn't use gpu registers for storing states

To do:

  • Update poseidon1 golang bindings
  • Update poseidon1 examples
  • Fix poseidon2 cuda test
  • Fix poseidon2 merkle tree builder test
  • Update keccak class with new design
  • Update keccak test
  • Check keccak correctness
  • Update tree builder rust wrappers
  • Leave doc comments

Future work:

  • Add keccak merkle tree builder externs
  • Add keccak rust tree builder wrappers
  • Write docs
  • Add example
  • Fix device output for tree builder

@ChickenLover ChickenLover force-pushed the feat/roman/tree-builder branch from df81a84 to a47c25e Compare May 28, 2024 14:02
@ChickenLover ChickenLover marked this pull request as ready for review May 29, 2024 08:03
Copy link
Collaborator

@yshekel yshekel left a comment

Choose a reason for hiding this comment

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

Great work!

I wrote some comments/questions and have a few more:
(1) I did not see the merkle trees for poseidon/poseidon2 in rust. Did I just miss it?
(2) I am not sure about where the tree builders are in terms of library structre. I mean is poseidon/poseidon2 in the field crate and keccak in hashes?
(3) (I may have asked this already but) Please clarify the FFI design and specifically how do you see it for V3? I think that basically the ffi functions (under api) are what is exposed to ffi and also what we expect backend to implement. On top of that we will wrap and expose nicer api in all langauges. Does that make sense to you and this design?

@ChickenLover
Copy link
Contributor Author

@yshekel

  1. There are no rust bindings for now
  2. Tree builder is just a template, so instantiations will be both in hash and in field libraries, depending on which hash it is
  3. Well, if we go with the current FFI design, then we will need to reimplement the tree builder for every backend. For hash functions - yes, that will do fine, but I am not sure what to do with the tree builder in that case

@yshekel
Copy link
Collaborator

yshekel commented May 29, 2024

@yshekel

  1. There are no rust bindings for now
  2. Tree builder is just a template, so instantiations will be both in hash and in field libraries, depending on which hash it is
  3. Well, if we go with the current FFI design, then we will need to reimplement the tree builder for every backend. For hash functions - yes, that will do fine, but I am not sure what to do with the tree builder in that case

If we can implement a generic tree builder (that is backend agnostic) then we can have the backend APIs implement only the hashes and then wrap it with the generic tree builder that calls the hashing APIs and builds the tree (and expose that to ffi in addition to hashes). I am not sure it's possible but maybe it is.
Otherwise we would have to define the tree builder a backend API and have the backend implement it, per hash. Internally it can reuse templates etc. to avoid fully reimplementing like you did basically.

ChickenLover and others added 2 commits June 2, 2024 22:45
Co-authored-by: Jeremy Felder <jeremy.felder1@gmail.com>
@ChickenLover ChickenLover marked this pull request as draft June 4, 2024 11:50
@LeonHibnik LeonHibnik requested a review from yshekel June 5, 2024 10:43
@ChickenLover ChickenLover marked this pull request as ready for review June 5, 2024 15:27
@ChickenLover ChickenLover merged commit 7fd9ed1 into main Jul 11, 2024
26 checks passed
@ChickenLover ChickenLover deleted the feat/roman/tree-builder branch July 11, 2024 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants