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

Add utils module #53

Merged
merged 5 commits into from
Sep 28, 2022
Merged

Add utils module #53

merged 5 commits into from
Sep 28, 2022

Conversation

s1fr0
Copy link
Contributor

@s1fr0 s1fr0 commented Sep 26, 2022

Addresses #52 by moving the rln Poseidon and Merkle Tree crates to utils.

Note that this branch is based on add-lfsr-poseidon which implements #52 and will be rebased on master when add-lfsr-poseidon is merged.

@s1fr0 s1fr0 added the track:zerokit Zerokit track (Applied ZK/Explorations) label Sep 26, 2022
@s1fr0 s1fr0 requested a review from oskarth September 26, 2022 11:16
@s1fr0 s1fr0 self-assigned this Sep 26, 2022
@s1fr0 s1fr0 requested a review from richard-ramos September 26, 2022 16:23
Copy link
Contributor

@oskarth oskarth left a comment

Choose a reason for hiding this comment

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

Thanks! Could you please update the CHANGELOG for next release with this item too?

Have you made sure it still works fine with nwaku (say)? Might be a good idea to add some integration tests to have more confidence in changes we make

Copy link
Member

@richard-ramos richard-ramos left a comment

Choose a reason for hiding this comment

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

A rebase is needed to pick up latest changes from rln's Cargo.toml. Latest version includes a parallel feature enabled by default, which is disabled for rln-wasm

utils/Cargo.toml Outdated
edition = "2021"

[dependencies]
ark-ff = { version = "0.3.0", default-features = false, features = ["parallel", "asm"] }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ark-ff = { version = "0.3.0", default-features = false, features = ["parallel", "asm"] }
ark-ff = { version = "0.3.0", default-features = false, features = ["asm"] }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 6d863d7

utils/Cargo.toml Outdated
ark-bn254 = { version = "0.3.0" }
num-traits = "0.2.11"
hex-literal = "0.3.4"
tiny-keccak = { version = "2.0.2", features = ["keccak"] }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
tiny-keccak = { version = "2.0.2", features = ["keccak"] }
tiny-keccak = { version = "2.0.2", features = ["keccak"] }
[features]
default = ["parallel"]
parallel = ["ark-ff/parallel"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 6d863d7

rln/Cargo.toml Outdated
@@ -30,13 +30,10 @@ num-traits = "0.2.11"
once_cell = "1.14.0"
rand = "0.8"
tiny-keccak = { version = "2.0.2", features = ["keccak"] }
utils = { path = "../utils/" }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
utils = { path = "../utils/" }
utils = { path = "../utils/", default-features = false }

Copy link
Member

Choose a reason for hiding this comment

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

Also, once this file is rebased, utils should be added to the parallel feature

parallel = ["ark-ec/parallel", "ark-ff/parallel", "ark-std/parallel", "ark-groth16/parallel", "utils/parallel"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 6d863d7

Base automatically changed from add-lfsr-poseidon to master September 27, 2022 15:18
@s1fr0
Copy link
Contributor Author

s1fr0 commented Sep 27, 2022

@richard-ramos I've implemented the changes you requested. Could you please have a last check? Thanks!

@s1fr0 s1fr0 requested a review from richard-ramos September 27, 2022 16:04
Copy link
Member

@richard-ramos richard-ramos left a comment

Choose a reason for hiding this comment

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

🎖️

@s1fr0 s1fr0 merged commit bbacc9d into master Sep 28, 2022
@s1fr0 s1fr0 deleted the add-utils-module branch September 28, 2022 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
track:zerokit Zerokit track (Applied ZK/Explorations)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate Poseidon round parameters and constants Move to a new utils module shared implementations
3 participants