From 9d535b949f1709b950d5337fe1b187173cb150fe Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Mon, 20 Apr 2020 16:29:41 -0230 Subject: [PATCH] Rename recipientBlacklistChecker function (#8365) --- app/scripts/controllers/transactions/index.js | 6 +- .../lib/recipient-blacklist-checker.js | 17 ++-- .../recipient-blacklist-checker-test.js | 84 ++++++++----------- 3 files changed, 41 insertions(+), 66 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 6774e93ba..ec297c0df 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -25,7 +25,7 @@ import NonceTracker from 'nonce-tracker' import * as txUtils from './lib/util' import cleanErrorStack from '../../lib/cleanErrorStack' import log from 'loglevel' -import recipientBlacklistChecker from './lib/recipient-blacklist-checker' +import { throwIfAccountIsBlacklisted } from './lib/recipient-blacklist-checker' import { TRANSACTION_TYPE_CANCEL, @@ -234,9 +234,7 @@ class TransactionController extends EventEmitter { this.emit('newUnapprovedTx', txMeta) try { - // check whether recipient account is blacklisted - recipientBlacklistChecker.checkAccount(txMeta.metamaskNetworkId, normalizedTxParams.to) - // add default tx params + throwIfAccountIsBlacklisted(txMeta.metamaskNetworkId, normalizedTxParams.to) txMeta = await this.addTxGasDefaults(txMeta, getCodeResponse) } catch (error) { log.warn(error) diff --git a/app/scripts/controllers/transactions/lib/recipient-blacklist-checker.js b/app/scripts/controllers/transactions/lib/recipient-blacklist-checker.js index c98e84361..8d048c58f 100644 --- a/app/scripts/controllers/transactions/lib/recipient-blacklist-checker.js +++ b/app/scripts/controllers/transactions/lib/recipient-blacklist-checker.js @@ -1,17 +1,12 @@ import blacklist from './recipient-blacklist' -/** @module*/ -export default { - checkAccount, -} - /** - * Checks if a specified account on a specified network is blacklisted. - @param {number} networkId - @param {string} account -*/ -function checkAccount (networkId, account) { - + * Checks if a specified account on a specified network is blacklisted + * @param {number} networkId + * @param {string} account + * @throws {Error} if the account is blacklisted on mainnet + */ +export function throwIfAccountIsBlacklisted (networkId, account) { const mainnetId = 1 if (networkId !== mainnetId) { return diff --git a/test/unit/app/controllers/transactions/recipient-blacklist-checker-test.js b/test/unit/app/controllers/transactions/recipient-blacklist-checker-test.js index c6adfad78..0f0a37074 100644 --- a/test/unit/app/controllers/transactions/recipient-blacklist-checker-test.js +++ b/test/unit/app/controllers/transactions/recipient-blacklist-checker-test.js @@ -1,71 +1,53 @@ import { strict as assert } from 'assert' -import recipientBlackListChecker from '../../../../../app/scripts/controllers/transactions/lib/recipient-blacklist-checker' +import { throwIfAccountIsBlacklisted } from '../../../../../app/scripts/controllers/transactions/lib/recipient-blacklist-checker' import { ROPSTEN_CODE, RINKEBY_CODE, KOVAN_CODE, GOERLI_CODE } from '../../../../../app/scripts/controllers/network/enums' -import KeyringController from 'eth-keyring-controller' describe('Recipient Blacklist Checker', function () { - describe('#checkAccount', function () { - let publicAccounts - - before(async function () { - const damnedMnemonic = 'candy maple cake sugar pudding cream honey rich smooth crumble sweet treat' - const keyringController = new KeyringController({}) - const Keyring = keyringController.getKeyringClassForType('HD Key Tree') - const opts = { - mnemonic: damnedMnemonic, - numberOfAccounts: 10, - } - const keyring = new Keyring(opts) - publicAccounts = await keyring.getAccounts() - }) + describe('#throwIfAccountIsBlacklisted', function () { + // Accounts from Ganache's original default seed phrase + const publicAccounts = [ + '0x627306090abab3a6e1400e9345bc60c78a8bef57', + '0xf17f52151ebef6c7334fad080c5704d77216b732', + '0xc5fdf4076b8f3a5357c5e395ab970b5b54098fef', + '0x821aea9a577a9b44299b9c15c88cf3087f3b5544', + '0x0d1d4e623d10f9fba5db95830f7d3839406c6af2', + '0x2932b7a2355d6fecc4b5c0b6bd44cc31df247a2e', + '0x2191ef87e392377ec08e7c08eb105ef5448eced5', + '0x0f4f2ac550a1b4e2280d04c21cea7ebd822934b5', + '0x6330a553fc93768f612722bb8c2ec78ac90b3bbc', + '0x5aeda56215b167893e80b4fe645ba6d5bab767de', + ] it('does not fail on test networks', function () { - let callCount = 0 const networks = [ROPSTEN_CODE, RINKEBY_CODE, KOVAN_CODE, GOERLI_CODE] for (const networkId of networks) { - publicAccounts.forEach((account) => { - recipientBlackListChecker.checkAccount(networkId, account) - callCount++ - }) + for (const account of publicAccounts) { + assert.doesNotThrow(() => throwIfAccountIsBlacklisted(networkId, account)) + } } - assert.equal(callCount, 40) }) it('fails on mainnet', function () { - const mainnetId = 1 - let callCount = 0 - publicAccounts.forEach((account) => { - try { - recipientBlackListChecker.checkAccount(mainnetId, account) - assert.fail('function should have thrown an error') - } catch (err) { - assert.equal(err.message, 'Recipient is a public account') - } - callCount++ - }) - assert.equal(callCount, 10) + for (const account of publicAccounts) { + assert.throws( + () => throwIfAccountIsBlacklisted(1, account), + { message: 'Recipient is a public account' }, + ) + } }) it('fails for public account - uppercase', function () { - const mainnetId = 1 - const publicAccount = '0X0D1D4E623D10F9FBA5DB95830F7D3839406C6AF2' - try { - recipientBlackListChecker.checkAccount(mainnetId, publicAccount) - assert.fail('function should have thrown an error') - } catch (err) { - assert.equal(err.message, 'Recipient is a public account') - } + assert.throws( + () => throwIfAccountIsBlacklisted(1, '0X0D1D4E623D10F9FBA5DB95830F7D3839406C6AF2'), + { message: 'Recipient is a public account' }, + ) }) - it('fails for public account - lowercase', async function () { - const mainnetId = 1 - const publicAccount = '0x0d1d4e623d10f9fba5db95830f7d3839406c6af2' - try { - await recipientBlackListChecker.checkAccount(mainnetId, publicAccount) - assert.fail('function should have thrown an error') - } catch (err) { - assert.equal(err.message, 'Recipient is a public account') - } + it('fails for public account - lowercase', function () { + assert.throws( + () => throwIfAccountIsBlacklisted(1, '0x0d1d4e623d10f9fba5db95830f7d3839406c6af2'), + { message: 'Recipient is a public account' }, + ) }) }) })