Setup-circuits command

[illuzen]

This lets users who do cargo install quantus-cli do wormhole commands, but it prompts them to generate the circuits first quantus setup-circuits

[n13]

Not sure (1) is actually true

DRY issues should be addressed to keep codebase sane

PR #96 Review: Setup-circuits command

Overall the intent is solid and well-aligned with a real UX pain: cargo install quantus-cli users shouldn't be forced to clone the repo to run wormhole commands. But there are several things that need to be addressed before this can land.

🔴 Blocking issues

1. The PR doesn't actually achieve its stated goal — build.rs is untouched.
build.rs still unconditionally calls qp_wormhole_circuit_builder::generate_all_circuit_binaries(...) and symlinks/copies into generated-bins/ on every build (see build.rs lines 36–74). That means cargo install quantus-cli will still run the build script and spend minutes generating ~1 GB of keys at install time. The setup-circuits subcommand becomes redundant unless build.rs is gated behind a cargo feature (e.g. build-circuits off by default, or check SKIP_CIRCUIT_BUILD/QUANTUS_SKIP_CIRCUIT_BUILD as the new default). Without that change, this PR adds a command but does not fix cargo install.

2. CI (rustfmt) is failing. The "Fast Checks (Format)" job red-X'd. Several real diffs rustfmt wants:

• src/circuits.rs:112 — is_circuit_version_error should use trailing || style (matches the repo's rustfmt config).
• src/cli/wormhole.rs:1125 — formatting regression on the CircuitBinsConfig::load call.
• src/lib.rs:51 — the re-export block needs a regroup.

Just run cargo +nightly fmt locally.

3. Clippy will (very likely) fail. .map_err(|e| crate::circuits::wrap_circuit_error(e)) appears ~9× in the diff and is a textbook redundant_closure lint. Replace with .map_err(crate::circuits::wrap_circuit_error). The Analysis (Clippy & Doc) job was skipped due to the fmt fail so this is latent.

🟠 DRY violations (explicit user rule)

4. wrap_circuit_error is duplicated almost verbatim.

pub fn wrap_circuit_error<E: std::fmt::Display>(error: E) -> QuantusError {
	let error_str = error.to_string();

	if is_circuit_version_error(&error_str) {
		QuantusError::Generic(format!(
			"Circuit binaries appear to be outdated or corrupted.\n\
             Run 'quantus setup-circuits --force' to regenerate them.\n\
             \n\
             Original error: {}",
			error_str
		))
	} else if error_str.contains("No such file") || error_str.contains("not found") {
        ...

The identical logic and identical message strings are reproduced in src/collect_rewards_lib.rs just to produce a CollectRewardsError instead of QuantusError. Factor the message-building into one helper (e.g. fn circuit_error_message(err: &str) -> String) and let each error type wrap the string. Two copies of the three format strings is exactly the pattern your rules say to avoid.

5. Two hard-coded lists of the same eight circuit files.
REQUIRED_CIRCUIT_FILES in src/circuits.rs lists 8 filenames, and print_circuit_file_info has its own local [(name, description); 8] array listing the same 8 filenames again. Make one const FILES: &[(&str, &str)] and derive both from it.

6. build.rs also hard-codes the same file names when hashing (print_bin_hash(&build_output_dir, "common.bin") …). If you're introducing a canonical list in src/circuits.rs, have build.rs consume it via a shared const, or move the list into the qp-wormhole-circuit-builder crate.

🟡 Correctness / UX concerns

7. The progress bar is cosmetic and misleading.

progress.set_message("Building wormhole leaf circuit...");
progress.set_progress(5);
// ...
progress.set_progress(10);

qp_wormhole_circuit_builder::generate_all_circuit_binaries(...)?;

progress.set_progress(100);
progress.finish();

The bar jumps to 10 % immediately, then sits there for the entire multi-minute generation (a single blocking call with no callback hook), then jumps to 100 %. It gives users the impression of a hang. Either (a) use a spinner (ProgressBar::new_spinner() with enable_steady_tick) since we genuinely have no progress signal, or (b) add a progress callback to generate_all_circuit_binaries upstream. A spinner is the smallest correct fix.

8. Silent failure risk — print_circuit_file_info swallows missing files.

for (filename, description) in files {
    let path = dir.join(filename);
    if let Ok(metadata) = std::fs::metadata(&path) {
        ...
    }
}

If generation succeeded but a file is missing, this loop silently skips it. Per the "fail early, no silent failure" rule, either log an error or re-validate with is_valid_circuit_dir(dir) after generation and error if not valid.

9. Redundant timing. generate_circuits does let start_time = Instant::now(); at line 217 while GenerationProgress::new() already stores its own start_time. Pick one.

10. bins_dir: bins_dir.to_string_lossy().to_string() in run_collect_rewards. CollectRewardsConfig::bins_dir is String, but a path being lossily re-encoded to UTF-8 is a quiet correctness hazard on non-UTF-8 locales (rare but real). Prefer making the config field a PathBuf while you're touching this code. Minor.

11. #[allow(dead_code)] pub fn circuits_exist — if nothing calls it, delete it (per "concise and minimal" rule). If you need a predicate, find_circuit_bins_dir().is_some() at the call site is one line.

🟢 Docs / discoverability

12. README isn't updated. The cargo install quantus-cli section in README.md:28 should tell users to run quantus setup-circuits afterwards before any wormhole command. Otherwise new users hit the error message and are left to discover the subcommand themselves.

13. The module docstring claims "Users must run quantus setup-circuits before using wormhole commands" — but as long as build.rs generates them, users inside the repo never need to. Reconcile the story once #1 is resolved.

Nits

• pub const DEFAULT_CIRCUIT_BINS_DIR: &str = ".quantus/circuit-bins" — other Quantus tooling tends to use ~/.quantus/ already (e.g. wallet directory). Worth confirming that .quantus/circuit-bins/ is the intended layout and not .quantus/cli/circuit-bins/ or similar for future-proofing.
• The comment // Step 1: Generate leaf circuit (0-30%) doesn't match the code (5 % → 10 % → 100 %). Per the user rule against narrative comments, drop it entirely.
• pub use circuits::{ ... CIRCUIT_BINS_ENV_VAR }; re-exports the env-var name but not DEFAULT_CIRCUIT_BINS_DIR / DEV_CIRCUIT_BINS_DIR — either export all three constants or none, for SDK consistency.

Suggested minimum-viable merge checklist

1. Gate build.rs behind an env var (default: skip during cargo install, keep for local cargo build). Otherwise the PR doesn't fix the stated problem.
2. cargo +nightly fmt and fix redundant closures.
3. Collapse the two wrap_circuit_error bodies into one shared message builder.
4. Swap the fake progress bar for a spinner, or add proper progress callbacks upstream.
5. Add a README note pointing cargo install users at quantus setup-circuits.

Happy to draft any of those fixes as follow-up commits if you want — just say the word.