Lints src/wallet and test/wallet (#1600)

* lint src/wallet/index

* lint generateFaucetWallet

* lint tests

* respond to comments

* change max-lines-per-function to 40

* remove * import

* fix TS issues

.eslintrc.js

@@ -2,14 +2,14 @@ module.exports = {
   root: true,
 
   // Make ESLint compatible with TypeScript
-  parser: "@typescript-eslint/parser",
+  parser: '@typescript-eslint/parser',
   parserOptions: {
     // Enable linting rules with type information from our tsconfig
     tsconfigRootDir: __dirname,
-    project: ["./tsconfig.eslint.json"],
+    project: ['./tsconfig.eslint.json'],
 
     // Allow the use of imports / ES modules
-    sourceType: "module",
+    sourceType: 'module',
 
     ecmaFeatures: {
       // Enable global strict mode
@@ -24,75 +24,79 @@ module.exports = {
   },
 
   plugins: [],
-  extends: ["@xrplf/eslint-config/base", "plugin:mocha/recommended"],
+  extends: ['@xrplf/eslint-config/base', 'plugin:mocha/recommended'],
   rules: {
     // Certain rippled APIs require snake_case naming
-    "@typescript-eslint/naming-convention": [
+    '@typescript-eslint/naming-convention': [
-      "error",
+      'error',
       {
-        selector: "interface",
+        selector: 'interface',
-        format: ["PascalCase", 'snake_case'],
+        format: ['PascalCase', 'snake_case'],
       },
     ],
-    "max-statements": ["warn", 25],
+    'max-lines-per-function': [
-    "id-length": ["error", { exceptions: ["_"] }], // exception for lodash
+      'warn',
+      { max: 40, skipBlankLines: 'true', skipComments: 'true' },
+    ],
+    'max-statements': ['warn', 25],
+    'id-length': ['error', { exceptions: ['_'] }], // exception for lodash
   },
   overrides: [
     {
-      files: ["test/**/*.ts"],
+      files: ['test/**/*.ts'],
       rules: {
         // Removed the max for test files and test helper files, since tests usually need to import more things
-        "import/max-dependencies": "off",
+        'import/max-dependencies': 'off',
 
         // describe blocks count as a function in Mocha tests, and can be insanely long
-        "max-lines-per-function": "off",
+        'max-lines-per-function': 'off',
 
         // Tests can be very long turns off max-line count
-        "max-lines": "off",
+        'max-lines': 'off',
 
         // We have lots of statements in tests
-        "max-statements": "off",
+        'max-statements': 'off',
 
         // We have lots of magic numbers in tests
-        "no-magic-number": "off",
+        'no-magic-number': 'off',
-        "@typescript-eslint/no-magic-numbers": "off",
+        '@typescript-eslint/no-magic-numbers': 'off',
 
         // We need to test things without type guards sometimes
-        "@typescript-eslint/no-unsafe-assignment": "off",
+        '@typescript-eslint/no-unsafe-assignment': 'off',
-        "@typescript-eslint/no-unsafe-call": "off",
+        '@typescript-eslint/no-unsafe-call': 'off',
-        "@typescript-eslint/consistent-type-assertions": "off",
+        '@typescript-eslint/consistent-type-assertions': 'off',
 
         // We need to mess with internal things to generate certain testing situations
-        "@typescript-eslint/no-unsafe-member-access": "off",
+        '@typescript-eslint/no-unsafe-member-access': 'off',
 
         // We need to be able to import xrpl-local
-        "node/no-extraneous-import": [
+        'node/no-extraneous-import': [
-          "error",
+          'error',
           {
-            allowModules: ["xrpl-local"],
+            allowModules: ['xrpl-local'],
           },
         ],
 
         // Tests are already in 2 callbacks, so max 3 is pretty restrictive
-        "max-nested-callbacks": "off",
+        'max-nested-callbacks': 'off',
       },
     },
     {
-      files: ["test/models/*.ts"],
+      files: ['test/models/*.ts'],
       rules: {
-        "@typescript-eslint/consistent-type-assertions": "off",
+        '@typescript-eslint/consistent-type-assertions': 'off',
-        "@typescript-eslint/no-explicit-any": "off",
+        '@typescript-eslint/no-explicit-any': 'off',
       },
     },
     {
-      files: [".eslintrc.js", "jest.config.js"],
+      files: ['.eslintrc.js', 'jest.config.js'],
       rules: {
         // Removed no-commonjs requirement as eslint must be in common js format
-        "import/no-commonjs": "off",
+        'import/no-commonjs': 'off',
 
         // Removed this as eslint prevents us from doing this differently
-        "import/unambiguous": "off",
+        'import/unambiguous': 'off',
       },
     },
   ],
-};
+}

src/wallet/generateFaucetWallet.ts

@@ -1,40 +1,43 @@
-import https = require('https')
+import { IncomingMessage } from 'http'
+import { request as httpsRequest, RequestOptions } from 'https'
 
 import { isValidClassicAddress } from 'ripple-address-codec'
 
 import type { Client } from '..'
-import { errors } from '../common'
 import { RippledError, XRPLFaucetError } from '../common/errors'
 import { GeneratedAddress } from '../utils/generateAddress'
 
 import Wallet from '.'
 
-export interface FaucetWallet {
+interface FaucetWallet {
   account: GeneratedAddress
   amount: number
   balance: number
 }
 
-export enum FaucetNetwork {
+// eslint-disable-next-line no-shadow -- Not previously declared
+enum FaucetNetwork {
   Testnet = 'faucet.altnet.rippletest.net',
   Devnet = 'faucet.devnet.rippletest.net',
 }
 
-const INTERVAL_SECONDS = 1 // Interval to check an account balance
+// Interval to check an account balance
-const MAX_ATTEMPTS = 20 // Maximum attempts to retrieve a balance
+const INTERVAL_SECONDS = 1
+// Maximum attempts to retrieve a balance
+const MAX_ATTEMPTS = 20
 
-//
+/**
-// Generates a random wallet with some amount of XRP (usually 1000 XRP).
+ * Generates a random wallet with some amount of XRP (usually 1000 XRP).
-//
+ *
-// @param client - Client.
+ * @param client - Client.
-// @param wallet - An existing XRPL Wallet to fund, if undefined, a new Wallet will be created.
+ * @param wallet - An existing XRPL Wallet to fund, if undefined, a new Wallet will be created.
-// @returns A Wallet on the Testnet or Devnet that contains some amount of XRP.
+ * @returns A Wallet on the Testnet or Devnet that contains some amount of XRP.
-// @throws When either Client isn't connected or unable to fund wallet address.
+ * @throws When either Client isn't connected or unable to fund wallet address.
-// z
+ */
 async function generateFaucetWallet(
   client: Client,
   wallet?: Wallet,
-): Promise<Wallet | void> {
+): Promise<Wallet | undefined> {
   if (!client.isConnected()) {
     throw new RippledError('Client not connected, cannot call faucet')
   }
@@ -46,7 +49,7 @@ async function generateFaucetWallet(
       : Wallet.generate()
 
   // Create the POST request body
-  const body: Uint8Array | undefined = new TextEncoder().encode(
+  const postBody = new TextEncoder().encode(
     JSON.stringify({
       destination: fundWallet.classicAddress,
     }),
@@ -59,83 +62,32 @@ async function generateFaucetWallet(
 
   // Check the address balance is not undefined and is a number
   const startingBalance =
-    addressToFundBalance && !isNaN(Number(addressToFundBalance))
+    addressToFundBalance && !Number.isNaN(Number(addressToFundBalance))
       ? Number(addressToFundBalance)
       : 0
 
-  const faucetUrl = getFaucetUrl(client)
-
   // Options to pass to https.request
-  const options = {
+  const options = getOptions(client, postBody)
-    hostname: faucetUrl,
-    port: 443,
-    path: '/accounts',
-    method: 'POST',
-    headers: {
-      'Content-Type': 'application/json',
-      'Content-Length': body ? body.length : 0,
-    },
-  }
 
   return new Promise((resolve, reject) => {
-    const request = https.request(options, (response) => {
+    const request = httpsRequest(options, (response) => {
-      const chunks: any[] = []
+      const chunks: Uint8Array[] = []
-      response.on('data', (d) => {
+      response.on('data', (data) => chunks.push(data))
-        chunks.push(d)
+      // eslint-disable-next-line @typescript-eslint/no-misused-promises -- not actually misused, different resolve/reject
-      })
+      response.on('end', async () =>
-      response.on('end', async () => {
+        onEnd(
-        const body = Buffer.concat(chunks).toString()
+          response,
-
+          chunks,
-        // "application/json; charset=utf-8"
+          client,
-        if (response.headers['content-type']?.startsWith('application/json')) {
+          startingBalance,
-          const faucetWallet: FaucetWallet = JSON.parse(body)
+          fundWallet,
-          const classicAddress = faucetWallet.account.classicAddress
+          resolve,
-
+          reject,
-          if (classicAddress) {
+        ),
-            try {
+      )
-              // Check at regular interval if the address is enabled on the XRPL and funded
-              const isFunded = await hasAddressBalanceIncreased(
-                client,
-                classicAddress,
-                startingBalance,
-              )
-
-              if (isFunded) {
-                resolve(fundWallet)
-              } else {
-                reject(
-                  new XRPLFaucetError(
-                    `Unable to fund address with faucet after waiting ${
-                      INTERVAL_SECONDS * MAX_ATTEMPTS
-                    } seconds`,
-                  ),
-                )
-              }
-            } catch (err) {
-              if (err instanceof Error) {
-                reject(new XRPLFaucetError(err.message))
-              }
-
-              reject(err)
-            }
-          } else {
-            reject(
-              new XRPLFaucetError(
-                `The faucet account classic address is undefined`,
-              ),
-            )
-          }
-        } else {
-          reject({
-            statusCode: response.statusCode,
-            contentType: response.headers['content-type'],
-            body,
-          })
-        }
-      })
     })
     // POST the body
-    request.write(body || '')
+    request.write(postBody)
 
     request.on('error', (error) => {
       reject(error)
@@ -145,12 +97,104 @@ async function generateFaucetWallet(
   })
 }
 
+function getOptions(client: Client, postBody: Uint8Array): RequestOptions {
+  return {
+    hostname: getFaucetUrl(client),
+    port: 443,
+    path: '/accounts',
+    method: 'POST',
+    headers: {
+      'Content-Type': 'application/json',
+      'Content-Length': postBody.length,
+    },
+  }
+}
+
+// eslint-disable-next-line max-params -- Helper function created for organizational purposes
+async function onEnd(
+  response: IncomingMessage,
+  chunks: Uint8Array[],
+  client: Client,
+  startingBalance: number,
+  fundWallet: Wallet,
+  resolve: (wallet?: Wallet) => void,
+  reject: (err: ErrorConstructor | Error | unknown) => void,
+): Promise<void> {
+  const body = Buffer.concat(chunks).toString()
+
+  // "application/json; charset=utf-8"
+  if (response.headers['content-type']?.startsWith('application/json')) {
+    await processSuccessfulResponse(
+      client,
+      body,
+      startingBalance,
+      fundWallet,
+      resolve,
+      reject,
+    )
+  } else {
+    reject(
+      new XRPLFaucetError(
+        `Content type is not \`application/json\`: ${JSON.stringify({
+          statusCode: response.statusCode,
+          contentType: response.headers['content-type'],
+          body,
+        })}`,
+      ),
+    )
+  }
+}
+
+// eslint-disable-next-line max-params -- Only used as a helper function
+async function processSuccessfulResponse(
+  client: Client,
+  body: string,
+  startingBalance: number,
+  fundWallet: Wallet,
+  resolve: (wallet?: Wallet) => void,
+  reject: (err: ErrorConstructor | Error | unknown) => void,
+): Promise<void> {
+  // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- We know this is safe and correct
+  const faucetWallet: FaucetWallet = JSON.parse(body)
+  const classicAddress = faucetWallet.account.classicAddress
+
+  if (!classicAddress) {
+    reject(new XRPLFaucetError(`The faucet account is undefined`))
+    return
+  }
+  try {
+    // Check at regular interval if the address is enabled on the XRPL and funded
+    const isFunded = await hasAddressBalanceIncreased(
+      client,
+      classicAddress,
+      startingBalance,
+    )
+
+    if (isFunded) {
+      resolve(fundWallet)
+    } else {
+      reject(
+        new XRPLFaucetError(
+          `Unable to fund address with faucet after waiting ${
+            INTERVAL_SECONDS * MAX_ATTEMPTS
+          } seconds`,
+        ),
+      )
+    }
+  } catch (err) {
+    if (err instanceof Error) {
+      reject(new XRPLFaucetError(err.message))
+    }
+    reject(err)
+  }
+}
+
 /**
  * Retrieves an XRPL address XRP balance.
  *
  * @param client - Client.
  * @param address - XRPL address.
- * @returns
+ * @returns The address's balance of XRP.
  */
 async function getAddressXrpBalance(
   client: Client,
@@ -166,7 +210,12 @@ async function getAddressXrpBalance(
     )
     return xrpBalance[0].value
   } catch (err) {
-    return `Unable to retrieve ${address} balance. Error: ${err}`
+    if (err instanceof Error) {
+      throw new XRPLFaucetError(
+        `Unable to retrieve ${address} balance. Error: ${err.message}`,
+      )
+    }
+    throw err
   }
 }
 
@@ -185,12 +234,13 @@ async function hasAddressBalanceIncreased(
 ): Promise<boolean> {
   return new Promise((resolve, reject) => {
     let attempts = MAX_ATTEMPTS
+    // eslint-disable-next-line @typescript-eslint/no-misused-promises -- Not actually misused here, different resolve
     const interval = setInterval(async () => {
       if (attempts < 0) {
         clearInterval(interval)
         resolve(false)
       } else {
-        attempts--
+        attempts -= 1
       }
 
       try {
@@ -201,11 +251,14 @@ async function hasAddressBalanceIncreased(
         }
       } catch (err) {
         clearInterval(interval)
-        reject(
+        if (err instanceof Error) {
-          new errors.XRPLFaucetError(
+          reject(
-            `Unable to check if the address ${address} balance has increased. Error: ${err}`,
+            new XRPLFaucetError(
-          ),
+              `Unable to check if the address ${address} balance has increased. Error: ${err.message}`,
-        )
+            ),
+          )
+        }
+        reject(err)
       }
     }, INTERVAL_SECONDS * 1000)
   })
@@ -217,7 +270,7 @@ async function hasAddressBalanceIncreased(
  * @param client - Client.
  * @returns A {@link FaucetNetwork}.
  */
-export function getFaucetUrl(client: Client) {
+function getFaucetUrl(client: Client): FaucetNetwork | undefined {
   const connectionUrl = client.connection.getUrl()
 
   // 'altnet' for Ripple Testnet server and 'testnet' for XRPL Labs Testnet server
@@ -233,3 +286,9 @@ export function getFaucetUrl(client: Client) {
 }
 
 export default generateFaucetWallet
+
+const _private = {
+  FaucetNetwork,
+  getFaucetUrl,
+}
+export { _private }

src/wallet/index.ts

@@ -11,23 +11,36 @@ import {
 
 import ECDSA from '../common/ecdsa'
 import { ValidationError } from '../common/errors'
+import { Transaction } from '../models/transactions'
 import { signOffline } from '../transaction/sign'
 import { SignOptions } from '../transaction/types'
 
+const DEFAULT_ALGORITHM: ECDSA = ECDSA.ed25519
+const DEFAULT_DERIVATION_PATH = "m/44'/144'/0'/0/0"
+
+function hexFromBuffer(buffer: Buffer): string {
+  return buffer.toString('hex').toUpperCase()
+}
+
 /**
  * A utility for deriving a wallet composed of a keypair (publicKey/privateKey).
  * A wallet can be derived from either a seed, mnemnoic, or entropy (array of random numbers).
  * It provides functionality to sign/verify transactions offline.
  */
 class Wallet {
-  readonly publicKey: string
+  public readonly publicKey: string
-  readonly privateKey: string
+  public readonly privateKey: string
-  readonly classicAddress: string
+  public readonly classicAddress: string
-  readonly seed?: string
+  public readonly seed?: string
-  private static readonly defaultAlgorithm: ECDSA = ECDSA.ed25519
-  private static readonly defaultDerivationPath: string = "m/44'/144'/0'/0/0"
 
-  constructor(publicKey: string, privateKey: string, seed?: string) {
+  /**
+   * Creates a new Wallet.
+   *
+   * @param publicKey - The public key for the account.
+   * @param privateKey - The private key used for signing transactions for the account.
+   * @param seed - (Optional) The seed used to derive the account keys.
+   */
+  public constructor(publicKey: string, privateKey: string, seed?: string) {
     this.publicKey = publicKey
     this.privateKey = privateKey
     this.classicAddress = deriveAddress(publicKey)
@@ -40,7 +53,7 @@ class Wallet {
    * @param algorithm - The digital signature algorithm to generate an address for.
    * @returns A new Wallet derived from a generated seed.
    */
-  static generate(algorithm: ECDSA = Wallet.defaultAlgorithm): Wallet {
+  public static generate(algorithm: ECDSA = DEFAULT_ALGORITHM): Wallet {
     const seed = generateSeed({ algorithm })
     return Wallet.fromSeed(seed)
   }
@@ -52,9 +65,9 @@ class Wallet {
    * @param algorithm - The digital signature algorithm to generate an address for.
    * @returns A Wallet derived from a seed.
    */
-  static fromSeed(
+  public static fromSeed(
     seed: string,
-    algorithm: ECDSA = Wallet.defaultAlgorithm,
+    algorithm: ECDSA = DEFAULT_ALGORITHM,
   ): Wallet {
     return Wallet.deriveWallet(seed, algorithm)
   }
@@ -65,10 +78,11 @@ class Wallet {
    * @param mnemonic - A string consisting of words (whitespace delimited) used to derive a wallet.
    * @param derivationPath - The path to derive a keypair (publicKey/privateKey) from a seed (that was converted from a mnemonic).
    * @returns A Wallet derived from a mnemonic.
+   * @throws ValidationError if unable to derive private key from mnemonic input.
    */
-  static fromMnemonic(
+  public static fromMnemonic(
     mnemonic: string,
-    derivationPath: string = Wallet.defaultDerivationPath,
+    derivationPath: string = DEFAULT_DERIVATION_PATH,
   ): Wallet {
     const seed = mnemonicToSeedSync(mnemonic)
     const masterNode = fromSeed(seed)
@@ -79,8 +93,8 @@ class Wallet {
       )
     }
 
-    const publicKey = Wallet.hexFromBuffer(node.publicKey)
+    const publicKey = hexFromBuffer(node.publicKey)
-    const privateKey = Wallet.hexFromBuffer(node.privateKey)
+    const privateKey = hexFromBuffer(node.privateKey)
     return new Wallet(publicKey, `00${privateKey}`)
   }
 
@@ -91,9 +105,9 @@ class Wallet {
    * @param algorithm - The digital signature algorithm to generate an address for.
    * @returns A Wallet derived from an entropy.
    */
-  static fromEntropy(
+  public static fromEntropy(
     entropy: Uint8Array | number[],
-    algorithm: ECDSA = Wallet.defaultAlgorithm,
+    algorithm: ECDSA = DEFAULT_ALGORITHM,
   ): Wallet {
     const options = {
       entropy: Uint8Array.from(entropy),
@@ -103,13 +117,16 @@ class Wallet {
     return Wallet.deriveWallet(seed, algorithm)
   }
 
-  private static hexFromBuffer(buffer: Buffer): string {
+  /**
-    return buffer.toString('hex').toUpperCase()
+   * Derive a Wallet from a seed.
-  }
+   *
-
+   * @param seed - The seed used to derive the wallet.
+   * @param algorithm - The algorithm used to do the derivation.
+   * @returns A Wallet derived from the seed.
+   */
   private static deriveWallet(
     seed: string,
-    algorithm: ECDSA = Wallet.defaultAlgorithm,
+    algorithm: ECDSA = DEFAULT_ALGORITHM,
   ): Wallet {
     const { publicKey, privateKey } = deriveKeypair(seed, { algorithm })
     return new Wallet(publicKey, privateKey, seed)
@@ -122,8 +139,8 @@ class Wallet {
    * @param options - Options to include for signing.
    * @returns A signed transaction.
    */
-  signTransaction(
+  public signTransaction(
-    transaction: any, // TODO: transaction should be typed with Transaction type.
+    transaction: Transaction,
     options: SignOptions = { signAs: '' },
   ): string {
     return signOffline(this, JSON.stringify(transaction), options)
@@ -136,7 +153,7 @@ class Wallet {
    * @param signedTransaction - A signed transaction (hex string of signTransaction result) to be verified offline.
    * @returns Returns true if a signedTransaction is valid.
    */
-  verifyTransaction(signedTransaction: string): boolean {
+  public verifyTransaction(signedTransaction: string): boolean {
     const tx = decode(signedTransaction)
     const messageHex: string = encodeForSigning(tx)
     const signature = tx.TxnSignature
@@ -147,14 +164,19 @@ class Wallet {
    * Gets an X-address in Testnet/Mainnet format.
    *
    * @param tag - A tag to be included within the X-address.
-   * @param test - A boolean to indicate if X-address should be in Testnet (true) or Mainnet (false) format.
+   * @param isTestnet - A boolean to indicate if X-address should be in Testnet (true) or Mainnet (false) format.
    * @returns An X-address.
    */
-  getXAddress(tag: number | false = false, test = false): string {
+  public getXAddress(tag: number | false = false, isTestnet = false): string {
-    return classicAddressToXAddress(this.classicAddress, tag, test)
+    return classicAddressToXAddress(this.classicAddress, tag, isTestnet)
   }
 
-  getClassicAddress(): string {
+  /**
+   * Gets the classic address of the account this wallet represents.
+   *
+   * @returns A classic address.
+   */
+  public getClassicAddress(): string {
     return deriveAddress(this.publicKey)
   }
 }

test/wallet/generateFaucetWallet.ts

@@ -1,8 +1,9 @@
 import { assert } from 'chai'
 
-import { getFaucetUrl, FaucetNetwork } from '../src/wallet/generateFaucetWallet'
+import { _private } from '../../src/wallet/generateFaucetWallet'
+import { setupClient, teardownClient } from '../setupClient'
 
-import { setupClient, teardownClient } from './setupClient'
+const { FaucetNetwork, getFaucetUrl } = _private
 
 describe('Get Faucet URL', function () {
   beforeEach(setupClient)

test/wallet/index.ts

@@ -1,6 +1,7 @@
 import { assert } from 'chai'
 
 import ECDSA from '../../src/common/ecdsa'
+import { Payment } from '../../src/models/transactions'
 import Wallet from '../../src/wallet'
 
 /**
@@ -155,7 +156,7 @@ describe('Wallet', function () {
     const address = 'rhvh5SrgBL5V8oeV9EpDuVszeJSSCEkbPc'
 
     it('signs a transaction offline', function () {
-      const txJSON = {
+      const txJSON: Payment = {
         TransactionType: 'Payment',
         Account: address,
         Destination: 'rQ3PTWGLCbPz8ZCicV5tCX3xuymojTng5r',