1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-11-22 18:00:18 +01:00

Limit number of transactions passed outside of TransactionController (#9010)

Refs #8572
Refs #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 #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]:5a3ae85b72/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]:https://github.com/MetaMask/metamask-extension/issues/8377
  [8572]:https://github.com/MetaMask/metamask-extension/issues/8572
  [8991]:https://github.com/MetaMask/metamask-extension/issues/8991
This commit is contained in:
Whymarrh Whitby 2020-07-16 17:52:41 -02:30 committed by GitHub
parent c8e50c6b15
commit c7fad8f400
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 247 additions and 7 deletions

View File

@ -37,6 +37,7 @@ import { hexToBn, bnToHex, BnMultiplyByFraction } from '../../lib/util'
import { TRANSACTION_NO_CONTRACT_ERROR_KEY } from '../../../../ui/app/helpers/constants/error-keys' 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 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 Transaction Controller is an aggregate of sub-controllers and trackers
@ -789,9 +790,7 @@ export default class TransactionController extends EventEmitter {
*/ */
_updateMemstore () { _updateMemstore () {
const unapprovedTxs = this.txStateManager.getUnapprovedTxList() const unapprovedTxs = this.txStateManager.getUnapprovedTxList()
const currentNetworkTxList = this.txStateManager.getFilteredTxList({ const currentNetworkTxList = this.txStateManager.getTxList(MAX_MEMSTORE_TX_LIST_SIZE)
metamaskNetworkId: this.getNetwork(),
})
this.memStore.updateState({ unapprovedTxs, currentNetworkTxList }) this.memStore.updateState({ unapprovedTxs, currentNetworkTxList })
} }
} }

View File

@ -57,12 +57,39 @@ export default class TransactionStateManager extends EventEmitter {
} }
/** /**
@returns {array} - of txMetas that have been filtered for only the current network * Returns the full tx list for the current network
*/ *
getTxList () { * 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 network = this.getNetwork()
const fullTxList = this.getFullTxList() 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
} }
/** /**

View File

@ -85,6 +85,220 @@ describe('TransactionStateManager', function () {
assert.ok(Array.isArray(result)) assert.ok(Array.isArray(result))
assert.equal(result.length, 0) 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 () { describe('#addTx', function () {