diff --git a/packages/permission-controller/src/SubjectMetadataController.ts b/packages/permission-controller/src/SubjectMetadataController.ts index dcd2b9cfa76..203f9893cae 100644 --- a/packages/permission-controller/src/SubjectMetadataController.ts +++ b/packages/permission-controller/src/SubjectMetadataController.ts @@ -209,6 +209,8 @@ export class SubjectMetadataController extends BaseController< this.subjectsWithoutPermissionsEncounteredSinceStartup.add(origin); this.update((draftState) => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore something about string-indexing draftState.subjectMetadata[origin] = newMetadata; if (typeof originToForget === 'string') { delete draftState.subjectMetadata[originToForget]; @@ -231,7 +233,8 @@ export class SubjectMetadataController extends BaseController< */ trimMetadataState(): void { this.update((draftState) => { - // @ts-expect-error ts(2589) + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore infinite type-resursion srsly return SubjectMetadataController.getTrimmedState( draftState, this.subjectHasPermissions, diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 5cfbdb33fb2..96d44c84cbe 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -23,7 +23,6 @@ import type { Provider, } from '@metamask/network-controller'; import { NetworkClientType, NetworkStatus } from '@metamask/network-controller'; -import * as NonceTrackerPackage from '@metamask/nonce-tracker'; import { errorCodes, providerErrors, rpcErrors } from '@metamask/rpc-errors'; import { createDeferredPromise } from '@metamask/utils'; import assert from 'assert'; @@ -294,9 +293,6 @@ function waitForTransactionFinished( const MOCK_PREFERENCES = { state: { selectedAddress: 'foo' } }; const INFURA_PROJECT_ID = '341eacb578dd44a1a049cbc5f6fd4035'; -const GOERLI_PROVIDER = new HttpProvider( - `https://goerli.infura.io/v3/${INFURA_PROJECT_ID}`, -); const MAINNET_PROVIDER = new HttpProvider( `https://mainnet.infura.io/v3/${INFURA_PROJECT_ID}`, ); @@ -331,24 +327,6 @@ const MOCK_NETWORK: MockNetwork = { }, subscribe: () => undefined, }; -const MOCK_NETWORK_WITHOUT_CHAIN_ID: MockNetwork = { - provider: GOERLI_PROVIDER, - blockTracker: buildMockBlockTracker('0x102833C'), - state: { - selectedNetworkClientId: NetworkType.goerli, - networksMetadata: { - [NetworkType.goerli]: { - EIPS: { 1559: false }, - status: NetworkStatus.Available, - }, - }, - providerConfig: { - type: NetworkType.goerli, - } as NetworkState['providerConfig'], - networkConfigurations: {}, - }, - subscribe: () => undefined, -}; const MOCK_MAINNET_NETWORK: MockNetwork = { provider: MAINNET_PROVIDER, blockTracker: buildMockBlockTracker('0x102833C'), @@ -572,11 +550,14 @@ describe('TransactionController', () => { ); const { messenger: givenRestrictedMessenger, ...otherOptions } = { - blockTracker: network.blockTracker, disableHistory: false, disableSendFlowHistory: false, disableSwaps: false, getCurrentNetworkEIP1559Compatibility: async () => false, + getGlobalProviderAndBlockTracker: () => ({ + provider: network.provider, + blockTracker: network.blockTracker, + }), getNetworkState: () => network.state, // TODO: Replace with a real type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -586,7 +567,6 @@ describe('TransactionController', () => { isMultichainEnabled: false, hooks: {}, onNetworkStateChange: network.subscribe, - provider: network.provider, sign: async (transaction: TypedTransaction) => transaction, transactionHistoryLimit: 40, ...givenOptions, @@ -780,21 +760,25 @@ describe('TransactionController', () => { return pendingTransactionTrackerMock; }); - multichainTrackingHelperClassMock.mockImplementation(({ provider }) => { - multichainTrackingHelperMock = { - getEthQuery: jest.fn().mockImplementation(() => { - return new EthQuery(provider); - }), - getProvider: jest.fn().mockImplementation(() => { - return provider; - }), - checkForPendingTransactionAndStartPolling: jest.fn(), - getNonceLock: getNonceLockSpy, - initialize: jest.fn(), - has: jest.fn().mockReturnValue(false), - } as unknown as jest.Mocked; - return multichainTrackingHelperMock; - }); + multichainTrackingHelperClassMock.mockImplementation( + ({ getGlobalProviderAndBlockTracker }) => { + multichainTrackingHelperMock = { + getEthQuery: jest.fn().mockImplementation(() => { + return new EthQuery( + getGlobalProviderAndBlockTracker()?.provider as Provider, + ); + }), + getProvider: jest.fn().mockImplementation(() => { + return getGlobalProviderAndBlockTracker()?.provider as Provider; + }), + checkForPendingTransactionAndStartPolling: jest.fn(), + getNonceLock: getNonceLockSpy, + initialize: jest.fn(), + has: jest.fn().mockReturnValue(false), + } as unknown as jest.Mocked; + return multichainTrackingHelperMock; + }, + ); defaultGasFeeFlowClassMock.mockImplementation(() => { defaultGasFeeFlowMock = { @@ -882,117 +866,6 @@ describe('TransactionController', () => { pendingTransactionTrackerMock.startIfPendingTransactions, ).toHaveBeenCalledTimes(1); }); - - describe('nonce tracker', () => { - it('uses external pending transactions', async () => { - const nonceTrackerMock = jest - .spyOn(NonceTrackerPackage, 'NonceTracker') - .mockImplementation(); - - const externalPendingTransactions = [ - { - from: '0x1', - }, - { from: '0x2' }, - ]; - - const getExternalPendingTransactions = jest - .fn() - .mockReturnValueOnce(externalPendingTransactions); - - setupController({ - options: { - getExternalPendingTransactions, - state: { - transactions: [ - { - ...TRANSACTION_META_MOCK, - chainId: MOCK_NETWORK.state.providerConfig.chainId, - status: TransactionStatus.submitted, - }, - ], - }, - }, - }); - - const pendingTransactions = - nonceTrackerMock.mock.calls[0][0].getPendingTransactions( - ACCOUNT_MOCK, - ); - - expect(nonceTrackerMock).toHaveBeenCalledTimes(1); - expect(pendingTransactions).toStrictEqual([ - expect.any(Object), - ...externalPendingTransactions, - ]); - expect(getExternalPendingTransactions).toHaveBeenCalledTimes(1); - expect(getExternalPendingTransactions).toHaveBeenCalledWith( - ACCOUNT_MOCK, - // This is undefined for the base nonceTracker - undefined, - ); - }); - }); - - describe('onBootCleanup', () => { - afterEach(() => { - updateGasMock.mockReset(); - updateGasFeesMock.mockReset(); - }); - - it('submits approved transactions for all chains', async () => { - const mockTransactionMeta = { - from: ACCOUNT_MOCK, - status: TransactionStatus.approved, - txParams: { - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - }, - }; - const mockedTransactions = [ - { - id: '123', - history: [{ ...mockTransactionMeta, id: '123' }], - chainId: toHex(5), - ...mockTransactionMeta, - }, - { - id: '456', - history: [{ ...mockTransactionMeta, id: '456' }], - chainId: toHex(1), - ...mockTransactionMeta, - }, - { - id: '789', - history: [{ ...mockTransactionMeta, id: '789' }], - chainId: toHex(16), - ...mockTransactionMeta, - }, - ]; - - const mockedControllerState = { - transactions: mockedTransactions, - methodData: {}, - lastFetchedBlockNumbers: {}, - }; - - const { controller } = setupController({ - options: { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - state: mockedControllerState as any, - }, - }); - - await flushPromises(); - - const { transactions } = controller.state; - - expect(transactions[0].status).toBe(TransactionStatus.submitted); - expect(transactions[1].status).toBe(TransactionStatus.submitted); - expect(transactions[2].status).toBe(TransactionStatus.submitted); - }); - }); }); describe('estimateGas', () => { @@ -1441,6 +1314,8 @@ describe('TransactionController', () => { to: ACCOUNT_MOCK, }); + await flushPromises(); + const expectedInitialSnapshot = { actionId: undefined, chainId: expect.any(String), @@ -2001,19 +1876,6 @@ describe('TransactionController', () => { await expectTransactionToFail(controller, 'No sign method defined'); }); - it('if no chainId defined', async () => { - const { controller } = setupController({ - network: MOCK_NETWORK_WITHOUT_CHAIN_ID, - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - }); - - await expectTransactionToFail(controller, 'No chainId defined'); - }); - it('if unexpected status', async () => { const { controller, messenger } = setupController(); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 06010fc3148..f51ebefc92c 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -15,10 +15,10 @@ import type { import { BaseController } from '@metamask/base-controller'; import { query, - NetworkType, ApprovalType, ORIGIN_METAMASK, convertHexToDecimal, + NetworkType, } from '@metamask/controller-utils'; import EthQuery from '@metamask/eth-query'; import type { @@ -280,7 +280,6 @@ export type PendingTransactionOptions = { * @property hooks.publish - Alternate logic to publish a transaction. */ export type TransactionControllerOptions = { - blockTracker: BlockTracker; disableHistory: boolean; disableSendFlowHistory: boolean; disableSwaps: boolean; @@ -293,6 +292,9 @@ export type TransactionControllerOptions = { getGasFeeEstimates?: ( options: FetchGasFeeEstimateOptions, ) => Promise; + getGlobalProviderAndBlockTracker: () => + | { provider: Provider; blockTracker: BlockTracker } + | undefined; getNetworkClientRegistry: NetworkController['getNetworkClientRegistry']; getNetworkState: () => NetworkState; getPermittedAccounts: (origin?: string) => Promise; @@ -304,7 +306,6 @@ export type TransactionControllerOptions = { messenger: TransactionControllerMessenger; onNetworkStateChange: (listener: (state: NetworkState) => void) => void; pendingTransactions?: PendingTransactionOptions; - provider: Provider; securityProviderRequest?: SecurityProviderRequest; sign?: ( transaction: TypedTransaction, @@ -590,10 +591,6 @@ export class TransactionController extends BaseController< private readonly approvingTransactionIds: Set = new Set(); - private readonly nonceTracker: NonceTracker; - - private readonly registry: MethodRegistry; - private readonly mutex = new Mutex(); private readonly gasFeeFlows: GasFeeFlow[]; @@ -639,6 +636,13 @@ export class TransactionController extends BaseController< #isSimulationEnabled: () => boolean; + #getGlobalProviderAndBlockTracker: () => + | { + provider: Provider; + blockTracker: BlockTracker; + } + | undefined; + #testGasFeeFlows: boolean; private readonly afterSign: ( @@ -695,14 +699,23 @@ export class TransactionController extends BaseController< } private async registryLookup(fourBytePrefix: string): Promise { - const registryMethod = await this.registry.lookup(fourBytePrefix); + const globalProvider = this.#getGlobalProviderAndBlockTracker()?.provider; + + if (!globalProvider) { + throw providerErrors.disconnected(); + } + + // @ts-expect-error the type in eth-method-registry is inappropriate and should be changed + const registry = new MethodRegistry({ provider: globalProvider }); + + const registryMethod = await registry.lookup(fourBytePrefix); if (!registryMethod) { return { registryMethod: '', parsedRegistryMethod: { name: undefined, args: undefined }, }; } - const parsedRegistryMethod = this.registry.parse(registryMethod); + const parsedRegistryMethod = registry.parse(registryMethod); return { registryMethod, parsedRegistryMethod }; } @@ -721,7 +734,6 @@ export class TransactionController extends BaseController< * Constructs a TransactionController. * * @param options - The controller options. - * @param options.blockTracker - The block tracker used to poll for new blocks data. * @param options.disableHistory - Whether to disable storing history in transaction metadata. * @param options.disableSendFlowHistory - Explicitly disable transaction metadata history. * @param options.disableSwaps - Whether to disable additional processing on swaps transactions. @@ -729,6 +741,7 @@ export class TransactionController extends BaseController< * @param options.getCurrentNetworkEIP1559Compatibility - Whether or not the network supports EIP-1559. * @param options.getExternalPendingTransactions - Callback to retrieve pending transactions from external sources. * @param options.getGasFeeEstimates - Callback to retrieve gas fee estimates. + * @param options.getGlobalProviderAndBlockTracker - Gets the global provider and block tracker. * @param options.getNetworkClientRegistry - Gets the network client registry. * @param options.getNetworkState - Gets the state of the network controller. * @param options.getPermittedAccounts - Get accounts that a given origin has permissions for. @@ -740,7 +753,6 @@ export class TransactionController extends BaseController< * @param options.messenger - The controller messenger. * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. * @param options.pendingTransactions - Configuration options for pending transaction support. - * @param options.provider - The provider used to create the underlying EthQuery instance. * @param options.securityProviderRequest - A function for verifying a transaction, whether it is malicious or not. * @param options.sign - Function used to sign transactions. * @param options.state - Initial state to set on this controller. @@ -749,7 +761,6 @@ export class TransactionController extends BaseController< * @param options.hooks - The controller hooks. */ constructor({ - blockTracker, disableHistory, disableSendFlowHistory, disableSwaps, @@ -757,6 +768,7 @@ export class TransactionController extends BaseController< getCurrentNetworkEIP1559Compatibility, getExternalPendingTransactions, getGasFeeEstimates, + getGlobalProviderAndBlockTracker, getNetworkClientRegistry, getNetworkState, getPermittedAccounts, @@ -768,7 +780,6 @@ export class TransactionController extends BaseController< messenger, onNetworkStateChange, pendingTransactions = {}, - provider, securityProviderRequest, sign, state, @@ -791,9 +802,6 @@ export class TransactionController extends BaseController< this.isSendFlowHistoryDisabled = disableSendFlowHistory ?? false; this.isHistoryDisabled = disableHistory ?? false; this.isSwapsDisabled = disableSwaps ?? false; - this.#isSimulationEnabled = isSimulationEnabled ?? (() => true); - // @ts-expect-error the type in eth-method-registry is inappropriate and should be changed - this.registry = new MethodRegistry({ provider }); this.getSavedGasFees = getSavedGasFees ?? ((_chainId) => undefined); this.getCurrentAccountEIP1559Compatibility = getCurrentAccountEIP1559Compatibility ?? (() => Promise.resolve(true)); @@ -809,6 +817,8 @@ export class TransactionController extends BaseController< this.#incomingTransactionOptions = incomingTransactions; this.#pendingTransactionOptions = pendingTransactions; this.#transactionHistoryLimit = transactionHistoryLimit; + this.#isSimulationEnabled = isSimulationEnabled ?? (() => true); + this.#getGlobalProviderAndBlockTracker = getGlobalProviderAndBlockTracker; this.sign = sign; this.#testGasFeeFlows = testGasFeeFlows === true; @@ -824,11 +834,6 @@ export class TransactionController extends BaseController< this.publish = hooks?.publish ?? (() => Promise.resolve({ transactionHash: undefined })); - this.nonceTracker = this.#createNonceTracker({ - provider, - blockTracker, - }); - const findNetworkClientIdByChainId = (chainId: Hex) => { return this.messagingSystem.call( `NetworkController:findNetworkClientIdByChainId`, @@ -838,10 +843,9 @@ export class TransactionController extends BaseController< this.#multichainTrackingHelper = new MultichainTrackingHelper({ isMultichainEnabled, - provider, - nonceTracker: this.nonceTracker, incomingTransactionOptions: incomingTransactions, findNetworkClientIdByChainId, + getGlobalProviderAndBlockTracker, getNetworkClientById: ((networkClientId: NetworkClientId) => { return this.messagingSystem.call( `NetworkController:getNetworkClientById`, @@ -873,13 +877,20 @@ export class TransactionController extends BaseController< }); this.incomingTransactionHelper = this.#createIncomingTransactionHelper({ - blockTracker, + getBlockTracker: () => getGlobalProviderAndBlockTracker()?.blockTracker, + getChainId: () => this.getChainId(), etherscanRemoteTransactionSource, }); + const getGlobalEthQuery = () => { + const globalProvider = getGlobalProviderAndBlockTracker()?.provider; + return globalProvider ? new EthQuery(globalProvider) : undefined; + }; + this.pendingTransactionTracker = this.#createPendingTransactionTracker({ - provider, - blockTracker, + getBlockTracker: () => getGlobalProviderAndBlockTracker()?.blockTracker, + getChainId: () => this.getChainId(), + getEthQuery: getGlobalEthQuery, }); this.gasFeeFlows = this.#getGasFeeFlows(); @@ -921,10 +932,9 @@ export class TransactionController extends BaseController< onNetworkStateChange(() => { log('Detected network change', this.getChainId()); this.pendingTransactionTracker.startIfPendingTransactions(); - this.onBootCleanup(); + this.initApprovedTransactions(); }); - this.onBootCleanup(); this.#checkForPendingTransactionAndStartPolling(); } @@ -943,6 +953,7 @@ export class TransactionController extends BaseController< */ async handleMethodData(fourBytePrefix: string): Promise { const releaseLock = await this.mutex.acquire(); + try { const { methodData } = this.state; const knownMethod = Object.keys(methodData).find( @@ -1046,8 +1057,9 @@ export class TransactionController extends BaseController< origin, ); - const chainId = this.getChainId(networkClientId); - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const chainId = this.#getChainIdOrThrow(networkClientId); + + const ethQuery = this.#getEthQueryOrThrow({ networkClientId, chainId, }); @@ -1078,7 +1090,12 @@ export class TransactionController extends BaseController< networkClientId, }; - await this.updateGasProperties(addedTransactionMeta); + const provider = this.#getProviderOrThrow({ + networkClientId, + chainId, + }); + + await this.updateGasProperties(addedTransactionMeta, ethQuery, provider); // Checks if a transaction already exists with a given actionId if (!existingTransactionMeta) { @@ -1299,10 +1316,11 @@ export class TransactionController extends BaseController< txParams: newTxParams, }); - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const ethQuery = this.#getEthQueryOrThrow({ networkClientId: transactionMeta.networkClientId, chainId: transactionMeta.chainId, }); + const hash = await this.publishTransactionForRetry( ethQuery, rawTx, @@ -1472,6 +1490,11 @@ export class TransactionController extends BaseController< networkClientId: transactionMeta.networkClientId, chainId: transactionMeta.chainId, }); + + if (!ethQuery) { + throw providerErrors.disconnected(); + } + const hash = await this.publishTransactionForRetry( ethQuery, rawTx, @@ -1538,9 +1561,10 @@ export class TransactionController extends BaseController< transaction: TransactionParams, networkClientId?: NetworkClientId, ) { - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const ethQuery = this.#getEthQueryOrThrow({ networkClientId, }); + const { estimatedGas, simulationFails } = await estimateGas( transaction, ethQuery, @@ -1561,9 +1585,10 @@ export class TransactionController extends BaseController< multiplier: number, networkClientId?: NetworkClientId, ) { - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const ethQuery = this.#getEthQueryOrThrow({ networkClientId, }); + const { blockGasLimit, estimatedGas, simulationFails } = await estimateGas( transaction, ethQuery, @@ -1639,7 +1664,9 @@ export class TransactionController extends BaseController< }); return; } - const currentChainId = this.getChainId(); + + const currentChainId = this.#getChainIdOrThrow(); + const newTransactions = this.state.transactions.filter( ({ chainId, txParams }) => { const isMatchingNetwork = ignoreNetwork || chainId === currentChainId; @@ -1912,10 +1939,16 @@ export class TransactionController extends BaseController< address: string, networkClientId?: NetworkClientId, ): Promise { - return this.#multichainTrackingHelper.getNonceLock( + const nonceLock = await this.#multichainTrackingHelper.getNonceLock( address, networkClientId, ); + + if (!nonceLock) { + throw providerErrors.disconnected(); + } + + return nonceLock; } /** @@ -1974,15 +2007,19 @@ export class TransactionController extends BaseController< ) as TransactionParams; const updatedTransaction = merge({}, transactionMeta, editableParams); - const provider = this.#multichainTrackingHelper.getProvider({ + const provider = this.#getProviderOrThrow({ + chainId: transactionMeta.chainId, + networkClientId: transactionMeta.networkClientId, + }); + const ethQuery = this.#getEthQueryOrThrow({ chainId: transactionMeta.chainId, networkClientId: transactionMeta.networkClientId, }); - const ethQuery = new EthQuery(provider); const { type } = await determineTransactionType( updatedTransaction.txParams, ethQuery, ); + updatedTransaction.type = type; await updateTransactionLayer1GasFee({ @@ -2167,7 +2204,8 @@ export class TransactionController extends BaseController< * Creates approvals for all unapproved transactions persisted. */ initApprovals() { - const chainId = this.getChainId(); + const chainId = this.#getChainIdOrThrow(); + const unapprovedTxs = this.state.transactions.filter( (transaction) => transaction.status === TransactionStatus.unapproved && @@ -2187,6 +2225,32 @@ export class TransactionController extends BaseController< } } + /** + * Sign and submit any previously approved transactions. + */ + initApprovedTransactions() { + const approvedTransactions = this.state.transactions.filter( + (transaction) => transaction.status === TransactionStatus.approved, + ); + + if (!approvedTransactions.length) { + return; + } + + log('Processing previously approved transactions', { + count: approvedTransactions.length, + }); + + for (const transactionMeta of approvedTransactions) { + if (this.beforeApproveOnInit(transactionMeta)) { + this.approveTransaction(transactionMeta.id).catch((error) => { + /* istanbul ignore next */ + console.error('Error while submitting persisted transaction', error); + }); + } + } + } + /** * Search transaction metadata for matching entries. * @@ -2210,7 +2274,8 @@ export class TransactionController extends BaseController< filterToCurrentNetwork?: boolean; limit?: number; } = {}): TransactionMeta[] { - const chainId = this.getChainId(); + const chainId = this.#getChainIdOrThrow(); + // searchCriteria is an object that might have values that aren't predicate // methods. When providing any other value type (string, number, etc), we // consider this shorthand for "check the value at key for strict equality @@ -2316,9 +2381,9 @@ export class TransactionController extends BaseController< this.gasFeeFlows, ) as GasFeeFlow; - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ - networkClientId, - chainId, + const ethQuery = this.#getEthQueryOrThrow({ + chainId: transactionMeta.chainId, + networkClientId: transactionMeta.networkClientId, }); const gasFeeControllerData = await this.getGasFeeEstimates({ @@ -2349,7 +2414,7 @@ export class TransactionController extends BaseController< chainId?: Hex; networkClientId?: NetworkClientId; }): Promise { - const provider = this.#multichainTrackingHelper.getProvider({ + const provider = this.#getProviderOrThrow({ networkClientId, chainId, }); @@ -2442,7 +2507,11 @@ export class TransactionController extends BaseController< }); } - private async updateGasProperties(transactionMeta: TransactionMeta) { + private async updateGasProperties( + transactionMeta: TransactionMeta, + ethQuery: EthQuery, + provider: Provider, + ) { const isEIP1559Compatible = (await this.getEIP1559Compatibility(transactionMeta.networkClientId)) && transactionMeta.txParams.type !== TransactionEnvelopeType.legacy; @@ -2451,16 +2520,6 @@ export class TransactionController extends BaseController< const isCustomNetwork = this.#isCustomNetwork(networkClientId); - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ - networkClientId, - chainId, - }); - - const provider = this.#multichainTrackingHelper.getProvider({ - networkClientId, - chainId, - }); - await updateGas({ ethQuery, chainId, @@ -2484,28 +2543,6 @@ export class TransactionController extends BaseController< }); } - private onBootCleanup() { - this.submitApprovedTransactions(); - } - - /** - * Force submit approved transactions for all chains. - */ - private submitApprovedTransactions() { - const approvedTransactions = this.state.transactions.filter( - (transaction) => transaction.status === TransactionStatus.approved, - ); - - for (const transactionMeta of approvedTransactions) { - if (this.beforeApproveOnInit(transactionMeta)) { - this.approveTransaction(transactionMeta.id).catch((error) => { - /* istanbul ignore next */ - console.error('Error while submitting persisted transaction', error); - }); - } - } - } - private async processApproval( transactionMeta: TransactionMeta, { @@ -2655,13 +2692,15 @@ export class TransactionController extends BaseController< this.approvingTransactionIds.delete(transactionId), ); + const getNonceLock = ( + address: string, + networkClientId?: NetworkClientId, + ) => + this.#multichainTrackingHelper.getNonceLock(address, networkClientId); + const [nonce, releaseNonce] = await getNextNonce( transactionMeta, - (address: string) => - this.#multichainTrackingHelper.getNonceLock( - address, - transactionMeta.networkClientId, - ), + getNonceLock, ); // must set transaction to submitted/failed before releasing lock @@ -2708,7 +2747,7 @@ export class TransactionController extends BaseController< return ApprovalState.NotApproved; } - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const ethQuery = this.#getEthQueryOrThrow({ networkClientId: transactionMeta.networkClientId, chainId: transactionMeta.chainId, }); @@ -2959,7 +2998,19 @@ export class TransactionController extends BaseController< return { meta: transaction, isCompleted }; } - private getChainId(networkClientId?: NetworkClientId): Hex { + /* + private getChainId(networkClientId?: NetworkClientId): Hex | undefined { + if (networkClientId) { + return this.messagingSystem.call( + `NetworkController:getNetworkClientById`, + networkClientId, + ).configuration.chainId; + } + + return this.getNetworkState()?.providerConfig.chainId; + } + */ + private getChainId(networkClientId?: NetworkClientId): Hex | undefined { const globalChainId = this.#getGlobalChainId(); const globalNetworkClientId = this.#getGlobalNetworkClientId(); @@ -3336,10 +3387,12 @@ export class TransactionController extends BaseController< private getNonceTrackerTransactions( status: TransactionStatus, address: string, - chainId: string = this.getChainId(), + chainId?: Hex, ) { + const finalChainId = chainId ?? (this.getChainId() as string); + return getAndFormatTransactionsForNonceTracker( - chainId, + finalChainId, address, status, this.state.transactions, @@ -3369,10 +3422,11 @@ export class TransactionController extends BaseController< return; } - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const ethQuery = this.#getEthQueryOrThrow({ networkClientId: transactionMeta.networkClientId, chainId: transactionMeta.chainId, }); + const { updatedTransactionMeta, approvalTransactionMeta } = await updatePostTransactionBalance(transactionMeta, { ethQuery, @@ -3412,27 +3466,29 @@ export class TransactionController extends BaseController< this, chainId, ), - getConfirmedTransactions: this.getNonceTrackerTransactions.bind( - this, - TransactionStatus.confirmed, - ), + getConfirmedTransactions: (address) => + this.getNonceTrackerTransactions( + TransactionStatus.confirmed, + address, + chainId, + ), }); } #createIncomingTransactionHelper({ - blockTracker, + getBlockTracker, + getChainId, etherscanRemoteTransactionSource, - chainId, }: { - blockTracker: BlockTracker; + getBlockTracker: () => BlockTracker | undefined; + getChainId: () => Hex | undefined; etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; - chainId?: Hex; }): IncomingTransactionHelper { const incomingTransactionHelper = new IncomingTransactionHelper({ - blockTracker, + getBlockTracker, + getChainId, getCurrentAccount: this.getSelectedAddress, getLastFetchedBlockNumbers: () => this.state.lastFetchedBlockNumbers, - getChainId: chainId ? () => chainId : this.getChainId.bind(this), isEnabled: this.#incomingTransactionOptions.isEnabled, queryEntireHistory: this.#incomingTransactionOptions.queryEntireHistory, remoteTransactionSource: etherscanRemoteTransactionSource, @@ -3446,29 +3502,26 @@ export class TransactionController extends BaseController< } #createPendingTransactionTracker({ - provider, - blockTracker, - chainId, + getBlockTracker, + getChainId, + getEthQuery, }: { - provider: Provider; - blockTracker: BlockTracker; - chainId?: Hex; + getBlockTracker: () => BlockTracker | undefined; + getChainId: () => Hex | undefined; + getEthQuery: () => EthQuery | undefined; }): PendingTransactionTracker { - const ethQuery = new EthQuery(provider); - const getChainId = chainId ? () => chainId : this.getChainId.bind(this); - const pendingTransactionTracker = new PendingTransactionTracker({ approveTransaction: async (transactionId: string) => { await this.approveTransaction(transactionId); }, - blockTracker, + getBlockTracker, getChainId, - getEthQuery: () => ethQuery, + getEthQuery, getTransactions: () => this.state.transactions, isResubmitEnabled: this.#pendingTransactionOptions.isResubmitEnabled, - getGlobalLock: () => + getGlobalLock: (chainId: Hex) => this.#multichainTrackingHelper.acquireNonceLockForChainIdKey({ - chainId: getChainId(), + chainId, }), publishTransaction: this.publishTransaction.bind(this), hooks: { @@ -3558,7 +3611,7 @@ export class TransactionController extends BaseController< } #getNonceTrackerPendingTransactions( - chainId: string | undefined, + chainId: Hex | undefined, address: string, ) { const standardPendingTransactions = this.getNonceTrackerTransactions( @@ -3571,6 +3624,7 @@ export class TransactionController extends BaseController< address, chainId, ); + return [...standardPendingTransactions, ...externalPendingTransactions]; } @@ -3843,4 +3897,52 @@ export class TransactionController extends BaseController< ).configuration.type === NetworkClientType.Custom ); } + + #getEthQueryOrThrow({ + networkClientId, + chainId, + }: { + networkClientId?: NetworkClientId; + chainId?: Hex; + }): EthQuery { + const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + networkClientId, + chainId, + }); + + if (!ethQuery) { + throw providerErrors.disconnected(); + } + + return ethQuery; + } + + #getChainIdOrThrow(networkClientId?: NetworkClientId): Hex { + const chainId = this.getChainId(networkClientId); + + if (!chainId) { + throw providerErrors.disconnected(); + } + + return chainId; + } + + #getProviderOrThrow({ + networkClientId, + chainId, + }: { + networkClientId?: NetworkClientId; + chainId?: Hex; + }): Provider { + const provider = this.#multichainTrackingHelper.getProvider({ + networkClientId, + chainId, + }); + + if (!provider) { + throw providerErrors.disconnected(); + } + + return provider; + } } diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 979f88c4525..9ba069d3ee2 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -48,6 +48,7 @@ import { import type { TransactionControllerActions, TransactionControllerEvents, + TransactionControllerOptions, } from './TransactionController'; import { TransactionController } from './TransactionController'; import type { TransactionMeta } from './types'; @@ -150,8 +151,7 @@ const setupController = async ( allowedEvents: ['NetworkController:stateChange'], }); - const options = { - blockTracker, + const options: TransactionControllerOptions = { disableHistory: false, disableSendFlowHistory: false, disableSwaps: false, @@ -163,6 +163,10 @@ const setupController = async ( false ); }, + getGlobalProviderAndBlockTracker: () => ({ + provider, + blockTracker, + }), getNetworkState: () => networkController.state, getNetworkClientRegistry: networkController.getNetworkClientRegistry.bind(networkController), @@ -174,7 +178,6 @@ const setupController = async ( onNetworkStateChange: () => { // noop }, - provider, sign: async (transaction: TypedTransaction) => transaction, transactionHistoryLimit: 40, ...givenOptions, @@ -206,123 +209,6 @@ describe('TransactionController Integration', () => { expect(transactionController).toBeDefined(); transactionController.destroy(); }); - - it('should submit all approved transactions in state', async () => { - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.goerli, - ), - mocks: [ - buildEthBlockNumberRequestMock('0x1'), - buildEthSendRawTransactionRequestMock( - '0x02e2050101018252089408f137f335ea1b8f193b8f6ea92561a60d23a2118080c0808080', - '0x1', - ), - ], - }); - - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.sepolia, - ), - mocks: [ - buildEthBlockNumberRequestMock('0x1'), - buildEthSendRawTransactionRequestMock( - '0x02e583aa36a70101018252089408f137f335ea1b8f193b8f6ea92561a60d23a2118080c0808080', - '0x1', - ), - ], - }); - - const { transactionController } = await setupController({ - isMultichainEnabled: true, - state: { - transactions: [ - { - actionId: undefined, - chainId: '0x5', - dappSuggestedGasFees: undefined, - deviceConfirmedOn: undefined, - id: 'ecfe8c60-ba27-11ee-8643-dfd28279a442', - origin: undefined, - securityAlertResponse: undefined, - status: TransactionStatus.approved, - time: 1706039113766, - txParams: { - from: '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207', - gas: '0x5208', - nonce: '0x1', - to: '0x08f137f335ea1b8f193b8f6ea92561a60d23a211', - value: '0x0', - maxFeePerGas: '0x1', - maxPriorityFeePerGas: '0x1', - }, - userEditedGasLimit: false, - verifiedOnBlockchain: false, - type: TransactionType.simpleSend, - networkClientId: 'goerli', - simulationFails: undefined, - originalGasEstimate: '0x5208', - defaultGasEstimates: { - gas: '0x5208', - maxFeePerGas: '0x1', - maxPriorityFeePerGas: '0x1', - gasPrice: undefined, - estimateType: 'dappSuggested', - }, - userFeeLevel: 'dappSuggested', - sendFlowHistory: [], - }, - { - actionId: undefined, - chainId: '0xaa36a7', - dappSuggestedGasFees: undefined, - deviceConfirmedOn: undefined, - id: 'c4cc0ff0-ba28-11ee-926f-55a7f9c2c2c6', - origin: undefined, - securityAlertResponse: undefined, - status: TransactionStatus.approved, - time: 1706039113766, - txParams: { - from: '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207', - gas: '0x5208', - nonce: '0x1', - to: '0x08f137f335ea1b8f193b8f6ea92561a60d23a211', - value: '0x0', - maxFeePerGas: '0x1', - maxPriorityFeePerGas: '0x1', - }, - userEditedGasLimit: false, - verifiedOnBlockchain: false, - type: TransactionType.simpleSend, - networkClientId: 'sepolia', - simulationFails: undefined, - originalGasEstimate: '0x5208', - defaultGasEstimates: { - gas: '0x5208', - maxFeePerGas: '0x1', - maxPriorityFeePerGas: '0x1', - gasPrice: undefined, - estimateType: 'dappSuggested', - }, - userFeeLevel: 'dappSuggested', - sendFlowHistory: [], - }, - ], - }, - }); - await advanceTime({ clock, duration: 1 }); - - expect(transactionController.state.transactions).toMatchObject([ - expect.objectContaining({ - status: 'submitted', - }), - expect.objectContaining({ - status: 'submitted', - }), - ]); - transactionController.destroy(); - }); }); describe('multichain transaction lifecycle', () => { diff --git a/packages/transaction-controller/src/helpers/GasFeePoller.ts b/packages/transaction-controller/src/helpers/GasFeePoller.ts index 1aaff0ea067..302646a15cc 100644 --- a/packages/transaction-controller/src/helpers/GasFeePoller.ts +++ b/packages/transaction-controller/src/helpers/GasFeePoller.ts @@ -37,7 +37,10 @@ export class GasFeePoller { options: FetchGasFeeEstimateOptions, ) => Promise; - #getProvider: (chainId: Hex, networkClientId?: NetworkClientId) => Provider; + #getProvider: ( + chainId: Hex, + networkClientId?: NetworkClientId, + ) => Provider | undefined; #getTransactions: () => TransactionMeta[]; @@ -72,7 +75,10 @@ export class GasFeePoller { getGasFeeControllerEstimates: ( options: FetchGasFeeEstimateOptions, ) => Promise; - getProvider: (chainId: Hex, networkClientId?: NetworkClientId) => Provider; + getProvider: ( + chainId: Hex, + networkClientId?: NetworkClientId, + ) => Provider | undefined; getTransactions: () => TransactionMeta[]; layer1GasFeeFlows: Layer1GasFeeFlow[]; onStateChange: (listener: () => void) => void; @@ -192,7 +198,18 @@ export class GasFeePoller { > { const { chainId, networkClientId } = transactionMeta; - const ethQuery = new EthQuery(this.#getProvider(chainId, networkClientId)); + const provider = this.#getProvider(chainId, networkClientId); + if (!provider) { + log('Provider not available', transactionMeta.id); + return undefined; + } + + const ethQuery = new EthQuery(provider); + if (!ethQuery) { + log('Provider not available', transactionMeta.id); + return undefined; + } + const gasFeeFlow = getGasFeeFlow(transactionMeta, this.#gasFeeFlows); if (gasFeeFlow) { @@ -236,7 +253,12 @@ export class GasFeePoller { transactionMeta: TransactionMeta, ): Promise { const { chainId, networkClientId } = transactionMeta; + const provider = this.#getProvider(chainId, networkClientId); + if (!provider) { + log('Provider not available', transactionMeta.id); + throw new Error('provider not available'); + } const layer1GasFee = await getTransactionLayer1GasFee({ layer1GasFeeFlows: this.#layer1GasFeeFlows, diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts index 49b39c4effc..52d18ca8b11 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts @@ -31,7 +31,7 @@ const BLOCK_TRACKER_MOCK = { } as unknown as jest.Mocked; const CONTROLLER_ARGS_MOCK = { - blockTracker: BLOCK_TRACKER_MOCK, + getBlockTracker: () => BLOCK_TRACKER_MOCK, getCurrentAccount: () => ADDRESS_MOCK, getLastFetchedBlockNumbers: () => ({}), getChainId: () => CHAIN_ID_MOCK, @@ -589,7 +589,7 @@ describe('IncomingTransactionHelper', () => { helper.start(); expect( - CONTROLLER_ARGS_MOCK.blockTracker.addListener, + CONTROLLER_ARGS_MOCK.getBlockTracker().addListener, ).toHaveBeenCalledTimes(1); }); @@ -603,7 +603,7 @@ describe('IncomingTransactionHelper', () => { helper.start(); expect( - CONTROLLER_ARGS_MOCK.blockTracker.addListener, + CONTROLLER_ARGS_MOCK.getBlockTracker().addListener, ).toHaveBeenCalledTimes(1); }); @@ -617,7 +617,7 @@ describe('IncomingTransactionHelper', () => { helper.start(); expect( - CONTROLLER_ARGS_MOCK.blockTracker.addListener, + CONTROLLER_ARGS_MOCK.getBlockTracker().addListener, ).not.toHaveBeenCalled(); }); @@ -632,7 +632,7 @@ describe('IncomingTransactionHelper', () => { helper.start(); expect( - CONTROLLER_ARGS_MOCK.blockTracker.addListener, + CONTROLLER_ARGS_MOCK.getBlockTracker().addListener, ).not.toHaveBeenCalled(); }); }); @@ -648,7 +648,7 @@ describe('IncomingTransactionHelper', () => { helper.stop(); expect( - CONTROLLER_ARGS_MOCK.blockTracker.removeListener, + CONTROLLER_ARGS_MOCK.getBlockTracker().removeListener, ).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index c6600b48931..43ca79fd369 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -33,7 +33,11 @@ export type IncomingTransactionOptions = { export class IncomingTransactionHelper { hub: EventEmitter; - #blockTracker: BlockTracker; + #currentBlockTracker?: BlockTracker; + + #getBlockTracker: () => BlockTracker | undefined; + + #getChainId: () => Hex | undefined; #getCurrentAccount: () => string; @@ -41,8 +45,6 @@ export class IncomingTransactionHelper { #getLocalTransactions: () => TransactionMeta[]; - #getChainId: () => Hex; - #isEnabled: () => boolean; #isRunning: boolean; @@ -60,22 +62,22 @@ export class IncomingTransactionHelper { #updateTransactions: boolean; constructor({ - blockTracker, + getBlockTracker, + getChainId, getCurrentAccount, getLastFetchedBlockNumbers, getLocalTransactions, - getChainId, isEnabled, queryEntireHistory, remoteTransactionSource, transactionLimit, updateTransactions, }: { - blockTracker: BlockTracker; + getBlockTracker: () => BlockTracker | undefined; + getChainId: () => Hex | undefined; getCurrentAccount: () => string; getLastFetchedBlockNumbers: () => Record; getLocalTransactions?: () => TransactionMeta[]; - getChainId: () => Hex; isEnabled?: () => boolean; queryEntireHistory?: boolean; remoteTransactionSource: RemoteTransactionSource; @@ -84,11 +86,11 @@ export class IncomingTransactionHelper { }) { this.hub = new EventEmitter(); - this.#blockTracker = blockTracker; + this.#getBlockTracker = getBlockTracker; + this.#getChainId = getChainId; this.#getCurrentAccount = getCurrentAccount; this.#getLastFetchedBlockNumbers = getLastFetchedBlockNumbers; this.#getLocalTransactions = getLocalTransactions || (() => []); - this.#getChainId = getChainId; this.#isEnabled = isEnabled ?? (() => true); this.#isRunning = false; this.#queryEntireHistory = queryEntireHistory ?? true; @@ -112,16 +114,25 @@ export class IncomingTransactionHelper { return; } - if (!this.#canStart()) { + const { blockTracker, chainId } = this.#getNetworkObjects(); + + if (!blockTracker || !chainId) { + log('Cannot start as network is not available'); + return; + } + + if (!this.#canStart(chainId)) { return; } - this.#blockTracker.addListener('latest', this.#onLatestBlock); + this.#currentBlockTracker = blockTracker; + this.#currentBlockTracker.addListener('latest', this.#onLatestBlock); this.#isRunning = true; } stop() { - this.#blockTracker.removeListener('latest', this.#onLatestBlock); + this.#currentBlockTracker?.removeListener('latest', this.#onLatestBlock); + this.#currentBlockTracker = undefined; this.#isRunning = false; } @@ -131,21 +142,28 @@ export class IncomingTransactionHelper { log('Checking for incoming transactions'); try { - if (!this.#canStart()) { + const { blockTracker, chainId } = this.#getNetworkObjects(); + + if (!blockTracker || !chainId) { + log('Cannot update as network client is not available'); + return; + } + + if (!this.#canStart(chainId)) { return; } const latestBlockNumber = parseInt( - latestBlockNumberHex || (await this.#blockTracker.getLatestBlock()), + latestBlockNumberHex || (await blockTracker.getLatestBlock()), 16, ); const additionalLastFetchedKeys = this.#remoteTransactionSource.getLastBlockVariations?.() ?? []; - const fromBlock = this.#getFromBlock(latestBlockNumber); + const fromBlock = this.#getFromBlock(latestBlockNumber, chainId); const address = this.#getCurrentAccount(); - const currentChainId = this.#getChainId(); + const currentChainId = chainId; let remoteTransactions = []; @@ -197,9 +215,11 @@ export class IncomingTransactionHelper { updated: updatedTransactions, }); } + this.#updateLastFetchedBlockNumber( remoteTransactions, additionalLastFetchedKeys, + currentChainId, ); } finally { releaseLock(); @@ -241,16 +261,21 @@ export class IncomingTransactionHelper { ); } - #getLastFetchedBlockNumberDec(): number { + #getLastFetchedBlockNumberDec(chainId: Hex): number { const additionalLastFetchedKeys = this.#remoteTransactionSource.getLastBlockVariations?.() ?? []; - const lastFetchedKey = this.#getBlockNumberKey(additionalLastFetchedKeys); + + const lastFetchedKey = this.#getBlockNumberKey( + additionalLastFetchedKeys, + chainId, + ); + const lastFetchedBlockNumbers = this.#getLastFetchedBlockNumbers(); return lastFetchedBlockNumbers[lastFetchedKey]; } - #getFromBlock(latestBlockNumber: number): number | undefined { - const lastFetchedBlockNumber = this.#getLastFetchedBlockNumberDec(); + #getFromBlock(latestBlockNumber: number, chainId: Hex): number | undefined { + const lastFetchedBlockNumber = this.#getLastFetchedBlockNumberDec(chainId); if (lastFetchedBlockNumber) { return lastFetchedBlockNumber + 1; @@ -264,6 +289,7 @@ export class IncomingTransactionHelper { #updateLastFetchedBlockNumber( remoteTxs: TransactionMeta[], additionalKeys: string[], + chainId: Hex, ) { let lastFetchedBlockNumber = -1; @@ -282,7 +308,7 @@ export class IncomingTransactionHelper { return; } - const lastFetchedKey = this.#getBlockNumberKey(additionalKeys); + const lastFetchedKey = this.#getBlockNumberKey(additionalKeys, chainId); const lastFetchedBlockNumbers = this.#getLastFetchedBlockNumbers(); const previousValue = lastFetchedBlockNumbers[lastFetchedKey]; @@ -299,20 +325,25 @@ export class IncomingTransactionHelper { }); } - #getBlockNumberKey(additionalKeys: string[]): string { - const currentChainId = this.#getChainId(); + #getBlockNumberKey(additionalKeys: string[], chainId: Hex): string { const currentAccount = this.#getCurrentAccount()?.toLowerCase(); - return [currentChainId, currentAccount, ...additionalKeys].join('#'); + return [chainId, currentAccount, ...additionalKeys].join('#'); } - #canStart(): boolean { + #canStart(chainId: Hex): boolean { const isEnabled = this.#isEnabled(); - const currentChainId = this.#getChainId(); const isSupportedNetwork = - this.#remoteTransactionSource.isSupportedNetwork(currentChainId); + this.#remoteTransactionSource.isSupportedNetwork(chainId); return isEnabled && isSupportedNetwork; } + + #getNetworkObjects() { + const blockTracker = this.#getBlockTracker(); + const chainId = this.#getChainId(); + + return { blockTracker, chainId }; + } } diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts index 5a03b5c55f9..e0536ae91e9 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts @@ -8,6 +8,7 @@ import { useFakeTimers } from 'sinon'; import { advanceTime } from '../../../../tests/helpers'; import { EtherscanRemoteTransactionSource } from './EtherscanRemoteTransactionSource'; import type { IncomingTransactionHelper } from './IncomingTransactionHelper'; +import type { MultichainTrackingHelperOptions } from './MultichainTrackingHelper'; import { MultichainTrackingHelper } from './MultichainTrackingHelper'; import type { PendingTransactionTracker } from './PendingTransactionTracker'; @@ -139,11 +140,12 @@ function newMultichainTrackingHelper( const mockNonceLock = { releaseLock: jest.fn() }; const mockNonceTrackers: Record> = {}; + const mockGetNonceLock = jest.fn().mockResolvedValue(mockNonceLock); const mockCreateNonceTracker = jest .fn() .mockImplementation(({ chainId }: { chainId: Hex }) => { const mockNonceTracker = { - getNonceLock: jest.fn().mockResolvedValue(mockNonceLock), + getNonceLock: mockGetNonceLock, } as unknown as jest.Mocked; mockNonceTrackers[chainId] = mockNonceTracker; return mockNonceTracker; @@ -155,13 +157,14 @@ function newMultichainTrackingHelper( > = {}; const mockCreateIncomingTransactionHelper = jest .fn() - .mockImplementation(({ chainId }: { chainId: Hex }) => { + .mockImplementation(({ getChainId }: { getChainId: () => Hex }) => { const mockIncomingTransactionHelper = { start: jest.fn(), stop: jest.fn(), update: jest.fn(), } as unknown as jest.Mocked; - mockIncomingTransactionHelpers[chainId] = mockIncomingTransactionHelper; + mockIncomingTransactionHelpers[getChainId()] = + mockIncomingTransactionHelper; return mockIncomingTransactionHelper; }); @@ -171,18 +174,18 @@ function newMultichainTrackingHelper( > = {}; const mockCreatePendingTransactionTracker = jest .fn() - .mockImplementation(({ chainId }: { chainId: Hex }) => { + .mockImplementation(({ getChainId }: { getChainId: () => Hex }) => { const mockPendingTransactionTracker = { start: jest.fn(), stop: jest.fn(), } as unknown as jest.Mocked; - mockPendingTransactionTrackers[chainId] = mockPendingTransactionTracker; + mockPendingTransactionTrackers[getChainId()] = + mockPendingTransactionTracker; return mockPendingTransactionTracker; }); - const options = { + const options: jest.Mocked = { isMultichainEnabled: true, - provider: MOCK_PROVIDERS.mainnet, nonceTracker: { getNonceLock: jest.fn().mockResolvedValue(mockNonceLock), }, @@ -194,6 +197,10 @@ function newMultichainTrackingHelper( updateTransactions: true, }, findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, + getGlobalProviderAndBlockTracker: () => ({ + provider: MOCK_PROVIDERS.mainnet, + blockTracker: MOCK_BLOCK_TRACKERS.mainnet, + }), getNetworkClientById: mockGetNetworkClientById, getNetworkClientRegistry: mockGetNetworkClientRegistry, removeIncomingTransactionHelperListeners: jest.fn(), @@ -210,6 +217,7 @@ function newMultichainTrackingHelper( return { helper, options, + mockGetNonceLock, mockNonceLock, mockNonceTrackers, mockIncomingTransactionHelpers, @@ -341,20 +349,31 @@ describe('MultichainTrackingHelper', () => { }); expect(options.createIncomingTransactionHelper).toHaveBeenCalledTimes(1); - expect(options.createIncomingTransactionHelper).toHaveBeenCalledWith({ - blockTracker: MOCK_BLOCK_TRACKERS.mainnet, - etherscanRemoteTransactionSource: expect.any( - EtherscanRemoteTransactionSource, - ), - chainId: '0x1', - }); + expect( + options.createIncomingTransactionHelper.mock.calls[0][0].getBlockTracker(), + ).toBe(MOCK_BLOCK_TRACKERS.mainnet); + expect( + options.createIncomingTransactionHelper.mock.calls[0][0].getChainId(), + ).toBe('0x1'); + expect(options.createIncomingTransactionHelper).toHaveBeenCalledWith( + expect.objectContaining({ + etherscanRemoteTransactionSource: expect.any( + EtherscanRemoteTransactionSource, + ), + }), + ); expect(options.createPendingTransactionTracker).toHaveBeenCalledTimes(1); - expect(options.createPendingTransactionTracker).toHaveBeenCalledWith({ - provider: MOCK_PROVIDERS.mainnet, - blockTracker: MOCK_BLOCK_TRACKERS.mainnet, - chainId: '0x1', - }); + expect( + options.createPendingTransactionTracker.mock.calls[0][0].getBlockTracker(), + ).toBe(MOCK_BLOCK_TRACKERS.mainnet); + expect( + options.createPendingTransactionTracker.mock.calls[0][0].getChainId(), + ).toBe('0x1'); + expect( + options.createPendingTransactionTracker.mock.calls[0][0].getEthQuery() + ?.provider, + ).toBe(MOCK_PROVIDERS.mainnet); expect(helper.has('mainnet')).toBe(true); }); @@ -525,7 +544,7 @@ describe('MultichainTrackingHelper', () => { expect(releaseLockForChainIdKey).not.toHaveBeenCalled(); expect(mockNonceLock.releaseLock).not.toHaveBeenCalled(); - nonceLock.releaseLock(); + nonceLock?.releaseLock(); expect(releaseLockForChainIdKey).toHaveBeenCalled(); expect(mockNonceLock.releaseLock).toHaveBeenCalled(); @@ -585,23 +604,23 @@ describe('MultichainTrackingHelper', () => { }); it('gets the nonce lock from the global NonceTracker', async () => { - const { options, helper } = newMultichainTrackingHelper({}); + const { helper, mockNonceTrackers } = newMultichainTrackingHelper({}); helper.initialize(); await helper.getNonceLock('0xdeadbeef'); - expect(options.nonceTracker.getNonceLock).toHaveBeenCalledWith( + expect(mockNonceTrackers['0x1'].getNonceLock).toHaveBeenCalledWith( '0xdeadbeef', ); }); it('throws an error if unable to acquire nonce lock from the global NonceTracker', async () => { - const { options, helper } = newMultichainTrackingHelper({}); + const { helper, mockGetNonceLock } = newMultichainTrackingHelper({}); helper.initialize(); - options.nonceTracker.getNonceLock.mockRejectedValue( + mockGetNonceLock.mockRejectedValue( 'failed to acquire lock from nonceTracker', ); @@ -634,7 +653,7 @@ describe('MultichainTrackingHelper', () => { }); it('gets the nonce lock from the global NonceTracker', async () => { - const { options, helper } = newMultichainTrackingHelper({ + const { helper, mockNonceTrackers } = newMultichainTrackingHelper({ isMultichainEnabled: false, }); @@ -642,30 +661,26 @@ describe('MultichainTrackingHelper', () => { await helper.getNonceLock('0xdeadbeef', '0xabc'); - expect(options.nonceTracker.getNonceLock).toHaveBeenCalledWith( - '0xdeadbeef', - ); + expect(Object.values(mockNonceTrackers)).toHaveLength(1); + expect( + Object.values(mockNonceTrackers)[0].getNonceLock, + ).toHaveBeenCalledWith('0xdeadbeef'); }); it('throws an error if unable to acquire nonce lock from the global NonceTracker', async () => { - const { options, helper } = newMultichainTrackingHelper({ + const { helper, mockGetNonceLock } = newMultichainTrackingHelper({ isMultichainEnabled: false, }); helper.initialize(); - options.nonceTracker.getNonceLock.mockRejectedValue( + mockGetNonceLock.mockRejectedValue( 'failed to acquire lock from nonceTracker', ); - // for some reason jest expect().rejects.toThrow doesn't work here - let error = ''; - try { - await helper.getNonceLock('0xdeadbeef', '0xabc'); - } catch (err: unknown) { - error = err as string; - } - expect(error).toBe('failed to acquire lock from nonceTracker'); + await expect(helper.getNonceLock('0xdeadbeef', '0xabc')).rejects.toBe( + 'failed to acquire lock from nonceTracker', + ); }); }); }); @@ -733,7 +748,7 @@ describe('MultichainTrackingHelper', () => { networkClientId: 'goerli', chainId: '0xa', }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.goerli); + expect(ethQuery?.provider).toBe(MOCK_PROVIDERS.goerli); expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); expect(options.getNetworkClientById).toHaveBeenCalledWith('goerli'); @@ -746,7 +761,7 @@ describe('MultichainTrackingHelper', () => { networkClientId: 'missingNetworkClientId', chainId: '0xa', }); - expect(ethQuery.provider).toBe( + expect(ethQuery?.provider).toBe( MOCK_PROVIDERS['customNetworkClientId-1'], ); @@ -761,24 +776,6 @@ describe('MultichainTrackingHelper', () => { 'customNetworkClientId-1', ); }); - - it('returns EthQuery with the fallback global provider if networkClientId and chainId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ - networkClientId: 'missingNetworkClientId', - chainId: '0xdeadbeef', - }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'missingNetworkClientId', - ); - expect(options.findNetworkClientIdByChainId).toHaveBeenCalledWith( - '0xdeadbeef', - ); - }); }); describe('when given only networkClientId', () => { @@ -786,25 +783,11 @@ describe('MultichainTrackingHelper', () => { const { options, helper } = newMultichainTrackingHelper(); const ethQuery = helper.getEthQuery({ networkClientId: 'goerli' }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.goerli); + expect(ethQuery?.provider).toBe(MOCK_PROVIDERS.goerli); expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); expect(options.getNetworkClientById).toHaveBeenCalledWith('goerli'); }); - - it('returns EthQuery with the fallback global provider if networkClientId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ - networkClientId: 'missingNetworkClientId', - }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'missingNetworkClientId', - ); - }); }); describe('when given only chainId', () => { @@ -812,7 +795,7 @@ describe('MultichainTrackingHelper', () => { const { options, helper } = newMultichainTrackingHelper(); const ethQuery = helper.getEthQuery({ chainId: '0xa' }); - expect(ethQuery.provider).toBe( + expect(ethQuery?.provider).toBe( MOCK_PROVIDERS['customNetworkClientId-1'], ); @@ -824,24 +807,13 @@ describe('MultichainTrackingHelper', () => { 'customNetworkClientId-1', ); }); - - it('returns EthQuery with the fallback global provider if chainId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ chainId: '0xdeadbeef' }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.findNetworkClientIdByChainId).toHaveBeenCalledWith( - '0xdeadbeef', - ); - }); }); it('returns EthQuery with the global provider when no arguments are provided', () => { const { options, helper } = newMultichainTrackingHelper(); const ethQuery = helper.getEthQuery(); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); + expect(ethQuery?.provider).toBe(MOCK_PROVIDERS.mainnet); expect(options.getNetworkClientById).not.toHaveBeenCalled(); }); @@ -855,13 +827,13 @@ describe('MultichainTrackingHelper', () => { networkClientId: 'goerli', chainId: '0x5', }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); + expect(ethQuery?.provider).toBe(MOCK_PROVIDERS.mainnet); ethQuery = helper.getEthQuery({ networkClientId: 'goerli' }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); + expect(ethQuery?.provider).toBe(MOCK_PROVIDERS.mainnet); ethQuery = helper.getEthQuery({ chainId: '0x5' }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); + expect(ethQuery?.provider).toBe(MOCK_PROVIDERS.mainnet); ethQuery = helper.getEthQuery(); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); + expect(ethQuery?.provider).toBe(MOCK_PROVIDERS.mainnet); expect(options.getNetworkClientById).not.toHaveBeenCalled(); }); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts index 4c2e3fdd646..403cfabdbd3 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts @@ -11,7 +11,7 @@ import type { NonceLock, NonceTracker } from '@metamask/nonce-tracker'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; -import { incomingTransactionsLogger as log } from '../logger'; +import { createModuleLogger, projectLogger } from '../logger'; import { EtherscanRemoteTransactionSource } from './EtherscanRemoteTransactionSource'; import type { IncomingTransactionHelper, @@ -19,6 +19,8 @@ import type { } from './IncomingTransactionHelper'; import type { PendingTransactionTracker } from './PendingTransactionTracker'; +const log = createModuleLogger(projectLogger, 'multichain'); + /** * Registry of network clients provided by the NetworkController */ @@ -28,11 +30,12 @@ type NetworkClientRegistry = ReturnType< export type MultichainTrackingHelperOptions = { isMultichainEnabled: boolean; - provider: Provider; - nonceTracker: NonceTracker; incomingTransactionOptions: IncomingTransactionOptions; findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; + getGlobalProviderAndBlockTracker: () => + | { provider: Provider; blockTracker: BlockTracker } + | undefined; getNetworkClientById: NetworkController['getNetworkClientById']; getNetworkClientRegistry: NetworkController['getNetworkClientRegistry']; @@ -48,14 +51,14 @@ export type MultichainTrackingHelperOptions = { chainId?: Hex; }) => NonceTracker; createIncomingTransactionHelper: (opts: { - blockTracker: BlockTracker; + getBlockTracker: () => BlockTracker | undefined; + getChainId: () => Hex | undefined; etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; - chainId?: Hex; }) => IncomingTransactionHelper; createPendingTransactionTracker: (opts: { - provider: Provider; - blockTracker: BlockTracker; - chainId?: Hex; + getBlockTracker: () => BlockTracker | undefined; + getChainId: () => Hex | undefined; + getEthQuery: () => EthQuery | undefined; }) => PendingTransactionTracker; onNetworkStateChange: ( listener: ( @@ -67,14 +70,16 @@ export type MultichainTrackingHelperOptions = { export class MultichainTrackingHelper { #isMultichainEnabled: boolean; - readonly #provider: Provider; - - readonly #nonceTracker: NonceTracker; + #globalNonceTracker: NonceTracker | undefined; readonly #incomingTransactionOptions: IncomingTransactionOptions; readonly #findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; + readonly #getGlobalProviderAndBlockTracker: () => + | { provider: Provider; blockTracker: BlockTracker } + | undefined; + readonly #getNetworkClientById: NetworkController['getNetworkClientById']; readonly #getNetworkClientRegistry: NetworkController['getNetworkClientRegistry']; @@ -94,15 +99,15 @@ export class MultichainTrackingHelper { }) => NonceTracker; readonly #createIncomingTransactionHelper: (opts: { - blockTracker: BlockTracker; - chainId?: Hex; + getBlockTracker: () => BlockTracker | undefined; + getChainId: () => Hex | undefined; etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; }) => IncomingTransactionHelper; readonly #createPendingTransactionTracker: (opts: { - provider: Provider; - blockTracker: BlockTracker; - chainId?: Hex; + getBlockTracker: () => BlockTracker | undefined; + getChainId: () => Hex | undefined; + getEthQuery: () => EthQuery | undefined; }) => PendingTransactionTracker; readonly #nonceMutexesByChainId = new Map>(); @@ -123,10 +128,9 @@ export class MultichainTrackingHelper { constructor({ isMultichainEnabled, - provider, - nonceTracker, incomingTransactionOptions, findNetworkClientIdByChainId, + getGlobalProviderAndBlockTracker, getNetworkClientById, getNetworkClientRegistry, removeIncomingTransactionHelperListeners, @@ -137,11 +141,10 @@ export class MultichainTrackingHelper { onNetworkStateChange, }: MultichainTrackingHelperOptions) { this.#isMultichainEnabled = isMultichainEnabled; - this.#provider = provider; - this.#nonceTracker = nonceTracker; this.#incomingTransactionOptions = incomingTransactionOptions; this.#findNetworkClientIdByChainId = findNetworkClientIdByChainId; + this.#getGlobalProviderAndBlockTracker = getGlobalProviderAndBlockTracker; this.#getNetworkClientById = getNetworkClientById; this.#getNetworkClientRegistry = getNetworkClientRegistry; @@ -186,8 +189,36 @@ export class MultichainTrackingHelper { }: { networkClientId?: NetworkClientId; chainId?: Hex; - } = {}): EthQuery { - return new EthQuery(this.getProvider({ networkClientId, chainId })); + } = {}): EthQuery | undefined { + if (!networkClientId && !chainId) { + const globalProvider = this.#getGlobalProviderAndBlockTracker()?.provider; + return globalProvider ? new EthQuery(globalProvider) : undefined; + } + let networkClient: NetworkClient | undefined; + + if (networkClientId) { + try { + networkClient = this.#getNetworkClientById(networkClientId); + } catch (err) { + log('Failed to get network client by networkClientId', networkClientId); + } + } + + if (!networkClient && chainId) { + try { + networkClientId = this.#findNetworkClientIdByChainId(chainId); + networkClient = this.#getNetworkClientById(networkClientId); + } catch (err) { + log('Failed to get network client by chainId', chainId); + } + } + if (networkClient) { + const provider = this.getProvider({ networkClientId, chainId }); + if (provider) { + return new EthQuery(provider); + } + } + return undefined; } getProvider({ @@ -196,17 +227,16 @@ export class MultichainTrackingHelper { }: { networkClientId?: NetworkClientId; chainId?: Hex; - } = {}): Provider { - if (!this.#isMultichainEnabled) { - return this.#provider; - } - + } = {}): Provider | undefined { const networkClient = this.#getNetworkClient({ networkClientId, chainId, }); - return networkClient?.provider || this.#provider; + return ( + networkClient?.provider || + this.#getGlobalProviderAndBlockTracker()?.provider + ); } /** @@ -249,9 +279,15 @@ export class MultichainTrackingHelper { async getNonceLock( address: string, networkClientId?: NetworkClientId, - ): Promise { + ): Promise { let releaseLockForChainIdKey: (() => void) | undefined; - let nonceTracker = this.#nonceTracker; + let nonceTracker: NonceTracker | undefined; + + if (!networkClientId || !this.#isMultichainEnabled) { + this.#globalNonceTracker ??= this.#getGlobalNonceTracker(); + nonceTracker = this.#globalNonceTracker; + } + if (networkClientId && this.#isMultichainEnabled) { const networkClient = this.#getNetworkClientById(networkClientId); releaseLockForChainIdKey = await this.acquireNonceLockForChainIdKey({ @@ -265,6 +301,10 @@ export class MultichainTrackingHelper { nonceTracker = trackers.nonceTracker; } + if (!nonceTracker) { + return undefined; + } + // Acquires the lock for the chainId + address and the nonceLock from the nonceTracker, then // couples them together by replacing the nonceLock's releaseLock method with // an anonymous function that calls releases both the original nonceLock and the @@ -377,11 +417,9 @@ export class MultichainTrackingHelper { return; } - const { - provider, - blockTracker, - configuration: { chainId }, - } = this.#getNetworkClientById(networkClientId); + const networkClient = this.#getNetworkClientById(networkClientId); + const { provider, blockTracker, configuration } = networkClient; + const { chainId } = configuration; let etherscanRemoteTransactionSource = this.#etherscanRemoteTransactionSourcesMap.get(chainId); @@ -403,15 +441,15 @@ export class MultichainTrackingHelper { }); const incomingTransactionHelper = this.#createIncomingTransactionHelper({ - blockTracker, + getBlockTracker: () => blockTracker, + getChainId: () => chainId, etherscanRemoteTransactionSource, - chainId, }); const pendingTransactionTracker = this.#createPendingTransactionTracker({ - provider, - blockTracker, - chainId, + getBlockTracker: () => blockTracker, + getChainId: () => chainId, + getEthQuery: () => new EthQuery(provider), }); this.#trackingMap.set(networkClientId, { @@ -443,6 +481,23 @@ export class MultichainTrackingHelper { }); }; + #getGlobalNonceTracker(): NonceTracker | undefined { + const globalProvider = this.#getGlobalProviderAndBlockTracker()?.provider; + + const globalBlockTracker = + this.#getGlobalProviderAndBlockTracker()?.blockTracker; + + if (!globalProvider || !globalBlockTracker) { + log('Cannot get nonce lock as selected network is not available'); + return undefined; + } + + return this.#createNonceTracker({ + provider: globalProvider, + blockTracker: globalBlockTracker, + }); + } + #getNetworkClient({ networkClientId, chainId, diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts index 8552832202b..be8f4da8384 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts @@ -90,7 +90,7 @@ describe('PendingTransactionTracker', () => { options = { approveTransaction: jest.fn(), - blockTracker, + getBlockTracker: () => blockTracker, failTransaction, getChainId: () => CHAIN_ID_MOCK, getEthQuery: () => ETH_QUERY_MOCK, diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index 9ccf4465304..e545e4f39ee 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -1,13 +1,12 @@ import { query } from '@metamask/controller-utils'; import type EthQuery from '@metamask/eth-query'; -import type { - BlockTracker, - NetworkClientId, -} from '@metamask/network-controller'; +import type { BlockTracker } from '@metamask/network-controller'; +import type { Hex } from '@metamask/utils'; +import { createModuleLogger } from '@metamask/utils'; import EventEmitter from 'events'; import { cloneDeep, merge } from 'lodash'; -import { createModuleLogger, projectLogger } from '../logger'; +import { projectLogger } from '../logger'; import type { TransactionMeta, TransactionReceipt } from '../types'; import { TransactionStatus, TransactionType } from '../types'; @@ -61,13 +60,21 @@ export class PendingTransactionTracker { #approveTransaction: (transactionId: string) => Promise; - #blockTracker: BlockTracker; + #beforeCheckPendingTransaction: (transactionMeta: TransactionMeta) => boolean; + + #beforePublish: (transactionMeta: TransactionMeta) => boolean; + + #currentBlockTracker?: BlockTracker; #droppedBlockCountByHash: Map; - #getChainId: () => string; + #getBlockTracker: () => BlockTracker | undefined; - #getEthQuery: (networkClientId?: NetworkClientId) => EthQuery; + #getChainId: () => Hex | undefined; + + #getEthQuery: () => EthQuery | undefined; + + #getGlobalLock: (chainId: Hex) => Promise<() => void>; #getTransactions: () => TransactionMeta[]; @@ -77,19 +84,13 @@ export class PendingTransactionTracker { // eslint-disable-next-line @typescript-eslint/no-explicit-any #listener: any; - #getGlobalLock: () => Promise<() => void>; - #publishTransaction: (ethQuery: EthQuery, rawTx: string) => Promise; #running: boolean; - #beforeCheckPendingTransaction: (transactionMeta: TransactionMeta) => boolean; - - #beforePublish: (transactionMeta: TransactionMeta) => boolean; - constructor({ approveTransaction, - blockTracker, + getBlockTracker, getChainId, getEthQuery, getTransactions, @@ -99,12 +100,12 @@ export class PendingTransactionTracker { hooks, }: { approveTransaction: (transactionId: string) => Promise; - blockTracker: BlockTracker; - getChainId: () => string; - getEthQuery: (networkClientId?: NetworkClientId) => EthQuery; + getBlockTracker: () => BlockTracker | undefined; + getChainId: () => Hex | undefined; + getEthQuery: () => EthQuery | undefined; getTransactions: () => TransactionMeta[]; isResubmitEnabled?: () => boolean; - getGlobalLock: () => Promise<() => void>; + getGlobalLock: (chainId: Hex) => Promise<() => void>; publishTransaction: (ethQuery: EthQuery, rawTx: string) => Promise; hooks?: { beforeCheckPendingTransaction?: ( @@ -116,8 +117,8 @@ export class PendingTransactionTracker { this.hub = new EventEmitter() as PendingTransactionTrackerEventEmitter; this.#approveTransaction = approveTransaction; - this.#blockTracker = blockTracker; this.#droppedBlockCountByHash = new Map(); + this.#getBlockTracker = getBlockTracker; this.#getChainId = getChainId; this.#getEthQuery = getEthQuery; this.#getTransactions = getTransactions; @@ -132,39 +133,60 @@ export class PendingTransactionTracker { } startIfPendingTransactions = () => { - const pendingTransactions = this.#getPendingTransactions(); + const { blockTracker, chainId } = this.#getNetworkObjects(); + + if (!blockTracker || !chainId) { + log('Unable to start as network is not available'); + return; + } + + this.#currentBlockTracker = blockTracker; + + const pendingTransactions = this.#getPendingTransactions(chainId); if (pendingTransactions.length) { - this.#start(); + this.#start(blockTracker); } else { this.stop(); } }; /** - * Force checks the network if the given transaction is confirmed and updates it's status. + * Force checks the network if the given transaction is confirmed and updates its status. * * @param txMeta - The transaction to check */ async forceCheckTransaction(txMeta: TransactionMeta) { - const releaseLock = await this.#getGlobalLock(); + let releaseLock: (() => void) | undefined; try { - await this.#checkTransaction(txMeta); + const { blockTracker, chainId, ethQuery } = this.#getNetworkObjects(); + + if (!blockTracker || !chainId || !ethQuery) { + log( + 'Cannot force check transaction as network not available', + txMeta.id, + ); + return; + } + + releaseLock = await this.#getGlobalLock(chainId); + + await this.#checkTransaction(txMeta, ethQuery, chainId); } catch (error) { /* istanbul ignore next */ - log('Failed to check transaction', error); + log('Failed to force check transaction', error); } finally { - releaseLock(); + releaseLock?.(); } } - #start() { + #start(blockTracker: BlockTracker) { if (this.#running) { return; } - this.#blockTracker.on('latest', this.#listener); + blockTracker.on('latest', this.#listener); this.#running = true; log('Started polling'); @@ -175,36 +197,48 @@ export class PendingTransactionTracker { return; } - this.#blockTracker.removeListener('latest', this.#listener); + this.#currentBlockTracker?.removeListener('latest', this.#listener); + this.#currentBlockTracker = undefined; this.#running = false; log('Stopped polling'); } async #onLatestBlock(latestBlockNumber: string) { - const releaseLock = await this.#getGlobalLock(); - try { - await this.#checkTransactions(); - } catch (error) { - /* istanbul ignore next */ - log('Failed to check transactions', error); - } finally { - releaseLock(); - } + const { blockTracker, chainId, ethQuery } = this.#getNetworkObjects(); - try { - await this.#resubmitTransactions(latestBlockNumber); + if (!blockTracker || !chainId || !ethQuery) { + log('Cannot process latest block as network not available'); + return; + } + + const releaseLock = await this.#getGlobalLock(chainId); + + try { + await this.#checkTransactions(chainId, ethQuery); + } catch (error) { + /* istanbul ignore next */ + log('Failed to check transactions', error); + } finally { + releaseLock(); + } + + try { + await this.#resubmitTransactions(latestBlockNumber, chainId, ethQuery); + } catch (error) { + /* istanbul ignore next */ + log('Failed to resubmit transactions', error); + } } catch (error) { - /* istanbul ignore next */ - log('Failed to resubmit transactions', error); + log('Failed to process latest block', error); } } - async #checkTransactions() { + async #checkTransactions(chainId: Hex, ethQuery: EthQuery) { log('Checking transactions'); - const pendingTransactions = this.#getPendingTransactions(); + const pendingTransactions = this.#getPendingTransactions(chainId); if (!pendingTransactions.length) { log('No pending transactions to check'); @@ -217,18 +251,24 @@ export class PendingTransactionTracker { }); await Promise.all( - pendingTransactions.map((tx) => this.#checkTransaction(tx)), + pendingTransactions.map((tx) => + this.#checkTransaction(tx, ethQuery, chainId), + ), ); } - async #resubmitTransactions(latestBlockNumber: string) { + async #resubmitTransactions( + latestBlockNumber: string, + chainId: Hex, + ethQuery: EthQuery, + ) { if (!this.#isResubmitEnabled() || !this.#running) { return; } log('Resubmitting transactions'); - const pendingTransactions = this.#getPendingTransactions(); + const pendingTransactions = this.#getPendingTransactions(chainId); if (!pendingTransactions.length) { log('No pending transactions to resubmit'); @@ -242,7 +282,7 @@ export class PendingTransactionTracker { for (const txMeta of pendingTransactions) { try { - await this.#resubmitTransaction(txMeta, latestBlockNumber); + await this.#resubmitTransaction(txMeta, latestBlockNumber, ethQuery); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { @@ -273,11 +313,14 @@ export class PendingTransactionTracker { async #resubmitTransaction( txMeta: TransactionMeta, latestBlockNumber: string, + ethQuery: EthQuery, ) { if (!this.#isResubmitDue(txMeta, latestBlockNumber)) { return; } + log('Resubmitting transaction', txMeta.id); + const { rawTx } = txMeta; if (!this.#beforePublish(txMeta)) { @@ -290,7 +333,6 @@ export class PendingTransactionTracker { return; } - const ethQuery = this.#getEthQuery(txMeta.networkClientId); await this.#publishTransaction(ethQuery, rawTx); const retryCount = (txMeta.retryCount ?? 0) + 1; @@ -331,7 +373,11 @@ export class PendingTransactionTracker { return blocksSinceFirstRetry >= requiredBlocksSinceFirstRetry; } - async #checkTransaction(txMeta: TransactionMeta) { + async #checkTransaction( + txMeta: TransactionMeta, + ethQuery: EthQuery, + chainId: Hex, + ) { const { hash, id } = txMeta; if (!hash && this.#beforeCheckPendingTransaction(txMeta)) { @@ -346,14 +392,14 @@ export class PendingTransactionTracker { return; } - if (this.#isNonceTaken(txMeta)) { + if (this.#isNonceTaken(txMeta, chainId)) { log('Nonce already taken', id); this.#dropTransaction(txMeta); return; } try { - const receipt = await this.#getTransactionReceipt(hash); + const receipt = await this.#getTransactionReceipt(ethQuery, hash); const isSuccess = receipt?.status === RECEIPT_STATUS_SUCCESS; const isFailure = receipt?.status === RECEIPT_STATUS_FAILURE; @@ -371,11 +417,15 @@ export class PendingTransactionTracker { const { blockNumber, blockHash } = receipt || {}; if (isSuccess && blockNumber && blockHash) { - await this.#onTransactionConfirmed(txMeta, { - ...receipt, - blockNumber, - blockHash, - }); + await this.#onTransactionConfirmed( + txMeta, + { + ...receipt, + blockNumber, + blockHash, + }, + ethQuery, + ); return; } @@ -393,7 +443,7 @@ export class PendingTransactionTracker { return; } - if (await this.#isTransactionDropped(txMeta)) { + if (await this.#isTransactionDropped(txMeta, ethQuery)) { this.#dropTransaction(txMeta); } } @@ -401,6 +451,7 @@ export class PendingTransactionTracker { async #onTransactionConfirmed( txMeta: TransactionMeta, receipt: SuccessfulTransactionReceipt, + ethQuery: EthQuery, ) { const { id } = txMeta; const { blockHash } = receipt; @@ -408,7 +459,7 @@ export class PendingTransactionTracker { log('Transaction confirmed', id); const { baseFeePerGas, timestamp: blockTimestamp } = - await this.#getBlockByHash(blockHash, false); + await this.#getBlockByHash(blockHash, false, ethQuery); const updatedTxMeta = cloneDeep(txMeta); updatedTxMeta.baseFeePerGas = baseFeePerGas; @@ -429,7 +480,7 @@ export class PendingTransactionTracker { this.hub.emit('transaction-confirmed', updatedTxMeta); } - async #isTransactionDropped(txMeta: TransactionMeta) { + async #isTransactionDropped(txMeta: TransactionMeta, ethQuery: EthQuery) { const { hash, id, @@ -441,7 +492,11 @@ export class PendingTransactionTracker { return false; } - const networkNextNonceHex = await this.#getNetworkTransactionCount(from); + const networkNextNonceHex = await this.#getNetworkTransactionCount( + from, + ethQuery, + ); + const networkNextNonceNumber = parseInt(networkNextNonceHex, 16); const nonceNumber = parseInt(nonce, 16); @@ -468,10 +523,10 @@ export class PendingTransactionTracker { return true; } - #isNonceTaken(txMeta: TransactionMeta): boolean { + #isNonceTaken(txMeta: TransactionMeta, chainId: Hex): boolean { const { id, txParams } = txMeta; - return this.#getCurrentChainTransactions().some( + return this.#getCurrentChainTransactions(chainId).some( (tx) => tx.id !== id && tx.txParams.from === txParams.from && @@ -481,8 +536,8 @@ export class PendingTransactionTracker { ); } - #getPendingTransactions(): TransactionMeta[] { - return this.#getCurrentChainTransactions().filter( + #getPendingTransactions(chainId: Hex): TransactionMeta[] { + return this.#getCurrentChainTransactions(chainId).filter( (tx) => tx.status === TransactionStatus.submitted && !tx.verifiedOnBlockchain && @@ -515,32 +570,43 @@ export class PendingTransactionTracker { } async #getTransactionReceipt( + ethQuery: EthQuery, txHash?: string, ): Promise { - return await query(this.#getEthQuery(), 'getTransactionReceipt', [txHash]); + return await query(ethQuery, 'getTransactionReceipt', [txHash]); } async #getBlockByHash( blockHash: string, includeTransactionDetails: boolean, + ethQuery: EthQuery, // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any ): Promise { - return await query(this.#getEthQuery(), 'getBlockByHash', [ + return await query(ethQuery, 'getBlockByHash', [ blockHash, includeTransactionDetails, ]); } - async #getNetworkTransactionCount(address: string): Promise { - return await query(this.#getEthQuery(), 'getTransactionCount', [address]); + async #getNetworkTransactionCount( + address: string, + ethQuery: EthQuery, + ): Promise { + return await query(ethQuery, 'getTransactionCount', [address]); } - #getCurrentChainTransactions(): TransactionMeta[] { - const currentChainId = this.#getChainId(); - + #getCurrentChainTransactions(currentChainId: Hex): TransactionMeta[] { return this.#getTransactions().filter( (tx) => tx.chainId === currentChainId, ); } + + #getNetworkObjects() { + const blockTracker = this.#getBlockTracker(); + const chainId = this.#getChainId(); + const ethQuery = this.#getEthQuery(); + + return { blockTracker, chainId, ethQuery }; + } } diff --git "a/packages/transaction-controller/src/helpers/\\" "b/packages/transaction-controller/src/helpers/\\" new file mode 100644 index 00000000000..f345c611612 --- /dev/null +++ "b/packages/transaction-controller/src/helpers/\\" @@ -0,0 +1,298 @@ +import EthQuery from '@metamask/eth-query'; +import type { + FetchGasFeeEstimateOptions, + GasFeeState, +} from '@metamask/gas-fee-controller'; +import type { NetworkClientId, Provider } from '@metamask/network-controller'; +import type { Hex } from '@metamask/utils'; +import { createModuleLogger } from '@metamask/utils'; +import EventEmitter from 'events'; + +import { projectLogger } from '../logger'; +import type { + GasFeeEstimates, + GasFeeFlow, + GasFeeFlowRequest, + Layer1GasFeeFlow, +} from '../types'; +import { TransactionStatus, type TransactionMeta } from '../types'; +import { getGasFeeFlow } from '../utils/gas-flow'; +import { getTransactionLayer1GasFee } from '../utils/layer1-gas-fee-flow'; + +const log = createModuleLogger(projectLogger, 'gas-fee-poller'); + +const INTERVAL_MILLISECONDS = 10000; + +/** + * Automatically polls and updates suggested gas fees on unapproved transactions. + */ +export class GasFeePoller { + hub: EventEmitter = new EventEmitter(); + + #findNetworkClientIdByChainId: (chainId: Hex) => NetworkClientId | undefined; + + #gasFeeFlows: GasFeeFlow[]; + + #getGasFeeControllerEstimates: ( + options: FetchGasFeeEstimateOptions, + ) => Promise; + + #getProvider: (chainId: Hex, networkClientId?: NetworkClientId) => Provider; + + #getTransactions: () => TransactionMeta[]; + + #layer1GasFeeFlows: Layer1GasFeeFlow[]; + + #timeout: ReturnType | undefined; + + #running = false; + + /** + * Constructs a new instance of the GasFeePoller. + * @param options - The options for this instance. + * @param options.findNetworkClientIdByChainId - Callback to find the network client ID by chain ID. + * @param options.gasFeeFlows - The gas fee flows to use to obtain suitable gas fees. + * @param options.getGasFeeControllerEstimates - Callback to obtain the default fee estimates. + * @param options.getProvider - Callback to obtain a provider instance. + * @param options.getTransactions - Callback to obtain the transaction data. + * @param options.layer1GasFeeFlows - The layer 1 gas fee flows to use to obtain suitable layer 1 gas fees. + * @param options.onStateChange - Callback to register a listener for controller state changes. + */ + constructor({ + findNetworkClientIdByChainId, + gasFeeFlows, + getGasFeeControllerEstimates, + getProvider, + getTransactions, + layer1GasFeeFlows, + onStateChange, + }: { + findNetworkClientIdByChainId: (chainId: Hex) => NetworkClientId | undefined; + gasFeeFlows: GasFeeFlow[]; + getGasFeeControllerEstimates: ( + options: FetchGasFeeEstimateOptions, + ) => Promise; + getProvider: (chainId: Hex, networkClientId?: NetworkClientId) => Provider; + getTransactions: () => TransactionMeta[]; + layer1GasFeeFlows: Layer1GasFeeFlow[]; + onStateChange: (listener: () => void) => void; + }) { + this.#findNetworkClientIdByChainId = findNetworkClientIdByChainId; + this.#gasFeeFlows = gasFeeFlows; + this.#layer1GasFeeFlows = layer1GasFeeFlows; + this.#getGasFeeControllerEstimates = getGasFeeControllerEstimates; + this.#getProvider = getProvider; + this.#getTransactions = getTransactions; + + onStateChange(() => { + const unapprovedTransactions = this.#getUnapprovedTransactions(); + + if (unapprovedTransactions.length) { + this.#start(); + } else { + this.#stop(); + } + }); + } + + #start() { + if (this.#running) { + return; + } + + // Intentionally not awaiting since this starts the timeout chain. + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this.#onTimeout(); + + this.#running = true; + + log('Started polling'); + } + + #stop() { + if (!this.#running) { + return; + } + + clearTimeout(this.#timeout); + + this.#timeout = undefined; + this.#running = false; + + log('Stopped polling'); + } + + async #onTimeout() { + await this.#updateUnapprovedTransactions(); + + // eslint-disable-next-line @typescript-eslint/no-misused-promises + this.#timeout = setTimeout(() => this.#onTimeout(), INTERVAL_MILLISECONDS); + } + + async #updateUnapprovedTransactions() { + const unapprovedTransactions = this.#getUnapprovedTransactions(); + + if (!unapprovedTransactions.length) { + return; + } + + log('Found unapproved transactions', unapprovedTransactions.length); + + const gasFeeControllerDataByChainId = await this.#getGasFeeControllerData( + unapprovedTransactions, + ); + + log('Retrieved gas fee controller data', gasFeeControllerDataByChainId); + + await Promise.all( + unapprovedTransactions.flatMap((tx) => { + const { chainId } = tx; + + const gasFeeControllerData = gasFeeControllerDataByChainId.get( + chainId, + ) as GasFeeState; + + return this.#updateUnapprovedTransaction(tx, gasFeeControllerData); + }), + ); + } + + async #updateUnapprovedTransaction( + transactionMeta: TransactionMeta, + gasFeeControllerData: GasFeeState, + ) { + const { id } = transactionMeta; + + const [gasFeeEstimatesResponse, layer1GasFee] = await Promise.all([ + this.#updateTransactionGasFeeEstimates( + transactionMeta, + gasFeeControllerData, + ), + this.#updateTransactionLayer1GasFee(transactionMeta), + ]); + + if (!gasFeeEstimatesResponse && !layer1GasFee) { + return; + } + + this.hub.emit('transaction-updated', { + transactionId: id, + gasFeeEstimates: gasFeeEstimatesResponse?.gasFeeEstimates, + gasFeeEstimatesLoaded: gasFeeEstimatesResponse?.gasFeeEstimatesLoaded, + layer1GasFee, + }); + } + + async #updateTransactionGasFeeEstimates( + transactionMeta: TransactionMeta, + gasFeeControllerData: GasFeeState, + ): Promise< + | { gasFeeEstimates?: GasFeeEstimates; gasFeeEstimatesLoaded: boolean } + | undefined + > { + const { chainId, networkClientId } = transactionMeta; + + const ethQuery = new EthQuery(this.#getProvider(chainId, networkClientId)); + const gasFeeFlow = getGasFeeFlow(transactionMeta, this.#gasFeeFlows); + + if (gasFeeFlow) { + log( + 'Found gas fee flow', + gasFeeFlow.constructor.name, + transactionMeta.id, + ); + } + + if (!ethQuery) { + log('Provider not available', transactionMeta.id); + return; + } + + const request: GasFeeFlowRequest = { + ethQuery, + gasFeeControllerData, + transactionMeta, + }; + + let gasFeeEstimates: GasFeeEstimates | undefined; + + if (gasFeeFlow) { + try { + const response = await gasFeeFlow.getGasFees(request); + gasFeeEstimates = response.estimates; + } catch (error) { + log('Failed to get suggested gas fees', transactionMeta.id, error); + } + } + + if (!gasFeeEstimates && transactionMeta.gasFeeEstimatesLoaded) { + return undefined; + } + + log('Updated gas fee estimates', { + gasFeeEstimates, + transaction: transactionMeta.id, + }); + + return { gasFeeEstimates, gasFeeEstimatesLoaded: true }; + } + + async #updateTransactionLayer1GasFee( + transactionMeta: TransactionMeta, + ): Promise { + const { chainId, networkClientId } = transactionMeta; + const provider = this.#getProvider(chainId, networkClientId); + + const layer1GasFee = await getTransactionLayer1GasFee({ + layer1GasFeeFlows: this.#layer1GasFeeFlows, + provider, + transactionMeta, + }); + + if (layer1GasFee) { + log('Updated layer 1 gas fee', layer1GasFee, transactionMeta.id); + } + + return layer1GasFee; + } + + #getUnapprovedTransactions() { + return this.#getTransactions().filter( + (tx) => tx.status === TransactionStatus.unapproved, + ); + } + + async #getGasFeeControllerData( + transactions: TransactionMeta[], + ): Promise> { + const networkClientIdsByChainId = new Map(); + + for (const transaction of transactions) { + const { chainId, networkClientId: transactionNetworkClientId } = + transaction; + + if (networkClientIdsByChainId.has(chainId)) { + continue; + } + + const networkClientId = + transactionNetworkClientId ?? + (this.#findNetworkClientIdByChainId(chainId) as string); + + networkClientIdsByChainId.set(chainId, networkClientId); + } + + log('Extracted network client IDs by chain ID', networkClientIdsByChainId); + + const entryPromises = Array.from(networkClientIdsByChainId.entries()).map( + async ([chainId, networkClientId]) => { + return [ + chainId, + await this.#getGasFeeControllerEstimates({ networkClientId }), + ] as const; + }, + ); + + return new Map(await Promise.all(entryPromises)); + } +} diff --git a/packages/transaction-controller/src/utils/nonce.ts b/packages/transaction-controller/src/utils/nonce.ts index b95a73a1682..0aa25393d75 100644 --- a/packages/transaction-controller/src/utils/nonce.ts +++ b/packages/transaction-controller/src/utils/nonce.ts @@ -3,6 +3,7 @@ import type { NonceLock, Transaction as NonceTrackerTransaction, } from '@metamask/nonce-tracker'; +import { providerErrors } from '@metamask/rpc-errors'; import { createModuleLogger, projectLogger } from '../logger'; import type { TransactionMeta, TransactionStatus } from '../types'; @@ -18,7 +19,7 @@ const log = createModuleLogger(projectLogger, 'nonce'); */ export async function getNextNonce( txMeta: TransactionMeta, - getNonceLock: (address: string) => Promise, + getNonceLock: (address: string) => Promise, ): Promise<[string, (() => void) | undefined]> { const { customNonceValue, @@ -38,6 +39,11 @@ export async function getNextNonce( } const nonceLock = await getNonceLock(from); + + if (!nonceLock) { + throw providerErrors.chainDisconnected(); + } + const nonce = toHex(nonceLock.nextNonce); const releaseLock = nonceLock.releaseLock.bind(nonceLock);