Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit number of transactions passed outside of TransactionController #9010

Merged
merged 1 commit into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions app/scripts/controllers/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { hexToBn, bnToHex, BnMultiplyByFraction } from '../../lib/util'
import { TRANSACTION_NO_CONTRACT_ERROR_KEY } from '../../../../ui/app/helpers/constants/error-keys'

const SIMPLE_GAS_COST = '0x5208' // Hex for 21000, cost of a simple send.
const MAX_MEMSTORE_TX_LIST_SIZE = 100 // Number of transactions (by unique nonces) to keep in memory
rekmarks marked this conversation as resolved.
Show resolved Hide resolved

/**
Transaction Controller is an aggregate of sub-controllers and trackers
Expand Down Expand Up @@ -789,9 +790,7 @@ export default class TransactionController extends EventEmitter {
*/
_updateMemstore () {
const unapprovedTxs = this.txStateManager.getUnapprovedTxList()
const currentNetworkTxList = this.txStateManager.getFilteredTxList({
metamaskNetworkId: this.getNetwork(),
})
const currentNetworkTxList = this.txStateManager.getTxList(MAX_MEMSTORE_TX_LIST_SIZE)
this.memStore.updateState({ unapprovedTxs, currentNetworkTxList })
}
}
35 changes: 31 additions & 4 deletions app/scripts/controllers/transactions/tx-state-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,39 @@ export default class TransactionStateManager extends EventEmitter {
}

/**
@returns {array} - of txMetas that have been filtered for only the current network
*/
getTxList () {
* Returns the full tx list for the current network
*
* The list is iterated backwards as new transactions are pushed onto it.
*
* @param {number} [limit] a limit for the number of transactions to return
* @returns {Object[]} The {@code txMeta}s, filtered to the current network
*/
getTxList (limit) {
const network = this.getNetwork()
const fullTxList = this.getFullTxList()
return fullTxList.filter((txMeta) => txMeta.metamaskNetworkId === network)

const nonces = new Set()
whymarrh marked this conversation as resolved.
Show resolved Hide resolved
const txs = []
for (let i = fullTxList.length - 1; i > -1; i--) {
const txMeta = fullTxList[i]
if (txMeta.metamaskNetworkId !== network) {
continue
}

if (limit !== undefined) {
const { nonce } = txMeta.txParams
if (!nonces.has(nonce)) {
if (nonces.size < limit) {
nonces.add(nonce)
} else {
continue
}
}
}

txs.unshift(txMeta)
}
return txs
}

/**
Expand Down
214 changes: 214 additions & 0 deletions test/unit/app/controllers/transactions/tx-state-manager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,220 @@ describe('TransactionStateManager', function () {
assert.ok(Array.isArray(result))
assert.equal(result.length, 0)
})

it('should return a full list of transactions', function () {
const submittedTx = {
id: 0,
metamaskNetworkId: currentNetworkId,
time: 0,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x0',
},
status: 'submitted',
}

const confirmedTx = {
id: 3,
metamaskNetworkId: currentNetworkId,
time: 3,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x3',
},
status: 'confirmed',
}

const txm = new TxStateManager({
initState: {
transactions: [
submittedTx,
confirmedTx,
],
},
getNetwork: () => currentNetworkId,
})

assert.deepEqual(txm.getTxList(), [
submittedTx,
confirmedTx,
])
})

it('should return a list of transactions, limited by N unique nonces when there are NO duplicates', function () {
const submittedTx0 = {
id: 0,
metamaskNetworkId: currentNetworkId,
time: 0,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x0',
},
status: 'submitted',
}

const unapprovedTx1 = {
id: 1,
metamaskNetworkId: currentNetworkId,
time: 1,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x1',
},
status: 'unapproved',
}

const approvedTx2 = {
id: 2,
metamaskNetworkId: currentNetworkId,
time: 2,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x2',
},
status: 'approved',
}

const confirmedTx3 = {
id: 3,
metamaskNetworkId: currentNetworkId,
time: 3,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x3',
},
status: 'confirmed',
}

const txm = new TxStateManager({
initState: {
transactions: [
submittedTx0,
unapprovedTx1,
approvedTx2,
confirmedTx3,
],
},
getNetwork: () => currentNetworkId,
})

assert.deepEqual(txm.getTxList(2), [
approvedTx2,
confirmedTx3,
])
})

it('should return a list of transactions, limited by N unique nonces when there ARE duplicates', function () {
const submittedTx0s = [
{
id: 0,
metamaskNetworkId: currentNetworkId,
time: 0,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x0',
},
status: 'submitted',
},
{
id: 0,
metamaskNetworkId: currentNetworkId,
time: 0,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x0',
},
status: 'submitted',
},
]

const unapprovedTx1 = {
id: 1,
metamaskNetworkId: currentNetworkId,
time: 1,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x1',
},
status: 'unapproved',
}

const approvedTx2s = [
{
id: 2,
metamaskNetworkId: currentNetworkId,
time: 2,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x2',
},
status: 'approved',
},
{
id: 2,
metamaskNetworkId: currentNetworkId,
time: 2,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x2',
},
status: 'approved',
},
]

const failedTx3s = [
{
id: 3,
metamaskNetworkId: currentNetworkId,
time: 3,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x3',
},
status: 'failed',
},
{
id: 3,
metamaskNetworkId: currentNetworkId,
time: 3,
txParams: {
from: '0xAddress',
to: '0xRecipient',
nonce: '0x3',
},
status: 'failed',
},
]

const txm = new TxStateManager({
initState: {
transactions: [
...submittedTx0s,
unapprovedTx1,
...approvedTx2s,
...failedTx3s,
],
},
getNetwork: () => currentNetworkId,
})

assert.deepEqual(txm.getTxList(2), [
...approvedTx2s,
...failedTx3s,
])
})
})

describe('#addTx', function () {
Expand Down