Skip to content

fix(cli)(#36): invoice pay --amount accepts human units#37

Open
vrogojin wants to merge 1 commit into
mainfrom
fix/issue-36-invoice-pay-human-units
Open

fix(cli)(#36): invoice pay --amount accepts human units#37
vrogojin wants to merge 1 commit into
mainfrom
fix/issue-36-invoice-pay-human-units

Conversation

@vrogojin
Copy link
Copy Markdown
Contributor

@vrogojin vrogojin commented Jun 5, 2026

Summary

Implementation

invoice-pay resolves the invoice via sphere.accounting!.getInvoice(invoiceId), reads terms.targets[targetIndex].assets[0], and:

  • fails fast on missing invoice / out-of-range target / NFT-asset target;
  • looks up decimals through resolveCoin(symbol);
  • pipes the raw value through toSmallestUnit(rawAmount, decimals);
  • rejects zero / non-positive with failWithHelp.

This mirrors exactly the conversion path PR #35 added for invoice-create / invoice-return.

Help block updates

  • description mentions the partial / over-payment use case
  • --amount flag doc spells out human units and gives a fractional example
  • examples use sphere invoice pay … form with human-readable amounts (0.5 UCT, 25 UCT)
  • new notes line reminds operators that NFT targets reject --amount
sphere payments send @bob 50 UCT             → 50 whole UCT      ✓ human units
sphere invoice create --asset 50 UCT          → 50 whole UCT      ✓ human units (PR #35)
sphere invoice return … --asset 50 UCT        → 50 whole UCT      ✓ human units (PR #35)
sphere invoice pay <id> --amount 50           → 50 whole UCT      ✓ NOW human units

Out of scope

  • --asset-index selector (target.assets[N] beyond N=0) — issue's "Out of scope" section, parked.
  • SDK-level units semantics — PayInvoiceParams.amount stays a smallest-unit string. All conversion happens in the CLI.

Test plan

  • npx tsc --noEmit — clean
  • npx vitest run — 126/126 pass (legacy-cli-ux static-guard tests still green)
  • node bin/sphere.mjs invoice pay --help — help block renders with new description / flag / examples / notes
  • npx tsup — ESM + CJS + DTS build clean
  • (optional follow-up) Extend manual-test-accounting-roundtrip.sh with a partial-payment leg exercising --amount — flagged in the issue as a nice-to-have to guard the new path

Closes #36.

Closes the cross-command inconsistency PR #35 deferred:

  payments send 50 UCT             → 50 whole UCT      ✓ human units
  invoice create --asset 50 UCT    → 50 whole UCT      ✓ human units (PR #35)
  invoice return … --asset 50 UCT  → 50 whole UCT      ✓ human units (PR #35)
  invoice pay <id> --amount 50     → 50 whole UCT      ✓ NOW human units

PR #35 left `invoice-pay --amount` parsing the raw value as smallest
units because the command takes only the amount (no coin token); the
coin is implicit in the invoice's target/asset entry. This patch
resolves the invoice via `sphere.accounting!.getInvoice(invoiceId)`,
reads the target's first asset, fails fast on NFT targets or missing
targets, looks up decimals through `resolveCoin(symbol)`, and pipes
the value through `toSmallestUnit()` — exactly the same conversion
path PR #35 used for create/return.

Side-effect on COMMAND_HELP:
  - description: mentions partial/over-payment use case (per the
    issue's "Why deferred" rationale)
  - --amount flag desc: documents human-unit semantics with a
    fractional example
  - examples: switched to `sphere invoice pay …` form with
    human-readable amounts (0.5 UCT, 25 UCT)
  - notes: explicit reminder that NFT targets reject --amount

Picked Option A from the issue (--amount <value> stays single-token,
coin inferred from the invoice). Option B (--amount <value> <coin>)
was rejected because forcing the user to re-type a coin the invoice
already pins adds friction without information; on `--asset` the
coin is being *declared* (create/return), on `pay` it's *fixed by
the invoice*.

== Verification ==

- npx tsc --noEmit: clean
- npx vitest run: 126/126 pass
- node bin/sphere.mjs invoice pay --help: renders cleanly with new
  description / flag / examples / notes
- npx tsup: clean ESM + CJS + DTS build

Issue: #36
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.

invoice pay --amount: accept human units (toSmallestUnit conversion) for cross-command consistency

1 participant