refactor: clean up Client and associated files (#1556)

* remove _PRIVATE

* make requestAll public

* un-type connection.request

* fix lodash imports

* add comments

* Rename files to camelCase

package.json

@@ -13,7 +13,7 @@
   "jsdelivr": "build/ripple-latest-min.js",
   "types": "dist/npm/index.d.ts",
   "browser": {
-    "ws": "./dist/npm/client/wswrapper.js",
+    "ws": "./dist/npm/client/wsWrapper.js",
     "https-proxy-agent": false
   },
   "directories": {

src/client/connection.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import {EventEmitter} from 'events'
 import {parse as parseURL} from 'url'
 import WebSocket from 'ws'
@@ -12,7 +12,7 @@ import {
   RippleError
 } from '../common/errors'
 import {ExponentialBackoff} from './backoff'
-import { Request, Response } from '../models/methods'
+import { Response } from '../models/methods'
 
 /**
  * ConnectionOptions is the configuration for the Connection class.
@@ -186,7 +186,7 @@ class RequestManager {
    * hung responses, and a promise that will resolve with the response once
    * the response is seen & handled.
    */
-  createRequest(data: Request, timeout: number): [string | number, string, Promise<Response>] {
+  createRequest(data: any, timeout: number): [string | number, string, Promise<any>] {
     const newId = data.id ? data.id : this.nextId++
     const newData = JSON.stringify({...data, id: newId})
     const timer = setTimeout(
@@ -473,7 +473,7 @@ export class Connection extends EventEmitter {
     await this.connect()
   }
 
-  async request<T extends Request, U extends Response>(request: T, timeout?: number): Promise<U> {
+  async request<T extends {command: string}>(request: T, timeout?: number): Promise<any> {
     if (!this._shouldBeConnected) {
       throw new NotConnectedError()
     }
@@ -486,7 +486,7 @@ export class Connection extends EventEmitter {
       this._requestManager.reject(id, error)
     })
 
-    return responsePromise as any
+    return responsePromise
   }
 
   /**

src/client/index.ts

@@ -2,7 +2,6 @@ import {EventEmitter} from 'events'
 import {
   constants,
   errors,
-  validate,
   txFlags,
 } from '../common'
 import { Connection, ConnectionUserOptions } from './connection'
@@ -98,8 +97,6 @@ import {
   RandomResponse
 } from '../models/methods'
 
-import RangeSet from './rangeset'
-import * as ledgerUtils from '../ledger/utils'
 import * as transactionUtils from '../transaction/utils'
 import * as schemaValidator from '../common/schema-validator'
 import {getFee} from '../common/fee'
@@ -172,21 +169,17 @@ type MarkerResponse = AccountChannelsResponse
                     | LedgerDataResponse
 
 class Client extends EventEmitter {
+  // Factor to multiply estimated fee by to provide a cushion in case the
+  // required fee rises during submission of a transaction. Defaults to 1.2.
   _feeCushion: number
+  // Maximum fee to use with transactions, in XRP. Must be a string-encoded
+  // number. Defaults to '2'.
   _maxFeeXRP: string
 
   // New in > 0.21.0
   // non-validated ledger versions are allowed, and passed to rippled as-is.
   connection: Connection
 
-  // these are exposed only for use by unit tests; they are not part of the client.
-  static _PRIVATE = {
-    validate,
-    RangeSet,
-    ledgerUtils,
-    schemaValidator
-  }
-
   constructor(server: string, options: ClientOptions = {}) {
     super()
     if (typeof server !== 'string' || !server.match("^(wss?|wss?\\+unix)://")) {
@@ -312,7 +305,7 @@ class Client extends EventEmitter {
 
   /**
    * Makes multiple paged requests to the client to return a given number of
-   * resources. _requestAll() will make multiple requests until the `limit`
+   * resources. requestAll() will make multiple requests until the `limit`
    * number of resources is reached (if no `limit` is provided, a single request
    * will be made).
    *
@@ -323,14 +316,14 @@ class Client extends EventEmitter {
    * general use. Instead, use rippled's built-in pagination and make multiple
    * requests as needed.
    */
-  async _requestAll(req: AccountChannelsRequest): Promise<AccountChannelsResponse[]>
+  async requestAll(req: AccountChannelsRequest): Promise<AccountChannelsResponse[]>
-  async _requestAll(req: AccountLinesRequest): Promise<AccountLinesResponse[]>
+  async requestAll(req: AccountLinesRequest): Promise<AccountLinesResponse[]>
-  async _requestAll(req: AccountObjectsRequest): Promise<AccountObjectsResponse[]>
+  async requestAll(req: AccountObjectsRequest): Promise<AccountObjectsResponse[]>
-  async _requestAll(req: AccountOffersRequest): Promise<AccountOffersResponse[]>
+  async requestAll(req: AccountOffersRequest): Promise<AccountOffersResponse[]>
-  async _requestAll(req: AccountTxRequest): Promise<AccountTxResponse[]>
+  async requestAll(req: AccountTxRequest): Promise<AccountTxResponse[]>
-  async _requestAll(req: BookOffersRequest): Promise<BookOffersResponse[]>
+  async requestAll(req: BookOffersRequest): Promise<BookOffersResponse[]>
-  async _requestAll(req: LedgerDataRequest): Promise<LedgerDataResponse[]>
+  async requestAll(req: LedgerDataRequest): Promise<LedgerDataResponse[]>
-  async _requestAll<T extends MarkerRequest, U extends MarkerResponse>(request: T, options: {collect?: string} = {}): Promise<U[]> {
+  async requestAll<T extends MarkerRequest, U extends MarkerResponse>(request: T, options: {collect?: string} = {}): Promise<U[]> {
     // The data under collection is keyed based on the command. Fail if command
     // not recognized and collection key not provided.
     const collectKey = options.collect || getCollectKeyFromCommand(request.command)

src/common/schema-validator.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import * as assert from 'assert'
 const {Validator} = require('jsonschema')
 import {ValidationError} from './errors'

src/common/validate.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import {ValidationError} from './errors'
 import {schemaValidate} from './schema-validator'
 

src/index.ts

@@ -9,6 +9,6 @@ export * from './models/methods'
 export * from './utils'
 
 // Broadcast client is experimental
-export {BroadcastClient} from './client/broadcast'
+export {BroadcastClient} from './client/broadcastClient'
 
 export * from './Wallet'

src/ledger/orderbook.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import * as utils from './utils'
 import {
   parseOrderbookOrder,
@@ -81,7 +81,7 @@ async function makeRequest(
     taker_gets: takerGets,
     taker_pays: takerPays
   })
-  return client._requestAll({command: 'book_offers',
+  return client.requestAll({command: 'book_offers',
     taker_gets: orderData.taker_gets,
     taker_pays: orderData.taker_pays,
     ledger_index: options.ledgerVersion || 'validated',

src/ledger/parse/fields.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import BigNumber from 'bignumber.js'
 import {constants} from '../../common'
 const AccountFields = constants.AccountFields

src/ledger/parse/ledger.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import {removeUndefined, rippleTimeToISOTime} from '../../utils'
 import parseTransaction from './transaction'
 import { TransactionAndMetadata } from '../../models/transactions'

src/ledger/parse/orderbook-order.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import {parseTimestamp, adjustQualityForXRP} from './utils'
 import {removeUndefined} from '../../utils'
 

src/ledger/parse/pathfind.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import parseAmount from './amount'
 import {Amount, RippledAmount} from '../../common/types/objects'
 import {Path, GetPaths, RippledPathsResponse} from '../pathfind-types'

src/ledger/parse/settings.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import * as assert from 'assert'
 import {constants} from '../../common'
 const AccountFlags = constants.AccountFlags

src/ledger/pathfind.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import BigNumber from 'bignumber.js'
 import {getXRPBalance, renameCounterpartyToIssuer} from './utils'
 import {

src/ledger/trustlines.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import {validate, ensureClassicAddress} from '../common'
 import parseAccountTrustline from './parse/account-trustline'
 import {Client} from '..'
@@ -29,7 +29,7 @@ async function getTrustlines(
   address = ensureClassicAddress(address)
 
   // 2. Make Request
-  const responses = await this._requestAll({command: 'account_lines',
+  const responses = await this.requestAll({command: 'account_lines',
     account: address,
     ledger_index: options.ledgerVersion ?? 'validated',
     limit: options.limit,

src/ledger/utils.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import * as assert from 'assert'
 import * as common from '../common'
 import {Connection} from '../client'

src/transaction/combine.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import binary from 'ripple-binary-codec'
 import BigNumber from 'bignumber.js'
 import {ValidationError} from '../common/errors'

src/transaction/payment.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import * as utils from './utils'
 const validate = utils.common.validate
 const paymentFlags = utils.common.txFlags.Payment

src/transaction/ticket.ts

@@ -1,4 +1,4 @@
-import * as _ from 'lodash'
+import _ from 'lodash'
 import * as utils from './utils'
 import {Prepare, TransactionJSON, Instructions} from './types'
 import {Client} from '..'

src/utils/index.ts

@@ -1,3 +1,9 @@
+import _ from 'lodash'
+import BigNumber from 'bignumber.js'
+import {RippledAmount} from '../common/types/objects'
+import {ValidationError} from '../common/errors'
+import {xAddressToClassicAddress} from 'ripple-address-codec'
+
 import { deriveKeypair, deriveAddress, deriveXAddress } from './derive'
 import computeLedgerHeaderHash from './ledgerHash'
 import signPaymentChannelClaim from './signPaymentChannelClaim'
@@ -18,12 +24,6 @@ import {
 } from './hashes'
 import { generateXAddress } from './generateAddress'
 
-import _ from 'lodash'
-import BigNumber from 'bignumber.js'
-import {RippledAmount} from '../common/types/objects'
-import {ValidationError} from '../common/errors'
-import {xAddressToClassicAddress} from 'ripple-address-codec'
-
 function isValidSecret(secret: string): boolean {
   try {
     deriveKeypair(secret)

test/client/preparePayment.ts

@@ -5,9 +5,8 @@ import requests from '../fixtures/requests'
 import {ValidationError} from 'xrpl-local/common/errors'
 import binary from 'ripple-binary-codec'
 import assert from 'assert-diff'
-import {Client} from 'xrpl-local'
+import * as schemaValidator from 'xrpl-local/common/schema-validator'
 
-const {schemaValidator} = Client._PRIVATE
 const instructionsWithMaxLedgerVersionOffset = {maxLedgerVersionOffset: 100}
 const {preparePayment: REQUEST_FIXTURES} = requests
 const {preparePayment: RESPONSE_FIXTURES} = responses

test/client/prepareTicket.ts

@@ -6,8 +6,8 @@ import rippled from '../fixtures/rippled'
 // import binary from 'ripple-binary-codec'
 // import assert from 'assert-diff'
 // import {Client} from 'xrpl-local'
+// import * as schemaValidator from 'xrpl-local/common/schema-validator'
 
-// const {schemaValidator} = Client._PRIVATE
 // const instructionsWithMaxLedgerVersionOffset = {maxLedgerVersionOffset: 100}
 // const {preparePayment: REQUEST_FIXTURES} = requests
 // const {preparePayment: RESPONSE_FIXTURES} = responses

test/client/sign.ts

@@ -1,12 +1,11 @@
 import assert from 'assert-diff'
-import {Client} from 'xrpl-local'
 import binary from 'ripple-binary-codec'
 import requests from '../fixtures/requests'
 import responses from '../fixtures/responses'
 import rippled from '../fixtures/rippled'
 import {TestSuite} from '../testUtils'
+import * as schemaValidator from 'xrpl-local/common/schema-validator'
 
-const {schemaValidator} = Client._PRIVATE
 const {sign: REQUEST_FIXTURES} = requests
 const {sign: RESPONSE_FIXTURES} = responses
 

test/connection.ts

@@ -4,8 +4,8 @@ import assert from 'assert-diff'
 import setupClient from './setupClient'
 import {Client} from 'xrpl-local'
 import {ignoreWebSocketDisconnect} from './testUtils'
+import {Connection} from 'xrpl-local/client'
 import rippled from './fixtures/rippled'
-const utils = Client._PRIVATE.ledgerUtils
 
 const TIMEOUT = 200000 // how long before each test case times out
 const isBrowser = (process as any).browser
@@ -29,7 +29,7 @@ describe('Connection', function () {
   afterEach(setupClient.teardown)
 
   it('default options', function () {
-    const connection: any = new utils.Connection('url')
+    const connection: any = new Connection('url')
     assert.strictEqual(connection._url, 'url')
     assert(connection._config.proxy == null)
     assert(connection._config.authorization == null)
@@ -52,7 +52,7 @@ describe('Connection', function () {
     it('as false', function () {
       const messages = []
       console.log = (id, message) => messages.push([id, message])
-      const connection: any = new utils.Connection('url', {trace: false})
+      const connection: any = new Connection('url', {trace: false})
       connection._ws = {send: function () {}}
       connection.request(mockedRequestData)
       connection._onMessage(mockedResponse)
@@ -62,7 +62,7 @@ describe('Connection', function () {
     it('as true', function () {
       const messages = []
       console.log = (id, message) => messages.push([id, message])
-      const connection: any = new utils.Connection('url', {trace: true})
+      const connection: any = new Connection('url', {trace: true})
       connection._ws = {send: function () {}}
       connection.request(mockedRequestData)
       connection._onMessage(mockedResponse)
@@ -71,7 +71,7 @@ describe('Connection', function () {
 
     it('as a function', function () {
       const messages = []
-      const connection: any = new utils.Connection('url', {
+      const connection: any = new Connection('url', {
         trace: (id, message) => messages.push([id, message])
       })
       connection._ws = {send: function () {}}
@@ -104,7 +104,7 @@ describe('Connection', function () {
         authorization: 'authorization',
         trustedCertificates: ['path/to/pem']
       }
-      const connection = new utils.Connection(
+      const connection = new Connection(
         this.client.connection._url,
         options
       )
@@ -124,7 +124,7 @@ describe('Connection', function () {
   })
 
   it('NotConnectedError', function () {
-    const connection = new utils.Connection('url')
+    const connection = new Connection('url')
     return connection.request({
         command: 'ledger', 
         ledger_index: 'validated'
@@ -148,7 +148,7 @@ describe('Connection', function () {
     }
 
     // Address where no one listens
-    const connection = new utils.Connection(
+    const connection = new Connection(
       'ws://testripple.circleci.com:129'
     )
     connection.on('error', done)
@@ -411,7 +411,7 @@ describe('Connection', function () {
   })
 
   it('Cannot connect because no server', function () {
-    const connection = new utils.Connection(undefined as string)
+    const connection = new Connection(undefined as string)
     return connection
       .connect()
       .then(() => {

test/rangeset.ts

@@ -1,6 +1,5 @@
 import assert from 'assert'
-import {Client} from 'xrpl-local'
+import RangeSet from 'xrpl-local/client/rangeSet'
-const RangeSet = Client._PRIVATE.RangeSet
 
 describe('RangeSet', function () {
   it('addRange()/addValue()', function () {

test/rippleClientPrivate.ts

@@ -6,8 +6,14 @@ import {assertRejects} from './testUtils'
 import addresses from './fixtures/addresses.json'
 import setupClient from './setupClient'
 import {toRippledAmount} from '../src'
+import * as schemaValidator from 'xrpl-local/common/schema-validator'
+import {validate} from 'xrpl-local/common'
+import {
+  renameCounterpartyToIssuerInOrder,
+  compareTransactions,
+  getRecursive
+} from 'xrpl-local/ledger/utils'
 
-const {validate, schemaValidator, ledgerUtils} = Client._PRIVATE
 const address = addresses.ACCOUNT
 assert.options.strict = true
 
@@ -141,23 +147,23 @@ describe('Client', function () {
       taker_pays: {issuer: '1', currency: 'XRP'}
     }
     assert.deepEqual(
-      ledgerUtils.renameCounterpartyToIssuerInOrder(order),
+      renameCounterpartyToIssuerInOrder(order),
       expected
     )
   })
 
   it('ledger utils - compareTransactions', async () => {
     // @ts-ignore
-    assert.strictEqual(ledgerUtils.compareTransactions({}, {}), 0)
+    assert.strictEqual(compareTransactions({}, {}), 0)
     let first: any = {outcome: {ledgerVersion: 1, indexInLedger: 100}}
     let second: any = {outcome: {ledgerVersion: 1, indexInLedger: 200}}
-    assert.strictEqual(ledgerUtils.compareTransactions(first, second), -1)
+    assert.strictEqual(compareTransactions(first, second), -1)
     first = {outcome: {ledgerVersion: 1, indexInLedger: 100}}
     second = {outcome: {ledgerVersion: 1, indexInLedger: 100}}
-    assert.strictEqual(ledgerUtils.compareTransactions(first, second), 0)
+    assert.strictEqual(compareTransactions(first, second), 0)
     first = {outcome: {ledgerVersion: 1, indexInLedger: 200}}
     second = {outcome: {ledgerVersion: 1, indexInLedger: 100}}
-    assert.strictEqual(ledgerUtils.compareTransactions(first, second), 1)
+    assert.strictEqual(compareTransactions(first, second), 1)
   })
 
   it('ledger utils - getRecursive', async () => {
@@ -170,6 +176,6 @@ describe('Client', function () {
         resolve({marker: 'A', results: [1]})
       })
     }
-    await assertRejects(ledgerUtils.getRecursive(getter, 10), Error)
+    await assertRejects(getRecursive(getter, 10), Error)
   })
 })

test/wallet/signTransaction.ts

@@ -1,8 +1,7 @@
-import {Client} from 'xrpl-local'
 import {TestSuite} from '../testUtils'
 import Wallet from '../../src/Wallet'
+import * as schemaValidator from 'xrpl-local/common/schema-validator'
 
-const {schemaValidator} = Client._PRIVATE
 const publicKey =
   '030E58CDD076E798C84755590AAF6237CA8FAE821070A59F648B517A30DC6F589D'
 const privateKey =

webpack.config.js

@@ -16,7 +16,7 @@ function getDefaultConfiguration() {
     filename: `ripple-lib.default.js`,
   },
   plugins: [
-    new webpack.NormalModuleReplacementPlugin(/^ws$/, './wswrapper'),
+    new webpack.NormalModuleReplacementPlugin(/^ws$/, './wsWrapper'),
     new webpack.NormalModuleReplacementPlugin(/^\.\/wallet$/, './wallet-web'),
     new webpack.NormalModuleReplacementPlugin(/^.*setup-api$/, './setup-api-web'),
     new webpack.ProvidePlugin({ process: 'process/browser' }),