fix(sdk-coin-xrp): partial-payment verify and explain fallthrough (CSHLD-1452) - #9478

Summary

Fixes 4 SDK-side findings from CR-1406 §5.5 in modules/sdk-coin-xrp. These are the SDK-layer sibling of CSHLD-763 (XRP Transaction Type Enforcement — Whitelist/Blacklist Audit).

Linear: https://linear.app/bitgo/issue/CSHLD-1452/sdk-xrp-fix-partial-payment-cross-currency-verify-explaintransaction

Why these fixes are needed (the indexer is balance-safe, but these guard other paths)

The XRP indexer computes balance entries from meta.AffectedNodes deltas, never from the Amount field — so balance correctness is already handled and no balance fix is needed. However, the SDK fixes below guard concerns the indexer cannot cover (the indexer only sees the confirmed on-chain result, after the fact):

verifyTransaction is a security check that runs before signing/broadcasting to catch a tampered or malicious prebuild; the indexer computing the correct balance after confirmation doesn't undo a wrong tx already signed. explainTransaction/recover() build the RecoveryInfo returned to the user; the indexer is not in that path.

Findings fixed

F1 — Partial payment (tfPartialPayment)

tfPartialPayment (0x00020000) allows an XRPL Payment to deliver less than Amount and still succeed; the actual delivered value is in meta.delivered_amount, not the signed blob. Previously Amount was read directly and reported as delivered.

• explainTransaction (coin-level) and explainPaymentTransaction (lib) now detect tfPartialPayment. When metadata is supplied, outputAmount/outputs[].amount use meta.delivered_amount; otherwise partialPayment: true is surfaced so consumers know the value is requested, not settled.
• verifyTransaction rejects tfPartialPayment prebuilds outright (BitGo never builds them — defense in depth).
• New TF_PARTIAL_PAYMENT = 0x00020000 constant, with a comment distinguishing it from the numerically-identical REQUIRE_DESTINATION_TAG_FLAG (same bit, different flag space — Payment tx flag vs AccountRoot ledger flag).
• New optional meta field on ExplainTransactionOptions; new PaymentTransactionExplanation type in the TransactionExplanation union.

F2 — Cross-currency / ICA payment verification

verifyTransaction skipped the amount comparison entirely whenever output.amount was an object (every cross-currency/token transfer went unverified).

• Removed the typeof output.amount !== 'object' skip.
• Added toBaseUnits() using getBaseFactor() (overridden by XrpToken to use the token's decimals, not the base coin's) so display-unit Amount.value is compared against base-unit recipient intent for both base XRP and tokens.
• Updated 2 existing token-transfer tests that relied on the skip (see "Updated existing tests" below).

F3 — explainTransaction() fallthrough (live AccountDelete bug)

The coin-level explainTransaction (xrp.ts:212) had branches only for AccountSet/TrustSet/MPTokenAuthorize and silently fell through to a Payment-shaped return for any other type — including AccountDelete (a supported enum member with no Amount field), which recover() invokes. Result: outputAmount: undefined, outputs[0].amount: undefined.

• Added explicit AccountDelete and SignerListSet branches + a default: throw, mirroring the safe switch in lib/transaction.ts:196-211.
• AccountDelete returns a 0-value placeholder (full sweep, exact amount unknown at build time), matching lib/transaction.ts.

F4 — AMM transaction types

XrpTransactionType enum has no AMM members; fromRawTransaction() correctly rejects unrecognized types today, but there's no structured handling for future AMM support.

• Added a stability doc-comment: string values are public API compared by downstream consumers, so AMM members must be added with exact XRPL names alongside switch-statement updates — never change string values.

Test results

127 passing, 27 failing

The 27 failures are pre-existing (identical on master): MPT/xrpl dependency issues (isMPTAmount is not a function, Invalid field TransactionType: MPTokenAuthorize) — none touch the changed code paths. The +8 passing vs master's 119 = my 8 new tests, all green:

• AccountDelete explain (no fallthrough)
• SignerListSet explain (no fallthrough)
• SetRegularKey explain → rejected as unsupported
• Partial payment flagged without metadata
• Partial payment uses delivered_amount from metadata (XRP)
• Partial payment uses issued-currency delivered_amount from metadata
• Cross-currency mismatch verify → rejected
• Partial-payment prebuild verify → rejected

Updated existing tests (2 token-transfer verify tests)

Two existing tests in test/unit/xrp.ts (should verify token transfers and should verify token transfers with recipient has dt) had their recipient amount changed from 1e82 (10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000) to 1e13 (10000000000000). This was required by the F2 fix, not a change in test intent:

• Before the fix, verifyTransaction skipped the amount comparison entirely whenever output.amount was an object (the IOU {value, currency, issuer} shape). So the recipient amount was never actually compared against the tx Amount — the 1e82 value was nonsense that passed only because the check was skipped. That's the very bug F2 fixes.
• After the fix, the comparison runs. toBaseUnits() converts the tx Amount.value from display units to base units using getBaseFactor(). The test's token is txrp:rlusd (15 decimal places), and the tx Amount.value is "0.01", so the correct base-unit recipient amount is 0.01 × 10^15 = 1e13 = 10000000000000.
• A new negative test (should fail to verify a token transfer when the amount does not match (object Amount)) uses 2e13 with the same 0.01 tx and confirms it is now rejected — proving the comparison actually runs and a mismatch is caught.

Files changed

Out of scope

• Indexer-layer txType registry/whitelist — that's CSHLD-763 itself, and balance correctness (already handled by the indexer via AffectedNodes).
• P2 reserve-requirement balance display.
• Actually adding AMM transaction support (no BitGo usage today).

🤖 Generated with Claude Code