feat!: crates refactor - Githubissues

mrain commented 5 months ago

Description

We carve out many primitives from jf-primitives repo, so that upstream users could selectively upgrade each component without breaking others.

Tasks:

[x] break jf-primitives into smaller crates
[x] specify CODEOWNERS
[x] fix all tests
[x] crate CHANGELOG for each crate
[x] documentations
[x] checking combinations of feature flags.
[x] update the scripts and github workflows

Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why.

[ ] Targeted PR against correct branch (main)
[ ] Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
[ ] Wrote unit tests
[ ] Updated relevant documentation in the code
[ ] Added a relevant changelog entry to the Pending section in CHANGELOG.md
[ ] Re-reviewed Files changed in the GitHub PR explorer

mrain commented 5 months ago

Also I'd like to ask if there's a better idea to place jf-primitives-core. It only contains PRF, CRHF and Commitment trait definitions which is used in jf-rescue crate.

mrain commented 5 months ago

Right now we have 2 decisions to make:

Should we completely remove jf-primitives. Right now it only contains aead, vrf and elgamal. We could have 3 more standalone crates.
- Or we could deprecate them for now because they are not in use. (Oh maybe vrf could be in use).
The role of VerificationResult.
- I saw that @ggutoski introduce this type alias a year ago: https://github.com/EspressoSystems/jellyfish/blob/8f48813ca52d964090dbf0de62f07f5e0c7e22c6/primitives/src/errors.rs#L18-L23
- It's used in commitment scheme, merkle tree and vdf. It feels to me that we should use it in every verify() function.
- Currently it stands in jf-traits and introduces a lot of unnecessary deps. Or we could define this type anywhere we implement related things. I very very slightly prefer the current way but would like to hear more thoughts.

(I wish I could create a poll here)

cc @EspressoSystems/jellyfish

ggutoski commented 5 months ago

Right now we have 2 decisions to make:

Should we completely remove jf-primitives. Right now it only contains aead, vrf and elgamal. We could have 3 more standalone crates.

Or we could deprecate them for now because they are not in use. (Oh maybe vrf could be in use).

The role of VerificationResult.

My vote: deprecate anything not in use, move everything else to its own crate, remove jf-primitives. Not a strong opinion though.

I saw that @ggutoski introduce this type alias a year ago: https://github.com/EspressoSystems/jellyfish/blob/8f48813ca52d964090dbf0de62f07f5e0c7e22c6/primitives/src/errors.rs#L18-L23

It's used in commitment scheme, merkle tree and vdf. It feels to me that we should use it in every verify() function.

Currently it stands in jf-traits and introduces a lot of unnecessary deps. Or we could define this type anywhere we implement related things. I very very slightly prefer the current way but would like to hear more thoughts.

My views of Rust error handling patterns are currently in flux. Currently I don't like custom error types such as this, in part for the reason you suggest: it introduces a lot of unnecessary deps. Error handling patterns are beyond the scope of this PR. For this PR I suggest we do the simplest, sane thing we can.

Regarding the specific question of VerificationResult, I don't have a strong opinion. I suppose we could keep it as-is for now. Eventually I'd like to delete cruft such as this.

philippecamacho commented 5 months ago

Great work @mrain! Thanks a lot. Would it be possible to update the README.md, describing at a high level the new structure?

philippecamacho commented 5 months ago

When I run cargo bench I got this failure:

est result: ok. 0 passed; 0 failed; 18 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running benches/bench.rs (target/nix_rustc/release/deps/plonk_benches-f083682ab5cd44f3)
thread 'main' panicked at /home/leloup/repositories/espressosys/jellyfish/plonk/src/proof_system/mod.rs:55:9:
not implemented: Should load from files in practice.
stack backtrace:
   0:     0x5c965e1a3406 - std::backtrace_rs::backtrace::libunwind::trace::hbee8a7973eeb6c93
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/libunwind.rs:104:5
   1:     0x5c965e1a3406 - std::backtrace_rs::backtrace::trace_unsynchronized::hc8ac75eea3aa6899
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x5c965e1a3406 - std::sys_common::backtrace::_print_fmt::hc7f3e3b5298b1083
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x5c965e1a3406 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hbb235daedd7c6190
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x5c965e1c1ee0 - core::fmt::rt::Argument::fmt::h76c38a80d925a410
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/rt.rs:142:9
   5:     0x5c965e1c1ee0 - core::fmt::write::h3ed6aeaa977c8e45
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/mod.rs:1120:17
   6:     0x5c965e1a18bf - std::io::Write::write_fmt::h78b18af5775fedb5
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/io/mod.rs:1810:15
   7:     0x5c965e1a31e4 - std::sys_common::backtrace::_print::h5d645a07e0fcfdbb
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x5c965e1a31e4 - std::sys_common::backtrace::print::h85035a511aafe7a8
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x5c965e1a4747 - std::panicking::default_hook::{{closure}}::hcce8cea212785a25
  10:     0x5c965e1a44a9 - std::panicking::default_hook::hf5fcb0f213fe709a
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:292:9
  11:     0x5c965e1a4bd8 - std::panicking::rust_panic_with_hook::h095fccf1dc9379ee
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:779:13
  12:     0x5c965e1a4a79 - std::panicking::begin_panic_handler::{{closure}}::h032ba12139b353db
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:649:13
  13:     0x5c965e1a3906 - std::sys_common::backtrace::__rust_end_short_backtrace::h9259bc2ff8fd0f76
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:171:18
  14:     0x5c965e1a4810 - rust_begin_unwind
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
  15:     0x5c965e180fd5 - core::panicking::panic_fmt::h784f20a50eaab275
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
  16:     0x5c965e1830cb - jf_plonk::proof_system::UniversalSNARK::universal_setup::h4169e74f94c05e2c
  17:     0x5c965e183e3f - plonk_benches::main::h5a8cfc99ffb51b28
  18:     0x5c965e1883c3 - std::sys_common::backtrace::__rust_begin_short_backtrace::hf54c38501376fbf2
  19:     0x5c965e188709 - std::rt::lang_start::{{closure}}::h1af7ae021c241fda
  20:     0x5c965e19f421 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h37600b1e5eea4ecd
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:284:13
  21:     0x5c965e19f421 - std::panicking::try::do_call::hb4bda49fa13a0c2b
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  22:     0x5c965e19f421 - std::panicking::try::h8bbf75149211aaaa
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  23:     0x5c965e19f421 - std::panic::catch_unwind::h8c78ec68ebea34cb
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  24:     0x5c965e19f421 - std::rt::lang_start_internal::{{closure}}::hffdf44a19fd9e220
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:48
  25:     0x5c965e19f421 - std::panicking::try::do_call::hcb3194972c74716d
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  26:     0x5c965e19f421 - std::panicking::try::hcdc6892c5f0dba4c
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  27:     0x5c965e19f421 - std::panic::catch_unwind::h4910beb4573f4776
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  28:     0x5c965e19f421 - std::rt::lang_start_internal::h6939038e2873596b
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:20
  29:     0x5c965e183e95 - main
  30:     0x7c5e942ee0ce - __libc_start_call_main
  31:     0x7c5e942ee189 - __libc_start_main@GLIBC_2.2.5
  32:     0x5c965e181705 - _start
  33:                0x0 - <unknown>
error: bench failed, to rerun pass `-p jf-plonk --bench plonk-benches`

[nix-shell:~/repositories/espressosys/jellyfish]$

Can you reproduce?

mrain commented 5 months ago

When I run cargo bench I got this failure:

est result: ok. 0 passed; 0 failed; 18 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running benches/bench.rs (target/nix_rustc/release/deps/plonk_benches-f083682ab5cd44f3)
thread 'main' panicked at /home/leloup/repositories/espressosys/jellyfish/plonk/src/proof_system/mod.rs:55:9:
not implemented: Should load from files in practice.
stack backtrace:
   ...
error: bench failed, to rerun pass `-p jf-plonk --bench plonk-benches`

[nix-shell:~/repositories/espressosys/jellyfish]$

Can you reproduce?

Problem fixed in b60d41f. We should use test-srs here.

Am working on README.md now.

EspressoSystems / jellyfish

feat!: crates refactor #556

Description