Lints src/common (#1595)

* lint errors (and remove unused error types)

* lint fee

* lint index

* lint ecdsa

* fix tests

* remove address tests from getFee

* fix more tests

* fix tests

* respond to comments

src/client/connection.ts

@@ -11,7 +11,7 @@ import {
   DisconnectedError,
   NotConnectedError,
   ConnectionError,
-  RippleError,
+  XrplError,
 } from '../common/errors'
 import { BaseRequest } from '../models/methods/baseMethod'
 
@@ -217,7 +217,7 @@ export class Connection extends EventEmitter {
     }
     if (this.ws != null) {
       return Promise.reject(
-        new RippleError('Websocket connection never cleaned up.', {
+        new XrplError('Websocket connection never cleaned up.', {
           state: this.state,
         }),
       )

src/client/index.ts

@@ -21,8 +21,8 @@ import {
 } from 'ripple-address-codec'
 
 import { constants, errors, txFlags, ensureClassicAddress } from '../common'
-import { RippledError, ValidationError } from '../common/errors'
-import { getFee } from '../common/fee'
+import { ValidationError, XrplError } from '../common/errors'
+import getFee from '../common/fee'
 import getBalances from '../ledger/balances'
 import { getOrderbook, formatBidsAndAsks } from '../ledger/orderbook'
 import getPaths from '../ledger/pathfind'
@@ -429,7 +429,7 @@ class Client extends EventEmitter {
       // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
       const singleResult = (singleResponse as U).result
       if (!(collectKey in singleResult)) {
-        throw new RippledError(`${collectKey} not in result`)
+        throw new XrplError(`${collectKey} not in result`)
       }
       // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- Should be true
       const collectedData = singleResult[collectKey]

src/common/ecdsa.ts

@@ -1,3 +1,4 @@
+// eslint-disable-next-line no-shadow -- No shadow here
 enum ECDSA {
   ed25519 = 'ed25519',
   secp256k1 = 'ecdsa-secp256k1',

src/common/errors.ts

@@ -1,22 +1,34 @@
+/* eslint-disable max-classes-per-file -- Errors can be defined in the same file */
 import { inspect } from 'util'
 
-class RippleError extends Error {
-  name: string
-  message: string
-  data?: any
+// TODO: replace all `new Error`s with `new XrplError`s
 
-  constructor(message = '', data?: any) {
+class XrplError extends Error {
+  public readonly name: string
+  public readonly message: string
+  public readonly data?: unknown
+
+  /**
+   * Construct an XrplError.
+   *
+   * @param message - The error message.
+   * @param data - The data that caused the error.
+   */
+  public constructor(message = '', data?: unknown) {
     super(message)
 
     this.name = this.constructor.name
     this.message = message
     this.data = data
-    if (Error.captureStackTrace) {
-      Error.captureStackTrace(this, this.constructor)
-    }
+    Error.captureStackTrace(this, this.constructor)
   }
 
-  toString() {
+  /**
+   * Converts the Error to a human-readable String form.
+   *
+   * @returns The String output of the Error.
+   */
+  public toString(): string {
     let result = `[${this.name}(${this.message}`
     if (this.data) {
       result += `, ${inspect(this.data)}`
@@ -25,21 +37,25 @@ class RippleError extends Error {
     return result
   }
 
-  // console.log in node uses util.inspect on object, and util.inspect allows
-  // us to customize its output:
-  // https://nodejs.org/api/util.html#util_custom_inspect_function_on_objects
-  inspect() {
+  /**
+   * Console.log in node uses util.inspect on object, and util.inspect allows
+   * us to customize its output:
+   * https://nodejs.org/api/util.html#util_custom_inspect_function_on_objects.
+   *
+   * @returns The String output of the Error.
+   */
+  public inspect(): string {
     return this.toString()
   }
 }
 
-class RippledError extends RippleError {}
+class RippledError extends XrplError {}
 
-class UnexpectedError extends RippleError {}
+class UnexpectedError extends XrplError {}
 
-class LedgerVersionError extends RippleError {}
+class LedgerVersionError extends XrplError {}
 
-class ConnectionError extends RippleError {}
+class ConnectionError extends XrplError {}
 
 class NotConnectedError extends ConnectionError {}
 
@@ -51,34 +67,23 @@ class TimeoutError extends ConnectionError {}
 
 class ResponseFormatError extends ConnectionError {}
 
-class ValidationError extends RippleError {}
+class ValidationError extends XrplError {}
 
-class XRPLFaucetError extends RippleError {}
+class XRPLFaucetError extends XrplError {}
 
-class NotFoundError extends RippleError {
-  constructor(message = 'Not found') {
+class NotFoundError extends XrplError {
+  /**
+   * Construct an XrplError.
+   *
+   * @param message - The error message. Defaults to "Not found".
+   */
+  public constructor(message = 'Not found') {
     super(message)
   }
 }
 
-class MissingLedgerHistoryError extends RippleError {
-  constructor(message?: string) {
-    super(message || 'Server is missing ledger history in the specified range')
-  }
-}
-
-class PendingLedgerVersionError extends RippleError {
-  constructor(message?: string) {
-    super(
-      message ||
-        "maxLedgerVersion is greater than server's most recent" +
-          ' validated ledger',
-    )
-  }
-}
-
 export {
-  RippleError,
+  XrplError,
   UnexpectedError,
   ConnectionError,
   RippledError,
@@ -89,8 +94,6 @@ export {
   ResponseFormatError,
   ValidationError,
   NotFoundError,
-  PendingLedgerVersionError,
-  MissingLedgerHistoryError,
   LedgerVersionError,
   XRPLFaucetError,
 }

src/common/fee.ts

@@ -1,17 +1,24 @@
 import BigNumber from 'bignumber.js'
-import _ from 'lodash'
 
 import type { Client } from '..'
 
-// This is a public API that can be called directly.
-// This is not used by the `prepare*` methods. See `src/transaction/utils.ts`
-async function getFee(this: Client, cushion?: number): Promise<string> {
-  if (cushion == null) {
-    cushion = this.feeCushion
-  }
-  if (cushion == null) {
-    cushion = 1.2
-  }
+const NUM_DECIMAL_PLACES = 6
+const BASE_10 = 10
+
+/**
+ * Calculates the current transaction fee for the ledger.
+ * Note: This is a public API that can be called directly.
+ * This is not used by the `prepare*` methods. See `src/transaction/utils.ts`.
+ *
+ * @param this - The Client used to connect to the ledger.
+ * @param cushion - The fee cushion to use.
+ * @returns The transaction fee.
+ */
+export default async function getFee(
+  this: Client,
+  cushion: number | null,
+): Promise<string> {
+  const feeCushion = cushion ?? this.feeCushion
 
   const serverInfo = (await this.request({ command: 'server_info' })).result
     .info
@@ -27,12 +34,10 @@ async function getFee(this: Client, cushion?: number): Promise<string> {
     // https://github.com/ripple/rippled/issues/3812#issuecomment-816871100
     serverInfo.load_factor = 1
   }
-  let fee = baseFeeXrp.times(serverInfo.load_factor).times(cushion)
+  let fee = baseFeeXrp.times(serverInfo.load_factor).times(feeCushion)
 
   // Cap fee to `this.maxFeeXRP`
   fee = BigNumber.min(fee, this.maxFeeXRP)
   // Round fee to 6 decimal places
-  return new BigNumber(fee.toFixed(6)).toString(10)
+  return new BigNumber(fee.toFixed(NUM_DECIMAL_PLACES)).toString(BASE_10)
 }
-
-export { getFee }

src/common/index.ts

@@ -3,6 +3,13 @@ import { xAddressToClassicAddress, isValidXAddress } from 'ripple-address-codec'
 import * as constants from './constants'
 import * as errors from './errors'
 
+/**
+ * If an address is an X-Address, converts it to a classic address.
+ *
+ * @param account - A classic address or X-address.
+ * @returns The account's classic address.
+ * @throws Error if the X-Address has an associated tag.
+ */
 export function ensureClassicAddress(account: string): string {
   if (isValidXAddress(account)) {
     const { classicAddress, tag } = xAddressToClassicAddress(account)

src/models/methods/serverInfo.ts

@@ -51,7 +51,7 @@ export interface ServerInfoResponse extends BaseResponse {
         job_types: JobType[]
         threads: number
       }
-      load_factor: number
+      load_factor?: number
       load_factor_local?: number
       load_factor_net?: number
       load_factor_cluster?: number

src/transaction/sign.ts

@@ -4,12 +4,12 @@ import binaryCodec from 'ripple-binary-codec'
 import keypairs from 'ripple-keypairs'
 
 import type { Client, Wallet } from '..'
+import { ValidationError } from '../common/errors'
 import { SignedTransaction } from '../common/types/objects'
 import { xrpToDrops } from '../utils'
 import { computeBinaryTransactionHash } from '../utils/hashes'
 
 import { SignOptions, KeyPair, TransactionJSON } from './types'
-import * as utils from './utils'
 
 function computeSignature(tx: object, privateKey: string, signAs?: string) {
   const signingData = signAs
@@ -28,7 +28,7 @@ function signWithKeypair(
 ): SignedTransaction {
   const tx = JSON.parse(txJSON)
   if (tx.TxnSignature || tx.Signers) {
-    throw new utils.common.errors.ValidationError(
+    throw new ValidationError(
       'txJSON must not contain "TxnSignature" or "Signers" properties',
     )
   }
@@ -148,7 +148,7 @@ function checkTxSerialization(serialized: string, tx: TransactionJSON): void {
   // ...And ensure it is equal to the original tx, except:
   // - It must have a TxnSignature or Signers (multisign).
   if (!decoded.TxnSignature && !decoded.Signers) {
-    throw new utils.common.errors.ValidationError(
+    throw new ValidationError(
       'Serialized transaction must have a TxnSignature or Signers property',
     )
   }
@@ -182,14 +182,15 @@ function checkTxSerialization(serialized: string, tx: TransactionJSON): void {
   })
 
   if (!_.isEqual(decoded, tx)) {
-    const error = new utils.common.errors.ValidationError(
-      'Serialized transaction does not match original txJSON. See `error.data`',
-    )
-    error.data = {
+    const data = {
       decoded,
       tx,
       diff: objectDiff(tx, decoded),
     }
+    const error = new ValidationError(
+      'Serialized transaction does not match original txJSON. See `error.data`',
+      data,
+    )
     throw error
   }
 }
@@ -208,7 +209,7 @@ function checkFee(client: Client, txFee: string): void {
   const fee = new BigNumber(txFee)
   const maxFeeDrops = xrpToDrops(client.maxFeeXRP)
   if (fee.isGreaterThan(maxFeeDrops)) {
-    throw new utils.common.errors.ValidationError(
+    throw new ValidationError(
       `"Fee" should not exceed "${maxFeeDrops}". ` +
         'To use a higher fee, set `maxFeeXRP` in the Client constructor.',
     )
@@ -234,9 +235,7 @@ function sign(
   }
   if (!keypair && !secret) {
     // Clearer message than 'ValidationError: instance is not exactly one from [subschema 0],[subschema 1]'
-    throw new utils.common.errors.ValidationError(
-      'sign: Missing secret or keypair.',
-    )
+    throw new ValidationError('sign: Missing secret or keypair.')
   }
   return signWithKeypair(this, txJSON, keypair || secret, options)
 }

test/client/errors.ts

@@ -6,9 +6,9 @@ describe('client errors', function () {
   beforeEach(setupClient.setup)
   afterEach(setupClient.teardown)
 
-  it('RippleError with data', async function () {
-    const error = new this.client.errors.RippleError('_message_', '_data_')
-    assert.strictEqual(error.toString(), "[RippleError(_message_, '_data_')]")
+  it('XrplError with data', async function () {
+    const error = new this.client.errors.XrplError('_message_', '_data_')
+    assert.strictEqual(error.toString(), "[XrplError(_message_, '_data_')]")
   })
 
   it('NotFoundError default message', async function () {

test/client/getFee.ts

@@ -2,69 +2,57 @@ import { assert } from 'chai'
 
 import rippled from '../fixtures/rippled'
 import setupClient from '../setupClient'
-import { addressTests } from '../testUtils'
 
 describe('client.getFee', function () {
   beforeEach(setupClient.setup)
   afterEach(setupClient.teardown)
 
-  addressTests.forEach(function (test) {
-    describe(test.type, function () {
-      it('getFee', async function () {
-        this.mockRippled.addResponse('server_info', rippled.server_info.normal)
-        const fee = await this.client.getFee()
-        assert.strictEqual(fee, '0.000012')
-      })
+  it('getFee', async function () {
+    this.mockRippled.addResponse('server_info', rippled.server_info.normal)
+    const fee = await this.client.getFee()
+    assert.strictEqual(fee, '0.000012')
+  })
 
-      it('getFee default', async function () {
-        this.mockRippled.addResponse('server_info', rippled.server_info.normal)
-        this.client.feeCushion = undefined as unknown as number
-        const fee = await this.client.getFee()
-        assert.strictEqual(fee, '0.000012')
-      })
+  it('getFee - high load_factor', async function () {
+    this.mockRippled.addResponse(
+      'server_info',
+      rippled.server_info.highLoadFactor,
+    )
+    const fee = await this.client.getFee()
+    assert.strictEqual(fee, '2')
+  })
 
-      it('getFee - high load_factor', async function () {
-        this.mockRippled.addResponse(
-          'server_info',
-          rippled.server_info.highLoadFactor,
-        )
-        const fee = await this.client.getFee()
-        assert.strictEqual(fee, '2')
-      })
+  it('getFee - high load_factor with custom maxFeeXRP', async function () {
+    this.mockRippled.addResponse(
+      'server_info',
+      rippled.server_info.highLoadFactor,
+    )
+    // Ensure that overriding with high maxFeeXRP of '51540' causes no errors.
+    // (fee will actually be 51539.607552)
+    this.client.maxFeeXRP = '51540'
+    const fee = await this.client.getFee()
+    assert.strictEqual(fee, '51539.607552')
+  })
 
-      it('getFee - high load_factor with custom maxFeeXRP', async function () {
-        this.mockRippled.addResponse(
-          'server_info',
-          rippled.server_info.highLoadFactor,
-        )
-        // Ensure that overriding with high maxFeeXRP of '51540' causes no errors.
-        // (fee will actually be 51539.607552)
-        this.client.maxFeeXRP = '51540'
-        const fee = await this.client.getFee()
-        assert.strictEqual(fee, '51539.607552')
-      })
+  it('getFee custom cushion', async function () {
+    this.mockRippled.addResponse('server_info', rippled.server_info.normal)
+    this.client.feeCushion = 1.4
+    const fee = await this.client.getFee()
+    assert.strictEqual(fee, '0.000014')
+  })
 
-      it('getFee custom cushion', async function () {
-        this.mockRippled.addResponse('server_info', rippled.server_info.normal)
-        this.client.feeCushion = 1.4
-        const fee = await this.client.getFee()
-        assert.strictEqual(fee, '0.000014')
-      })
+  // This is not recommended since it may result in attempting to pay
+  // less than the base fee. However, this test verifies the existing behavior.
+  it('getFee cushion less than 1.0', async function () {
+    this.mockRippled.addResponse('server_info', rippled.server_info.normal)
+    this.client.feeCushion = 0.9
+    const fee = await this.client.getFee()
+    assert.strictEqual(fee, '0.000009')
+  })
 
-      // This is not recommended since it may result in attempting to pay
-      // less than the base fee. However, this test verifies the existing behavior.
-      it('getFee cushion less than 1.0', async function () {
-        this.mockRippled.addResponse('server_info', rippled.server_info.normal)
-        this.client.feeCushion = 0.9
-        const fee = await this.client.getFee()
-        assert.strictEqual(fee, '0.000009')
-      })
-
-      it('getFee reporting', async function () {
-        this.mockRippled.addResponse('server_info', rippled.server_info.normal)
-        const fee = await this.client.getFee()
-        assert.strictEqual(fee, '0.000012')
-      })
-    })
+  it('getFee reporting', async function () {
+    this.mockRippled.addResponse('server_info', rippled.server_info.normal)
+    const fee = await this.client.getFee()
+    assert.strictEqual(fee, '0.000012')
   })
 })

test/client/getPaths.ts

@@ -97,7 +97,7 @@ describe('client.getPaths', function () {
         ...REQUEST_FIXTURES.normal,
         source: { address: addresses.NOTFOUND },
       }),
-      this.client.errors.RippleError,
+      this.client.errors.XrplError,
     )
   })
   // 'send all', function () {

test/connection.ts

@@ -11,7 +11,7 @@ import {
   DisconnectedError,
   NotConnectedError,
   ResponseFormatError,
-  RippleError,
+  XrplError,
   TimeoutError,
 } from '../src/common/errors'
 
@@ -207,11 +207,11 @@ describe('Connection', function () {
   })
 
   it('DisconnectedError on initial onOpen send', async function () {
-    // _onOpen previously could throw PromiseRejectionHandledWarning: Promise rejection was handled asynchronously
+    // onOpen previously could throw PromiseRejectionHandledWarning: Promise rejection was handled asynchronously
     // do not rely on the client.setup hook to test this as it bypasses the case, disconnect client connection first
     await this.client.disconnect()
 
-    // stub _onOpen to only run logic relevant to test case
+    // stub onOpen to only run logic relevant to test case
     this.client.connection.onOpen = () => {
       // overload websocket send on open when _ws exists
       this.client.connection.ws.send = function (_0, _1, _2) {
@@ -420,7 +420,7 @@ describe('Connection', function () {
       new Client({
         servers: ['wss://server1.com', 'wss://server2.com'],
       } as any)
-    }, RippleError)
+    }, XrplError)
   })
 
   it('connect throws error', function (done) {

test/integration/integration.ts

@@ -4,7 +4,6 @@ import _ from 'lodash'
 import { isValidXAddress } from 'ripple-address-codec'
 
 import { Client } from 'xrpl-local'
-import { errors } from 'xrpl-local/common'
 import { isValidSecret } from 'xrpl-local/utils'
 
 import { generateXAddress } from '../../src/utils/generateAddress'
@@ -46,24 +45,7 @@ function verifyTransaction(testcase, hash, type, options, txData, account) {
       }
       return { txJSON: JSON.stringify(txData), id: hash, tx: data }
     })
-    .catch(async (error) => {
-      if (error instanceof errors.PendingLedgerVersionError) {
-        console.log('NOT VALIDATED YET...')
-        return new Promise((resolve, reject) => {
-          setTimeout(
-            () =>
-              verifyTransaction(
-                testcase,
-                hash,
-                type,
-                options,
-                txData,
-                account,
-              ).then(resolve, reject),
-            INTERVAL,
-          )
-        })
-      }
+    .catch((error) => {
       console.log(error.stack)
       assert(false, `Transaction not successful: ${error.message}`)
     })