From fe919315d4c321791d41331fa946209b60f9dddf Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 13 Sep 2021 15:45:32 -0400 Subject: [PATCH] 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 | 68 ++--- src/wallet/generateFaucetWallet.ts | 247 +++++++++++------- src/wallet/index.ts | 78 ++++-- .../generateFaucetWallet.ts} | 5 +- test/wallet/index.ts | 3 +- 5 files changed, 244 insertions(+), 157 deletions(-) rename test/{walletGeneration.ts => wallet/generateFaucetWallet.ts} (85%) diff --git a/.eslintrc.js b/.eslintrc.js index f0abaa73..02a8bbd6 100644 --- a/.eslintrc.js +++ b/.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": [ - "error", + '@typescript-eslint/naming-convention': [ + 'error', { - selector: "interface", - format: ["PascalCase", 'snake_case'], + selector: 'interface', + format: ['PascalCase', 'snake_case'], }, ], - "max-statements": ["warn", 25], - "id-length": ["error", { exceptions: ["_"] }], // exception for lodash + 'max-lines-per-function': [ + '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", - "@typescript-eslint/no-magic-numbers": "off", + 'no-magic-number': '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-call": "off", - "@typescript-eslint/consistent-type-assertions": "off", + '@typescript-eslint/no-unsafe-assignment': 'off', + '@typescript-eslint/no-unsafe-call': '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": [ - "error", + 'node/no-extraneous-import': [ + '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/no-explicit-any": "off", + '@typescript-eslint/consistent-type-assertions': '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', }, }, ], -}; +} diff --git a/src/wallet/generateFaucetWallet.ts b/src/wallet/generateFaucetWallet.ts index 117738b6..2251df3d 100644 --- a/src/wallet/generateFaucetWallet.ts +++ b/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 -const MAX_ATTEMPTS = 20 // Maximum attempts to retrieve a balance +// Interval to check an account 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). -// -// @param client - Client. -// @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. -// @throws When either Client isn't connected or unable to fund wallet address. -// z +/** + * Generates a random wallet with some amount of XRP (usually 1000 XRP). + * + * @param client - Client. + * @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. + * @throws When either Client isn't connected or unable to fund wallet address. + */ async function generateFaucetWallet( client: Client, wallet?: Wallet, -): Promise { +): Promise { 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 = { - hostname: faucetUrl, - port: 443, - path: '/accounts', - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'Content-Length': body ? body.length : 0, - }, - } + const options = getOptions(client, postBody) return new Promise((resolve, reject) => { - const request = https.request(options, (response) => { - const chunks: any[] = [] - response.on('data', (d) => { - chunks.push(d) - }) - response.on('end', async () => { - const body = Buffer.concat(chunks).toString() - - // "application/json; charset=utf-8" - if (response.headers['content-type']?.startsWith('application/json')) { - const faucetWallet: FaucetWallet = JSON.parse(body) - const classicAddress = faucetWallet.account.classicAddress - - 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, - }) - } - }) + const request = httpsRequest(options, (response) => { + const chunks: Uint8Array[] = [] + response.on('data', (data) => chunks.push(data)) + // eslint-disable-next-line @typescript-eslint/no-misused-promises -- not actually misused, different resolve/reject + response.on('end', async () => + onEnd( + response, + chunks, + client, + startingBalance, + fundWallet, + resolve, + reject, + ), + ) }) // 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 { + 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 { + // 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 { 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( - new errors.XRPLFaucetError( - `Unable to check if the address ${address} balance has increased. Error: ${err}`, - ), - ) + if (err instanceof Error) { + reject( + 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 } diff --git a/src/wallet/index.ts b/src/wallet/index.ts index 22e30d48..e306a84a 100644 --- a/src/wallet/index.ts +++ b/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 - readonly privateKey: string - readonly classicAddress: string - readonly seed?: string - private static readonly defaultAlgorithm: ECDSA = ECDSA.ed25519 - private static readonly defaultDerivationPath: string = "m/44'/144'/0'/0/0" + public readonly publicKey: string + public readonly privateKey: string + public readonly classicAddress: string + public readonly seed?: string - 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 privateKey = Wallet.hexFromBuffer(node.privateKey) + const publicKey = hexFromBuffer(node.publicKey) + 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( - transaction: any, // TODO: transaction should be typed with Transaction type. + public signTransaction( + 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 { - return classicAddressToXAddress(this.classicAddress, tag, test) + public getXAddress(tag: number | false = false, isTestnet = false): string { + 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) } } diff --git a/test/walletGeneration.ts b/test/wallet/generateFaucetWallet.ts similarity index 85% rename from test/walletGeneration.ts rename to test/wallet/generateFaucetWallet.ts index 75a78e69..b1677f69 100644 --- a/test/walletGeneration.ts +++ b/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) diff --git a/test/wallet/index.ts b/test/wallet/index.ts index 610af7d1..7f28e809 100644 --- a/test/wallet/index.ts +++ b/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',