Skip to content

Commit

Permalink
Limit number of transactions passed outside of TransactionController
Browse files Browse the repository at this point in the history 
Refs MetaMask#8572
Refs MetaMask#8991

This change limits the number of transactions (`txMeta`s) that are passed
outside of the `TransactionController`, resulting in shorter serialization and
deserialization times when state is moved between the background and UI
contexts.

`TransactionController#_updateMemstore`
---------------------------------------

The `currentNetworkTxList` state of the `TransactionController` is used
externally (i.e. outside of the controller) as the canonical source for
the full transaction history. Prior to this change, the method would iterate
the full transaction history and possibly return all of it.

This change limits it to `MAX_MEMSTORE_TX_LIST_SIZE` to make sure that:

1. Calls to `_updateMemstore` are fast(er)
2. Passing `currentNetworkTxList` around is fast(er)

(Shown in MetaMask#8377, `_updateMemstore`, is called _frequently_ when a transaction
is pending.)

The list is iterated backwards because it is possible that new transactions are
at the end of the list. [1]

Results
-------

In profiles before this change, with ~3k transactions locally,
`PortDuplexStream._onMessage` took up to ~4.5s to complete when the set of
transactions is included. [2]

In profiles after this change, `PortDuplexStream._onMessage` took ~90ms to
complete. [3]

Before vs. after profile screenshots:

![Profile 1][2]
![Profile 2][3]

  [1]:https://github.com/MetaMask/metamask-extension/blob/5a3ae85b728096cb45c8cc6822249eed5555ee25/app/scripts/controllers/transactions/tx-state-manager.js#L172-L174
  [2]:https://user-images.githubusercontent.com/1623628/87613203-36f51d80-c6e7-11ea-89bc-11a1cc2f3b1e.png
  [3]:https://user-images.githubusercontent.com/1623628/87613215-3bb9d180-c6e7-11ea-8d85-aff3acbd0374.png
  [8337]:MetaMask#8377
  [8572]:MetaMask#8572
  [8991]:MetaMask#8991
  • Loading branch information
@whymarrh
whymarrh committed Jul 16, 2020
1 parent acaa648 commit a0b1ef0
Show file tree
Hide file tree
Showing 3 changed files with 247 additions and 7 deletions.
5 changes: 2 additions & 3 deletions app/scripts/controllers/transactions/index.js
View file
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

/**
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
View file
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()
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
View file
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

0 comments on commit a0b1ef0

Please sign in to comment.