-
Notifications
You must be signed in to change notification settings - Fork 7
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
refactor(RLN:) Remove dependencies and add new APIs #45
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
a8ab705
refactor(rln): removing unused crates/dependencies
s1fr0 ea46351
cargo fmt
s1fr0 7a4f042
refactor(rln): removed more dependencies; curve/fields as parameters
s1fr0 180b050
refactor(rln): use poseidon-rs hash instead of semaphore-rs poseidon
s1fr0 e96620a
chore(rln): remove deps
s1fr0 0165295
refactor(rln): use exclusively arkworks Fr
s1fr0 1563f80
refactor(rln): integrate poseidon-rs implementation to work with arkw…
s1fr0 f7fd696
Merge branch 'master' into poseidon-integration
s1fr0 10f5f6c
fix(rln): remove previous poseidon-rs wrapper
s1fr0 dada2d1
feat(rln): add features to select MT; remove prints if not in debug mode
s1fr0 f1cde12
fix(rln): collect test parameters in a vector
s1fr0 4339edf
feat(RLN): add `new_with_params` (#36)
richard-ramos c79136e
chore(rln): simplify read wasm
s1fr0 562ce32
fix(rln): remove unused dependencies
s1fr0 021553a
cargo fmt
s1fr0 c2e56b6
fix(rln): update dependencies, fix commit
s1fr0 0e0bd19
refactor(rln): restore ark-circom original dep
s1fr0 cb1b24d
Merge branch 'master' into minor-refactors
s1fr0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,28 +9,34 @@ crate-type = ["cdylib", "rlib", "staticlib"] | |
[dependencies] | ||
|
||
# ZKP Generation | ||
ark-ec = { version = "0.3.0", default-features = false, features = ["parallel"] } | ||
ark-ff = { version = "0.3.0", default-features = false, features = ["parallel", "asm"] } | ||
ark-std = { version = "0.3.0", default-features = false, features = ["parallel"] } | ||
ark-bn254 = { version = "0.3.0" } | ||
ark-groth16 = { git = "https://github.com/arkworks-rs/groth16", rev = "765817f", features = ["parallel"] } | ||
ark-relations = { version = "0.3.0", default-features = false, features = [ "std" ] } | ||
ark-serialize = { version = "0.3.0", default-features = false } | ||
ark-circom = { git = "https://github.com/gakonst/ark-circom", features = ["circom-2"] } | ||
wasmer = { version = "2.0" } | ||
ark-circom = { git = "https://github.com/gakonst/ark-circom", rev = "06eb075", features = ["circom-2"] } | ||
#ark-circom = { git = "https://github.com/vacp2p/ark-circom", branch = "no-ethers-core", features = ["circom-2"] } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we might want ethers in future anyway, so we can deal with ZK smart contracts etc from Zerokit too |
||
wasmer = "2.3.0" | ||
|
||
# error handling | ||
color-eyre = "0.5" | ||
color-eyre = "0.5.11" | ||
thiserror = "1.0.0" | ||
|
||
# utilities | ||
hex-literal = "0.3" | ||
num-bigint = { version = "0.4", default-features = false, features = ["rand"] } | ||
once_cell = "1.8" | ||
cfg-if = "1.0" | ||
num-bigint = { version = "0.4.3", default-features = false, features = ["rand"] } | ||
num-traits = "0.2.11" | ||
once_cell = "1.14.0" | ||
rand = "0.8" | ||
tiny-keccak = "2.0.2" | ||
num-traits = "0.2.15" | ||
tiny-keccak = { version = "2.0.2", features = ["keccak"] } | ||
|
||
# serialization | ||
serde = { version = "1.0.103", default-features = false, features = ["derive"] } | ||
serde_json = "1.0.48" | ||
serde_json = "1.0.48" | ||
|
||
[dev-dependencies] | ||
|
||
hex-literal = "0.3.4" | ||
|
||
[features] | ||
fullmerkletree = [] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oskarth The vacp2p fork is a subset of the original ark-circom, which is lighter in terms of dependencies while guaranteeing zerokit the functionalities it needs. I was unsure which dep activate by default (I have pros and cons for both), so I kept the original one. But feel free to change it (the difference is ~60 dependencies less compiled)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah original one is better unless our fork is a must. We should upstream change we make, or at least start discussion.
I feel like there should be some form of dead code compilation that can be done here? So it doesn't matter how many dependencies are in ark-circom for zerokit binary size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can get a warning for unused functions/code (
dead_code
). To detect unused dependencies you need special tools like udeps, so I suspect the compiler doesn't figure it out automatically which one are unused and skips their compilation. But I might be wrong, I tried to search something like what you said but with no luck.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What matters here is the final compilation size, not if it is part of compilation process (CI isn't that slow). I could be wrong, but I think this gets remove with e.g. https://doc.rust-lang.org/rustc/codegen-options/index.html#link-dead-code
For wasm there are tools like https://rustwasm.github.io/docs/book/reference/code-size.html#the-twiggy-code-size-profiler
Can continue this discussion in relevant issue though