From e7d60012ec56f27ecce4012335eeb6443c380b87 Mon Sep 17 00:00:00 2001 From: Jordi Parra Crespo Date: Thu, 19 Dec 2024 11:57:36 +0100 Subject: [PATCH 1/3] fix(xrpl): autofill not validating correctly DeliverMax and Amount when both passed as objects --- packages/xrpl/HISTORY.md | 2 ++ packages/xrpl/src/client/index.ts | 3 +- .../xrpl/src/models/transactions/common.ts | 32 +++++++++++++++++ packages/xrpl/test/client/autofill.test.ts | 36 +++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/xrpl/HISTORY.md b/packages/xrpl/HISTORY.md index e1854eeebe..cc682c1f17 100644 --- a/packages/xrpl/HISTORY.md +++ b/packages/xrpl/HISTORY.md @@ -8,11 +8,13 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr * parseTransactionFlags as a utility function in the xrpl package to streamline transactions flags-to-map conversion * Added new MPT transaction definitions (XLS-33) * New `MPTAmount` type support for `Payment` and `Clawback` transactions +* New util `areAmountsEqual` to check if 2 amounts are strictly equal ### Fixed * `TransactionStream` model supports APIv2 * `TransactionStream` model includes `close_time_iso` field * `Ledger` model includes `close_time_iso` field +* `autofill` function in client not validating amounts correctly ## 4.0.0 (2024-07-15) diff --git a/packages/xrpl/src/client/index.ts b/packages/xrpl/src/client/index.ts index 722b169f2a..5b18dd53e4 100644 --- a/packages/xrpl/src/client/index.ts +++ b/packages/xrpl/src/client/index.ts @@ -47,6 +47,7 @@ import type { OnEventToListenerMap, } from '../models/methods/subscribe' import type { SubmittableTransaction } from '../models/transactions' +import { areAmountsEqual } from '../models/transactions/common' import { setTransactionFlagsToNumber } from '../models/utils/flags' import { ensureClassicAddress, @@ -699,7 +700,7 @@ class Client extends EventEmitter { // eslint-disable-next-line @typescript-eslint/ban-ts-comment -- ignore type-assertions on the DeliverMax property // @ts-expect-error -- DeliverMax property exists only at the RPC level, not at the protocol level // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- This is a valid null check for Amount - if (tx.Amount != null && tx.Amount !== tx.DeliverMax) { + if (tx.Amount != null && !areAmountsEqual(tx.Amount, tx.DeliverMax)) { return Promise.reject( new ValidationError( 'PaymentTransaction: Amount and DeliverMax fields must be identical when both are provided', diff --git a/packages/xrpl/src/models/transactions/common.ts b/packages/xrpl/src/models/transactions/common.ts index eb5b56fa1a..cd75368a94 100644 --- a/packages/xrpl/src/models/transactions/common.ts +++ b/packages/xrpl/src/models/transactions/common.ts @@ -1,3 +1,4 @@ +import BigNumber from 'bignumber.js' import { isValidClassicAddress, isValidXAddress } from 'ripple-address-codec' import { TRANSACTION_TYPES } from 'ripple-binary-codec' @@ -168,6 +169,37 @@ export function isAmount(amount: unknown): amount is Amount { ) } +/** + * Check if two amounts are equal. + * + * @param amount1 - The first amount to compare. + * @param amount2 - The second amount to compare. + * @returns Whether the two amounts are equal. + * @throws When the amounts are not valid. + */ +export function areAmountsEqual(amount1: unknown, amount2: unknown): boolean { + const isAmount1Invalid = !isAmount(amount1) + if (isAmount1Invalid || !isAmount(amount2)) { + throw new ValidationError( + `Amount: invalid field. Expected Amount but received ${JSON.stringify( + isAmount1Invalid ? amount1 : amount2, + )}`, + ) + } + + if (isString(amount1) && isString(amount2)) { + return new BigNumber(amount1).eq(amount2) + } + + if (isRecord(amount1) && isRecord(amount2)) { + return Object.entries(amount1).every( + ([key, value]) => amount2[key] === value, + ) + } + + return false +} + /** * Verify the form and type of an XChainBridge at runtime. * diff --git a/packages/xrpl/test/client/autofill.test.ts b/packages/xrpl/test/client/autofill.test.ts index 8c2d9b5ec8..f8378d1854 100644 --- a/packages/xrpl/test/client/autofill.test.ts +++ b/packages/xrpl/test/client/autofill.test.ts @@ -6,6 +6,7 @@ import { EscrowFinish, Payment, Transaction, + IssuedCurrencyAmount, } from '../../src' import { ValidationError } from '../../src/errors' import rippled from '../fixtures/rippled' @@ -98,6 +99,25 @@ describe('client.autofill', function () { assert.strictEqual('DeliverMax' in txResult, false) }) + it('Validate Payment transaction API v2: Payment Transaction: differing DeliverMax and Amount fields using amount objects', async function () { + // @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions + paymentTx.DeliverMax = { + currency: 'USD', + value: AMOUNT, + issuer: 'r9vbV3EHvXWjSkeQ6CAcYVPGeq7TuiXY2X', + } + paymentTx.Amount = { + currency: 'USD', + value: AMOUNT, + issuer: 'r9vbV3EHvXWjSkeQ6CAcYVPGeq7TuiXY2X', + } + + const txResult = await testContext.client.autofill(paymentTx) + + assert.strictEqual((txResult.Amount as IssuedCurrencyAmount).value, AMOUNT) + assert.strictEqual('DeliverMax' in txResult, false) + }) + it('Validate Payment transaction API v2: Payment Transaction: differing DeliverMax and Amount fields', async function () { // @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions paymentTx.DeliverMax = '6789' @@ -106,6 +126,22 @@ describe('client.autofill', function () { await assertRejects(testContext.client.autofill(paymentTx), ValidationError) }) + it('Validate Payment transaction API v2: Payment Transaction: differing DeliverMax and Amount fields using objects', async function () { + // @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions + paymentTx.DeliverMax = { + currency: 'USD', + value: '31415', + issuer: 'r9vbV3EHvXWjSkeQ6CAcYVPGeq7TuiXY2X', + } + paymentTx.Amount = { + currency: 'USD', + value: '27182', + issuer: 'r9vbV3EHvXWjSkeQ6CAcYVPGeq7TuiXY2X', + } + + await assertRejects(testContext.client.autofill(paymentTx), ValidationError) + }) + it('should not autofill if fields are present', async function () { const tx: Transaction = { TransactionType: 'DepositPreauth', From ef8efd72b45050098c272959ca1bac6fa4a23481 Mon Sep 17 00:00:00 2001 From: Jordi Parra Crespo Date: Wed, 8 Jan 2025 09:49:55 +0100 Subject: [PATCH 2/3] fix(xrpl.js): improve areAmountsEqual validator --- packages/xrpl/src/models/transactions/common.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/xrpl/src/models/transactions/common.ts b/packages/xrpl/src/models/transactions/common.ts index da1328a6b3..4cebfd0cdb 100644 --- a/packages/xrpl/src/models/transactions/common.ts +++ b/packages/xrpl/src/models/transactions/common.ts @@ -214,9 +214,18 @@ export function areAmountsEqual(amount1: unknown, amount2: unknown): boolean { return new BigNumber(amount1).eq(amount2) } - if (isRecord(amount1) && isRecord(amount2)) { - return Object.entries(amount1).every( - ([key, value]) => amount2[key] === value, + if (isIssuedCurrency(amount1) && isIssuedCurrency(amount2)) { + return ( + amount1.currency === amount2.currency && + amount1.issuer === amount2.issuer && + new BigNumber(amount1.value).eq(amount2.value) + ) + } + + if (isMPTAmount(amount1) && isMPTAmount(amount2)) { + return ( + amount1.mpt_issuance_id === amount2.mpt_issuance_id && + new BigNumber(amount1.value).eq(amount2.value) ) } From 3bfec9dac14bd5ea31230dc6d179595e70e3750c Mon Sep 17 00:00:00 2001 From: Jordi Parra Crespo Date: Wed, 8 Jan 2025 09:57:33 +0100 Subject: [PATCH 3/3] test(xrpl): fix description in autofill test --- packages/xrpl/test/client/autofill.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/xrpl/test/client/autofill.test.ts b/packages/xrpl/test/client/autofill.test.ts index f8378d1854..2e154d26c3 100644 --- a/packages/xrpl/test/client/autofill.test.ts +++ b/packages/xrpl/test/client/autofill.test.ts @@ -99,7 +99,7 @@ describe('client.autofill', function () { assert.strictEqual('DeliverMax' in txResult, false) }) - it('Validate Payment transaction API v2: Payment Transaction: differing DeliverMax and Amount fields using amount objects', async function () { + it('Validate Payment transaction API v2: Payment Transaction: identical DeliverMax and Amount fields using amount objects', async function () { // @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions paymentTx.DeliverMax = { currency: 'USD',