diff --git a/packages/xrpl/HISTORY.md b/packages/xrpl/HISTORY.md index f2b79844b0..0d386a3fd7 100644 --- a/packages/xrpl/HISTORY.md +++ b/packages/xrpl/HISTORY.md @@ -7,6 +7,7 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr ### BREAKING CHANGES: * `ED25519` is the default signing-algorithm used in the `Wallet.fromMnemonic` method. Users can explicitly specify `ecdsa-secp256k1` to retrieve the cryptographic material created using older versions of this package. * `Client.getServerInfo()` and `Client.connect()` now throw if the `server_info` request fails, or if the response succeeds but does not include a `network_id`. Previously, these failures were swallowed and only logged via `console.error`, leaving `client.networkID` undefined and causing `autofill()` to omit the `NetworkID` field — producing transactions valid on the wrong network. Servers running rippled <1.11 (which do not return `network_id`) will now fail to connect; upgrade to rippled 1.11+ or set `client.networkID` manually after construction. +* `dropsToXrp()` now returns a `BigNumber` instead of a JavaScript `number`, and `Client.getXrpBalance()` returns `Promise` instead of `Promise`. The previous implementation called `.toNumber()`, which silently dropped precision for amounts approaching the XRP supply (~10^17 drops): `xrpToDrops(dropsToXrp('9999999999999999'))` returned `'9999999999999998'`, losing one drop on the round-trip. Callers can call `.toString()` for a decimal string or `.toNumber()` for a JS number (only safe for amounts well below the supply). ([#3316](https://github.com/XRPLF/xrpl.js/issues/3316)) ### Added * Add new fields to `ServerDefinitionsResponse`: `ACCOUNT_SET_FLAGS`, `LEDGER_ENTRY_FLAGS`, `LEDGER_ENTRY_FORMATS`, `TRANSACTION_FLAGS`, and `TRANSACTION_FORMATS`, reflecting new sections returned by `server_definitions` in rippled. diff --git a/packages/xrpl/src/Wallet/fundWallet.ts b/packages/xrpl/src/Wallet/fundWallet.ts index 174034aa5f..b9dfe0f05c 100644 --- a/packages/xrpl/src/Wallet/fundWallet.ts +++ b/packages/xrpl/src/Wallet/fundWallet.ts @@ -95,7 +95,7 @@ export async function getStartingBalance( ): Promise { let startingBalance = 0 try { - startingBalance = Number(await client.getXrpBalance(classicAddress)) + startingBalance = (await client.getXrpBalance(classicAddress)).toNumber() } catch { // startingBalance remains '0' } @@ -264,7 +264,7 @@ async function getUpdatedBalance( try { let newBalance try { - newBalance = Number(await client.getXrpBalance(address)) + newBalance = (await client.getXrpBalance(address)).toNumber() } catch { /* newBalance remains undefined */ } diff --git a/packages/xrpl/src/client/index.ts b/packages/xrpl/src/client/index.ts index 420d60058f..387927051c 100644 --- a/packages/xrpl/src/client/index.ts +++ b/packages/xrpl/src/client/index.ts @@ -1,6 +1,7 @@ /* eslint-disable jsdoc/require-jsdoc -- Request has many aliases, but they don't need unique docs */ /* eslint-disable max-lines -- Client is a large file w/ lots of imports/exports */ +import BigNumber from 'bignumber.js' import { EventEmitter } from 'eventemitter3' import { @@ -936,7 +937,9 @@ class Client extends EventEmitter { * @param [options] - Additional options for fetching the balance (optional). * @param [options.ledger_hash] - The hash of the ledger to retrieve the balance from (optional). * @param [options.ledger_index] - The index of the ledger to retrieve the balance from (optional). - * @returns A promise that resolves with the XRP balance as a number. + * @returns A promise that resolves with the XRP balance as a `BigNumber`. + * Call `.toString()` for a decimal string or `.toNumber()` to convert to a + * JavaScript number (may lose precision for very large amounts). */ public async getXrpBalance( address: string, @@ -944,7 +947,7 @@ class Client extends EventEmitter { ledger_hash?: string ledger_index?: LedgerIndex } = {}, - ): Promise { + ): Promise { const xrpRequest: AccountInfoRequest = { command: 'account_info', account: address, @@ -1018,7 +1021,7 @@ class Client extends EventEmitter { const balances: Balance[] = [] // get XRP balance - let xrpPromise: Promise = Promise.resolve(0) + let xrpPromise: Promise = Promise.resolve(new BigNumber(0)) if (!options.peer) { xrpPromise = this.getXrpBalance(address, { ledger_hash: options.ledger_hash, @@ -1043,7 +1046,7 @@ class Client extends EventEmitter { const accountLinesBalance = linesResponses.flatMap((response) => formatBalances(response.result.lines), ) - if (xrpBalance !== 0) { + if (!xrpBalance.isZero()) { balances.push({ currency: 'XRP', value: xrpBalance.toString() }) } balances.push(...accountLinesBalance) @@ -1250,9 +1253,9 @@ class Client extends EventEmitter { let startingBalance = 0 if (existingWallet) { try { - startingBalance = Number( - await this.getXrpBalance(walletToFund.classicAddress), - ) + startingBalance = ( + await this.getXrpBalance(walletToFund.classicAddress) + ).toNumber() } catch { /* startingBalance remains what it was previously */ } diff --git a/packages/xrpl/src/utils/xrpConversion.ts b/packages/xrpl/src/utils/xrpConversion.ts index ffe25eeca2..cfaeaa06f7 100644 --- a/packages/xrpl/src/utils/xrpConversion.ts +++ b/packages/xrpl/src/utils/xrpConversion.ts @@ -10,12 +10,21 @@ const SANITY_CHECK = /^-?[0-9.]+$/u /** * Convert Drops to XRP. * + * Returns a `BigNumber` (rather than a JavaScript `number`) so that the full + * precision of the drops value is preserved across the conversion. For + * amounts approaching the XRP supply (~10^17 drops), an IEEE-754 double + * cannot represent every drop exactly, which silently lost up to one drop + * on each `xrpToDrops(dropsToXrp(value))` round-trip. See xrpl.js issue + * #3316. + * * @param dropsToConvert - Drops to convert to XRP. This can be a string, number, or BigNumber. - * @returns Amount in XRP. + * @returns Amount in XRP, as a `BigNumber`. Call `.toString()` for a decimal + * string or `.toNumber()` to convert to a JavaScript number (may lose + * precision for very large amounts). * @throws When drops amount is invalid. * @category Utilities */ -export function dropsToXrp(dropsToConvert: BigNumber.Value): number { +export function dropsToXrp(dropsToConvert: BigNumber.Value): BigNumber { /* * Converting to BigNumber and then back to string should remove any * decimal point followed by zeros, e.g. '1.00'. @@ -50,7 +59,13 @@ export function dropsToXrp(dropsToConvert: BigNumber.Value): number { ) } - return new BigNumber(drops).dividedBy(DROPS_PER_XRP).toNumber() + /* + * Return a `BigNumber` rather than calling `.toNumber()` so that the + * result preserves the full precision of the input. Drops are at most 6 + * decimal places of XRP, so the division terminates exactly within + * BigNumber's default precision. + */ + return new BigNumber(drops).dividedBy(DROPS_PER_XRP) } /** diff --git a/packages/xrpl/test/client/getXrpBalance.test.ts b/packages/xrpl/test/client/getXrpBalance.test.ts index 783eaa45d9..ac17b0f210 100644 --- a/packages/xrpl/test/client/getXrpBalance.test.ts +++ b/packages/xrpl/test/client/getXrpBalance.test.ts @@ -30,7 +30,7 @@ describe('client.getXrpBalance', function () { ) testContext.mockRippled!.addResponse('ledger', rippled.ledger.normal) const result = await testContext.client.getXrpBalance(testcase.address) - assert.equal(result, 922.913243) + assert.isTrue(result.isEqualTo('922.913243')) }) }) }) diff --git a/packages/xrpl/test/utils/dropsToXrp.test.ts b/packages/xrpl/test/utils/dropsToXrp.test.ts index 180512ce7d..8b7c2eab90 100644 --- a/packages/xrpl/test/utils/dropsToXrp.test.ts +++ b/packages/xrpl/test/utils/dropsToXrp.test.ts @@ -1,77 +1,84 @@ import BigNumber from 'bignumber.js' import { assert } from 'chai' -import { dropsToXrp } from '../../src/utils' +import { dropsToXrp, xrpToDrops } from '../../src/utils' describe('dropsToXrp', function () { it('works with a typical amount', function () { const xrp = dropsToXrp('2000000') - assert.strictEqual(xrp, 2, '2 million drops equals 2 XRP') + assert.isTrue(xrp.isEqualTo('2'), '2 million drops equals 2 XRP') }) it('works with fractions', function () { let xrp = dropsToXrp('3456789') - assert.strictEqual(xrp, 3.456789, '3,456,789 drops equals 3.456789 XRP') + assert.isTrue( + xrp.isEqualTo('3.456789'), + '3,456,789 drops equals 3.456789 XRP', + ) xrp = dropsToXrp('3400000') - assert.strictEqual(xrp, 3.4, '3,400,000 drops equals 3.4 XRP') + assert.isTrue(xrp.isEqualTo('3.4'), '3,400,000 drops equals 3.4 XRP') xrp = dropsToXrp('1') - assert.strictEqual(xrp, 0.000001, '1 drop equals 0.000001 XRP') + assert.isTrue(xrp.isEqualTo('0.000001'), '1 drop equals 0.000001 XRP') xrp = dropsToXrp('1.0') - assert.strictEqual(xrp, 0.000001, '1.0 drops equals 0.000001 XRP') + assert.isTrue(xrp.isEqualTo('0.000001'), '1.0 drops equals 0.000001 XRP') xrp = dropsToXrp('1.00') - assert.strictEqual(xrp, 0.000001, '1.00 drops equals 0.000001 XRP') + assert.isTrue(xrp.isEqualTo('0.000001'), '1.00 drops equals 0.000001 XRP') }) it('works with zero', function () { let xrp = dropsToXrp('0') - assert.strictEqual(xrp, 0, '0 drops equals 0 XRP') + assert.isTrue(xrp.isZero(), '0 drops equals 0 XRP') // negative zero is equivalent to zero xrp = dropsToXrp('-0') - assert.strictEqual(xrp, 0, '-0 drops equals 0 XRP') + assert.isTrue(xrp.isZero(), '-0 drops equals 0 XRP') xrp = dropsToXrp('0.00') - assert.strictEqual(xrp, 0, '0.00 drops equals 0 XRP') + assert.isTrue(xrp.isZero(), '0.00 drops equals 0 XRP') xrp = dropsToXrp('000000000') - assert.strictEqual(xrp, 0, '000000000 drops equals 0 XRP') + assert.isTrue(xrp.isZero(), '000000000 drops equals 0 XRP') }) it('works with a negative value', function () { const xrp = dropsToXrp('-2000000') - assert.strictEqual(xrp, -2, '-2 million drops equals -2 XRP') + assert.isTrue(xrp.isEqualTo('-2'), '-2 million drops equals -2 XRP') }) it('works with a value ending with a decimal point', function () { let xrp = dropsToXrp('2000000.') - assert.strictEqual(xrp, 2, '2000000. drops equals 2 XRP') + assert.isTrue(xrp.isEqualTo('2'), '2000000. drops equals 2 XRP') xrp = dropsToXrp('-2000000.') - assert.strictEqual(xrp, -2, '-2000000. drops equals -2 XRP') + assert.isTrue(xrp.isEqualTo('-2'), '-2000000. drops equals -2 XRP') }) it('works with BigNumber objects', function () { let xrp = dropsToXrp(new BigNumber(2000000)) - assert.strictEqual(xrp, 2, '(BigNumber) 2 million drops equals 2 XRP') + assert.isTrue( + xrp.isEqualTo('2'), + '(BigNumber) 2 million drops equals 2 XRP', + ) xrp = dropsToXrp(new BigNumber(-2000000)) - assert.strictEqual(xrp, -2, '(BigNumber) -2 million drops equals -2 XRP') + assert.isTrue( + xrp.isEqualTo('-2'), + '(BigNumber) -2 million drops equals -2 XRP', + ) xrp = dropsToXrp(new BigNumber(2345678)) - assert.strictEqual( - xrp, - 2.345678, + assert.isTrue( + xrp.isEqualTo('2.345678'), '(BigNumber) 2,345,678 drops equals 2.345678 XRP', ) xrp = dropsToXrp(new BigNumber(-2345678)) - assert.strictEqual( - xrp, - -2.345678, + assert.isTrue( + xrp.isEqualTo('-2.345678'), '(BigNumber) -2,345,678 drops equals -2.345678 XRP', ) }) @@ -79,16 +86,18 @@ describe('dropsToXrp', function () { it('works with a number', function () { // This is not recommended. Use strings or BigNumber objects to avoid precision errors. let xrp = dropsToXrp(2000000) - assert.strictEqual(xrp, 2, '(number) 2 million drops equals 2 XRP') + assert.isTrue(xrp.isEqualTo('2'), '(number) 2 million drops equals 2 XRP') xrp = dropsToXrp(-2000000) - assert.strictEqual(xrp, -2, '(number) -2 million drops equals -2 XRP') + assert.isTrue( + xrp.isEqualTo('-2'), + '(number) -2 million drops equals -2 XRP', + ) }) it('works with scientific notation', function () { const xrp = dropsToXrp('1e6') - assert.strictEqual( - xrp, - 1, + assert.isTrue( + xrp.isEqualTo('1'), '(scientific notation string) 1e6 drops equals 1 XRP', ) }) @@ -130,4 +139,19 @@ describe('dropsToXrp', function () { dropsToXrp('...') }, /dropsToXrp: invalid value '\.\.\.'/u) }) + + // Regression test for xrpl.js#3316: dropsToXrp previously returned a JS + // number, which silently lost precision for amounts approaching the XRP + // supply (~10^17 drops). The round-trip xrpToDrops(dropsToXrp(d)) should + // return exactly `d` for every valid drops amount. + it('round-trips large amounts without precision loss (issue #3316)', function () { + const drops = '9999999999999999' + const xrp = dropsToXrp(drops) + assert.isTrue( + xrp.isEqualTo('9999999999.999999'), + 'large drops amounts are converted without losing the trailing drop', + ) + const roundTripped = xrpToDrops(xrp) + assert.strictEqual(roundTripped, drops) + }) })