Simplify the error check

* fix serialize/deserialize verficiation bug in Wallet.sign() when signing a non-XRP Payment transaction with an amount that contains trailing insignificant zeros

CONTRIBUTING.md

@@ -128,12 +128,12 @@ npm uninstall abbrev -w xrpl
 ### Release
 
 1. Ensure that all tests passed on the last CI that ran on `main`.
-2. Open a PR to update the docs if docs were modified.
 ___
 NOW WE ARE READY TO PUBLISH! No new code changes happen manually now.
 ___
-3. Checkout `main` and `git pull`.
-4. Create a new branch to capture updates that take place during this process. `git checkout -b <BRANCH_NAME>`
+2. Checkout `main` and `git pull`.
+3. Create a new branch to capture updates that take place during this process. `git checkout -b <BRANCH_NAME>`
+4. Run `npm run docgen` if the docs were modified in this release to update them.
 5. Run `npm run build` to triple check the build still works
 6. Run `npx lerna version --no-git-tag-version` - This creates a draft PR and release tags for the new version.
 7. For each changed package, pick what the new version should be. Lerna will bump the versions, commit version bumps to `main`, and create a new git tag for each published package.

package.json

@@ -47,13 +47,13 @@
     "https-browserify": "^1.0.0",
     "jest": "^26.0.1",
     "lerna": "^4.0.0",
-    "mocha": "^9",
+    "mocha": "^10",
     "npm-run-all": "^4.1.5",
     "nyc": "^15",
     "path-browserify": "1.0.1",
     "prettier": "^2.3.2",
     "process": "^0.11.10",
-    "puppeteer": "13.0.1",
+    "puppeteer": "^13.7.0",
     "run-script-os": "^1.1.6",
     "source-map-support": "^0.5.16",
     "stream-browserify": "^3.0.0",

packages/xrpl/HISTORY.md

@@ -3,15 +3,21 @@
 Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xrpl-announce) for release announcements. We recommend that xrpl.js (ripple-lib) users stay up-to-date with the latest stable release.
 
 ## Unreleased
+### Fixed
+* Client.disconnect() now stops the heartbeat health check as well
+* Updated dependencies which had a warning when running `npm audit`
+* Infinite error/reconnect in browser if an exception was raised during the initial websocket connection event.
+* Errors during reliable submission with no error message now properly show the preliminary result instead of a type error
+* Fixed serialize/deserialize verification bug in `Wallet.sign()` when signing a non-XRP Payment with an amount that contains trailing insignificant zeros
 
 ## 2.2.3 (2022-05-04)
 ### Fixed
-* Fixed fromMnemonic having no way to decode mnemonics from rippled's `wallet_propose` method.
+* Fixed fromMnemonic having no way to decode mnemonics from rippled's `wallet_propose` method
 
 ## 2.2.2 (2022-05-02)
 ### Added
 * Export deriveAddress from ripple-keypairs in xrpl.js
-* Deprecated BroadcastClient as it does not solve the reliabile connection problem.
+* Deprecated BroadcastClient as it does not solve the reliable connection problem
 
 ### Fixed
 * Added missing `Owner` field to NFTokenBurn type definition
@@ -20,7 +26,7 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr
 
 ## 2.2.1 (2022-04-21)
 ### Fixed
-* Fix return field of nft offer
+* Fix return field of NFT offer
 
 ## 2.2.0 (2022-04-19)
 ### Added
@@ -31,7 +37,7 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr
 ### Fixed
 * Type of TrustSet transaction edited, specifically LimitAmount property type (fixed typescript issue)
 * Remove unnecessary console.warn for partial payments (#1783, #1784, #1896)
-* Matched 1.9.0's breaking changes to NFT fields.
+* Matched 1.9.0's breaking changes to NFT fields
 
 ## 2.1.1 (2021-12-23)
 ### Fixed

packages/xrpl/src/Wallet/fundWallet.ts

@@ -1,3 +1,4 @@
+/* eslint-disable max-lines -- Relatively simple logic, but verbose */
 import { IncomingMessage } from 'http'
 import { request as httpsRequest, RequestOptions } from 'https'
 
@@ -29,7 +30,8 @@ const INTERVAL_SECONDS = 1
 const MAX_ATTEMPTS = 20
 
 /**
- * Generates a random wallet with some amount of XRP (usually 1000 XRP).
+ * Only usable on test networks.
+ * Fills this wallet with some amount of XRP (usually 1000 XRP).
  *
  * @example
  * ```typescript
@@ -41,15 +43,17 @@ const MAX_ATTEMPTS = 20
  * @param this - Client.
  * @param wallet - An existing XRPL Wallet to fund. If undefined or null, a new Wallet will be created.
  * @param options - See below.
- * @param options.faucetHost - A custom host for a faucet server. On devnet and
- * testnet, `fundWallet` will attempt to determine the correct server
- * automatically. In other environments, or if you would like to customize the
- * faucet host in devnet or testnet, you should provide the host using this
- * option.
- * @returns A Wallet on the Testnet or Devnet that contains some amount of XRP,
+ * @param options.faucetHost - A custom host for a faucet server. `fundWallet` will automatically
+ * determine the correct server for devnet and testnet as long as `faucetHost` is null or undefined.
+ * In other environments, you should provide the host using this option.
+ * Ex. Passing in faucetHost = `faucet.devnet.rippletest.net` will send a request
+ * to https://faucet.devnet.rippletest.net/accounts:443
+ * @returns A Wallet on a test network that contains some amount of XRP,
  * and that wallet's balance in XRP.
- * @throws When either Client isn't connected or unable to fund wallet address.
+ * @throws When the Client isn't connected, when it is unable to fund the wallet address,
+ * or if `faucetHost` is incorrectly formatted.
  */
+// eslint-disable-next-line max-lines-per-function -- Error checking makes for a better user experience.
 async function fundWallet(
   this: Client,
   wallet?: Wallet | null,
@@ -88,6 +92,14 @@ async function fundWallet(
     /* startingBalance remains '0' */
   }
 
+  if (
+    options?.faucetHost != null &&
+    /^(http|https|ws|wss):\/\//.test(options.faucetHost)
+  ) {
+    throw new XRPLFaucetError(
+      `Invalid format for faucetHost. Should not include ws(s)/http(s) prefix. Received '${options.faucetHost}'.`,
+    )
+  }
   // Options to pass to https.request
   const httpOptions = getHTTPOptions(this, postBody, options?.faucetHost)
 

packages/xrpl/src/Wallet/index.ts

@@ -1,4 +1,5 @@
 /* eslint-disable max-lines -- There are lots of equivalent constructors which make sense to have here. */
+import BigNumber from 'bignumber.js'
 import { fromSeed } from 'bip32'
 import { mnemonicToSeedSync } from 'bip39'
 import _ from 'lodash'
@@ -22,6 +23,7 @@ import {
   sign,
 } from 'ripple-keypairs'
 
+import type { Client } from '../client'
 import ECDSA from '../ECDSA'
 import { ValidationError } from '../errors'
 import { Transaction } from '../models/transactions'
@@ -318,13 +320,17 @@ class Wallet {
       multisignAddress = this.classicAddress
     }
 
-    if (transaction.TxnSignature || transaction.Signers) {
+    const tx = { ...transaction }
+
+    if (tx.TxnSignature || tx.Signers) {
       throw new ValidationError(
         'txJSON must not contain "TxnSignature" or "Signers" properties',
       )
     }
 
-    const txToSignAndEncode = { ...transaction }
+    removeTrailingZeros(tx)
+
+    const txToSignAndEncode = { ...tx }
 
     txToSignAndEncode.SigningPubKey = multisignAddress ? '' : this.publicKey
 
@@ -346,7 +352,7 @@ class Wallet {
       )
     }
     const serialized = encode(txToSignAndEncode)
-    this.checkTxSerialization(serialized, transaction)
+    this.checkTxSerialization(serialized, tx)
     return {
       tx_blob: serialized,
       hash: hashSignedTx(serialized),
@@ -377,6 +383,34 @@ class Wallet {
     return classicAddressToXAddress(this.classicAddress, tag, isTestnet)
   }
 
+  /**
+   * Only usable on test networks.
+   * Funds an existing wallet using a faucet.
+   * This is an alias for Client.fundWallet()
+   *
+   * @param this - An existing XRPL Wallet to fund.
+   * @param client - An XRPL client.
+   * @param options - See below.
+   * @param options.faucetHost - A custom host for a faucet server. `fundWallet` will automatically
+   * determine the correct server for devnet and testnet as long as `faucetHost` is null or undefined.
+   * In other environments, you should provide the host using this option.
+   * Here's an example of how to format `faucetHost`: `faucet.devnet.rippletest.net`
+   * @returns This wallet and that its new balance in XRP.
+   * @throws When either Client isn't connected or unable to fund wallet address.
+   */
+  public async fundWallet(
+    this: Wallet,
+    client: Client,
+    options?: {
+      faucetHost?: string
+    },
+  ): Promise<{
+    wallet: Wallet
+    balance: number
+  }> {
+    return client.fundWallet(this, options)
+  }
+
   /**
    *  Decode a serialized transaction, remove the fields that are added during the signing process,
    *  and verify that it matches the transaction prior to signing. This gives the user a sanity check
@@ -473,4 +507,26 @@ function computeSignature(
   return sign(encodeForSigning(tx), privateKey)
 }
 
+/**
+ * Remove trailing insignificant zeros for non-XRP Payment amount.
+ * This resolves the serialization mismatch bug when encoding/decoding a non-XRP Payment transaction
+ * with an amount that contains trailing insignificant zeros; for example, '123.4000' would serialize
+ * to '123.4' and cause a mismatch.
+ *
+ * @param tx - The transaction prior to signing.
+ */
+function removeTrailingZeros(tx: Transaction): void {
+  if (
+    tx.TransactionType === 'Payment' &&
+    typeof tx.Amount !== 'string' &&
+    tx.Amount.value.includes('.') &&
+    tx.Amount.value.endsWith('0')
+  ) {
+    // eslint-disable-next-line no-param-reassign -- Required to update Transaction.Amount.value
+    tx.Amount = { ...tx.Amount }
+    // eslint-disable-next-line no-param-reassign -- Required to update Transaction.Amount.value
+    tx.Amount.value = new BigNumber(tx.Amount.value).toString()
+  }
+}
+
 export default Wallet

packages/xrpl/src/client/connection.ts

@@ -273,6 +273,7 @@ export class Connection extends EventEmitter {
    * @returns A promise containing either `undefined` or a disconnected code, that resolves when the connection is destroyed.
    */
   public async disconnect(): Promise<number | undefined> {
+    this.clearHeartbeatInterval()
     if (this.reconnectTimeoutID !== null) {
       clearTimeout(this.reconnectTimeoutID)
       this.reconnectTimeoutID = null
@@ -420,6 +421,7 @@ export class Connection extends EventEmitter {
    * @returns A promise that resolves to void when the connection is fully established.
    * @throws Error if the websocket initialized is somehow null.
    */
+  // eslint-disable-next-line max-lines-per-function -- Many error code conditionals to check.
   private async onceOpen(connectionTimeoutID: NodeJS.Timeout): Promise<void> {
     if (this.ws == null) {
       throw new XrplError('onceOpen: ws is null')
@@ -434,7 +436,7 @@ export class Connection extends EventEmitter {
       this.emit('error', 'websocket', error.message, error),
     )
     // Handle a closed connection: reconnect if it was unexpected
-    this.ws.once('close', (code, reason) => {
+    this.ws.once('close', (code?: number, reason?: Buffer) => {
       if (this.ws == null) {
         throw new XrplError('onceClose: ws is null')
       }
@@ -447,9 +449,31 @@ export class Connection extends EventEmitter {
       )
       this.ws.removeAllListeners()
       this.ws = null
-      this.emit('disconnected', code)
-      // If this wasn't a manual disconnect, then lets reconnect ASAP.
-      if (code !== INTENTIONAL_DISCONNECT_CODE) {
+
+      if (code === undefined) {
+        const reasonText = reason ? reason.toString() : 'undefined'
+        // eslint-disable-next-line no-console -- The error is helpful for debugging.
+        console.error(
+          `Disconnected but the disconnect code was undefined (The given reason was ${reasonText}).` +
+            `This could be caused by an exception being thrown during a 'connect' callback. ` +
+            `Disconnecting with code 1011 to indicate an internal error has occurred.`,
+        )
+
+        /*
+         * Error code 1011 represents an Internal Error according to
+         * https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code
+         */
+        const internalErrorCode = 1011
+        this.emit('disconnected', internalErrorCode)
+      } else {
+        this.emit('disconnected', code)
+      }
+
+      /*
+       * If this wasn't a manual disconnect, then lets reconnect ASAP.
+       * Code can be undefined if there's an exception while connecting.
+       */
+      if (code !== INTENTIONAL_DISCONNECT_CODE && code !== undefined) {
         this.intentionalDisconnect()
       }
     })

packages/xrpl/src/sugar/submit.ts

@@ -148,7 +148,7 @@ async function waitForFinalTransactionOutcome(
     .catch(async (error) => {
       // error is of an unknown type and hence we assert type to extract the value we need.
       // eslint-disable-next-line @typescript-eslint/consistent-type-assertions,@typescript-eslint/no-unsafe-member-access -- ^
-      const message = error.data.error as string
+      const message = error?.data?.error as string
       if (message === 'txnNotFound') {
         return waitForFinalTransactionOutcome(
           client,

packages/xrpl/test/integration/fundWallet.ts

@@ -6,6 +6,7 @@ import {
   isValidClassicAddress,
   isValidXAddress,
   dropsToXrp,
+  XRPLFaucetError,
 } from 'xrpl-local'
 // how long before each test case times out
 const TIMEOUT = 60000
@@ -69,6 +70,53 @@ describe('fundWallet', function () {
     await api.disconnect()
   })
 
+  it('can fund an existing Wallet using the Wallet alias', async function () {
+    const api = new Client('wss://s.devnet.rippletest.net:51233')
+
+    await api.connect()
+    const { wallet, balance } = await api.fundWallet()
+
+    assert.notEqual(wallet, undefined)
+    assert(isValidClassicAddress(wallet.classicAddress))
+    assert(isValidXAddress(wallet.getXAddress()))
+
+    const info = await api.request({
+      command: 'account_info',
+      account: wallet.classicAddress,
+    })
+
+    assert.equal(dropsToXrp(info.result.account_data.Balance), balance)
+
+    const { balance: newBalance } = await wallet.fundWallet(api)
+
+    const afterSent = await api.request({
+      command: 'account_info',
+      account: wallet.classicAddress,
+    })
+    assert.equal(dropsToXrp(afterSent.result.account_data.Balance), newBalance)
+
+    await api.disconnect()
+  })
+  it('throws when given an incorrectly formatted faucetHost', async function () {
+    const api = new Client('ws://xls20-sandbox.rippletest.net:51233')
+
+    let errorHappened = false
+    await api.connect()
+
+    // Using try/catch instead of assert.throws because 'await' has to be top-level
+    try {
+      await api.fundWallet(null, {
+        faucetHost: 'https://faucet-nft.ripple.com/',
+      })
+    } catch (error) {
+      assert.ok(error instanceof XRPLFaucetError)
+      errorHappened = true
+    }
+
+    assert.ok(errorHappened)
+    await api.disconnect()
+  })
+
   it('can generate and fund wallets using a custom host', async function () {
     const api = new Client('ws://xls20-sandbox.rippletest.net:51233')
 

packages/xrpl/test/wallet/index.ts

@@ -589,6 +589,56 @@ describe('Wallet', function () {
         wallet.sign(payment)
       }, /^1.1234567 is an illegal amount/u)
     })
+
+    it('sign handles non-XRP amount with a trailing zero', async function () {
+      const payment: Transaction = {
+        TransactionType: 'Payment',
+        Account: 'r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59',
+        Destination: 'rQ3PTWGLCbPz8ZCicV5tCX3xuymojTng5r',
+        Amount: {
+          currency: 'FOO',
+          issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
+          value: '123.40',
+        },
+        Flags: 2147483648,
+        Sequence: 23,
+        LastLedgerSequence: 8819954,
+        Fee: '12',
+      }
+      const result = wallet.sign(payment)
+      const expectedResult = {
+        tx_blob:
+          '12000022800000002400000017201B008694F261D504625103A72000000000000000000000000000464F4F00000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F5447446304402206EBFC9B8061C3F82D521506CE62B6BBA99995B2175BFE0E1BC516775932AECEB0220172B9CE9C0EFB3F4870E19B79B4E817DD376E5785F034AB792708F92282C65F781145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648',
+        hash: '6235E5A3CC14DB97F75CAE2A4B5AA9B4134B7AD48D7DD8C15473D81631435FE4',
+      }
+
+      assert.deepEqual(result, expectedResult)
+    })
+
+    it('sign handles non-XRP amount with trailing zeros', async function () {
+      const payment: Transaction = {
+        TransactionType: 'Payment',
+        Account: 'r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59',
+        Destination: 'rQ3PTWGLCbPz8ZCicV5tCX3xuymojTng5r',
+        Amount: {
+          currency: 'FOO',
+          issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
+          value: '123.000',
+        },
+        Flags: 2147483648,
+        Sequence: 23,
+        LastLedgerSequence: 8819954,
+        Fee: '12',
+      }
+      const result = wallet.sign(payment)
+      const expectedResult = {
+        tx_blob:
+          '12000022800000002400000017201B008694F261D5045EADB112E000000000000000000000000000464F4F00000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F54474473045022100C0C77D7D6D6453F0C5EDFF61DE60B5D3D6952C8F30D51543560936D72FA103B00220258CBFCEAC4D2DB5CC2B9417EB46225943E9F4B92944B303ADB810002530BFFB81145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648',
+        hash: 'FADCD5EE33C01103AA129FCF0923637D543DB56250CD57A1A308EC386A211CBB',
+      }
+
+      assert.deepEqual(result, expectedResult)
+    })
   })
 
   describe('verifyTransaction', function () {