Skip to content

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

Description

@vrogojin

Context

PR #35 (fix(cli): bump SDK pin past #397 + treat invoice amounts as human units, merged 2026-06-05) made invoice create --asset <amount> <coin> and invoice return --asset <amount> <coin> interpret the amount as human units (e.g. 7 UCT = 7 whole UCT), auto-converting to smallest units via toSmallestUnit(amount, decimals). That matches payments send 7 UCT's existing convention.

invoice pay --amount was intentionally deferred from PR #35 — it's the one remaining inconsistency. Today:

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 atoms (!!!)    ✗ smallest units

The user filing this issue: "why do we need --amount? Is it needed for custom payment amount, or we don't want to cover the whole invoice?" — yes, exactly that. --amount is for partial / over-payment. Per PayInvoiceParams.amount doc: "defaults to remaining needed to cover the asset". Without --amount, the SDK pays exactly what's left to cover the asset.

Legitimate use cases:

  1. Partial payment — pay half now, half later. Invoice stays in PARTIAL state.
  2. Overpayment — tip / round-up. Triggers receiver-side auto-return if enabled, otherwise extra is kept.
  3. Multi-target invoice, paying only your share — combined with --target-index, one of N payers covers part of target #0.

Why deferred

invoice pay --amount <value> takes only the amount — no coin token. The coin is implicit in the invoice. To convert human → smallest, the CLI needs the invoice's coin (to get decimals from TokenRegistry):

const invoiceId = await resolveInvoiceId(sphere, idOrPrefix, 'invoice-pay');
const invoice = sphere.accounting!.getInvoice(invoiceId);
const targetIndex = ;
const assetIndex = ;          // currently always 0
const asset = invoice.terms.targets[targetIndex].assets[assetIndex];
if (!('coin' in asset)) failWithHelp('invoice-pay', '--amount is only valid for coin-asset targets, not NFT');
const [coinSymbol /* or coinId */] = asset.coin;
const { decimals } = resolveCoin(coinSymbol);
const smallest = toSmallestUnit(humanAmount, decimals);

This wasn't blocking the soak (manual-test-accounting-roundtrip.sh pays the full invoice, no --amount), so PR #35 stayed scoped.

Proposal

Two equivalent shapes — pick one for the cross-command "feel":

Option A (recommended) — keep current flag, but accept human units

sphere invoice pay <id> --amount 50          # 50 UCT (coin from invoice)
sphere invoice pay <id> --amount 0.5         # 0.5 UCT
  • Pros: minimal user disruption; the coin is unambiguous (invoice fixes it).
  • Cons: differs from --asset <amount> <coin> shape on create/return. Less canonical.

Option B — --amount <value> <coin> (two tokens, matches --asset)

sphere invoice pay <id> --amount 50 UCT
  • Pros: identical shape to --asset everywhere else. Strictest "all canonical".
  • Cons: requires typing the coin even though it's redundant (the CLI ignores user-supplied coin if it mismatches the invoice's coin — error or silent override?). Operator friction.

My read: Option A — the coin context is already pinned by the invoice; making the user retype it adds friction without information. The --asset <amount> <coin> shape on create/return makes sense because the user is declaring the coin; on pay, the user is paying against an already-declared coin.

Acceptance

  • sphere invoice pay <id> --amount 50 (Option A) or --amount 50 UCT (Option B) interprets 50 as human units of the invoice's coin.
  • --amount 0.5 works for fractional payments (matches payments send behaviour).
  • Validation rejects zero / negative / non-numeric with a failWithHelp block.
  • NFT targets reject --amount with a clear error (--amount only applies to coin-asset targets).
  • Update COMMAND_HELP['invoice-pay'] flags + examples + notes to reflect the new shape.
  • Update the static UX guard in src/legacy/legacy-cli-ux.test.ts if any new patterns need pinning.
  • (Optional) Update manual-test-accounting-roundtrip.sh to exercise --amount in a partial-payment scenario — would catch any future regression.

Out of scope

  • --asset-index (which asset within a target) selector — currently hard-coded to 0. If multi-asset targets become real, that's its own issue.
  • Changing the SDK's PayInvoiceParams.amount units semantic — that stays as smallest-unit string (BigInt-safe). All conversion happens in the CLI.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions