From b6ba24e82d8ac046ff0ed9407bbe16436e047c8d Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Wed, 2 Oct 2024 10:05:07 -0400 Subject: [PATCH 1/3] Integration test -- Validate highest AssetPrice value appropos serialization --- packages/xrpl/src/models/common/index.ts | 2 +- .../xrpl/src/models/transactions/common.ts | 2 +- .../transactions/oracleSet.test.ts | 20 ++++++++++++++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/xrpl/src/models/common/index.ts b/packages/xrpl/src/models/common/index.ts index 98298d978f..ff4d41bea5 100644 --- a/packages/xrpl/src/models/common/index.ts +++ b/packages/xrpl/src/models/common/index.ts @@ -186,7 +186,7 @@ export interface PriceData { * The asset price after applying the Scale precision level. It's not included if the last update transaction didn't include * the BaseAsset/QuoteAsset pair. */ - AssetPrice?: number | string + AssetPrice?: bigint | number | string /** * The scaling factor to apply to an asset price. For example, if Scale is 6 and original price is 0.155, then the scaled diff --git a/packages/xrpl/src/models/transactions/common.ts b/packages/xrpl/src/models/transactions/common.ts index 5af72d038a..e4f5282c14 100644 --- a/packages/xrpl/src/models/transactions/common.ts +++ b/packages/xrpl/src/models/transactions/common.ts @@ -81,7 +81,7 @@ export function isString(str: unknown): str is string { * @returns Whether the number is properly formed. */ export function isNumber(num: unknown): num is number { - return typeof num === 'number' + return typeof num === 'number' || typeof num === 'bigint' } /** diff --git a/packages/xrpl/test/integration/transactions/oracleSet.test.ts b/packages/xrpl/test/integration/transactions/oracleSet.test.ts index 5927963d26..2dfbeefd04 100644 --- a/packages/xrpl/test/integration/transactions/oracleSet.test.ts +++ b/packages/xrpl/test/integration/transactions/oracleSet.test.ts @@ -13,6 +13,10 @@ import { testTransaction } from '../utils' // how long before each test case times out const TIMEOUT = 20000 +// Upper bound admissible value for AssetPrice field +// large numeric values necessarily have to use str type in Javascript +// number type uses double-precision floating point representation, hence represents a smaller range of values +const MAX_64_BIT_UNSIGNED_INT = '18446744073709551615' describe('OracleSet', function () { let testContext: XrplIntegrationTestContext @@ -39,6 +43,14 @@ describe('OracleSet', function () { Scale: 3, }, }, + { + PriceData: { + BaseAsset: 'XRP', + QuoteAsset: 'INR', + AssetPrice: BigInt(MAX_64_BIT_UNSIGNED_INT), + Scale: 3, + }, + }, ], Provider: stringToHex('chainlink'), URI: '6469645F6578616D706C65', @@ -62,12 +74,18 @@ describe('OracleSet', function () { assert.equal(oracle.Owner, testContext.wallet.classicAddress) assert.equal(oracle.AssetClass, tx.AssetClass) assert.equal(oracle.Provider, tx.Provider) - assert.equal(oracle.PriceDataSeries.length, 1) + assert.equal(oracle.PriceDataSeries.length, 2) assert.equal(oracle.PriceDataSeries[0].PriceData.BaseAsset, 'XRP') assert.equal(oracle.PriceDataSeries[0].PriceData.QuoteAsset, 'USD') assert.equal(oracle.PriceDataSeries[0].PriceData.AssetPrice, '2e4') assert.equal(oracle.PriceDataSeries[0].PriceData.Scale, 3) assert.equal(oracle.Flags, 0) + + // validate the serialization of large AssetPrice values + assert.equal( + oracle.PriceDataSeries[1].PriceData.AssetPrice, + 'ffffffffffffffff', + ) }, TIMEOUT, ) From cd6c99808c662adc3771ff6d4f62e051d6ac56e3 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Wed, 2 Oct 2024 21:56:46 -0400 Subject: [PATCH 2/3] update HISTORY file --- packages/xrpl/HISTORY.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/xrpl/HISTORY.md b/packages/xrpl/HISTORY.md index e6deb8457b..6f77836bd3 100644 --- a/packages/xrpl/HISTORY.md +++ b/packages/xrpl/HISTORY.md @@ -7,6 +7,9 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr ### Added * parseTransactionFlags as a utility function in the xrpl package to streamline transactions flags-to-map conversion +### Fixed +* Expanded the supported range of values in `OracleSet` transaction's `AssetPrice` field. The change enables values in range [0, 18446744073709551615], both inclusive. + ## 4.0.0 (2024-07-15) ### BREAKING CHANGES From d53dd5eb6726177d218f3da11bc9eb266b4c0085 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Thu, 3 Oct 2024 11:06:51 -0400 Subject: [PATCH 3/3] CodeRabbit suggestions: Refactor parseAmountValue function to use bigint, instead of number type. Include unit tests for comprehensive coverage of this function. --- .../models/transactions/NFTokenAcceptOffer.ts | 10 ++-- .../models/transactions/NFTokenCreateOffer.ts | 11 +++- .../xrpl/src/models/transactions/common.ts | 12 +++-- .../test/models/NFTokenCreateOffer.test.ts | 51 ++++++++++++++++++- 4 files changed, 75 insertions(+), 9 deletions(-) diff --git a/packages/xrpl/src/models/transactions/NFTokenAcceptOffer.ts b/packages/xrpl/src/models/transactions/NFTokenAcceptOffer.ts index 1a33f46e5f..4909641667 100644 --- a/packages/xrpl/src/models/transactions/NFTokenAcceptOffer.ts +++ b/packages/xrpl/src/models/transactions/NFTokenAcceptOffer.ts @@ -71,9 +71,13 @@ export interface NFTokenAcceptOfferMetadata extends TransactionMetadataBase { } function validateNFTokenBrokerFee(tx: Record): void { - const value = parseAmountValue(tx.NFTokenBrokerFee) - if (Number.isNaN(value)) { - throw new ValidationError('NFTokenAcceptOffer: invalid NFTokenBrokerFee') + let value: bigint + try { + value = parseAmountValue(tx.NFTokenBrokerFee) + } catch { + throw new ValidationError( + 'NFTokenAcceptOffer: invalid NFTokenBrokerFee. BigInt constructor could not parse NFTokenBrokerFee', + ) } if (value <= 0) { diff --git a/packages/xrpl/src/models/transactions/NFTokenCreateOffer.ts b/packages/xrpl/src/models/transactions/NFTokenCreateOffer.ts index 9575d1b6be..b7a2a46269 100644 --- a/packages/xrpl/src/models/transactions/NFTokenCreateOffer.ts +++ b/packages/xrpl/src/models/transactions/NFTokenCreateOffer.ts @@ -107,7 +107,16 @@ function validateNFTokenBuyOfferCases(tx: Record): void { ) } - if (parseAmountValue(tx.Amount) <= 0) { + let parsedAmount: bigint + try { + parsedAmount = parseAmountValue(tx.Amount) + } catch { + throw new ValidationError( + 'NFTokenCreateOffer: Invalid Amount, Amount field could not be parsed by BigInt constructor', + ) + } + + if (parsedAmount <= 0) { throw new ValidationError( 'NFTokenCreateOffer: Amount must be greater than 0 for buy offers', ) diff --git a/packages/xrpl/src/models/transactions/common.ts b/packages/xrpl/src/models/transactions/common.ts index e4f5282c14..eb436317ca 100644 --- a/packages/xrpl/src/models/transactions/common.ts +++ b/packages/xrpl/src/models/transactions/common.ts @@ -356,13 +356,17 @@ export function validateBaseTransaction(common: Record): void { * * @param amount - An Amount to parse for its value. * @returns The parsed amount value, or NaN if the amount count not be parsed. + * @throws ValidationError, if the input Amount is invalid + * @throws SyntaxError, if Amount cannot be parsed by BigInt constructor */ -export function parseAmountValue(amount: unknown): number { +export function parseAmountValue(amount: unknown): bigint { if (!isAmount(amount)) { - return NaN + throw new ValidationError( + 'parseAmountValue: Specified input Amount is invalid', + ) } if (typeof amount === 'string') { - return parseFloat(amount) + return BigInt(amount) } - return parseFloat(amount.value) + return BigInt(amount.value) } diff --git a/packages/xrpl/test/models/NFTokenCreateOffer.test.ts b/packages/xrpl/test/models/NFTokenCreateOffer.test.ts index 6260861608..081b22dc66 100644 --- a/packages/xrpl/test/models/NFTokenCreateOffer.test.ts +++ b/packages/xrpl/test/models/NFTokenCreateOffer.test.ts @@ -1,10 +1,59 @@ import { assert } from 'chai' -import { validate, ValidationError, NFTokenCreateOfferFlags } from '../../src' +import { + validate, + ValidationError, + NFTokenCreateOfferFlags, + IssuedCurrencyAmount, +} from '../../src' +import { parseAmountValue } from '../../src/models/transactions/common' const NFTOKEN_ID = '00090032B5F762798A53D543A014CAF8B297CFF8F2F937E844B17C9E00000003' +describe('parseAmountValue', function () { + it(`validate large amount values`, function () { + // (Upper bound of created XRP tokens) minus 12 drops + assert.equal( + parseAmountValue('99999999999999988'), + BigInt('99999999999999988'), + ) + + // Token Amounts or Issued Currencies are represented using 54 bits of precision in the XRP Ledger + // Docs: https://xrpl.org/docs/references/protocol/binary-format#token-amount-format + const highest_iou_amount: IssuedCurrencyAmount = { + currency: 'ABC', + issuer: 'rIssuerAddress', + // 54 bits can be used to safely represent a value of (2**54 - 1) + value: '18014398509481983', + } + + assert.equal( + parseAmountValue(highest_iou_amount), + BigInt('18014398509481983'), + ) + }) + + it(`validate non-positive amount values`, function () { + assert.equal(parseAmountValue('0'), BigInt(0)) + assert.equal(parseAmountValue('-1234'), BigInt(-1234)) + }) + + it(`validate invalid amount values`, function () { + assert.throws( + () => parseAmountValue(1234), + ValidationError, + 'parseAmountValue: Specified input Amount is invalid', + ) + + assert.throws( + () => parseAmountValue('abcd'), + SyntaxError, + 'Cannot convert abcd to a BigInt', + ) + }) +}) + /** * NFTokenCreateOffer Transaction Verification Testing. *