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

Feature: Parameter Generation for Poseidon Hash #57

Merged
merged 43 commits into from
Jun 2, 2022

Conversation

BoyuanFeng
Copy link
Contributor

@BoyuanFeng BoyuanFeng commented May 2, 2022

Integrate from Plonk Prototype to Manta-rs

Major changes:

  • Extend the Specification trait in manta-pay/src/crypto/hash/poseidon.rs to 1) separate fields for parameters (public constants) and states (private variables); 2) generically support addition, multiplication, etc
  • Rewrite parameter generation following the Specification trait such that the code can work for generic fields
  • Update Poseidon hash implementation to match #constraints between manta-rs and Plonk Prototype. Saw 20~30% saving in #constraints and latency than previous manta-rs, in terms of Poseidon hash, mint, reclaim, and private transfer.

Changes in #51 are moved here.
Closes: #22

@BoyuanFeng BoyuanFeng self-assigned this May 2, 2022
@tsunrise tsunrise self-requested a review May 3, 2022 07:51
Copy link
Contributor

@tsunrise tsunrise left a comment

Choose a reason for hiding this comment

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

Just took a very rough look at the code. Have a few comments. Will do it again in a day or two!

manta-pay/src/crypto/hash/poseidon.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@bhgomes bhgomes left a comment

Choose a reason for hiding this comment

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

Small review right now, more comments later, but please address the current comments first. Thanks!

manta-pay/src/crypto/hash/mod.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@tsunrise tsunrise left a comment

Choose a reason for hiding this comment

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

reviewed hasher and part of lfsr, will take a look at remaining part later

manta-pay/src/crypto/hash/mod.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@tsunrise tsunrise left a comment

Choose a reason for hiding this comment

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

nice work! left some minor comments

@bhgomes
Copy link
Contributor

bhgomes commented May 23, 2022

CI breaking issue: rust-lang/rust-clippy#8867

manta-pay/src/crypto/hash/poseidon/mod.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/mod.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/mod.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/hasher.rs Outdated Show resolved Hide resolved
tsunrise
tsunrise previously approved these changes May 28, 2022
Copy link
Contributor

@tsunrise tsunrise left a comment

Choose a reason for hiding this comment

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

LGTM with some minor comments. I pre-approve this so that my review will not block you
(I will make a PR for #74 once this one is merged)

manta-pay/src/crypto/hash/poseidon/parameter/matrix.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/parameter/mds.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/parameter/mds.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/parameter/mod.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/mod.rs Show resolved Hide resolved
bhgomes
bhgomes previously approved these changes Jun 2, 2022
manta-pay/src/crypto/hash/poseidon/parameter/matrix.rs Outdated Show resolved Hide resolved
manta-pay/src/crypto/hash/poseidon/parameter/mod.rs Outdated Show resolved Hide resolved
@bhgomes bhgomes requested review from tsunrise and removed request for tsunrise June 2, 2022 04:10
tsunrise
tsunrise previously approved these changes Jun 2, 2022
Copy link
Contributor

@tsunrise tsunrise left a comment

Choose a reason for hiding this comment

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

LGTM

manta-pay/src/crypto/hash/poseidon/round_constants.rs Outdated Show resolved Hide resolved
@BoyuanFeng BoyuanFeng dismissed stale reviews from tsunrise and bhgomes via 6061df0 June 2, 2022 20:04
@BoyuanFeng BoyuanFeng force-pushed the poseidon_optimization branch from 791640e to 6061df0 Compare June 2, 2022 20:04
Boyuan Feng and others added 20 commits June 2, 2022 16:09
* feat: add proving and verifying benchmark
* fix: remove let bindings for clippy false positives
Co-authored-by: Brandon H. Gomes <bhgomes@pm.me>
* feat: generalized ledger infrastructure and add correct pruning
* feat: add more randomness/concurrency to simulation
* feat: add iteration features for manta_util::array
* feat: add sign method to separate post into pieces
* feat: use more sharding-optimal coin selection algorithm
* feat: add self-transfers to simulation
* feat: add GenerateReceivingKeys, FlushToPublic, and SelfTransfer to sim
* feat: add zero-transaction to simulation
* feat: add simulation action
* initial implementation
* support benchmarking verify time
* headless wasm timer
* headless wasm & clean browser bench code
* add conditional compilation
* add ci
@BoyuanFeng BoyuanFeng force-pushed the poseidon_optimization branch from 6061df0 to f9eeaaa Compare June 2, 2022 20:09
@bhgomes bhgomes requested review from tsunrise and bhgomes June 2, 2022 22:37
@BoyuanFeng BoyuanFeng merged commit 98d0091 into main Jun 2, 2022
@BoyuanFeng BoyuanFeng deleted the poseidon_optimization branch June 2, 2022 22:41
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.

Use Fastest Poseidon Implementation
3 participants