Skip to content

fix(cdk-cli): parse --unit consistently and scope balance by currency#1876

Closed
b-l-u-e wants to merge 1 commit intocashubtc:mainfrom
b-l-u-e:fix/cdk-cli-currency-unit-balance
Closed

fix(cdk-cli): parse --unit consistently and scope balance by currency#1876
b-l-u-e wants to merge 1 commit intocashubtc:mainfrom
b-l-u-e:fix/cdk-cli-currency-unit-balance

Conversation

@b-l-u-e
Copy link
Copy Markdown
Contributor

@b-l-u-e b-l-u-e commented Apr 11, 2026

This PR improves how the global --unit flag is parsed and how balance uses it, so the CLI no longer misleads users about which currency unit they are acting on. Plural aliases and a typo check align input with Cashu’s standard units; --unit is global so it works both before and after the subcommand.

issues i faced

Ambiguous or misleading unit handling

A typo or odd string could still parse as a custom unit, so what i typed did not match what i saw on the screen. For example, a mistyped unit could still show sat balances as if nothing was wrong:

cdk --unit satt balance
 http://127.0.0.1:8085 100 sat

Total balance across all wallets: 179 sat

balance and --unit

--unit did not line up with what i expected for balance. Asking for non-matching unit could still show sat totals. And putting --unit after the subcommand failed entirely:

cdk --unit dollarz balance
 http://127.0.0.1:8085 100 sat

Total balance across all wallets: 179 sat
cdk balance --unit usd
error: unexpected argument '--unit' found

Usage: cdk-cli balance

For more information, try '--help'.

Whereas putting --unit before the command still did not filter to that unit when you only held sat:

cdk --unit usd balance
0: http://127.0.0.1:8085 100 sat
1: https://fake.thesimplekid.dev 79 sat

Total balance across all wallets: 179 sat

Empty --unit

Passing an empty value looked like “no unit,” but the tool still behaved like a normal sat run, which is not the same as omitting --unit on purpose:

cdk --unit "" balance
0: http://127.0.0.1:8085 100 sat

Total balance across all wallets: 179 sat

What this PR changes

  • Stricter parsing: plural aliases, explicit error for satt, clear error for empty --unit (omit the flag for default sat).
  • balance respects the resolved unit (default sat when --unit is omitted).
  • global = true on --unit so cdk balance --unit usd and cdk --unit usd balance are equivalent.

@github-project-automation github-project-automation Bot moved this to Backlog in CDK Apr 11, 2026
Signed-off-by: b-l-u-e <8102260+blue@users.noreply.github.com>
@b-l-u-e b-l-u-e force-pushed the fix/cdk-cli-currency-unit-balance branch from 55e0a11 to 609da8b Compare April 11, 2026 06:35
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.55%. Comparing base (87f5c7c) to head (609da8b).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1876      +/-   ##
==========================================
- Coverage   63.59%   63.55%   -0.05%     
==========================================
  Files         329      329              
  Lines       54881    54881              
==========================================
- Hits        34904    34878      -26     
- Misses      19977    20003      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@lescuer97 lescuer97 left a comment

Choose a reason for hiding this comment

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

logic change

/// `--unit usd` for USD-only wallets).
pub async fn balance(
wallet_repository: &WalletRepository,
filter_unit: &CurrencyUnit,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you want this to be and Option and filter if it's not None

@thesimplekid
Copy link
Copy Markdown
Collaborator

Close for #1930

@github-project-automation github-project-automation Bot moved this from Backlog to Done in CDK Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants