Resolves TODOs in ./src/client (#1617)

* fix: fixes TODO's in ./src/client

src/client/broadcastClient.ts

@@ -19,13 +19,13 @@ export default class BroadcastClient extends Client {
       (server) => new Client(server, options),
     )
 
-    // exposed for testing
     this.clients = clients
     this.getMethodNames().forEach((name: string) => {
       this[name] = async (...args): Promise<unknown> =>
-        // eslint-disable-next-line max-len -- Need a long comment, TODO: figure out how to avoid this weirdness
-        /* eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call -- Types are outlined in Client class */
+        /* eslint-disable @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call -- Generates types
+          from the Client */
         Promise.race(clients.map(async (client) => client[name](...args)))
+      /* eslint-enable @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call */
     })
 
     // connection methods must be overridden to apply to all client instances

src/client/connection.ts

@@ -1,8 +1,6 @@
 /* eslint-disable max-lines -- Connection is a big class */
 import { EventEmitter } from 'events'
 import { Agent } from 'http'
-// eslint-disable-next-line node/no-deprecated-api -- TODO: resolve this
-import { parse as parseURL } from 'url'
 
 import _ from 'lodash'
 import WebSocket from 'ws'
@@ -57,35 +55,49 @@ export const INTENTIONAL_DISCONNECT_CODE = 4000
 type WebsocketState = 0 | 1 | 2 | 3
 
 function getAgent(url: string, config: ConnectionOptions): Agent | undefined {
-  // TODO: replace deprecated method
-  if (config.proxy != null) {
-    const parsedURL = parseURL(url)
-    const parsedProxyURL = parseURL(config.proxy)
-    const proxyOverrides = _.omitBy(
-      {
-        secureEndpoint: parsedURL.protocol === 'wss:',
-        secureProxy: parsedProxyURL.protocol === 'https:',
-        auth: config.proxyAuthorization,
-        ca: config.trustedCertificates,
-        key: config.key,
-        passphrase: config.passphrase,
-        cert: config.certificate,
-      },
-      (value) => value == null,
-    )
-    const proxyOptions = { ...parsedProxyURL, ...proxyOverrides }
-    let HttpsProxyAgent
-    try {
-      // eslint-disable-next-line max-len -- Long eslint-disable-next-line TODO: figure out how to make this nicer
-      // eslint-disable-next-line import/max-dependencies, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-require-imports, node/global-require, global-require, -- Necessary for the `require`
-      HttpsProxyAgent = require('https-proxy-agent')
-    } catch (_error) {
-      throw new Error('"proxy" option is not supported in the browser')
-    }
-    // eslint-disable-next-line @typescript-eslint/consistent-type-assertions, @typescript-eslint/no-unsafe-call -- Necessary
-    return new HttpsProxyAgent(proxyOptions) as unknown as Agent
+  if (config.proxy == null) {
+    return undefined
   }
-  return undefined
+
+  const parsedURL = new URL(url)
+  const parsedProxyURL = new URL(config.proxy)
+
+  const proxyOptions = _.omitBy(
+    {
+      secureEndpoint: parsedURL.protocol === 'wss:',
+      secureProxy: parsedProxyURL.protocol === 'https:',
+      auth: config.proxyAuthorization,
+      ca: config.trustedCertificates,
+      key: config.key,
+      passphrase: config.passphrase,
+      cert: config.certificate,
+      href: parsedProxyURL.href,
+      origin: parsedProxyURL.origin,
+      protocol: parsedProxyURL.protocol,
+      username: parsedProxyURL.username,
+      password: parsedProxyURL.password,
+      host: parsedProxyURL.host,
+      hostname: parsedProxyURL.hostname,
+      port: parsedProxyURL.port,
+      pathname: parsedProxyURL.pathname,
+      search: parsedProxyURL.search,
+      hash: parsedProxyURL.hash,
+    },
+    (value) => value == null,
+  )
+
+  let HttpsProxyAgent: new (opt: typeof proxyOptions) => Agent
+  try {
+    /* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-require-imports,
+      node/global-require, global-require, -- Necessary for the `require` */
+    HttpsProxyAgent = require('https-proxy-agent')
+    /* eslint-enable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-require-imports,
+      node/global-require, global-require */
+  } catch (_error) {
+    throw new Error('"proxy" option is not supported in the browser')
+  }
+
+  return new HttpsProxyAgent(proxyOptions)
 }
 
 /**
@@ -244,8 +256,9 @@ export class Connection extends EventEmitter {
     this.ws.on('error', () => clearTimeout(connectionTimeoutID))
     this.ws.on('close', (reason) => this.onConnectionFailed(reason))
     this.ws.on('close', () => clearTimeout(connectionTimeoutID))
-    // eslint-disable-next-line @typescript-eslint/no-misused-promises -- TODO: resolve this
-    this.ws.once('open', async () => this.onceOpen(connectionTimeoutID))
+    this.ws.once('open', () => {
+      void this.onceOpen(connectionTimeoutID)
+    })
     return this.connectionManager.awaitConnection()
   }
 
@@ -473,11 +486,9 @@ export class Connection extends EventEmitter {
    */
   private startHeartbeatInterval(): void {
     this.clearHeartbeatInterval()
-    this.heartbeatIntervalID = setInterval(
-      // eslint-disable-next-line @typescript-eslint/no-misused-promises -- TODO: resolve this
-      async () => this.heartbeat(),
-      this.config.timeout,
-    )
+    this.heartbeatIntervalID = setInterval(() => {
+      void this.heartbeat()
+    }, this.config.timeout)
   }
 
   /**

src/client/index.ts

@@ -1,4 +1,3 @@
-/* eslint-disable @typescript-eslint/member-ordering -- TODO: remove when instance methods aren't members */
 /* eslint-disable max-lines -- This might not be necessary later, but this file needs to be big right now */
 import * as assert from 'assert'
 import { EventEmitter } from 'events'
@@ -138,21 +137,6 @@ function clamp(value: number, min: number, max: number): number {
   return Math.min(Math.max(value, min), max)
 }
 
-/**
- * It returns a function that prepends params to the given func.
- * A sugar function for JavaScript .bind() without the "this" (keyword) binding.
- *
- * @param func - A function to prepend params.
- * @param params - Parameters to prepend to a function.
- * @returns A function bound with params.
- */
-// TODO Need to refactor prepend so TS can infer the correct function signature type
-// eslint-disable-next-line @typescript-eslint/ban-types -- expected param types
-function prepend(func: Function, ...params: unknown[]): Function {
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-return -- safe to return
-  return func.bind(null, ...params)
-}
-
 interface MarkerRequest extends BaseRequest {
   limit?: number
   marker?: unknown
@@ -517,23 +501,23 @@ class Client extends EventEmitter {
     return this.connection.isConnected()
   }
 
-  // TODO: Use prepend for other instance methods as well.
-  public autofill = prepend(autofill, this)
+  public autofill = autofill
+
+  public getFee = getFee
 
   // @deprecated Use autofill instead
-  public prepareTransaction = prepend(autofill, this)
+  public prepareTransaction = autofill
 
-  public getFee = prepend(getFee, this)
-  public submitTransaction = prepend(submitTransaction, this)
+  public submitTransaction = submitTransaction
 
-  public submitSignedTransaction = prepend(submitSignedTransaction, this)
+  public submitSignedTransaction = submitSignedTransaction
 
-  public getBalances = prepend(getBalances, this)
-  public getOrderbook = prepend(getOrderbook, this)
+  public getBalances = getBalances
+  public getOrderbook = getOrderbook
 
   public combine = combine
 
-  public generateFaucetWallet = prepend(generateFaucetWallet, this)
+  public generateFaucetWallet = generateFaucetWallet
 
   public errors = errors
 }

src/client/wsWrapper.ts

@@ -1,4 +1,3 @@
-/* eslint-disable import/no-unused-modules -- This is used by webpack */
 /* eslint-disable max-classes-per-file -- Needs to be a wrapper for ws */
 import { EventEmitter } from 'events'
 
@@ -28,7 +27,7 @@ interface WSWrapperOptions {
  * Provides `EventEmitter` interface for native browser `WebSocket`,
  * same, as `ws` package provides.
  */
-export default class WSWrapper extends EventEmitter {
+class WSWrapper extends EventEmitter {
   public static CONNECTING = 0
   public static OPEN = 1
   public static CLOSING = 2
@@ -97,3 +96,5 @@ export default class WSWrapper extends EventEmitter {
     return this.ws.readyState
   }
 }
+
+export default WSWrapper

src/sugar/autofill.ts

@@ -20,13 +20,13 @@ interface ClassicAccountAndTag {
 /**
  * Autofills fields in a transaction.
  *
- * @param client - A client.
+ * @param this - A client.
  * @param transaction - A transaction to autofill fields.
  * @param signersCount - The expected number of signers for this transaction. Used for multisign.
  * @returns An autofilled transaction.
  */
 async function autofill<T extends Transaction>(
-  client: Client,
+  this: Client,
   transaction: T,
   signersCount?: number,
 ): Promise<T> {
@@ -38,13 +38,13 @@ async function autofill<T extends Transaction>(
 
   const promises: Array<Promise<void>> = []
   if (tx.Sequence == null) {
-    promises.push(setNextValidSequenceNumber(client, tx))
+    promises.push(setNextValidSequenceNumber(this, tx))
   }
   if (tx.Fee == null) {
-    promises.push(calculateFeePerTransactionType(client, tx, signersCount))
+    promises.push(calculateFeePerTransactionType(this, tx, signersCount))
   }
   if (tx.LastLedgerSequence == null) {
-    promises.push(setLatestValidatedLedgerSequence(client, tx))
+    promises.push(setLatestValidatedLedgerSequence(this, tx))
   }
 
   return Promise.all(promises).then(() => tx)

src/sugar/balances.ts

@@ -36,7 +36,7 @@ interface GetBalancesOptions {
  * @returns An array of XRP/non-XRP balances.
  */
 async function getBalances(
-  client: Client,
+  this: Client,
   account: string,
   options: GetBalancesOptions = {},
 ): Promise<Balance[]> {
@@ -47,9 +47,9 @@ async function getBalances(
     ledger_index: options.ledger_index ?? 'validated',
     ledger_hash: options.ledger_hash,
   }
-  const balance = await client
-    .request(xrpRequest)
-    .then((response) => response.result.account_data.Balance)
+  const balance = await this.request(xrpRequest).then(
+    (response) => response.result.account_data.Balance,
+  )
   const xrpBalance = { currency: 'XRP', value: dropsToXrp(balance) }
   // 2. Get Non-XRP Balance
   const linesRequest: AccountLinesRequest = {
@@ -60,7 +60,7 @@ async function getBalances(
     peer: options.peer,
     limit: options.limit,
   }
-  const responses = await client.requestAll(linesRequest)
+  const responses = await this.requestAll(linesRequest)
   const accountLinesBalance = _.flatMap(responses, (response) =>
     formatBalances(response.result.lines),
   )

src/sugar/fee.ts

@@ -15,12 +15,12 @@ const BASE_10 = 10
  * @returns The transaction fee.
  */
 export default async function getFee(
-  client: Client,
+  this: Client,
   cushion?: number,
 ): Promise<string> {
-  const feeCushion = cushion ?? client.feeCushion
+  const feeCushion = cushion ?? this.feeCushion
 
-  const serverInfo = (await client.request({ command: 'server_info' })).result
+  const serverInfo = (await this.request({ command: 'server_info' })).result
     .info
 
   const baseFee = serverInfo.validated_ledger?.base_fee_xrp
@@ -37,7 +37,7 @@ export default async function getFee(
   let fee = baseFeeXrp.times(serverInfo.load_factor).times(feeCushion)
 
   // Cap fee to `client.maxFeeXRP`
-  fee = BigNumber.min(fee, client.maxFeeXRP)
+  fee = BigNumber.min(fee, this.maxFeeXRP)
   // Round fee to 6 decimal places
   return new BigNumber(fee.toFixed(NUM_DECIMAL_PLACES)).toString(BASE_10)
 }

src/sugar/orderbook.ts

@@ -10,6 +10,8 @@ import {
   TakerAmount,
 } from '../models/methods/bookOffers'
 
+const DEFAULT_LIMIT = 20
+
 function sortOffers(offers: BookOffer[]): BookOffer[] {
   return offers.sort((offerA, offerB) => {
     const qualityA = offerA.quality ?? 0
@@ -34,36 +36,33 @@ interface OrderbookOptions {
 /**
  * Fetch orderbook (buy/sell orders) between two accounts.
  *
- * @param client - Client.
- * @param taker_pays - Specs of the currency account taking the offer pays.
- * @param taker_gets - Specs of the currency account taking the offer receives.
- * @param takerPays
- * @param takerGets
+ * @param this - Client.
+ * @param takerPays - Specs of the currency account taking the offer pays.
+ * @param takerGets - Specs of the currency account taking the offer receives.
  * @param options - Options to include for getting orderbook between payer and receiver.
  * @returns An object containing buy and sell objects.
  */
 // eslint-disable-next-line max-params -- Function needs 4 params.
 async function getOrderbook(
-  client: Client,
+  this: Client,
   takerPays: TakerAmount,
   takerGets: TakerAmount,
-  options: OrderbookOptions,
+  options: OrderbookOptions = {},
 ): Promise<Orderbook> {
   const request: BookOffersRequest = {
     command: 'book_offers',
     taker_pays: takerPays,
     taker_gets: takerGets,
-    ledger_index: options.ledger_index,
+    ledger_index: options.ledger_index ?? 'validated',
     ledger_hash: options.ledger_hash,
-    limit: options.limit,
+    limit: options.limit ?? DEFAULT_LIMIT,
     taker: options.taker,
   }
   // 2. Make Request
-  const directOfferResults = await client.requestAll(request)
+  const directOfferResults = await this.requestAll(request)
   request.taker_gets = takerPays
   request.taker_pays = takerGets
-  const reverseOfferResults = await client.requestAll(request)
-
+  const reverseOfferResults = await this.requestAll(request)
   // 3. Return Formatted Response
   const directOffers = _.flatMap(
     directOfferResults,

src/sugar/submit.ts

@@ -5,8 +5,6 @@ import { ValidationError } from '../errors'
 import { Transaction } from '../models/transactions'
 import { sign } from '../wallet/signer'
 
-import autofill from './autofill'
-
 /**
  * Submits an unsigned transaction.
  * Steps performed on a transaction:
@@ -14,33 +12,32 @@ import autofill from './autofill'
  *    2. Sign & Encode.
  *    3. Submit.
  *
- * @param client - A Client.
+ * @param this - A Client.
  * @param wallet - A Wallet to sign a transaction.
  * @param transaction - A transaction to autofill, sign & encode, and submit.
  * @returns A promise that contains SubmitResponse.
  * @throws RippledError if submit request fails.
  */
 async function submitTransaction(
-  client: Client,
+  this: Client,
   wallet: Wallet,
   transaction: Transaction,
 ): Promise<SubmitResponse> {
-  // TODO: replace with client.autofill(transaction) once prepend refactor is fixed.
-  const tx = await autofill(client, transaction)
+  const tx = await this.autofill(transaction)
   const signedTxEncoded = sign(wallet, tx)
-  return submitSignedTransaction(client, signedTxEncoded)
+  return this.submitSignedTransaction(signedTxEncoded)
 }
 
 /**
  * Encodes and submits a signed transaction.
  *
- * @param client - A Client.
+ * @param this - A Client.
  * @param signedTransaction - A signed transaction to encode (if not already) and submit.
  * @returns A promise that contains SubmitResponse.
  * @throws RippledError if submit request fails.
  */
 async function submitSignedTransaction(
-  client: Client,
+  this: Client,
   signedTransaction: Transaction | string,
 ): Promise<SubmitResponse> {
   if (!isSigned(signedTransaction)) {
@@ -55,7 +52,7 @@ async function submitSignedTransaction(
     command: 'submit',
     tx_blob: signedTxEncoded,
   }
-  return client.request(request)
+  return this.request(request)
 }
 
 function isSigned(transaction: Transaction | string): boolean {