Skip to content

Commit

Permalink
Rename max history size constant and expose it.
Browse files Browse the repository at this point in the history
This will be useful in writing a migration to eliminate huge transaction
histories.
  • Loading branch information
Gudahtt committed Jul 26, 2024
1 parent b116d35 commit e3251bc
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 19 deletions.
2 changes: 2 additions & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add `DISPLAYED_TRANSACTION_HISTORY_PATHS` constant, representing the transaction history paths that may be used for display ([#4555](https://github.com/MetaMask/core/pull/4555))
- This was exported so that it might be used to ensure display logic and internal history logic remains in-sync.
- Any paths listed here will have their timestamps preserved. Unlisted paths may be compressed by the controller to minimize history size, losing the timestamp.
- Add `MAX_TRANSACTION_HISTORY_LENGTH` constant, representing the expected maximum size of the `history` property for a given transaction ([#4555](https://github.com/MetaMask/core/pull/4555))
- Note that this is not strictly enforced, the length may exceed this number of all entries are "displayed" entries, but we expect this to be extremely improbable in practice.

### Fixed

Expand Down
5 changes: 4 additions & 1 deletion packages/transaction-controller/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ export {
WalletDevice,
} from './types';
export type { EtherscanTransactionMeta } from './utils/etherscan';
export { DISPLAYED_TRANSACTION_HISTORY_PATHS } from './utils/history';
export {
DISPLAYED_TRANSACTION_HISTORY_PATHS,
MAX_TRANSACTION_HISTORY_LENGTH,
} from './utils/history';
export { determineTransactionType } from './utils/transaction-type';
export { mergeGasFeeEstimates } from './utils/gas-flow';
export {
Expand Down
52 changes: 36 additions & 16 deletions packages/transaction-controller/src/utils/history.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import {
type TransactionMeta,
type TransactionHistoryEntry,
} from '../types';
import { MAX_HISTORY_LENGTH, updateTransactionHistory } from './history';
import {
MAX_TRANSACTION_HISTORY_LENGTH,
updateTransactionHistory,
} from './history';

describe('History', () => {
describe('updateTransactionHistory', () => {
Expand Down Expand Up @@ -80,7 +83,7 @@ describe('History', () => {
partialTransaction: {
history: generateMockHistory({
originalTransaction,
length: MAX_HISTORY_LENGTH,
length: MAX_TRANSACTION_HISTORY_LENGTH,
}),
// This is the changed value
txParams: { from: generateAddress(123) },
Expand All @@ -94,10 +97,12 @@ describe('History', () => {
note: 'Mock non-displayed change',
op: 'replace',
path: '/txParams/from',
value: generateAddress(MAX_HISTORY_LENGTH - 1),
value: generateAddress(MAX_TRANSACTION_HISTORY_LENGTH - 1),
},
]);
expect(mockTransaction.history).toHaveLength(MAX_HISTORY_LENGTH);
expect(mockTransaction.history).toHaveLength(
MAX_TRANSACTION_HISTORY_LENGTH,
);

const updatedTransaction = updateTransactionHistory(
mockTransaction,
Expand Down Expand Up @@ -132,21 +137,25 @@ describe('History', () => {
],
],
});
expect(updatedTransaction.history).toHaveLength(MAX_HISTORY_LENGTH);
expect(updatedTransaction.history).toHaveLength(
MAX_TRANSACTION_HISTORY_LENGTH,
);
});
});

describe('when history is past max size with a single non-displayed entry at the end', () => {
it('merges a non-displayed entry when adding a new entry after max history size is reached', () => {
const originalTransaction = createMinimalMockTransaction();
// This matches the last gas price change in the mock history
const mockTransactionGasPrice = toHex(MAX_HISTORY_LENGTH - 1);
const mockTransactionGasPrice = toHex(
MAX_TRANSACTION_HISTORY_LENGTH - 1,
);
const mockTransaction = createMockTransaction({
partialTransaction: {
history: generateMockHistory({
numberOfDisplayEntries: MAX_HISTORY_LENGTH - 1,
numberOfDisplayEntries: MAX_TRANSACTION_HISTORY_LENGTH - 1,
originalTransaction,
length: MAX_HISTORY_LENGTH,
length: MAX_TRANSACTION_HISTORY_LENGTH,
}),
txParams: {
// This is the changed value
Expand All @@ -168,7 +177,9 @@ describe('History', () => {
},
]);
const mockTransactionClone = cloneDeep(mockTransaction);
expect(mockTransaction.history).toHaveLength(MAX_HISTORY_LENGTH);
expect(mockTransaction.history).toHaveLength(
MAX_TRANSACTION_HISTORY_LENGTH,
);

const updatedTransaction = updateTransactionHistory(
mockTransaction,
Expand All @@ -179,7 +190,10 @@ describe('History', () => {
expect(updatedTransaction).toStrictEqual({
...mockTransaction,
history: [
...mockTransactionClone.history.slice(0, MAX_HISTORY_LENGTH - 1),
...mockTransactionClone.history.slice(
0,
MAX_TRANSACTION_HISTORY_LENGTH - 1,
),
// This is the new merged entry:
[
{
Expand All @@ -197,7 +211,9 @@ describe('History', () => {
],
],
});
expect(updatedTransaction.history).toHaveLength(MAX_HISTORY_LENGTH);
expect(updatedTransaction.history).toHaveLength(
MAX_TRANSACTION_HISTORY_LENGTH,
);
});
});

Expand All @@ -207,9 +223,9 @@ describe('History', () => {
const mockTransaction = createMockTransaction({
partialTransaction: {
history: generateMockHistory({
numberOfDisplayEntries: MAX_HISTORY_LENGTH - 1,
numberOfDisplayEntries: MAX_TRANSACTION_HISTORY_LENGTH - 1,
originalTransaction,
length: MAX_HISTORY_LENGTH,
length: MAX_TRANSACTION_HISTORY_LENGTH,
}),
txParams: {
from: originalTransaction.txParams.from,
Expand All @@ -226,10 +242,12 @@ describe('History', () => {
note: 'Mock displayed change',
op: 'replace',
path: '/txParams/gasPrice',
value: toHex(MAX_HISTORY_LENGTH - 1),
value: toHex(MAX_TRANSACTION_HISTORY_LENGTH - 1),
},
]);
expect(mockTransaction.history).toHaveLength(MAX_HISTORY_LENGTH);
expect(mockTransaction.history).toHaveLength(
MAX_TRANSACTION_HISTORY_LENGTH,
);

const updatedTransaction = updateTransactionHistory(
mockTransaction,
Expand All @@ -253,7 +271,9 @@ describe('History', () => {
],
],
});
expect(updatedTransaction.history).toHaveLength(MAX_HISTORY_LENGTH + 1);
expect(updatedTransaction.history).toHaveLength(
MAX_TRANSACTION_HISTORY_LENGTH + 1,
);
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/transaction-controller/src/utils/history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {
/**
* The maximum allowed length of the `transaction.history` property.
*/
export const MAX_HISTORY_LENGTH = 100;
export const MAX_TRANSACTION_HISTORY_LENGTH = 100;

/**
* A list of trarnsaction history paths that may be used for display. These entries will not be
Expand Down Expand Up @@ -75,7 +75,7 @@ export function updateTransactionHistory(
newHistoryEntry,
] as TransactionHistory;

if (updatedHistory.length > MAX_HISTORY_LENGTH) {
if (updatedHistory.length > MAX_TRANSACTION_HISTORY_LENGTH) {
updatedHistory = compressTransactionHistory(updatedHistory);
}

Expand Down

0 comments on commit e3251bc

Please sign in to comment.