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

workspace tooling overhaul #54

Merged
merged 13 commits into from
Feb 6, 2024
Merged

workspace tooling overhaul #54

merged 13 commits into from
Feb 6, 2024

Conversation

slumber
Copy link
Contributor

@slumber slumber commented Jan 31, 2024

No description provided.

@slumber
Copy link
Contributor Author

slumber commented Jan 31, 2024

Development cli usage

# from the workspace
cargo install --path tools/tools-dev --force # to override previous installation

List of all commands:

Usage: cargo nexus <COMMAND>

Commands:
  clean     # new for dev only
  config    # new for dev only
  pp        # new for dev only
  node      # new for dev only
  new      
  run      
  prove    
  request  
  verify   
  help     Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help

The cli is structured in a way that:

  1. It doesn't depend on most crates from the workspace, so we don't have to reinstall it during development. Instead, it invokes corresponding binaries from the workspace.
  2. It defines the list of available commands as a library, it's a TODO to implement cli that we're going to ship. In short, it should reuse struct Cli from the dev crate and handle command::common::Command enum. Dev implementations will not be available as they depend on compiling inside workspace. (with exceptions: for example new command can be reused, the rest is inside dev::common_impl).
  3. (should be) easily extendable

@slumber
Copy link
Contributor Author

slumber commented Jan 31, 2024

config crate defines configurations used throughout workspace, such as vm or network. The way it works is:

  1. Config layout and default values are defined in config/bases/*.toml
  2. These files are compiled with tools-dev into a one big .config.env file. (done automatically but can use cargo nexus config to overwrite)
  3. When needed, configs are deserialized from env (currently using dotenv, probably we should just inject the env file into process).

.config.env is not checked into git, so you can change values inside freely.

@slumber
Copy link
Contributor Author

slumber commented Jan 31, 2024

CLI usage:

Basics:

cargo nexus new # outside of workspace, otherwise cargo treats crate as its member
cargo run
cargo nexus run [-v] # practically the same, can print trace

Local prove

cargo nexus prove local # --release to run in release, --bin to specify the binary

That's it, the command will compile configurations and generate nexus-public-{par/seq}-{k}.zst into local cache (/target/nexus-cache by default)

Network prove

# in a separate terminal, can run from anywere, e.g. $HOME
cargo nexus node 
# another terminal
cargo nexus node -p # pcd node

Once they're done reading PP file:

cargo nexus prove

#  you've got something like <HASH> 0 0 incomplete

When you see proof OK in node terminal, request the proof:

cargo nexus request e29d4c1bf1ba497291d5f0fe20f0cf60e71d0aad08af4861a229e5dad427e90a # <- HASH

Verify

Not supported, prover crate doesn't know how to decode proofs (yet)

@danielmarinq
Copy link
Contributor

danielmarinq commented Feb 1, 2024

This looks awesome. Three suggestions:

  • If the user runs cargo nexus prove without release mode -r, then print a message that says that they're proving in debug mode and that proving release mode artifacts should be significantly faster.
  • Have prove local be the default, and have prove network be the optional one. Further, print that local prove is likely slower.
  • When proving on the network, an error is printed if the network is not live on localhost. Parsing through this error might be unintuitive. Perhaps we can print a more helpful message like "No Nexus network detected at 127.0.0.1:8000".

@slumber
Copy link
Contributor Author

slumber commented Feb 2, 2024

This looks awesome. Three suggestions:

  • If the user runs cargo nexus prove without release mode -r, then print a message that says that they're proving in debug mode and that proving release mode artifacts should be significantly faster.
  • Have prove local be the default, and have prove network be the optional one. Further, print that local prove is likely slower.
  • When proving on the network, an error is printed if the network is not live on localhost. Parsing through this error might be unintuitive. Perhaps we can print a more helpful message like "No Nexus network detected at 127.0.0.1:8000".

These are great points. 1) is important and I'll fix it in this PR, although I believe profile handling should be reworked.
2) preserves previous behavior, and I'd like not to tweak it in this PR.
3) network-client is the crate to make such logs.

@slumber slumber marked this pull request as ready for review February 2, 2024 18:25
@JensNexus
Copy link
Contributor

  • If the user runs cargo nexus prove without release mode -r, then print a message that says that they're proving in debug mode and that proving release mode artifacts should be significantly faster.

Shouldn't release mode then be the default, and debug mode the one you have to specify with --debug if you want it?

Copy link
Contributor

@govereau govereau left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few minor questions.

@danielmarinq
Copy link
Contributor

  • If the user runs cargo nexus prove without release mode -r, then print a message that says that they're proving in debug mode and that proving release mode artifacts should be significantly faster.

Shouldn't release mode then be the default, and debug mode the one you have to specify with --debug if you want it?

Yes I think this is a better idea

@slumber
Copy link
Contributor Author

slumber commented Feb 5, 2024

  • If the user runs cargo nexus prove without release mode -r, then print a message that says that they're proving in debug mode and that proving release mode artifacts should be significantly faster.

Shouldn't release mode then be the default, and debug mode the one you have to specify with --debug if you want it?

Yes I think this is a better idea

I'd like to avoid making this change in this PR, especially because we had a discussion about our own Rust target

@slumber slumber merged commit 94409a4 into main Feb 6, 2024
4 checks passed
@slumber slumber deleted the slumber-tooling-overhaul branch February 6, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants