fix(xrpl): return string from dropsToXrp to preserve precision (#3316)

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<BigNumber>` instead of `Promise<number>`. 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.

packages/xrpl/src/Wallet/fundWallet.ts

@@ -95,7 +95,7 @@ export async function getStartingBalance(
 ): Promise<number> {
   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 */
         }

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,15 +937,17 @@ class Client extends EventEmitter<EventTypes> {
    * @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,
     options: {
       ledger_hash?: string
       ledger_index?: LedgerIndex
     } = {},
-  ): Promise<number> {
+  ): Promise<BigNumber> {
     const xrpRequest: AccountInfoRequest = {
       command: 'account_info',
       account: address,
@@ -1018,7 +1021,7 @@ class Client extends EventEmitter<EventTypes> {
     const balances: Balance[] = []
 
     // get XRP balance
-    let xrpPromise: Promise<number> = Promise.resolve(0)
+    let xrpPromise: Promise<BigNumber> = 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<EventTypes> {
         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<EventTypes> {
     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 */
       }

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)
 }
 
 /**

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'))
       })
     })
   })

packages/xrpl/test/utils/dropsToXrp.test.ts

@@ -1,94 +1,103 @@
 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',
     )
   })
 
   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)
+  })
 })