From 325bae09a435223105c51a26abd8988eac54cca3 Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Fri, 29 May 2026 17:35:27 +0000 Subject: [PATCH] sdk: add from_context constructors and wire the CLI to use them Build the serviceability and geolocation SDK clients directly from a resolved RFC-20 CliContext instead of round-tripping its values back through DZClient::new's config-file re-resolution and moniker conversion. - Add DZClient::from_context and GeoClient::from_context, gated behind a new `cli-context` cargo feature so non-CLI SDK consumers keep a dependency-light default build. - Preserve keypair precedence (CLI flag > DOUBLEZERO_KEYPAIR > stdin > context keypair path > default); the context path is only the low-precedence fallback so the env var still wins. - Wire the doublezero binary to from_context, removing the legacy bridge. --- CHANGELOG.md | 4 + Cargo.lock | 1 + client/doublezero/Cargo.toml | 2 +- client/doublezero/src/main.rs | 25 ++-- smartcontract/sdk/rs/Cargo.toml | 5 + smartcontract/sdk/rs/src/client.rs | 108 ++++++++++++++++++ smartcontract/sdk/rs/src/config.rs | 2 +- .../sdk/rs/src/geolocation/client.rs | 32 ++++++ 8 files changed, 162 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ead1a9e8b5..68775e46d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,10 @@ All notable changes to this project will be documented in this file. - Migrate the 11 `device` and `device interface` verbs (device `create`, `update`, `list`, `get`, `delete`, `set-health` plus interface `create`, `update`, `list`, `get`, `delete`) and the 14 `link` and `topology` verbs (link `accept`, `delete`, `wan create`, `dzx create`, `get`, `latency`, `list`, `set-health`, `update` plus topology `assign-node-segments`, `clear`, `create`, `delete`, `list`) to the RFC-20 `pub async fn execute(self, ctx: &CliContext, client, out)` signature. Signature-only sweep: verb bodies (including `--wait` polling via `poll_for_*_activated`, the per-verb requirement checks, and the `Signature:` writes) are unchanged. Test files gain a per-file `block_on` shim and `cli_context_default_for_tests()` import so the existing sync `#[test]` bodies can drive the now-async `execute`. `controlplane/doublezero-admin`, the unified `doublezero` binary, and the serviceability dispatcher all forward `&ctx` and await every device, interface, link, and topology arm. All 345 unit tests pass byte-identically (92 in the migrated modules: device 46, link 29, topology 17). Helper adoption (`require!`, `print_signature`, `render_collection`, `render_record`) lands opportunistically in follow-up PRs; the `--wait` polling flow on `device create/update`, `device interface create/update`, `link wan-create`/`dzx-create`/`accept`/`update` needs special handling there since the post-signature poll has to be preserved. - Migrate all six `accesspass` verbs (`set`, `close`, `list`, `get`, `user-balances`, `fund`) and all six `resource` verbs (`allocate`, `create`, `deallocate`, `get`, `close`, `verify`), plus the eight leaf single-file verbs (`address`, `balance`, `init`, `migrate`, `keygen`, `export`, `config get`, `config set`), to the RFC-20 `pub async fn execute(self, ctx: &CliContext, client, out) -> eyre::Result<()>` signature. The five small leaf verbs (`address`, `balance`, `init`, `migrate`, `keygen`) also adopt the `require!` macro and (where applicable) the `print_signature` helper since their bodies were one-line readiness checks paired with a single `Signature:` write. The larger and more idiosyncratic verbs (`config get`/`set` which manipulate the persisted YAML, `export` which serializes the whole graph, the accesspass and resource verbs which contain bespoke output and progress-spinner logic) keep their existing bodies for now and only get the signature flip; helper adoption for those lands opportunistically in follow-up PRs. `controlplane/doublezero-admin`, the unified `doublezero` binary, and the serviceability dispatcher all forward `&ctx` and await every accesspass, resource, and leaf-verb arm. `config get`/`set` tests gain a per-file `block_on` shim and a `cli_context_default_for_tests()` import so the existing sync `#[test]` bodies can still drive the now-async `execute`. The bespoke `accesspass fund` signature (`R: BufRead` for stdin) is preserved — only the `_ctx` parameter is inserted after `self`. Behavior is byte-identical: table layouts, JSON schemas, `Signature:` lines, the `fund` interactive flow, and the `config` text output all match the pre-refactor strings exactly; all 345 unit tests pass without assertion changes. - Migrate all eight `tenant` verbs (`create`, `update`, `list`, `get`, `delete`, `administrator add`, `administrator remove`, `update-payment-status`) and all six `permission` verbs (`set`, `suspend`, `resume`, `delete`, `get`, `list`) to the RFC-20 conforming shape on top of the shared CLI helpers. Every verb is now `pub async fn execute(self, ctx: &CliContext, client: &C, out: &mut W) -> eyre::Result<()>`, consumes the helpers (`require!`, `render_collection`, `render_record`, `print_signature`), and tenant verbs that accept a pubkey-or-code identifier (`update`, `delete`, `add-administrator`, `remove-administrator`, `update-payment-status`) route through a new `resolve_tenant_pk` helper in `smartcontract/cli/src/helpers.rs`. The duplicate-code precondition in `tenant create` is preserved, as is the `administrator = "me"` short-circuit. `tenant delete`'s bespoke two-line output ("✓ Tenant 'X' deleted successfully\n Signature: ..."), its cascade-delete progress spinners, and its reference-count polling loop are preserved with manual `writeln!` calls and an explanatory comment. `permission set`'s bespoke two-line aligned output ("Signature: ..." + "Permissions: ...") is preserved the same way. Permission verbs derive the on-chain PDA from `user_payer` rather than going through a pubkey-or-code resolver. `controlplane/doublezero-admin`, the unified `doublezero` binary, and the serviceability dispatcher all forward `&ctx` and await every tenant and permission arm. Behavior is byte-identical: table layout, JSON schema, `Signature:` line shape, and `--json` / `--json-compact` semantics match pre-refactor output exactly; all 18 tenant and 18 permission unit tests pass without assertion changes. +- SDK (Rust) + - Add `DZClient::from_context` and `GeoClient::from_context`, which build clients directly from a resolved RFC-20 `CliContext` instead of re-reading `~/.config/doublezero/cli/config.yml` and re-applying moniker conversion. The context already carries the fully resolved ledger RPC/WS URLs and program IDs, so these constructors consume them verbatim, making the context the single source of truth and removing the double-resolution the binary previously incurred. Keypair precedence is preserved exactly (CLI flag > `DOUBLEZERO_KEYPAIR` > stdin > context keypair path > default): the raw `--keypair` flag is passed as the highest-precedence source and the context keypair path is used only as the low-precedence fallback, so the env var still wins. The new constructors and their `doublezero-cli-core` dependency are gated behind a `cli-context` cargo feature so non-CLI SDK consumers (controlplane, telemetry, e2e) keep a dependency-light default build. `DZClient::new` / `GeoClient::new` are unchanged for callers that do not build a `CliContext` (e.g. `controlplane/doublezero-admin`). +- CLI + - Construct the serviceability and geolocation SDK clients in the `doublezero` binary via `DZClient::from_context` / `GeoClient::from_context`, replacing the legacy `DZClient::new(Option, ...)` bridge. The binary no longer round-trips the already-resolved `CliContext` values back through the SDK's config-file re-resolution. No user-facing command, flag, or output change. ## [v0.24.0](https://github.com/malbeclabs/doublezero/compare/client/v0.23.0...client/v0.24.0) - 2026-05-22 diff --git a/Cargo.lock b/Cargo.lock index 2cfab1266a..28dba3c9d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1772,6 +1772,7 @@ dependencies = [ "chrono", "directories-next", "dirs-next", + "doublezero-cli-core", "doublezero-config", "doublezero-geolocation", "doublezero-program-common", diff --git a/client/doublezero/Cargo.toml b/client/doublezero/Cargo.toml index 48858fb4bc..6f3a95ed5e 100644 --- a/client/doublezero/Cargo.toml +++ b/client/doublezero/Cargo.toml @@ -40,7 +40,7 @@ tabled.workspace = true tokio.workspace = true # Dependencies from this workspace -doublezero_sdk.workspace = true +doublezero_sdk = { workspace = true, features = ["cli-context"] } doublezero-geolocation-cli.workspace = true doublezero-serviceability-cli.workspace = true doublezero-cli-core.workspace = true diff --git a/client/doublezero/src/main.rs b/client/doublezero/src/main.rs index 5f0f0ef180..ec6b6de6da 100644 --- a/client/doublezero/src/main.rs +++ b/client/doublezero/src/main.rs @@ -199,19 +199,15 @@ async fn main() -> eyre::Result<()> { std::process::exit(1); }); - // Bridge to the legacy `DZClient::new(Option, ...)` signature. - // CliContext now carries the fully resolved values for URL/WS/program-ID, - // so we forward them directly. The keypair argument is an exception: it - // must reflect only the `--keypair` CLI flag so that `DZClient::new`'s - // internal `load_keypair` precedence chain (CLI flag > `DOUBLEZERO_KEYPAIR` - // env var > stdin > persisted config) is preserved. Passing the layered - // ctx value here would mask the env var, which the e2e contributor-auth - // suite relies on for negative-authz checks. - let url = Some(ctx.ledger_rpc_url.clone()); - let ws = Some(ctx.ledger_ws_rpc_url.clone()); - let program_id = Some(ctx.serviceability_program_id.to_string()); - - let dzclient = DZClient::new(url.clone(), ws, program_id, app.keypair.clone())?; + // Build the SDK client directly from the resolved `CliContext`. The context + // already carries the fully resolved URL/WS/program-ID, so `from_context` + // consumes them verbatim (no config-file re-read, no moniker conversion). + // The keypair argument reflects only the `--keypair` CLI flag so that the + // SDK's `load_keypair` precedence chain (CLI flag > `DOUBLEZERO_KEYPAIR` + // env var > stdin > context keypair path > default) is preserved. Passing + // the layered ctx value as the CLI source would mask the env var, which the + // e2e contributor-auth suite relies on for negative-authz checks. + let dzclient = DZClient::from_context(&ctx, app.keypair.clone())?; let client = CliCommandImpl::new(&dzclient); let stdout = std::io::stdout(); @@ -267,8 +263,7 @@ async fn main() -> eyre::Result<()> { // Geolocation module crate (doublezero-geolocation-cli per RFC-20) Command::Geolocation(args) => { - let geo_client = - GeoClient::new(url.clone(), app.geo_program_id.clone(), app.keypair.clone())?; + let geo_client = GeoClient::from_context(&ctx, app.keypair.clone())?; let svc_program_id = *dzclient.get_program_id(); let (globalstate_pk, _) = get_globalstate_pda(&svc_program_id); let geo_cli = GeoCliCommandImpl::new(&geo_client, &dzclient, globalstate_pk); diff --git a/smartcontract/sdk/rs/Cargo.toml b/smartcontract/sdk/rs/Cargo.toml index 2267c31b8e..afb6df8bb9 100644 --- a/smartcontract/sdk/rs/Cargo.toml +++ b/smartcontract/sdk/rs/Cargo.toml @@ -22,6 +22,7 @@ bincode.workspace = true chrono.workspace = true directories-next.workspace = true dirs-next.workspace = true +doublezero-cli-core = { workspace = true, optional = true } doublezero-config.workspace = true doublezero-geolocation = { workspace = true, features = ["no-entrypoint"] } doublezero-program-common.workspace = true @@ -51,3 +52,7 @@ log.workspace = true [features] default-mainnet-beta = [] +# Enables `from_context` constructors that build clients directly from a +# resolved `CliContext`. Gated so non-CLI SDK consumers don't pull in the +# CLI-only dependency tree (clap, tabled, tracing-subscriber). +cli-context = ["dep:doublezero-cli-core"] diff --git a/smartcontract/sdk/rs/src/client.rs b/smartcontract/sdk/rs/src/client.rs index 22db2d1849..7933cdd8d9 100644 --- a/smartcontract/sdk/rs/src/client.rs +++ b/smartcontract/sdk/rs/src/client.rs @@ -112,6 +112,47 @@ impl DZClient { }) } + /// Build a `DZClient` from a resolved RFC-20 [`CliContext`]. + /// + /// Unlike [`DZClient::new`], this performs no `config.yml` read and no + /// moniker conversion: the context already carries the fully resolved + /// ledger RPC/WS URLs and serviceability program ID, so they are consumed + /// verbatim. This makes the context the single source of truth and removes + /// the double-resolution the binary previously incurred. + /// + /// `keypair` is the raw `--keypair` CLI flag (or `None`). It is passed as + /// the highest-precedence source to [`load_keypair`] so the standard chain + /// (CLI flag > `DOUBLEZERO_KEYPAIR` > stdin > config path > default) is + /// preserved. The context's `keypair_path` is used only as the lower- + /// precedence config/default path; passing it as the CLI source would mask + /// the env var. + #[cfg(feature = "cli-context")] + pub fn from_context( + ctx: &doublezero_cli_core::CliContext, + keypair: Option, + ) -> eyre::Result { + let rpc_url = ctx.ledger_rpc_url.clone(); + let rpc_ws_url = ctx.ledger_ws_rpc_url.clone(); + + let client = RpcClient::new_with_commitment(rpc_url.clone(), CommitmentConfig::confirmed()); + + let default_path = ctx + .keypair_path + .clone() + .unwrap_or_else(default_keypair_path); + let payer = load_keypair(keypair, None, default_path) + .ok() + .map(|r| r.keypair); + + Ok(DZClient { + rpc_url, + client, + rpc_ws_url, + payer, + program_id: ctx.serviceability_program_id, + }) + } + pub fn get_rpc(&self) -> &String { &self.rpc_url } @@ -705,3 +746,70 @@ impl DoubleZeroClient for DZClient { Ok(transactions) } } + +#[cfg(all(test, feature = "cli-context"))] +mod cli_context_tests { + use super::*; + use doublezero_cli_core::CliContextBuilder; + use doublezero_config::Environment; + use serial_test::serial; + use std::io::Write; + + const ENV_KEYPAIR: &str = "DOUBLEZERO_KEYPAIR"; + + #[test] + #[serial] + fn from_context_uses_resolved_values_without_config_read() { + let pid = Pubkey::new_unique(); + let ctx = CliContextBuilder::new() + .with_env(Environment::Devnet) + .with_ledger_rpc_url("http://localhost:8899/") + .with_serviceability_program_id(pid) + .build() + .unwrap(); + + let client = DZClient::from_context(&ctx, None).unwrap(); + + // Resolved values consumed verbatim from the context. + assert_eq!(client.get_rpc().as_str(), "http://localhost:8899/"); + // WS derived from the RPC override by scheme swap (no env-default WS). + assert_eq!(client.get_ws().as_str(), "ws://localhost:8899/"); + assert_eq!(client.get_program_id(), &pid); + } + + /// Guards the masking hazard: `from_context` must pass the context's + /// keypair path only as the low-precedence fallback, never as the CLI + /// source, so `DOUBLEZERO_KEYPAIR` still wins over it. + #[test] + #[serial] + fn from_context_env_keypair_wins_over_context_path() { + let kp = Keypair::new(); + let dir = tempfile::tempdir().unwrap(); + let kp_path = dir.path().join("env-key.json"); + let json = serde_json::to_string(&kp.to_bytes().to_vec()).unwrap(); + std::fs::File::create(&kp_path) + .unwrap() + .write_all(json.as_bytes()) + .unwrap(); + + // Context carries a bogus keypair path. If it were used as the CLI + // source it would win and fail to load; correct behavior is for the + // env var to win. + let ctx = CliContextBuilder::new() + .with_env(Environment::Devnet) + .with_ledger_rpc_url("http://localhost:8899/") + .with_serviceability_program_id(Pubkey::new_unique()) + .with_keypair_path(PathBuf::from("/nonexistent/bogus.json")) + .build() + .unwrap(); + + std::env::set_var(ENV_KEYPAIR, &kp_path); + let client = DZClient::from_context(&ctx, None).unwrap(); + std::env::remove_var(ENV_KEYPAIR); + + assert_eq!( + client.payer_keypair().map(|k| k.pubkey()), + Some(kp.pubkey()) + ); + } +} diff --git a/smartcontract/sdk/rs/src/config.rs b/smartcontract/sdk/rs/src/config.rs index 23d7a180c2..dcba87e6df 100644 --- a/smartcontract/sdk/rs/src/config.rs +++ b/smartcontract/sdk/rs/src/config.rs @@ -60,7 +60,7 @@ pub struct ClientConfig { pub geo_program_id: Option, } -fn default_keypair_path() -> PathBuf { +pub(crate) fn default_keypair_path() -> PathBuf { let mut keypair_path = dirs_next::home_dir().unwrap_or_default(); keypair_path.extend([".config", "doublezero", "id.json"]); keypair_path diff --git a/smartcontract/sdk/rs/src/geolocation/client.rs b/smartcontract/sdk/rs/src/geolocation/client.rs index 5181d363c8..d315d916b4 100644 --- a/smartcontract/sdk/rs/src/geolocation/client.rs +++ b/smartcontract/sdk/rs/src/geolocation/client.rs @@ -115,6 +115,38 @@ impl GeoClient { program_id, }) } + + /// Build a `GeoClient` from a resolved RFC-20 [`CliContext`]. + /// + /// Mirrors [`crate::DZClient::from_context`]: no `config.yml` read and no + /// moniker conversion. The ledger RPC URL and the geolocation program ID + /// are taken verbatim from the context (the context already folds the + /// `--geo-program-id` flag and any persisted value). Keypair precedence is + /// preserved exactly as in [`crate::DZClient::from_context`]. + #[cfg(feature = "cli-context")] + pub fn from_context( + ctx: &doublezero_cli_core::CliContext, + keypair: Option, + ) -> eyre::Result { + let client = RpcClient::new_with_commitment( + ctx.ledger_rpc_url.clone(), + CommitmentConfig::confirmed(), + ); + + let default_path = ctx + .keypair_path + .clone() + .unwrap_or_else(crate::config::default_keypair_path); + let payer = load_keypair(keypair, None, default_path) + .ok() + .map(|r| r.keypair); + + Ok(GeoClient { + client, + payer, + program_id: ctx.geolocation_program_id, + }) + } } impl GeolocationClient for GeoClient {