From 2636d6c7d8557d0ae57b994ad8b894ddd8913347 Mon Sep 17 00:00:00 2001 From: satyakwok Date: Tue, 12 May 2026 01:34:00 +0200 Subject: [PATCH 1/2] fix(xrpl): use deep amount comparison in handleDeliverMax (#3313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `handleDeliverMax` used `tx.Amount !== tx.DeliverMax` (reference equality). For IOU and MPT amounts (objects like `{currency, issuer, value}`), two distinct objects with identical fields always compare not-equal, so every IOU payment that set both `Amount` and `DeliverMax` as separate object literals threw a spurious `PaymentTransaction: Amount and DeliverMax fields must be identical when both are provided` validation error. Reuse the existing `amountsEqual` helper from `client/partialPayment.ts` (handles XRP / IOU / MPT and uses `BigNumber` for value comparison so `"1"` and `"1.0"` are treated as equal). Exported it for cross-module use; same callable, no behaviour change inside `partialPayment.ts`. Adds two regression tests in `test/client/autofill.test.ts`: - identical IOU `Amount` and `DeliverMax` as separate objects → must autofill cleanly (this is the bug; fails on `main`). - differing IOU `value` with same currency/issuer → must still throw (companion check that the fix didn't loosen validation). All 23 autofill unit tests pass. --- packages/xrpl/src/client/partialPayment.ts | 11 +++++++++- packages/xrpl/src/sugar/autofill.ts | 7 +++++- packages/xrpl/test/client/autofill.test.ts | 25 ++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/xrpl/src/client/partialPayment.ts b/packages/xrpl/src/client/partialPayment.ts index e039f76584..f46c1315eb 100644 --- a/packages/xrpl/src/client/partialPayment.ts +++ b/packages/xrpl/src/client/partialPayment.ts @@ -28,7 +28,16 @@ const WARN_PARTIAL_PAYMENT_CODE = 2001 /* eslint-disable complexity -- check different token types */ /* eslint-disable @typescript-eslint/consistent-type-assertions -- known currency type */ -function amountsEqual( +/** + * Returns true when two transaction amounts represent the same on-ledger value. + * + * Handles all three amount shapes (XRP string, IOU object, MPT object) and uses + * `BigNumber` for the numeric `value` field so `"1.0"` and `"1"` compare equal. + * Exported so callers outside this module (notably `handleDeliverMax` in + * `sugar/autofill.ts`) can avoid reference-equality bugs on IOU/MPT objects — + * see issue #3313. + */ +export function amountsEqual( amt1: Amount | MPTAmount, amt2: Amount | MPTAmount, ): boolean { diff --git a/packages/xrpl/src/sugar/autofill.ts b/packages/xrpl/src/sugar/autofill.ts index bb7809ac98..e19747058e 100644 --- a/packages/xrpl/src/sugar/autofill.ts +++ b/packages/xrpl/src/sugar/autofill.ts @@ -4,6 +4,7 @@ import BigNumber from 'bignumber.js' import { xAddressToClassicAddress, isValidXAddress } from 'ripple-address-codec' import { type Client } from '..' +import { amountsEqual } from '../client/partialPayment' import { ValidationError, XrplError } from '../errors' import { LoanBroker } from '../models/ledger' import { @@ -478,8 +479,12 @@ export function handleDeliverMax(tx: Payment): void { // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition, no-param-reassign -- needed here tx.Amount ??= tx.DeliverMax + // Use `amountsEqual` rather than `!==` so two distinct objects with identical + // currency/issuer/value fields don't trigger a spurious validation error. The previous + // reference-equality check rejected every IOU payment whose caller built `Amount` and + // `DeliverMax` as separate object literals (issue #3313). // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- needed here - if (tx.Amount != null && tx.Amount !== tx.DeliverMax) { + if (tx.Amount != null && !amountsEqual(tx.Amount, tx.DeliverMax)) { throw new ValidationError( 'PaymentTransaction: Amount and DeliverMax fields must be identical when both are provided', ) diff --git a/packages/xrpl/test/client/autofill.test.ts b/packages/xrpl/test/client/autofill.test.ts index b68b2ddd08..355bb41158 100644 --- a/packages/xrpl/test/client/autofill.test.ts +++ b/packages/xrpl/test/client/autofill.test.ts @@ -105,6 +105,31 @@ describe('client.autofill', function () { await assertRejects(testContext.client.autofill(paymentTx), ValidationError) }) + it('Validate Payment transaction API v2: Payment Transaction: identical IOU DeliverMax and Amount as separate objects (#3313)', async function () { + // Regression test: the previous `tx.Amount !== tx.DeliverMax` used reference equality, + // which always returned false for two distinct IssuedCurrencyAmount object literals + // even when their fields were identical — every IOU payment that set both fields was + // rejected. + const issuer = 'r9vbV3EHvXWjSkeQ6CAcYVPGeq7TuiXY2X' + paymentTx.Amount = { currency: 'USD', issuer, value: '100' } + paymentTx.DeliverMax = { currency: 'USD', issuer, value: '100' } + + const txResult = await testContext.client.autofill(paymentTx) + + assert.deepEqual(txResult.Amount, { currency: 'USD', issuer, value: '100' }) + assert.strictEqual('DeliverMax' in txResult, false) + }) + + it('Validate Payment transaction API v2: Payment Transaction: differing IOU DeliverMax and Amount values still rejected', async function () { + // Companion to the case above: ensure the fix didn't loosen validation. Identical + // currency/issuer but different `value` must still throw. + const issuer = 'r9vbV3EHvXWjSkeQ6CAcYVPGeq7TuiXY2X' + paymentTx.Amount = { currency: 'USD', issuer, value: '100' } + paymentTx.DeliverMax = { currency: 'USD', issuer, value: '200' } + + await assertRejects(testContext.client.autofill(paymentTx), ValidationError) + }) + it('should not autofill if fields are present', async function () { const tx: Transaction = { TransactionType: 'DepositPreauth', From d1519c0f94ca6381de0fefcf018601de0dd4db51 Mon Sep 17 00:00:00 2001 From: satyakwok Date: Tue, 12 May 2026 03:03:53 +0200 Subject: [PATCH 2/2] test(autofill): correct comment wording on #3313 regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit caught it: the previous comment said `!==` returned `false` for distinct object literals; it actually returns `true` (reference inequality), which is precisely why the validation throw fired. No code change — comment-only correction. --- packages/xrpl/test/client/autofill.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/xrpl/test/client/autofill.test.ts b/packages/xrpl/test/client/autofill.test.ts index 355bb41158..edc9f45477 100644 --- a/packages/xrpl/test/client/autofill.test.ts +++ b/packages/xrpl/test/client/autofill.test.ts @@ -107,9 +107,9 @@ describe('client.autofill', function () { it('Validate Payment transaction API v2: Payment Transaction: identical IOU DeliverMax and Amount as separate objects (#3313)', async function () { // Regression test: the previous `tx.Amount !== tx.DeliverMax` used reference equality, - // which always returned false for two distinct IssuedCurrencyAmount object literals - // even when their fields were identical — every IOU payment that set both fields was - // rejected. + // which always returned true for two distinct IssuedCurrencyAmount object literals + // (different references, even with identical fields) — every IOU payment that set both + // fields as separate objects therefore tripped the validation throw. const issuer = 'r9vbV3EHvXWjSkeQ6CAcYVPGeq7TuiXY2X' paymentTx.Amount = { currency: 'USD', issuer, value: '100' } paymentTx.DeliverMax = { currency: 'USD', issuer, value: '100' }