From ad2fbcba4aa2b75b0c3fadda16844d7f4fccc7d0 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:31:10 -0500 Subject: [PATCH] fix(cli): name each candidate's environment when --env is not given Without --env, the wallet shortlist for a BOLT11 send lists the whole organization, but its pasted flags and quote command named no environment, so the payment or quote failed with "Exactly one --env is required". Each candidate now adds its wallet's own --env when the scope selects none, and keeps a given --env. An invoice with a BOLT11 prefix but an unreadable amount no longer reads as missing the prefix, and the no-candidate hint names at most five excluded wallets and counts the rest, which --json still lists. Refs #34 --- CHANGELOG.md | 1 + docs/guides/payments.md | 2 +- src/api.rs | 38 ++++++++++++++++++++- src/wallet_shortlist.rs | 72 ++++++++++++++++++++++++++++++++++++++-- tests/cli_integration.rs | 55 +++++++++++++++++++++++++++++- 5 files changed, 162 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac692be..9684db1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes - In human output, an accepted `payments receive` or `quotes create` names the command that shows the invoice, address, or quote, with the organization, environment, and account that the command used. - A 403 to `VOLTAGE_API_KEY` says that the key came from that variable and where the organization came from, and suggests `--account NAME` or unsetting the variable. `--json` reports both as `error.detail.credential_sources`. - Without a picker, `payments send --invoice` without `--wallet` names the wallets in scope that match the invoice's network and currency and have funds, counting their credit limit, with `--wallet ID` and the scope flags ready to paste, or says why each wallet does not qualify. A BTC wallet's funds must cover the invoice's amount, and a USD wallet comes with the quote it needs. `--json` reports them in `error.detail`. The list is a shortlist, not a guarantee. +- Without `--env`, each wallet the shortlist names comes with its own `--env`, in its flags and its quote command, so the pasted command does not fail for a missing environment. The hint names at most five wallets that do not qualify; `--json` lists all of them. - The wallet picker shows a wallet's currency when its balances report one, and typing a currency filters on it. A wallet with no balance yet shows no currency. ### Changed diff --git a/docs/guides/payments.md b/docs/guides/payments.md index 49bdb11..374ff1e 100644 --- a/docs/guides/payments.md +++ b/docs/guides/payments.md @@ -15,7 +15,7 @@ Amounts are decimals in the unit you name with `--unit` (`msats`, `sats`, `btc`, ## Choose a wallet for an invoice -On a terminal, a payment without `--wallet` offers a wallet picker. Elsewhere, such as with `--json`, `--no-input`, or in a script, `payments send --invoice` without `--wallet` lists the organization's wallets (in the `--env` environment when one is given) and names those that could pay. A wallet qualifies when it is active, is on the invoice's network, holds the `--currency` currency (or BTC or USD without `--currency`), and has funds: its available balance plus its credit limit (`limit`) is positive, since the API lets a payment draw the balance down to minus the limit. A BTC balance's funds must also cover the invoice's amount. A USD balance pays through a quote, so its amount is not compared, and the candidate has `needs_quote: true` and a `quote` command that converts the invoice's amount with `--unit sats --to usd`; add `--quote QUOTE_ID` to the send. The invoice prefix gives the network: `lnbc` is mainnet, `lntb` testnet, `lntbs` signet, which includes mutinynet, and `lnbcrt` regtest, which no wallet uses. The error then exits with code 2. Its hint gives `--wallet ID` with the scope flags for each qualifying wallet, or says why each wallet does not qualify. `--json` reports the same in `error.detail`: +On a terminal, a payment without `--wallet` offers a wallet picker. Elsewhere, such as with `--json`, `--no-input`, or in a script, `payments send --invoice` without `--wallet` lists the organization's wallets (in the `--env` environment when one is given) and names those that could pay. A wallet qualifies when it is active, is on the invoice's network, holds the `--currency` currency (or BTC or USD without `--currency`), and has funds: its available balance plus its credit limit (`limit`) is positive, since the API lets a payment draw the balance down to minus the limit. A BTC balance's funds must also cover the invoice's amount. A USD balance pays through a quote, so its amount is not compared, and the candidate has `needs_quote: true` and a `quote` command that converts the invoice's amount with `--unit sats --to usd`; add `--quote QUOTE_ID` to the send. The invoice prefix gives the network: `lnbc` is mainnet, `lntb` testnet, `lntbs` signet, which includes mutinynet, and `lnbcrt` regtest, which no wallet uses. The error then exits with code 2. Its hint gives `--wallet ID` with the scope flags for each qualifying wallet, or says why each wallet does not qualify, naming up to five and counting the rest. Without `--env`, each wallet's flags and quote command add that wallet's own `--env`, which the payment and the quote need. An invoice whose prefix or amount the CLI cannot read is not matched, and nothing is listed. `--json` reports the same in `error.detail`: ```json {"invoice": {"network": "signet", "amount_msats": 1000}, diff --git a/src/api.rs b/src/api.rs index c7428d6..343add4 100644 --- a/src/api.rs +++ b/src/api.rs @@ -946,8 +946,20 @@ fn next_step(invocation: &ApiInvocation, scope: &Scope, id: Option) -> Opt /// The effective scope as flags, so a command in a hint reaches the same organization, /// environment, and saved credential however the original command selected them. pub(crate) fn scope_flags(scope: &Scope) -> String { + scope_flags_in(scope, None) +} + +/// `scope_flags`, naming `env` when the scope selects no environment: a resource listed +/// across an organization lives in one environment, which a later command must name. +pub(crate) fn scope_flags_in(scope: &Scope, env: Option<&str>) -> String { let org = scope.org.map(|org| format!(" --org {org}")); - let envs = scope.envs.iter().map(|env| format!(" --env {env}")); + let fallback = env.filter(|_| scope.envs.is_empty()).map(str::to_owned); + let envs = scope + .envs + .iter() + .map(Uuid::to_string) + .chain(fallback) + .map(|env| format!(" --env {env}")); let account = scope .account .as_deref() @@ -1611,6 +1623,30 @@ mod tests { assert_eq!(scope_flags(&Scope::default()), ""); } + #[test] + fn scope_flags_name_a_listed_environment_only_when_none_is_selected() { + let org = Uuid::from_u128(1); + let chosen = Uuid::from_u128(2); + let none = Scope { + org: Some(org), + ..Scope::default() + }; + assert_eq!( + scope_flags_in(&none, Some("env-3")), + format!(" --org {org} --env env-3") + ); + assert_eq!(scope_flags_in(&none, None), format!(" --org {org}")); + let selected = Scope { + org: Some(org), + envs: vec![chosen], + ..Scope::default() + }; + assert_eq!( + scope_flags_in(&selected, Some("env-3")), + format!(" --org {org} --env {chosen}") + ); + } + #[test] fn credential_rejections_name_the_next_step_for_their_own_credential() { let account = diff --git a/src/wallet_shortlist.rs b/src/wallet_shortlist.rs index f87c37f..0cd437f 100644 --- a/src/wallet_shortlist.rs +++ b/src/wallet_shortlist.rs @@ -25,6 +25,9 @@ use serde::Serialize; use serde_json::Value; use uuid::Uuid; +/// How many excluded wallets the hint names; `--json` lists every one. +const EXCLUDED_IN_HINT: usize = 5; + /// What the shortlist does and does not promise, in the hint and in `--json`. const BASIS: &str = "These wallets match the invoice's network and currency and have funds, counting their credit limit; a BTC wallet's funds also cover the invoice's amount. That is a shortlist, not a guarantee: fees and line-of-credit rules are checked only when the payment is sent."; @@ -49,7 +52,7 @@ pub async fn require( check_payment(flags, invocation.alias)?; let Some(invoice) = Invoice::parse(invoice) else { return Err(Error::usage(WALLET_REQUIRED).with_hint(format!( - "The invoice does not start with a BOLT11 prefix (lnbc, lntb, lntbs, or lnbcrt), so no wallet can be matched to it. {WALLET_HINT}" + "The invoice is not a BOLT11 invoice the CLI can read: it must start with lnbc, lntb, lntbs, or lnbcrt, then an optional amount. No wallet can be matched to it. {WALLET_HINT}" ))); }; let skip = "Or pass --wallet ID to skip listing wallets."; @@ -202,7 +205,6 @@ pub struct Shortlist { impl Shortlist { fn new(invoice: Invoice, currency: Option, listed: &Value, scope: &Scope) -> Self { - let scope = api::scope_flags(scope); let items = listed .as_array() .or_else(|| listed.get("items").and_then(Value::as_array)) @@ -227,6 +229,9 @@ impl Shortlist { let network = text(wallet, "network").map(str::to_owned); match shortlist.qualify(wallet) { Ok(currency) => { + // Without --env the listing spans the organization, and a payment or a + // quote needs the wallet's own environment. + let scope = api::scope_flags_in(scope, text(wallet, "environment_id")); let needs_quote = currency != "btc"; let quote = needs_quote.then(|| shortlist.quote(wallet, &scope)); shortlist.candidates.push(Candidate { @@ -343,9 +348,10 @@ impl Shortlist { if self.excluded.is_empty() { return "No wallets are in this scope. Create one with `voltage wallets create`, or choose another scope with --org and --env.".into(); } - let reasons: Vec = self + let mut reasons: Vec = self .excluded .iter() + .take(EXCLUDED_IN_HINT) .map(|excluded| { let why = match excluded.reason { Exclusion::Inactive => "is inactive".to_owned(), @@ -362,6 +368,14 @@ impl Shortlist { format!("{} ({}) {why}", excluded.name, excluded.wallet_id) }) .collect(); + if let Some(more) = self + .excluded + .len() + .checked_sub(EXCLUDED_IN_HINT) + .filter(|n| *n > 0) + { + reasons.push(format!("and {more} more, listed in --json as `excluded`")); + } format!( "No wallet in this scope is active on {network} with {} funds: {}. Fund a matching wallet, create one with `voltage wallets create`, or choose another scope with --org and --env.", self.currencies(), @@ -394,6 +408,7 @@ mod tests { const A: &str = "11111111-1111-4111-8111-111111111111"; const B: &str = "22222222-2222-4222-8222-222222222222"; + const C: &str = "33333333-3333-4333-8333-333333333333"; fn invoice(text: &str) -> Option { Invoice::parse(text) @@ -658,6 +673,57 @@ mod tests { let empty = Shortlist::new(signet(), None, &json!([]), &scope); assert!(empty.hint().starts_with("No wallets are in this scope.")); + // Without --env the listing spans the organization, so each candidate's flags, and a + // USD candidate's quote, name the wallet's own environment. + let mut placed = wallet(A, "signet", json!([balance("usd", 1_000, false)])); + placed["environment_id"] = json!(C); + let found = Shortlist::new(signet(), None, &json!([placed]), &scope); + let candidate = &found.candidates[0]; + assert_eq!(candidate.flags, format!("--wallet {A} --org {B} --env {C}")); + assert!( + candidate + .quote + .as_deref() + .unwrap() + .ends_with(&format!(" --org {B} --env {C} --execute")), + "{candidate:?}" + ); + // An --env given is kept, not replaced by the wallet's. + let chosen = Scope { + org: Some(uuid(B)), + envs: vec![uuid(A)], + ..Scope::default() + }; + let mut placed = wallet(A, "signet", json!([balance("btc", 1_000, false)])); + placed["environment_id"] = json!(C); + let found = Shortlist::new(signet(), None, &json!([placed]), &chosen); + assert_eq!( + found.candidates[0].flags, + format!("--wallet {A} --org {B} --env {A}") + ); + + // A long list of excluded wallets is cut short in the hint, and kept whole in --json. + let many: Vec = (0..EXCLUDED_IN_HINT + 2) + .map(|_| wallet(A, "mainnet", json!([]))) + .collect(); + let capped = Shortlist::new(signet(), None, &json!(many), &scope); + let hint = capped.hint(); + assert_eq!( + hint.matches("is on mainnet").count(), + EXCLUDED_IN_HINT, + "{hint}" + ); + assert!( + hint.contains("and 2 more, listed in --json as `excluded`"), + "{hint}" + ); + assert_eq!(capped.excluded.len(), EXCLUDED_IN_HINT + 2); + let exact: Vec = (0..EXCLUDED_IN_HINT) + .map(|_| wallet(A, "mainnet", json!([]))) + .collect(); + let hint = Shortlist::new(signet(), None, &json!(exact), &scope).hint(); + assert!(!hint.contains("more, listed"), "{hint}"); + let mut inactive = wallet(B, "signet", json!([])); inactive["active"] = json!(false); let listed = json!([ diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 21d0992..4ee31b8 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -2949,6 +2949,51 @@ async fn a_bolt11_send_without_a_wallet_shortlists_the_wallets_that_could_pay() assert_eq!(listings().await, 3); } +#[tokio::test(flavor = "multi_thread")] +async fn a_bolt11_send_without_an_environment_names_each_candidates_environment() { + let (server, dir) = fixture().await; + Mock::given(method("GET")) + .and(path(wallets_path(ORG))) + .respond_with(ok(json!([ + {"id": WALLET, "name": "ops", "active": true, "network": "mutinynet", + "environment_id": ENV, + "balances": [{"currency": "btc", "available": {"amount": 5_000, "negative": false}}]} + ]))) + .expect(1) + .mount(&server) + .await; + let output = cli(dir.path(), &server) + .env_remove("VOLTAGE_EXECUTE") + .args([ + "payments", + "send", + "--org", + ORG, + "--invoice", + "lntbs10n1qqq", + ]) + .assert() + .code(2); + let error = json_error(&output); + let flags = format!("--wallet {WALLET} --org {ORG} --env {ENV}"); + assert_eq!(error["error"]["detail"]["candidates"][0]["flags"], flags); + // The listing spanned the organization: no environment was sent. + let listed = server.received_requests().await.unwrap(); + let listing = listed + .iter() + .find(|request| request.url.path() == wallets_path(ORG)) + .unwrap(); + assert!( + !listing + .url + .query_pairs() + .any(|(key, _)| key == "environment_id"), + "{}", + listing.url + ); + server.verify().await; +} + #[tokio::test(flavor = "multi_thread")] async fn a_bolt11_send_without_a_wallet_fails_before_listing_when_it_cannot_shortlist() { let (server, dir) = fixture().await; @@ -2971,7 +3016,15 @@ async fn a_bolt11_send_without_a_wallet_fails_before_listing_when_it_cannot_shor error["error"]["hint"] .as_str() .unwrap() - .starts_with("The invoice does not start with a BOLT11 prefix") + .starts_with("The invoice is not a BOLT11 invoice the CLI can read") + ); + // So is one with a BOLT11 prefix but an amount it cannot read. + let error = json_error(&send(&["--org", ORG, "--invoice", "lnbc1p1qqq"]).code(2)); + assert!( + error["error"]["hint"] + .as_str() + .unwrap() + .starts_with("The invoice is not a BOLT11 invoice the CLI can read") ); // A local mistake is reported first. let error = json_error(&send(&["--org", ORG, "--invoice", "lnbc1", "--unit", "sats"]).code(2));