Skip to content

Commit

Permalink
fix: transaction resubmit on multiple endpoints (#5262)
Browse files Browse the repository at this point in the history
## Explanation

Update `PendingTransactionTracker` to filter pending transactions using
the network client ID of the instance.

This prevents multiple instances with different network client IDs, but
the same chain, from processing the same transactions.

## References

## Changelog

### `@metamask/transaction-controller`

- **FIXED**: Prevent resubmit on multiple endpoints.

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
  • Loading branch information
matthewwalsh0 authored Feb 4, 2025
1 parent 97958dd commit 78c420e
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 12 deletions.
2 changes: 1 addition & 1 deletion eslint-warning-thresholds.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"import-x/namespace": 189,
"import-x/no-named-as-default": 1,
"import-x/no-named-as-default-member": 8,
"import-x/order": 206,
"import-x/order": 202,
"jest/no-conditional-in-test": 113,
"jest/prefer-lowercase-title": 2,
"jest/prefer-strict-equal": 2,
Expand Down
3 changes: 3 additions & 0 deletions packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3320,17 +3320,20 @@ export class TransactionController extends BaseController<
provider,
blockTracker,
chainId,
networkClientId,
}: {
provider: Provider;
blockTracker: BlockTracker;
chainId: Hex;
networkClientId: NetworkClientId;
}): PendingTransactionTracker {
const ethQuery = new EthQuery(provider);

const pendingTransactionTracker = new PendingTransactionTracker({
blockTracker,
getChainId: () => chainId,
getEthQuery: () => ethQuery,
getNetworkClientId: () => networkClientId,
getTransactions: () => this.state.transactions,
isResubmitEnabled: this.#pendingTransactionOptions.isResubmitEnabled,
getGlobalLock: () =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ describe('MultichainTrackingHelper', () => {
provider: MOCK_PROVIDERS.mainnet,
blockTracker: MOCK_BLOCK_TRACKERS.mainnet,
chainId: '0x1',
networkClientId: 'mainnet',
});

expect(helper.has('mainnet')).toBe(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import type { NonceLock, NonceTracker } from '@metamask/nonce-tracker';
import type { Hex } from '@metamask/utils';
import { Mutex } from 'async-mutex';

import { createModuleLogger, projectLogger } from '../logger';
import type { PendingTransactionTracker } from './PendingTransactionTracker';
import { createModuleLogger, projectLogger } from '../logger';

/**
* Registry of network clients provided by the NetworkController
Expand Down Expand Up @@ -39,6 +39,7 @@ export type MultichainTrackingHelperOptions = {
provider: Provider;
blockTracker: BlockTracker;
chainId: Hex;
networkClientId: NetworkClientId;
}) => PendingTransactionTracker;
onNetworkStateChange: (
listener: (
Expand Down Expand Up @@ -68,6 +69,7 @@ export class MultichainTrackingHelper {
provider: Provider;
blockTracker: BlockTracker;
chainId: Hex;
networkClientId: NetworkClientId;
}) => PendingTransactionTracker;

readonly #nonceMutexesByChainId = new Map<Hex, Map<string, Mutex>>();
Expand Down Expand Up @@ -319,6 +321,7 @@ export class MultichainTrackingHelper {
provider,
blockTracker,
chainId,
networkClientId,
});

this.#trackingMap.set(networkClientId, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ import type EthQuery from '@metamask/eth-query';
import type { BlockTracker } from '@metamask/network-controller';
import { freeze } from 'immer';

import type { TransactionMeta } from '../types';
import { TransactionStatus } from '../types';
import { PendingTransactionTracker } from './PendingTransactionTracker';
import { TransactionPoller } from './TransactionPoller';
import type { TransactionMeta } from '../types';
import { TransactionStatus } from '../types';

const ID_MOCK = 'testId';
const CHAIN_ID_MOCK = '0x1';
const NETWORK_CLIENT_ID_MOCK = 'testNetworkClientId';
const NONCE_MOCK = '0x2';
const BLOCK_NUMBER_MOCK = '0x123';

Expand All @@ -18,6 +19,7 @@ const ETH_QUERY_MOCK = {} as unknown as EthQuery;
const TRANSACTION_SUBMITTED_MOCK = {
id: ID_MOCK,
chainId: CHAIN_ID_MOCK,
networkClientId: NETWORK_CLIENT_ID_MOCK,
hash: '0x1',
rawTx: '0x987',
status: TransactionStatus.submitted,
Expand Down Expand Up @@ -114,6 +116,7 @@ describe('PendingTransactionTracker', () => {
blockTracker,
getChainId: jest.fn(() => CHAIN_ID_MOCK),
getEthQuery: jest.fn(() => ETH_QUERY_MOCK),
getNetworkClientId: jest.fn(() => NETWORK_CLIENT_ID_MOCK),
getTransactions: jest.fn(),
getGlobalLock: jest.fn(() => Promise.resolve(jest.fn())),
publishTransaction: jest.fn(),
Expand Down Expand Up @@ -223,7 +226,7 @@ describe('PendingTransactionTracker', () => {
},
{
...TRANSACTION_SUBMITTED_MOCK,
chainId: '0x2',
networkClientId: 'other-network-client-id',
},
{
...TRANSACTION_SUBMITTED_MOCK,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import type {
import EventEmitter from 'events';
import { cloneDeep, merge } from 'lodash';

import { TransactionPoller } from './TransactionPoller';
import { createModuleLogger, projectLogger } from '../logger';
import type { TransactionMeta, TransactionReceipt } from '../types';
import { TransactionStatus, TransactionType } from '../types';
import { TransactionPoller } from './TransactionPoller';

/**
* We wait this many blocks before emitting a 'transaction-dropped' event
Expand Down Expand Up @@ -72,6 +72,8 @@ export class PendingTransactionTracker {

#getEthQuery: (networkClientId?: NetworkClientId) => EthQuery;

readonly #getNetworkClientId: () => NetworkClientId;

#getTransactions: () => TransactionMeta[];

#isResubmitEnabled: () => boolean;
Expand Down Expand Up @@ -101,6 +103,7 @@ export class PendingTransactionTracker {
blockTracker,
getChainId,
getEthQuery,
getNetworkClientId,
getTransactions,
isResubmitEnabled,
getGlobalLock,
Expand All @@ -110,6 +113,7 @@ export class PendingTransactionTracker {
blockTracker: BlockTracker;
getChainId: () => string;
getEthQuery: (networkClientId?: NetworkClientId) => EthQuery;
getNetworkClientId: () => string;
getTransactions: () => TransactionMeta[];
isResubmitEnabled?: () => boolean;
getGlobalLock: () => Promise<() => void>;
Expand All @@ -129,17 +133,22 @@ export class PendingTransactionTracker {
this.#droppedBlockCountByHash = new Map();
this.#getChainId = getChainId;
this.#getEthQuery = getEthQuery;
this.#getNetworkClientId = getNetworkClientId;
this.#getTransactions = getTransactions;
this.#isResubmitEnabled = isResubmitEnabled ?? (() => true);
this.#listener = this.#onLatestBlock.bind(this);
this.#log = createModuleLogger(log, getChainId());
this.#getGlobalLock = getGlobalLock;
this.#publishTransaction = publishTransaction;
this.#running = false;
this.#transactionPoller = new TransactionPoller(blockTracker);
this.#beforePublish = hooks?.beforePublish ?? (() => true);
this.#beforeCheckPendingTransaction =
hooks?.beforeCheckPendingTransaction ?? (() => true);

this.#log = createModuleLogger(
log,
`${getChainId()}:${getNetworkClientId()}`,
);
}

startIfPendingTransactions = () => {
Expand Down Expand Up @@ -478,7 +487,7 @@ export class PendingTransactionTracker {
#isNonceTaken(txMeta: TransactionMeta): boolean {
const { id, txParams } = txMeta;

return this.#getCurrentChainTransactions().some(
return this.#getChainTransactions().some(
(tx) =>
tx.id !== id &&
tx.txParams.from === txParams.from &&
Expand All @@ -489,7 +498,7 @@ export class PendingTransactionTracker {
}

#getPendingTransactions(): TransactionMeta[] {
return this.#getCurrentChainTransactions().filter(
return this.#getNetworkClientTransactions().filter(
(tx) =>
tx.status === TransactionStatus.submitted &&
!tx.verifiedOnBlockchain &&
Expand Down Expand Up @@ -543,11 +552,15 @@ export class PendingTransactionTracker {
return await query(this.#getEthQuery(), 'getTransactionCount', [address]);
}

#getCurrentChainTransactions(): TransactionMeta[] {
const currentChainId = this.#getChainId();
#getChainTransactions(): TransactionMeta[] {
const chainId = this.#getChainId();
return this.#getTransactions().filter((tx) => tx.chainId === chainId);
}

#getNetworkClientTransactions(): TransactionMeta[] {
const networkClientId = this.#getNetworkClientId();
return this.#getTransactions().filter(
(tx) => tx.chainId === currentChainId,
(tx) => tx.networkClientId === networkClientId,
);
}
}

0 comments on commit 78c420e

Please sign in to comment.