MetaMask / MetaMask/metamask-extension
Deletion of transactions from state should include nonce grouped transactions with Dropped or Rejected statuses
@jpuri is already working on this.
Since Mar 7, 2022.
- Dominant language
- TypeScript
- Stars
- 13.2k
- Forks
- 5.6k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 451
Description
Currently, we have the following logic in the tx state manager for deleting transactions from state upon the addition of a new transaction:
```
// checks if the length of the tx history is longer then desired persistence
// limit and then if it is removes the oldest confirmed or rejected tx.
// Pending or unapproved transactions will not be removed by this
// operation. For safety of presenting a fully functional transaction UI
// representation, this function will not break apart transactions with the
// same nonce, per network. Not accounting for transactions of the same
// nonce and network combo can result in confusing or broken experiences
// in the UI.
//
// TODO: we are already limiting what we send to the UI, and in the future
// we will send UI only collected groups of transactions *per page* so at
// some point in the future, this persistence limit can be adjusted. When
// we do that I think we should figure out a better storage solution for
// transaction history entries.
const nonceNetworkSet = new Set();
const txsToDelete = transactions
.reverse()
.filter((tx) => {
const { nonce } = tx.txParams;
const { chainId, metamaskNetworkId, status } = tx;
const key = `${nonce}-${chainId ?? metamaskNetworkId}`;
if (nonceNetworkSet.has(key)) {
return false;
} else if (
nonceNetworkSet.size < txHistoryLimit - 1 ||
getFinalStates().includes(status) === false
) {
nonceNetworkSet.add(key);
return false;
}
return true;
})
.map((tx) => tx.id);
this._deleteTransactions(txsToDelete);
```
This ultimately ends up preserving many transactions that share the same nonce in state. The problem with this is that these transactions are never deleted, but the `txHistoryLimit` remains at 40, so only transactions with a unique nonce per chain are deleted from state. This leads to new transactions being deleted from state relatively quickly. This can make it hard to diagnose problems users have had with past transactions.
We can clear more room within our `txHistoryLimit` by implementing a strategy by which we can successfully delete transactions that share nonces with other transactions from the same change (in the users state). A proposed strategy is: allow transactions that share a nonce+chainId with another transaction to be deleted if that transaction's status is `DROPPED` or `REJECTED`
This will cause old transactions of relatively little relevance to the user to be deleted from state on a regular basis.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.