Remove recipient blocklist checker (#8943)

It seems that this blocklist checker never worked correctly. Ever since
the initial commit, it was comparing the Number `1` to the `networkId`,
which is a string. Additionally, even if it did throw, the transaction
continued unhindered. The user could still approve it, and there was no
indication shown to the user that anything went wrong. Also some of the
blocklist entries were incorrectly mixed-case, and were never hit.

We can remove this for now, and re-add it later on after we rewrite the
transaction controller.
feature/default_network_editable
Mark Stacey 4 years ago committed by GitHub
parent 652db3fd36
commit 8ff1d05df3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      app/scripts/controllers/transactions/index.js
  2. 19
      app/scripts/controllers/transactions/lib/recipient-blocklist-checker.js
  3. 17
      app/scripts/controllers/transactions/lib/recipient-blocklist.js
  4. 53
      test/unit/app/controllers/transactions/recipient-blocklist-checker-test.js
  5. 17
      test/unit/app/controllers/transactions/tx-controller-test.js

@ -25,7 +25,6 @@ import NonceTracker from 'nonce-tracker'
import * as txUtils from './lib/util'
import cleanErrorStack from '../../lib/cleanErrorStack'
import log from 'loglevel'
import { throwIfAccountIsBlocked } from './lib/recipient-blocklist-checker'
import {
TRANSACTION_TYPE_CANCEL,
@ -246,7 +245,6 @@ export default class TransactionController extends EventEmitter {
this.emit('newUnapprovedTx', txMeta)
try {
throwIfAccountIsBlocked(txMeta.metamaskNetworkId, normalizedTxParams.to)
txMeta = await this.addTxGasDefaults(txMeta, getCodeResponse)
} catch (error) {
log.warn(error)

@ -1,19 +0,0 @@
import blocklist from './recipient-blocklist'
import { MAINNET_NETWORK_ID } from '../../network/enums'
/**
* Checks if a specified account on a specified network is blocked
* @param {number} networkId
* @param {string} account
* @throws {Error} if the account is blocked on mainnet
*/
export function throwIfAccountIsBlocked (networkId, account) {
if (networkId !== MAINNET_NETWORK_ID) {
return
}
const accountToCheck = account.toLowerCase()
if (blocklist.includes(accountToCheck)) {
throw new Error('Recipient is a public account')
}
}

@ -1,17 +0,0 @@
const blocklist = [
// IDEX phisher
'0x9bcb0A9d99d815Bb87ee3191b1399b1Bcc46dc77',
// Ganache default seed phrases
'0x627306090abab3a6e1400e9345bc60c78a8bef57',
'0xf17f52151ebef6c7334fad080c5704d77216b732',
'0xc5fdf4076b8f3a5357c5e395ab970b5b54098fef',
'0x821aea9a577a9b44299b9c15c88cf3087f3b5544',
'0x0d1d4e623d10f9fba5db95830f7d3839406c6af2',
'0x2932b7a2355d6fecc4b5c0b6bd44cc31df247a2e',
'0x2191ef87e392377ec08e7c08eb105ef5448eced5',
'0x0f4f2ac550a1b4e2280d04c21cea7ebd822934b5',
'0x6330a553fc93768f612722bb8c2ec78ac90b3bbc',
'0x5aeda56215b167893e80b4fe645ba6d5bab767de',
]
export default blocklist

@ -1,53 +0,0 @@
import { strict as assert } from 'assert'
import { throwIfAccountIsBlocked } from '../../../../../app/scripts/controllers/transactions/lib/recipient-blocklist-checker'
import { ROPSTEN_NETWORK_ID, RINKEBY_NETWORK_ID, KOVAN_NETWORK_ID, GOERLI_NETWORK_ID } from '../../../../../app/scripts/controllers/network/enums'
describe('Recipient Blocklist Checker', function () {
describe('#throwIfAccountIsBlocked', 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 () {
const networks = [ROPSTEN_NETWORK_ID, RINKEBY_NETWORK_ID, KOVAN_NETWORK_ID, GOERLI_NETWORK_ID]
for (const networkId of networks) {
for (const account of publicAccounts) {
assert.doesNotThrow(() => throwIfAccountIsBlocked(networkId, account))
}
}
})
it('fails on mainnet', function () {
for (const account of publicAccounts) {
assert.throws(
() => throwIfAccountIsBlocked('1', account),
{ message: 'Recipient is a public account' },
)
}
})
it('fails for public account - uppercase', function () {
assert.throws(
() => throwIfAccountIsBlocked('1', '0X0D1D4E623D10F9FBA5DB95830F7D3839406C6AF2'),
{ message: 'Recipient is a public account' },
)
})
it('fails for public account - lowercase', function () {
assert.throws(
() => throwIfAccountIsBlocked('1', '0x0d1d4e623d10f9fba5db95830f7d3839406c6af2'),
{ message: 'Recipient is a public account' },
)
})
})
})

@ -198,27 +198,10 @@ describe('Transaction Controller', function () {
.catch(done)
})
it('should fail if recipient is public', async function () {
txController.networkStore = new ObservableStore('1')
await assert.rejects(
() => txController.addUnapprovedTransaction({ from: selectedAddress, to: '0x0d1d4e623D10F9FBA5Db95830F7d3839406C6AF2' }),
{ message: 'Recipient is a public account' },
)
})
it("should fail if the from address isn't the selected address", async function () {
await assert.rejects(() => txController.addUnapprovedTransaction({ from: '0x0d1d4e623D10F9FBA5Db95830F7d3839406C6AF2' }))
})
it('should not fail if recipient is public but not on mainnet', function (done) {
txController.once('newUnapprovedTx', (txMetaFromEmit) => {
assert.ok(txMetaFromEmit, 'txMeta is falsy')
done()
})
txController.addUnapprovedTransaction({ from: selectedAddress, to: '0x0d1d4e623D10F9FBA5Db95830F7d3839406C6AF2' })
.catch(done)
})
it('should fail if netId is loading', async function () {
txController.networkStore = new ObservableStore('loading')
await assert.rejects(

Loading…
Cancel
Save