Skip to content

cli/serviceability: rewrite location get to rfc-20 conforming pattern#3757

Open
juan-malbeclabs wants to merge 1 commit into
jo/4-cli-rename-serviceabilityfrom
jo/5-cli-location-get-conforming-verb
Open

cli/serviceability: rewrite location get to rfc-20 conforming pattern#3757
juan-malbeclabs wants to merge 1 commit into
jo/4-cli-rename-serviceabilityfrom
jo/5-cli-location-get-conforming-verb

Conversation

@juan-malbeclabs
Copy link
Copy Markdown
Contributor

@juan-malbeclabs juan-malbeclabs commented May 22, 2026

RFC-20 implementation stack

This PR is part of a 9-PR chain delivering RFC-20: CLI standardization. Each PR's diff is only its own contribution; reviewers should consume them in order.

# PR Scope
1 #3753 doublezero-cli-core foundation crate + solana_l1_rpc_url
2 #3754 --solana-url + --log-verbose global flags + tracing init
3 #3755 CliContext built in main + centralized error rendering
4 #3756 rename doublezero_clidoublezero-serviceability-cli
5 #3757 rewrite location get as the async + CliContext reference verb
6 #3758 docs/cli-standard.md + CLAUDE.md pointer
7 #3759 move per-resource subcommand wrappers into the module crate
8 #3760 add ServiceabilityCommand enum + async dispatcher
9 #3761 #[command(flatten)] + collapse binary dispatch

This PR: #3757 — position 5 of 9. Previous: #3756 · Next: #3758


Summary of Changes

  • Migrates location get to the RFC-20 conforming verb pattern as the project's reference (smartcontract/cli/src/location/get.rs). GetLocationCliCommand::execute is now async fn, takes &CliContext as its first non-self argument, and emits a tracing::debug! event so -v surfaces what the verb is doing.
  • Updates the verb's unit test to consume the shared doublezero_cli_core::testing::cli_context_default_for_tests() helper and exercise the new async signature via a small tokio current-thread runtime in the existing #[test]. Backend stays mocked through MockCliCommand (auto-generated by #[automock]).
  • Updates the binary dispatch arms in client/doublezero and controlplane/doublezero-admin to .await the new method. The doublezero-admin binary gets a doublezero-cli-core dep plus a small CliContext build at startup (matching the doublezero binary's pattern) so the new verb is callable end-to-end.
  • The verb's user-facing args, flags, table layout, and JSON schema are unchanged. Other location verbs (Create, Update, List, Delete) keep their current sync signatures and migrate opportunistically per RFC-20's grandfathering clause.

Diff Breakdown

Category Files Lines (+/-) Net
Core logic 3 +43 / -11 +32
Tests 1 +14 / -3 +11
Config/build 2 +2 / -0 +2
Docs 1 +1 / -0 +1
Generated 1 +14 / -7 +7
Total 7 +74 / -21 +53

One verb migrated end-to-end (async + CliContext + tracing + shared test helper); two binaries updated to .await the new dispatch arm.

Key files (click to expand)
  • smartcontract/cli/src/location/get.rs - converts execute to async fn (self, ctx, client, out), adds the tracing::debug! event, rewrites the unit test to use cli_context_default_for_tests() and a small tokio current-thread runtime around the awaited call.
  • client/doublezero/src/main.rs - one-line change: LocationCommands::Get(args) => args.execute(&ctx, &client, &mut handle).await.
  • controlplane/doublezero-admin/src/main.rs - same one-line dispatch change plus a CliContextBuilder::new().with_env(...).build()? block at startup so &ctx is available.
  • controlplane/doublezero-admin/Cargo.toml, smartcontract/cli/Cargo.toml - add doublezero-cli-core and tracing deps respectively.

Testing Verification

  • cargo test -p doublezero-serviceability-cli location::get passes (the rewritten test exercises pubkey lookup, code lookup, and the not-found error path through the async signature).
  • make rust-test green workspace-wide.
  • make rust-lint clean.
  • doublezero location get --help shows the same --code and --json flags as before; behavior unchanged from the user's perspective.
  • Targets jo/4-cli-rename-serviceability; the diff shown is only this PR's contribution.

@vihu
Copy link
Copy Markdown
Contributor

vihu commented May 22, 2026

doublezero-admin now constructs a CliContext, but it only seeds .with_env(env_for_ctx) and drops the explicit admin globals (--url, --ws, --program-id, --keypair) that are still passed into DZClient.

Today location get only uses ctx.env for a debug log, so this is not a current output regression. But this PR makes location get the reference RFC-20 pattern, and future migrated verbs are expected to read backend/signing inputs from CliContext. In doublezero-admin, that would make migrated verbs see default env-derived values instead of the actual overrides used by DZClient.

Can we either mirror the client/doublezero/src/main.rs context-building behavior here or hold off on passing a partial context from doublezero-admin until it carries the same resolved inputs?

@juan-malbeclabs juan-malbeclabs force-pushed the jo/4-cli-rename-serviceability branch 2 times, most recently from f2a46ac to 475a322 Compare May 24, 2026 14:41
@juan-malbeclabs juan-malbeclabs force-pushed the jo/5-cli-location-get-conforming-verb branch from 8d806a7 to 5056078 Compare May 24, 2026 14:41
juan-malbeclabs added a commit that referenced this pull request May 26, 2026
…3753)

## RFC-20 implementation stack

This PR is part of a 9-PR chain delivering [RFC-20: CLI
standardization](https://github.com/malbeclabs/doublezero/blob/main/rfcs/rfc20-cli-standardization.md).
Each PR's diff is **only its own contribution**; reviewers should
consume them in order.

| # | PR | Scope |
|---|----|-------|
| **1** | [#3753](#3753) |
`doublezero-cli-core` foundation crate + `solana_l1_rpc_url` |
| 2 | [#3754](#3754) |
`--solana-url` + `--log-verbose` global flags + tracing init |
| 3 | [#3755](#3755) |
`CliContext` built in `main` + centralized error rendering |
| 4 | [#3756](#3756) |
rename `doublezero_cli` → `doublezero-serviceability-cli` |
| 5 | [#3757](#3757) |
rewrite `location get` as the async + `CliContext` reference verb |
| 6 | [#3758](#3758) |
`docs/cli-standard.md` + `CLAUDE.md` pointer |
| 7 | [#3759](#3759) | move
per-resource subcommand wrappers into the module crate |
| 8 | [#3760](#3760) | add
`ServiceabilityCommand` enum + async dispatcher |
| 9 | [#3761](#3761) |
`#[command(flatten)]` + collapse binary dispatch |

**This PR: #3753** — position **1** of 9. Previous: (none — base is
`main`) · Next:
[#3754](#3754)

---

## Summary of Changes
- Adds `doublezero-cli-core` (`crates/doublezero-cli-core/`), the shared
library crate every `doublezero-<module>-cli` will reuse per
[RFC-20](../rfcs/rfc20-cli-standardization.md). Ships `CliContext` +
`CliContextBuilder`, `RequirementCheck` bitflags (bit values aligned
with the legacy `CHECK_ID_JSON | CHECK_BALANCE |
CHECK_FOUNDATION_ALLOWLIST` constants), the shared validators
(`validate_pubkey`, `validate_pubkey_or_code`, `validate_code`,
`validate_parse_bandwidth`, `validate_parse_delay_ms`,
`validate_parse_jitter_ms`, `validate_parse_delay_override_ms`), the
`DisplayVec` formatter, a `tracing` + `tracing-subscriber`
`init_logging(verbosity)` helper that writes to stderr, and `testing`
helpers (`cli_context_for_tests`, `cli_context_default_for_tests`).
- Adds `solana_l1_rpc_url` to `doublezero-config::NetworkConfig` with
the per-environment defaults from RFC-20 §Environments (mainnet-beta ->
Solana mainnet-beta; testnet and devnet -> Solana testnet; local ->
`http://localhost:8899`) plus a `DZ_SOLANA_RPC_URL` env-var override
mirroring the existing `DZ_LEDGER_RPC_URL` / `DZ_LEDGER_WS_RPC_URL`
overrides.
- Migrates the shared `validators.rs` and `formatters.rs` out of
`smartcontract/cli/` into the new core crate. Existing import paths
continue to compile via thin `pub use` re-export shims in
`smartcontract/cli/src/{validators,formatters}.rs`, so the rest of the
workspace is unaffected.

## Diff Breakdown
| Category     | Files | Lines (+/-) | Net  |
|--------------|-------|-------------|------|
| Core logic   |     7 | +618 / -0   | +618 |
| Tests        |     1 | +64 / -0    |  +64 |
| Scaffolding  |     4 | +50 / -167  | -117 |
| Config/build |     3 | +33 / -0    |  +33 |
| Generated    |     1 | +20 / -0    |  +20 |
| Docs         |     1 | +2 / -1     |   +1 |
| **Total**    |    17 | +787 / -168 | +619 |

Introduces ~620 lines of shared CLI utility code in the new core crate
while shrinking `smartcontract/cli` by replacing its validator and
formatter implementations with thin re-export shims; no behavior change.

<details>
<summary>Key files (click to expand)</summary>

- `crates/doublezero-cli-core/src/context.rs` - `CliContext`,
`OutputFormat`, and the `CliContextBuilder` that resolves `--env`
defaults from `doublezero-config` and applies per-field overrides.
- `crates/doublezero-cli-core/src/validators.rs` - shared `clap`
value-parsers (pubkey, code, pubkey-or-code, bandwidth, delay, jitter,
delay-override) with their unit tests; moved verbatim from
`smartcontract/cli/src/validators.rs`.
- `crates/doublezero-cli-core/src/requirements.rs` - `RequirementCheck`
bitflags type (KEYPAIR / BALANCE / FOUNDATION_ALLOWLIST) with bit values
preserved for ABI continuity with the legacy `u8` constants.
- `crates/doublezero-cli-core/src/testing.rs` -
`cli_context_for_tests()` / `cli_context_default_for_tests()` helpers
for module-crate verb tests.
- `crates/doublezero-cli-core/src/formatters.rs` - `DisplayVec` +
`stringify_vec` moved from `smartcontract/cli`.
- `crates/doublezero-cli-core/src/error.rs` - `Result` alias,
`CliError`, `render_eyre` helper for chain-of-causes rendering.
- `crates/doublezero-cli-core/src/logging.rs` -
`init_logging(verbosity)` via `tracing-subscriber` with stderr writer;
honors `RUST_LOG` when set.
- `config/src/env.rs` - adds `solana_l1_rpc_url` to `NetworkConfig`,
wires it through per environment, and adds `DZ_SOLANA_RPC_URL` override
plus tests.

</details>

## Testing Verification
- `cargo test -p doublezero-cli-core` passes (16 tests across
validators, requirements bitflags, context builder, testing helpers,
formatters).
- `cargo test -p doublezero-config` passes (9 tests including two new
ones covering the Solana L1 URL resolution per environment and the
`DZ_SOLANA_RPC_URL` override).
- `make rust-test` green, including the program-accounts-compat run.
- `make rust-lint` clean.
- Verified existing `smartcontract/cli` consumers continue to compile
against the shim files (`use doublezero_cli::validators::*` and friends
resolve through the re-export).
@juan-malbeclabs juan-malbeclabs force-pushed the jo/4-cli-rename-serviceability branch from 475a322 to 5e5f2df Compare May 26, 2026 17:07
@juan-malbeclabs juan-malbeclabs force-pushed the jo/5-cli-location-get-conforming-verb branch from 5056078 to dfe9382 Compare May 26, 2026 17:07
juan-malbeclabs added a commit that referenced this pull request May 26, 2026
## RFC-20 implementation stack

This PR is part of a 9-PR chain delivering [RFC-20: CLI
standardization](https://github.com/malbeclabs/doublezero/blob/main/rfcs/rfc20-cli-standardization.md).
Each PR's diff is **only its own contribution**; reviewers should
consume them in order.

| # | PR | Scope |
|---|----|-------|
| 1 | [#3753](#3753) |
`doublezero-cli-core` foundation crate + `solana_l1_rpc_url` |
| **2** | [#3754](#3754) |
`--solana-url` + `--log-verbose` global flags + tracing init |
| 3 | [#3755](#3755) |
`CliContext` built in `main` + centralized error rendering |
| 4 | [#3756](#3756) |
rename `doublezero_cli` → `doublezero-serviceability-cli` |
| 5 | [#3757](#3757) |
rewrite `location get` as the async + `CliContext` reference verb |
| 6 | [#3758](#3758) |
`docs/cli-standard.md` + `CLAUDE.md` pointer |
| 7 | [#3759](#3759) | move
per-resource subcommand wrappers into the module crate |
| 8 | [#3760](#3760) | add
`ServiceabilityCommand` enum + async dispatcher |
| 9 | [#3761](#3761) |
`#[command(flatten)]` + collapse binary dispatch |

**This PR: #3754** — position **2** of 9. Previous:
[#3753](#3753) · Next:
[#3755](#3755)

---

## Summary of Changes
- Adds the `--solana-url <SOLANA_RPC_URL>` global flag to `doublezero`
per RFC-20 §Global flags. Distinct from `--url`, which continues to
override the DZ ledger transport; `--solana-url` targets the Solana L1
transport. The flag is parsed and exposed on the binary's `App` struct;
per-verb consumption lands when verbs migrate to construct typed Solana
L1 clients from `CliContext` (follow-up PRs).
- Adds the `--verbose` / `-v` (repeatable) global flag and initializes
the `tracing` subscriber at startup via
`doublezero_cli_core::init_logging(verbosity)`. Default level is `warn`;
`-v` raises to `debug`, `-vv` to `trace`. Honors the `RUST_LOG` env var
when set. Diagnostic logs go to stderr so `--json` output on stdout
remains parseable.
- Replaces the `println!("using keypair: ...")` startup line with a
`tracing::info!` event so the keypair confirmation now appears only at
`-v` or higher and no longer pollutes parseable stdout.

## Diff Breakdown
| Category     | Files | Lines (+/-) | Net  |
|--------------|-------|-------------|------|
| Core logic   |     1 | +13 / -1    |  +12 |
| Config/build |     2 | +3 / -0     |   +3 |
| Docs         |     1 | +2 / -0     |   +2 |
| **Total**    |     4 | +18 / -1    |  +17 |

Three new global flags wired into the binary and the logging facade; no
changes to subcommand dispatch or verb behavior.

<details>
<summary>Key files (click to expand)</summary>

- `client/doublezero/src/main.rs` - adds `solana_url` and `verbose`
fields to the `App` struct, calls
`doublezero_cli_core::init_logging(app.verbose)` first thing in `main`,
and replaces the `println!` keypair confirmation with `tracing::info!`.
- `client/doublezero/Cargo.toml` - depends on `doublezero-cli-core` and
`tracing`.

</details>

## Testing Verification
- `make rust-lint` clean.
- `doublezero --help` shows the new `--solana-url` and `--verbose` /
`-v` global flags with the expected help text.
- `doublezero --keypair /tmp/fake.json --env local -v address` emits a
`tracing::info!` event with the keypair path on stderr while stdout
shows only the address; no `-v` keeps stderr empty.
- Built on top of #3753 (foundation crate); this PR targets
`jo/1-cli-core-foundation` and the diff shown is its own contribution
only.
@juan-malbeclabs juan-malbeclabs force-pushed the jo/4-cli-rename-serviceability branch from 5e5f2df to 38ed34b Compare May 27, 2026 15:08
@juan-malbeclabs juan-malbeclabs force-pushed the jo/5-cli-location-get-conforming-verb branch from dfe9382 to 82adaf7 Compare May 27, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants