From 3ae0555b5b0c7371e18e5e045ff17654361413d6 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 11 Oct 2023 12:47:57 -0500 Subject: [PATCH 01/25] integrating multichain controller upgrades --- package.json | 1 + src/SmartTransactionsController.test.ts | 82 +++++++++++++-- src/SmartTransactionsController.ts | 128 +++++++++++++++++++----- src/constants.ts | 1 + src/index.test.ts | 1 + yarn.lock | 8 ++ 6 files changed, 191 insertions(+), 30 deletions(-) diff --git a/package.json b/package.json index 21c2336..de54c13 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "@metamask/base-controller": "^3.2.1", "@metamask/controller-utils": "^5.0.0", "@metamask/network-controller": "^15.0.0", + "@metamask/polling-controller": "^0.1.0", "bignumber.js": "^9.0.1", "fast-json-patch": "^3.1.0", "lodash": "^4.17.21" diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 4ac7adb..34f3576 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -195,6 +195,7 @@ const createStateAfterPending = () => { uuid: 'uuid1', status: 'pending', cancellable: true, + chainId: '0x1', statusMetadata: { cancellationFeeWei: 0, cancellationReason: 'not_cancelled', @@ -223,6 +224,7 @@ const createStateAfterSuccess = () => { uuid: 'uuid2', status: 'success', cancellable: false, + chainId: '0x1', statusMetadata: { cancellationFeeWei: 36777567771000, cancellationReason: 'not_cancelled', @@ -269,6 +271,7 @@ describe('SmartTransactionsController', () => { provider: jest.fn(), confirmExternalTransaction: confirmExternalMock, trackMetaMetricsEvent: trackMetaMetricsEventSpy, + getNetworkClientById: jest.fn(), }); // eslint-disable-next-line jest/prefer-spy-on smartTransactionsController.subscribe = jest.fn(); @@ -283,7 +286,7 @@ describe('SmartTransactionsController', () => { it('initializes with default config', () => { expect(smartTransactionsController.config).toStrictEqual({ interval: DEFAULT_INTERVAL, - supportedChainIds: [CHAIN_IDS.ETHEREUM, CHAIN_IDS.RINKEBY], + supportedChainIds: [CHAIN_IDS.ETHEREUM, CHAIN_IDS.GOERLI], chainId: CHAIN_IDS.ETHEREUM, clientId: 'default', }); @@ -300,20 +303,44 @@ describe('SmartTransactionsController', () => { approvalTxFees: undefined, tradeTxFees: undefined, }, + feesByChainId: { + [CHAIN_IDS.ETHEREUM]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + [CHAIN_IDS.GOERLI]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + }, liveness: true, + livenessByChainId: { + [CHAIN_IDS.ETHEREUM]: true, + [CHAIN_IDS.GOERLI]: true, + }, }, }); }); describe('onNetworkChange', () => { it('is triggered', () => { - networkListener({ providerConfig: { chainId: '52' } } as NetworkState); - expect(smartTransactionsController.config.chainId).toBe('52'); + networkListener({ + providerConfig: { chainId: '0x32', type: 'rpc', ticker: 'CET' }, + selectedNetworkClientId: 'networkClientId', + networkConfigurations: {}, + networksMetadata: {}, + } as NetworkState); + expect(smartTransactionsController.config.chainId).toBe('0x32'); }); it('calls poll', () => { const checkPollSpy = jest.spyOn(smartTransactionsController, 'checkPoll'); - networkListener({ providerConfig: { chainId: '2' } } as NetworkState); + networkListener({ + providerConfig: { chainId: '0x32', type: 'rpc', ticker: 'CET' }, + selectedNetworkClientId: 'networkClientId', + networkConfigurations: {}, + networksMetadata: {}, + } as NetworkState); expect(checkPollSpy).toHaveBeenCalled(); }); }); @@ -354,7 +381,12 @@ describe('SmartTransactionsController', () => { 'updateSmartTransactions', ); expect(updateSmartTransactionsSpy).not.toHaveBeenCalled(); - networkListener({ providerConfig: { chainId: '56' } } as NetworkState); + networkListener({ + providerConfig: { chainId: '0x32', type: 'rpc', ticker: 'CET' }, + selectedNetworkClientId: 'networkClientId', + networkConfigurations: {}, + networksMetadata: {}, + } as NetworkState); expect(updateSmartTransactionsSpy).not.toHaveBeenCalled(); }); }); @@ -489,7 +521,11 @@ describe('SmartTransactionsController', () => { nock(API_BASE_URL) .get(`/networks/${ethereumChainIdDec}/batchStatus?uuids=uuid1`) .reply(200, pendingBatchStatusApiResponse); - await smartTransactionsController.fetchSmartTransactionsStatus(uuids); + + await smartTransactionsController.fetchSmartTransactionsStatus( + uuids, + CHAIN_IDS.ETHEREUM, + ); const pendingState = createStateAfterPending()[0]; const pendingTransaction = { ...pendingState, history: [pendingState] }; expect(smartTransactionsController.state).toStrictEqual({ @@ -502,7 +538,21 @@ describe('SmartTransactionsController', () => { approvalTxFees: undefined, tradeTxFees: undefined, }, + feesByChainId: { + [CHAIN_IDS.ETHEREUM]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + [CHAIN_IDS.GOERLI]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + }, liveness: true, + livenessByChainId: { + [CHAIN_IDS.ETHEREUM]: true, + [CHAIN_IDS.GOERLI]: true, + }, }, }); }); @@ -524,7 +574,11 @@ describe('SmartTransactionsController', () => { nock(API_BASE_URL) .get(`/networks/${ethereumChainIdDec}/batchStatus?uuids=uuid2`) .reply(200, successBatchStatusApiResponse); - await smartTransactionsController.fetchSmartTransactionsStatus(uuids); + + await smartTransactionsController.fetchSmartTransactionsStatus( + uuids, + CHAIN_IDS.ETHEREUM, + ); const successState = createStateAfterSuccess()[0]; const successTransaction = { ...successState, history: [successState] }; expect(smartTransactionsController.state).toStrictEqual({ @@ -541,6 +595,20 @@ describe('SmartTransactionsController', () => { tradeTxFees: undefined, }, liveness: true, + feesByChainId: { + '0x1': { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + '0x5': { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + }, + livenessByChainId: { + '0x1': true, + '0x5': true, + }, }, }); }); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 25d511c..0b31b9e 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -1,10 +1,11 @@ -import { - BaseConfig, - BaseController, - BaseState, -} from '@metamask/base-controller'; +import { BaseConfig, BaseState } from '@metamask/base-controller'; import { safelyExecute } from '@metamask/controller-utils'; -import { NetworkState } from '@metamask/network-controller'; +import { + NetworkState, + NetworkController, + NetworkClientId, +} from '@metamask/network-controller'; +import { PollingControllerV1 } from '@metamask/polling-controller'; import { BigNumber } from 'bignumber.js'; import { BigNumber as ethersBigNumber } from '@ethersproject/bignumber'; import { Web3Provider } from '@ethersproject/providers'; @@ -46,19 +47,23 @@ export type SmartTransactionsControllerConfig = BaseConfig & { supportedChainIds: string[]; }; +type FeeEstimates = { + approvalTxFees: IndividualTxFees | undefined; + tradeTxFees: IndividualTxFees | undefined; +}; + export type SmartTransactionsControllerState = BaseState & { smartTransactionsState: { smartTransactions: Record; userOptIn: boolean | undefined; liveness: boolean | undefined; - fees: { - approvalTxFees: IndividualTxFees | undefined; - tradeTxFees: IndividualTxFees | undefined; - }; + fees: FeeEstimates; + feesByChainId: Record; + livenessByChainId: Record; }; }; -export default class SmartTransactionsController extends BaseController< +export default class SmartTransactionsController extends PollingControllerV1< SmartTransactionsControllerConfig, SmartTransactionsControllerState > { @@ -72,6 +77,8 @@ export default class SmartTransactionsController extends BaseController< private trackMetaMetricsEvent: any; + private getNetworkClientById: NetworkController['getNetworkClientById']; + /* istanbul ignore next */ private async fetch(request: string, options?: RequestInit) { const { clientId } = this.config; @@ -93,6 +100,7 @@ export default class SmartTransactionsController extends BaseController< provider, confirmExternalTransaction, trackMetaMetricsEvent, + getNetworkClientById, }: { onNetworkStateChange: ( listener: (networkState: NetworkState) => void, @@ -101,6 +109,7 @@ export default class SmartTransactionsController extends BaseController< provider: any; confirmExternalTransaction: any; trackMetaMetricsEvent: any; + getNetworkClientById: NetworkController['getNetworkClientById']; }, config?: Partial, state?: Partial, @@ -111,7 +120,7 @@ export default class SmartTransactionsController extends BaseController< interval: DEFAULT_INTERVAL, chainId: CHAIN_IDS.ETHEREUM, clientId: 'default', - supportedChainIds: [CHAIN_IDS.ETHEREUM, CHAIN_IDS.RINKEBY], + supportedChainIds: [CHAIN_IDS.ETHEREUM, CHAIN_IDS.GOERLI], }; this.defaultState = { @@ -123,6 +132,20 @@ export default class SmartTransactionsController extends BaseController< tradeTxFees: undefined, }, liveness: true, + livenessByChainId: { + [CHAIN_IDS.ETHEREUM]: true, + [CHAIN_IDS.GOERLI]: true, + }, + feesByChainId: { + [CHAIN_IDS.ETHEREUM]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + [CHAIN_IDS.GOERLI]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + }, }, }; @@ -130,6 +153,7 @@ export default class SmartTransactionsController extends BaseController< this.ethersProvider = new Web3Provider(provider); this.confirmExternalTransaction = confirmExternalTransaction; this.trackMetaMetricsEvent = trackMetaMetricsEvent; + this.getNetworkClientById = getNetworkClientById; this.initialize(); this.initializeSmartTransactionsForChainId(); @@ -145,6 +169,19 @@ export default class SmartTransactionsController extends BaseController< this.subscribe((currentState: any) => this.checkPoll(currentState)); } + executePoll(networkClientId: string): Promise { + // if this is going to be truly UI driven polling we shouldn't really reach here + // with a networkClientId that is not supported, but for now I'll add a check in case + // wondering if we should add some kind of predicate to the polling controller to check whether + // we should poll or not + const chainId = this.getChainId(networkClientId); + if (!this.config.supportedChainIds.includes(chainId)) { + return Promise.resolve(); + } + + return this.updateSmartTransactions(networkClientId); + } + checkPoll(state: any) { const { smartTransactions } = state.smartTransactionsState; const currentSmartTransactions = smartTransactions[this.config.chainId]; @@ -253,6 +290,7 @@ export default class SmartTransactionsController extends BaseController< } updateSmartTransaction(smartTransaction: SmartTransaction): void { + // todo add networkClientId logic here const { chainId } = this.config; const { smartTransactionsState } = this.state; const { smartTransactions } = smartTransactionsState; @@ -330,9 +368,11 @@ export default class SmartTransactionsController extends BaseController< }); } - async updateSmartTransactions() { + async updateSmartTransactions( + networkClientId?: NetworkClientId, + ): Promise { + const chainId = this.getChainId(networkClientId); const { smartTransactions } = this.state.smartTransactionsState; - const { chainId } = this.config; const currentSmartTransactions = smartTransactions?.[chainId]; @@ -341,7 +381,7 @@ export default class SmartTransactionsController extends BaseController< .map((smartTransaction) => smartTransaction.uuid); if (transactionsToUpdate.length > 0) { - this.fetchSmartTransactionsStatus(transactionsToUpdate); + this.fetchSmartTransactionsStatus(transactionsToUpdate, chainId); } } @@ -420,9 +460,8 @@ export default class SmartTransactionsController extends BaseController< // ! Ask backend API to accept list of uuids as params async fetchSmartTransactionsStatus( uuids: string[], + chainId: string, ): Promise { - const { chainId } = this.config; - const params = new URLSearchParams({ uuids: uuids.join(','), }); @@ -436,6 +475,7 @@ export default class SmartTransactionsController extends BaseController< Object.entries(data).forEach(([uuid, stxStatus]) => { this.updateSmartTransaction({ + chainId, statusMetadata: stxStatus as SmartTransactionsStatus, status: calculateStatus(stxStatus as SmartTransactionsStatus), cancellable: isSmartTransactionCancellable( @@ -477,8 +517,9 @@ export default class SmartTransactionsController extends BaseController< async getFees( tradeTx: UnsignedTransaction, approvalTx: UnsignedTransaction, + networkClientId?: NetworkClientId, ): Promise { - const { chainId } = this.config; + const chainId = this.getChainId(networkClientId); const transactions = []; let unsignedTradeTransactionWithNonce; if (approvalTx) { @@ -520,6 +561,22 @@ export default class SmartTransactionsController extends BaseController< }, }, }); + + if (networkClientId) { + this.update({ + smartTransactionsState: { + ...this.state.smartTransactionsState, + feesByChainId: { + ...this.state.smartTransactionsState.feesByChainId, + [chainId]: { + approvalTxFees, + tradeTxFees, + }, + }, + }, + }); + } + return { approvalTxFees, tradeTxFees, @@ -532,12 +589,14 @@ export default class SmartTransactionsController extends BaseController< txParams, signedTransactions, signedCanceledTransactions, + networkClientId, }: { signedTransactions: SignedTransaction[]; signedCanceledTransactions: SignedCanceledTransaction[]; txParams?: any; + networkClientId?: NetworkClientId; }) { - const { chainId } = this.config; + const chainId = this.getChainId(networkClientId); const data = await this.fetch( getAPIRequestURL(APIType.SUBMIT_TRANSACTIONS, chainId), { @@ -583,19 +642,29 @@ export default class SmartTransactionsController extends BaseController< return data; } + getChainId(networkClientId?: NetworkClientId): string { + if (!networkClientId) { + return this.config.chainId; + } + return this.getNetworkClientById(networkClientId).configuration.chainId; + } + // TODO: This should return if the cancellation was on chain or not (for nonce management) // After this successful call client must update nonce representative // in transaction controller external transactions list - async cancelSmartTransaction(uuid: string): Promise { - const { chainId } = this.config; + async cancelSmartTransaction( + uuid: string, + networkClientId?: NetworkClientId, + ): Promise { + const chainId = this.getChainId(networkClientId); await this.fetch(getAPIRequestURL(APIType.CANCEL, chainId), { method: 'POST', body: JSON.stringify({ uuid }), }); } - async fetchLiveness(): Promise { - const { chainId } = this.config; + async fetchLiveness(networkClientId?: NetworkClientId): Promise { + const chainId = this.getChainId(networkClientId); let liveness = false; try { const response = await this.fetch( @@ -612,6 +681,19 @@ export default class SmartTransactionsController extends BaseController< liveness, }, }); + + if (networkClientId) { + this.update({ + smartTransactionsState: { + ...this.state.smartTransactionsState, + livenessByChainId: { + ...this.state.smartTransactionsState.livenessByChainId, + [chainId]: liveness, + }, + }, + }); + } + return liveness; } diff --git a/src/constants.ts b/src/constants.ts index 070e289..3b8266c 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,6 +1,7 @@ export const API_BASE_URL = 'https://transaction.metaswap.codefi.network'; export const CHAIN_IDS = { ETHEREUM: '0x1', + GOERLI: '0x5', RINKEBY: '0x4', BSC: '0x38', }; diff --git a/src/index.test.ts b/src/index.test.ts index 411d2c1..678b078 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -10,6 +10,7 @@ describe('default export', () => { provider: jest.fn(), confirmExternalTransaction: jest.fn(), trackMetaMetricsEvent: jest.fn(), + getNetworkClientById: jest.fn(), }); expect(controller).toBeInstanceOf(SmartTransactionsController); jest.clearAllTimers(); diff --git a/yarn.lock b/yarn.lock index 0eda61c..0356c33 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1336,6 +1336,7 @@ __metadata: "@metamask/eslint-config-nodejs": ^10.0.0 "@metamask/eslint-config-typescript": ^10.0.0 "@metamask/network-controller": ^15.0.0 + "@metamask/polling-controller": ^0.1.0 "@types/jest": ^26.0.24 "@types/lodash": ^4.14.194 "@types/node": ^16.18.31 @@ -1755,6 +1756,13 @@ __metadata: languageName: node linkType: hard +"@types/uuid@npm:^8.3.0": + version: 8.3.4 + resolution: "@types/uuid@npm:8.3.4" + checksum: 6f11f3ff70f30210edaa8071422d405e9c1d4e53abbe50fdce365150d3c698fe7bbff65c1e71ae080cbfb8fded860dbb5e174da96fdbbdfcaa3fb3daa474d20f + languageName: node + linkType: hard + "@types/yargs-parser@npm:*": version: 21.0.0 resolution: "@types/yargs-parser@npm:21.0.0" From bcab24bd4b58d185219f18cc39faf37ff17cada7 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 13 Oct 2023 09:56:27 -0500 Subject: [PATCH 02/25] add test --- package.json | 2 +- src/SmartTransactionsController.test.ts | 209 +++++++++++++++++++++++- src/SmartTransactionsController.ts | 8 +- yarn.lock | 21 ++- 4 files changed, 228 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index de54c13..179c294 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "@metamask/base-controller": "^3.2.1", "@metamask/controller-utils": "^5.0.0", "@metamask/network-controller": "^15.0.0", - "@metamask/polling-controller": "^0.1.0", + "@metamask/polling-controller": "^0.2.0", "bignumber.js": "^9.0.1", "fast-json-patch": "^3.1.0", "lodash": "^4.17.21" diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 34f3576..4a321f6 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -1,11 +1,21 @@ import nock from 'nock'; import { NetworkState } from '@metamask/network-controller'; +import { convertHexToDecimal } from '@metamask/controller-utils'; import SmartTransactionsController, { DEFAULT_INTERVAL, } from './SmartTransactionsController'; import { API_BASE_URL, CHAIN_IDS } from './constants'; import { SmartTransaction, SmartTransactionStatuses } from './types'; - +import * as utils from './utils'; + +/** + * Resolve all pending promises. + * This method is used for async tests that use fake timers. + * See https://stackoverflow.com/a/58716087 and https://jestjs.io/docs/timer-mocks. + */ +function flushPromises(): Promise { + return new Promise(jest.requireActual('timers').setImmediate); +} const confirmExternalMock = jest.fn(); jest.mock('@ethersproject/bytes', () => ({ @@ -252,11 +262,11 @@ const testHistory = [ const ethereumChainIdDec = parseInt(CHAIN_IDS.ETHEREUM, 16); const trackMetaMetricsEventSpy = jest.fn(); +const getNetworkClientByIdSpy = jest.fn(); describe('SmartTransactionsController', () => { let smartTransactionsController: SmartTransactionsController; let networkListener: (networkState: NetworkState) => void; - beforeEach(() => { smartTransactionsController = new SmartTransactionsController({ onNetworkStateChange: (listener) => { @@ -271,7 +281,7 @@ describe('SmartTransactionsController', () => { provider: jest.fn(), confirmExternalTransaction: confirmExternalMock, trackMetaMetricsEvent: trackMetaMetricsEventSpy, - getNetworkClientById: jest.fn(), + getNetworkClientById: getNetworkClientByIdSpy, }); // eslint-disable-next-line jest/prefer-spy-on smartTransactionsController.subscribe = jest.fn(); @@ -803,4 +813,197 @@ describe('SmartTransactionsController', () => { expect(actual).toBe(false); }); }); + + describe('startPollingByNetworkClientId', () => { + it('starts and stops calling smart transactions batch status api endpoint with the correct chainId at the interval passed via the constructor', async () => { + // mock this to a noop because it causes an extra fetch call to the API upon state changes + jest + .spyOn(smartTransactionsController, 'checkPoll') + .mockImplementation(() => undefined); + + const defaultState = { + smartTransactions: { + [CHAIN_IDS.ETHEREUM]: [], + }, + userOptIn: undefined, + fees: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + feesByChainId: { + [CHAIN_IDS.ETHEREUM]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + [CHAIN_IDS.GOERLI]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + }, + liveness: true, + livenessByChainId: { + [CHAIN_IDS.ETHEREUM]: true, + [CHAIN_IDS.GOERLI]: true, + }, + }; + + // pending transactions in state are required to test polling + smartTransactionsController.update({ + smartTransactionsState: { + ...defaultState, + smartTransactions: { + '0x1': [ + { + uuid: 'uuid1', + status: 'pending', + cancellable: true, + chainId: '0x1', + }, + ], + '0x5': [ + { + uuid: 'uuid2', + status: 'pending', + cancellable: true, + chainId: '0x5', + }, + ], + }, + }, + }); + + getNetworkClientByIdSpy.mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + configuration: { + chainId: CHAIN_IDS.ETHEREUM, + }, + }; + case 'goerli': + return { + configuration: { + chainId: CHAIN_IDS.GOERLI, + }, + }; + default: + throw new Error('Invalid network client id'); + } + }); + + jest.useFakeTimers(); + const handleFetchSpy = jest.spyOn(utils, 'handleFetch'); + + const mainnetPollingToken = + smartTransactionsController.startPollingByNetworkClientId('mainnet'); + + await Promise.all([ + jest.advanceTimersByTime(DEFAULT_INTERVAL), + flushPromises(), + ]); + + expect(handleFetchSpy.mock.calls[0]).toStrictEqual( + expect.arrayContaining([ + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.ETHEREUM, + )}/batchStatus?uuids=uuid1`, + ]), + ); + + smartTransactionsController.startPollingByNetworkClientId('goerli'); + await jest.advanceTimersByTime(DEFAULT_INTERVAL); + + expect( + JSON.stringify(handleFetchSpy.mock.calls.map((arg) => arg[0])), + ).toStrictEqual( + JSON.stringify([ + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.ETHEREUM, + )}/batchStatus?uuids=uuid1`, + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.ETHEREUM, + )}/batchStatus?uuids=uuid1`, + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.GOERLI, + )}/batchStatus?uuids=uuid2`, + ]), + ); + + // stop the mainnet polling + smartTransactionsController.stopPollingByPollingToken( + mainnetPollingToken, + ); + + // cycle two polling intervals + await jest.advanceTimersByTime(DEFAULT_INTERVAL); + await jest.advanceTimersByTime(DEFAULT_INTERVAL); + + // check that the mainnet polling has stopped while the goerli polling continues + expect( + JSON.stringify(handleFetchSpy.mock.calls.map((arg) => arg[0])), + ).toStrictEqual( + JSON.stringify([ + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.ETHEREUM, + )}/batchStatus?uuids=uuid1`, + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.ETHEREUM, + )}/batchStatus?uuids=uuid1`, + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.GOERLI, + )}/batchStatus?uuids=uuid2`, + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.GOERLI, + )}/batchStatus?uuids=uuid2`, + `${API_BASE_URL}/networks/${convertHexToDecimal( + CHAIN_IDS.GOERLI, + )}/batchStatus?uuids=uuid2`, + ]), + ); + + // TODO figure this out: + // having trouble getting the state to update in this test + // await jest.runOnlyPendingTimers(); + // const pendingState = createStateAfterPending()[0]; + // const pendingTransaction = { ...pendingState, history: [pendingState] }; + // await Promise.all([jest.runOnlyPendingTimers(), flushPromises()]); + // expect(smartTransactionsController.state).toStrictEqual({ + // smartTransactionsState: { + // smartTransactions: { + // [CHAIN_IDS.ETHEREUM]: [pendingTransaction], + // }, + // userOptIn: undefined, + // fees: { + // approvalTxFees: undefined, + // tradeTxFees: undefined, + // }, + // feesByChainId: { + // [CHAIN_IDS.ETHEREUM]: { + // approvalTxFees: undefined, + // tradeTxFees: undefined, + // }, + // [CHAIN_IDS.GOERLI]: { + // approvalTxFees: undefined, + // tradeTxFees: undefined, + // }, + // }, + // liveness: true, + // livenessByChainId: { + // [CHAIN_IDS.ETHEREUM]: true, + // [CHAIN_IDS.GOERLI]: true, + // }, + // }, + // }); + + // cleanup + smartTransactionsController.update({ + smartTransactionsState: { + ...defaultState, + }, + }); + + smartTransactionsController.stopAllPolling(); + jest.clearAllTimers(); + }); + }); }); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 0b31b9e..6d7ee07 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -148,7 +148,7 @@ export default class SmartTransactionsController extends PollingControllerV1< }, }, }; - + this.setIntervalLength(this.config.interval || DEFAULT_INTERVAL); this.getNonceLock = getNonceLock; this.ethersProvider = new Web3Provider(provider); this.confirmExternalTransaction = confirmExternalTransaction; @@ -169,7 +169,7 @@ export default class SmartTransactionsController extends PollingControllerV1< this.subscribe((currentState: any) => this.checkPoll(currentState)); } - executePoll(networkClientId: string): Promise { + _executePoll(networkClientId: string): Promise { // if this is going to be truly UI driven polling we shouldn't really reach here // with a networkClientId that is not supported, but for now I'll add a check in case // wondering if we should add some kind of predicate to the polling controller to check whether @@ -178,7 +178,6 @@ export default class SmartTransactionsController extends PollingControllerV1< if (!this.config.supportedChainIds.includes(chainId)) { return Promise.resolve(); } - return this.updateSmartTransactions(networkClientId); } @@ -375,7 +374,6 @@ export default class SmartTransactionsController extends PollingControllerV1< const { smartTransactions } = this.state.smartTransactionsState; const currentSmartTransactions = smartTransactions?.[chainId]; - const transactionsToUpdate: string[] = currentSmartTransactions .filter(isSmartTransactionPending) .map((smartTransaction) => smartTransaction.uuid); @@ -465,14 +463,12 @@ export default class SmartTransactionsController extends PollingControllerV1< const params = new URLSearchParams({ uuids: uuids.join(','), }); - const url = `${getAPIRequestURL( APIType.BATCH_STATUS, chainId, )}?${params.toString()}`; const data = await this.fetch(url); - Object.entries(data).forEach(([uuid, stxStatus]) => { this.updateSmartTransaction({ chainId, diff --git a/yarn.lock b/yarn.lock index 0356c33..acf5d3b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1303,6 +1303,23 @@ __metadata: languageName: node linkType: hard +"@metamask/polling-controller@npm:^0.2.0": + version: 0.2.0 + resolution: "@metamask/polling-controller@npm:0.2.0" + dependencies: + "@metamask/base-controller": ^3.2.3 + "@metamask/controller-utils": ^5.0.2 + "@metamask/network-controller": ^15.0.0 + "@metamask/utils": ^8.1.0 + "@types/uuid": ^8.3.0 + fast-json-stable-stringify: ^2.1.0 + uuid: ^8.3.2 + peerDependencies: + "@metamask/network-controller": ^15.0.0 + checksum: 4dee3e49b23ba2b92055816dcc68b8e468405d9b00528d14b01c490058dea6e7aae943b19a007adbbbe06aa9a5b61d961211f9de82c8b55c7622599de78eb76e + languageName: node + linkType: hard + "@metamask/rpc-errors@npm:^6.0.0, @metamask/rpc-errors@npm:^6.1.0": version: 6.1.0 resolution: "@metamask/rpc-errors@npm:6.1.0" @@ -1336,7 +1353,7 @@ __metadata: "@metamask/eslint-config-nodejs": ^10.0.0 "@metamask/eslint-config-typescript": ^10.0.0 "@metamask/network-controller": ^15.0.0 - "@metamask/polling-controller": ^0.1.0 + "@metamask/polling-controller": ^0.2.0 "@types/jest": ^26.0.24 "@types/lodash": ^4.14.194 "@types/node": ^16.18.31 @@ -4035,7 +4052,7 @@ __metadata: languageName: node linkType: hard -"fast-json-stable-stringify@npm:2.x, fast-json-stable-stringify@npm:^2.0.0": +"fast-json-stable-stringify@npm:2.x, fast-json-stable-stringify@npm:^2.0.0, fast-json-stable-stringify@npm:^2.1.0": version: 2.1.0 resolution: "fast-json-stable-stringify@npm:2.1.0" checksum: b191531e36c607977e5b1c47811158733c34ccb3bfde92c44798929e9b4154884378536d26ad90dfecd32e1ffc09c545d23535ad91b3161a27ddbb8ebe0cbecb From b7d2bb4b28e3ad19c1f7a5a85ea4e5332662f941 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 13 Oct 2023 14:10:20 -0500 Subject: [PATCH 03/25] cleanup --- src/SmartTransactionsController.test.ts | 34 ------------------------- 1 file changed, 34 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 4a321f6..26b1a83 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -961,40 +961,6 @@ describe('SmartTransactionsController', () => { ]), ); - // TODO figure this out: - // having trouble getting the state to update in this test - // await jest.runOnlyPendingTimers(); - // const pendingState = createStateAfterPending()[0]; - // const pendingTransaction = { ...pendingState, history: [pendingState] }; - // await Promise.all([jest.runOnlyPendingTimers(), flushPromises()]); - // expect(smartTransactionsController.state).toStrictEqual({ - // smartTransactionsState: { - // smartTransactions: { - // [CHAIN_IDS.ETHEREUM]: [pendingTransaction], - // }, - // userOptIn: undefined, - // fees: { - // approvalTxFees: undefined, - // tradeTxFees: undefined, - // }, - // feesByChainId: { - // [CHAIN_IDS.ETHEREUM]: { - // approvalTxFees: undefined, - // tradeTxFees: undefined, - // }, - // [CHAIN_IDS.GOERLI]: { - // approvalTxFees: undefined, - // tradeTxFees: undefined, - // }, - // }, - // liveness: true, - // livenessByChainId: { - // [CHAIN_IDS.ETHEREUM]: true, - // [CHAIN_IDS.GOERLI]: true, - // }, - // }, - // }); - // cleanup smartTransactionsController.update({ smartTransactionsState: { From e81084bbceb0107813e45b94da469e3c5be48a2d Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 13 Oct 2023 14:33:52 -0500 Subject: [PATCH 04/25] need to move confirmSmartTransaction tests into another test since confirmSmartTransaction is now private --- src/SmartTransactionsController.test.ts | 2 +- src/SmartTransactionsController.ts | 82 ++++++++++++++----------- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 26b1a83..7a165b4 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -724,7 +724,7 @@ describe('SmartTransactionsController', () => { history: testHistory, }; await smartTransactionsController.confirmSmartTransaction( - successfulStx as SmartTransaction, + successfulStx as SmartTransaction, ); expect(trackMetaMetricsEventSpy).toHaveBeenCalled(); }); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 6d7ee07..e545bea 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -288,12 +288,14 @@ export default class SmartTransactionsController extends PollingControllerV1< return currentIndex === -1 || currentIndex === undefined; } - updateSmartTransaction(smartTransaction: SmartTransaction): void { - // todo add networkClientId logic here - const { chainId } = this.config; + updateSmartTransaction( + smartTransaction: SmartTransaction, + chainId: string, + ): void { + const currentChainId = chainId || this.config.chainId; const { smartTransactionsState } = this.state; const { smartTransactions } = smartTransactionsState; - const currentSmartTransactions = smartTransactions[chainId]; + const currentSmartTransactions = smartTransactions[currentChainId]; const currentIndex = currentSmartTransactions?.findIndex( (stx) => stx.uuid === smartTransaction.uuid, ); @@ -329,7 +331,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ...smartTransactionsState, smartTransactions: { ...smartTransactionsState.smartTransactions, - [chainId]: nextSmartTransactions, + [currentChainId]: nextSmartTransactions, }, }, }); @@ -347,7 +349,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ...currentSmartTransaction, ...smartTransaction, }; - this.confirmSmartTransaction(nextSmartTransaction); + this.#confirmSmartTransaction(nextSmartTransaction, currentChainId); } this.update({ @@ -355,13 +357,13 @@ export default class SmartTransactionsController extends PollingControllerV1< ...smartTransactionsState, smartTransactions: { ...smartTransactionsState.smartTransactions, - [chainId]: smartTransactionsState.smartTransactions[chainId].map( - (item, index) => { - return index === currentIndex - ? { ...item, ...smartTransaction } - : item; - }, - ), + [currentChainId]: smartTransactionsState.smartTransactions[ + currentChainId + ].map((item, index) => { + return index === currentIndex + ? { ...item, ...smartTransaction } + : item; + }), }, }, }); @@ -383,7 +385,10 @@ export default class SmartTransactionsController extends PollingControllerV1< } } - async confirmSmartTransaction(smartTransaction: SmartTransaction) { + async #confirmSmartTransaction( + smartTransaction: SmartTransaction, + chainId: string, + ) { const txHash = smartTransaction.statusMetadata?.minedHash; try { const transactionReceipt = @@ -441,10 +446,13 @@ export default class SmartTransactionsController extends PollingControllerV1< category: 'swaps', }); - this.updateSmartTransaction({ - ...smartTransaction, - confirmed: true, - }); + this.updateSmartTransaction( + { + ...smartTransaction, + confirmed: true, + }, + chainId, + ); } } catch (e) { this.trackMetaMetricsEvent({ @@ -470,15 +478,17 @@ export default class SmartTransactionsController extends PollingControllerV1< const data = await this.fetch(url); Object.entries(data).forEach(([uuid, stxStatus]) => { - this.updateSmartTransaction({ + this.updateSmartTransaction( + { + statusMetadata: stxStatus as SmartTransactionsStatus, + status: calculateStatus(stxStatus as SmartTransactionsStatus), + cancellable: isSmartTransactionCancellable( + stxStatus as SmartTransactionsStatus, + ), + uuid, + }, chainId, - statusMetadata: stxStatus as SmartTransactionsStatus, - status: calculateStatus(stxStatus as SmartTransactionsStatus), - cancellable: isSmartTransactionCancellable( - stxStatus as SmartTransactionsStatus, - ), - uuid, - }); + ); }); return data; @@ -621,16 +631,18 @@ export default class SmartTransactionsController extends PollingControllerV1< } const { nonceDetails } = nonceLock; - this.updateSmartTransaction({ + this.updateSmartTransaction( + { + nonceDetails, + preTxBalance, + status: SmartTransactionStatuses.PENDING, + time, + txParams, + uuid: data.uuid, + cancellable: true, + }, chainId, - nonceDetails, - preTxBalance, - status: SmartTransactionStatuses.PENDING, - time, - txParams, - uuid: data.uuid, - cancellable: true, - }); + ); } finally { nonceLock.releaseLock(); } From cb5585f91ed7321688e2f0eedd6020758dcf62c7 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 13 Oct 2023 14:39:59 -0500 Subject: [PATCH 05/25] actually for now leave it as a public method --- src/SmartTransactionsController.test.ts | 8 +++++--- src/SmartTransactionsController.ts | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 7a165b4..68e5a0e 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -205,7 +205,6 @@ const createStateAfterPending = () => { uuid: 'uuid1', status: 'pending', cancellable: true, - chainId: '0x1', statusMetadata: { cancellationFeeWei: 0, cancellationReason: 'not_cancelled', @@ -234,7 +233,6 @@ const createStateAfterSuccess = () => { uuid: 'uuid2', status: 'success', cancellable: false, - chainId: '0x1', statusMetadata: { cancellationFeeWei: 36777567771000, cancellationReason: 'not_cancelled', @@ -661,6 +659,7 @@ describe('SmartTransactionsController', () => { }; smartTransactionsController.updateSmartTransaction( updateTransaction as SmartTransaction, + CHAIN_IDS.ETHEREUM, ); expect( @@ -693,6 +692,7 @@ describe('SmartTransactionsController', () => { }; smartTransactionsController.updateSmartTransaction( updateTransaction as SmartTransaction, + CHAIN_IDS.ETHEREUM, ); expect(confirmSpy).toHaveBeenCalled(); }); @@ -711,6 +711,7 @@ describe('SmartTransactionsController', () => { }; await smartTransactionsController.confirmSmartTransaction( successfulStx as SmartTransaction, + CHAIN_IDS.ETHEREUM, ); expect(confirmExternalMock).toHaveBeenCalled(); }); @@ -724,7 +725,8 @@ describe('SmartTransactionsController', () => { history: testHistory, }; await smartTransactionsController.confirmSmartTransaction( - successfulStx as SmartTransaction, + successfulStx as SmartTransaction, + CHAIN_IDS.ETHEREUM, ); expect(trackMetaMetricsEventSpy).toHaveBeenCalled(); }); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index e545bea..67d738d 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -349,7 +349,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ...currentSmartTransaction, ...smartTransaction, }; - this.#confirmSmartTransaction(nextSmartTransaction, currentChainId); + this.confirmSmartTransaction(nextSmartTransaction, currentChainId); } this.update({ @@ -385,7 +385,8 @@ export default class SmartTransactionsController extends PollingControllerV1< } } - async #confirmSmartTransaction( + // TODO make this a private method? + async confirmSmartTransaction( smartTransaction: SmartTransaction, chainId: string, ) { From fb7048063415c0ab2d92ee9d89ea9db31e4515d4 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 13 Oct 2023 14:42:31 -0500 Subject: [PATCH 06/25] cleanup --- src/SmartTransactionsController.ts | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 67d738d..ff8bc3a 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -292,10 +292,9 @@ export default class SmartTransactionsController extends PollingControllerV1< smartTransaction: SmartTransaction, chainId: string, ): void { - const currentChainId = chainId || this.config.chainId; const { smartTransactionsState } = this.state; const { smartTransactions } = smartTransactionsState; - const currentSmartTransactions = smartTransactions[currentChainId]; + const currentSmartTransactions = smartTransactions[chainId]; const currentIndex = currentSmartTransactions?.findIndex( (stx) => stx.uuid === smartTransaction.uuid, ); @@ -331,7 +330,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ...smartTransactionsState, smartTransactions: { ...smartTransactionsState.smartTransactions, - [currentChainId]: nextSmartTransactions, + [chainId]: nextSmartTransactions, }, }, }); @@ -349,7 +348,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ...currentSmartTransaction, ...smartTransaction, }; - this.confirmSmartTransaction(nextSmartTransaction, currentChainId); + this.confirmSmartTransaction(nextSmartTransaction, chainId); } this.update({ @@ -357,13 +356,13 @@ export default class SmartTransactionsController extends PollingControllerV1< ...smartTransactionsState, smartTransactions: { ...smartTransactionsState.smartTransactions, - [currentChainId]: smartTransactionsState.smartTransactions[ - currentChainId - ].map((item, index) => { - return index === currentIndex - ? { ...item, ...smartTransaction } - : item; - }), + [chainId]: smartTransactionsState.smartTransactions[chainId].map( + (item, index) => { + return index === currentIndex + ? { ...item, ...smartTransaction } + : item; + }, + ), }, }, }); From c34e0b3095b48b59cefa1e9458ffa964b6ca0cbb Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 13 Oct 2023 16:36:24 -0500 Subject: [PATCH 07/25] fix typeerror? --- src/SmartTransactionsController.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index ff8bc3a..ae6f802 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -310,7 +310,7 @@ export default class SmartTransactionsController extends PollingControllerV1< if (isNewSmartTransaction) { // add smart transaction - const cancelledNonceIndex = currentSmartTransactions.findIndex( + const cancelledNonceIndex = currentSmartTransactions?.findIndex( (stx: SmartTransaction) => stx.txParams?.nonce === smartTransaction.txParams?.nonce && stx.status?.startsWith('cancelled'), @@ -322,9 +322,9 @@ export default class SmartTransactionsController extends PollingControllerV1< cancelledNonceIndex > -1 ? currentSmartTransactions .slice(0, cancelledNonceIndex) - .concat(currentSmartTransactions.slice(cancelledNonceIndex + 1)) - .concat(historifiedSmartTransaction) - : currentSmartTransactions.concat(historifiedSmartTransaction); + ?.concat(currentSmartTransactions.slice(cancelledNonceIndex + 1)) + ?.concat(historifiedSmartTransaction) + : currentSmartTransactions?.concat(historifiedSmartTransaction); this.update({ smartTransactionsState: { ...smartTransactionsState, From 34a90c703359860b0498cdedda243f24d69216f5 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 16 Oct 2023 10:30:34 -0500 Subject: [PATCH 08/25] add test + cleanup --- src/SmartTransactionsController.test.ts | 146 ++++++++++++++---------- src/SmartTransactionsController.ts | 4 +- 2 files changed, 85 insertions(+), 65 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 68e5a0e..b9a6af4 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -40,7 +40,7 @@ jest.mock('@ethersproject/providers', () => ({ const addressFrom = '0x268392a24B6b093127E8581eAfbD1DA228bAdAe3'; -const createUnsignedTransaction = () => { +const createUnsignedTransaction = (chainId: number) => { return { from: addressFrom, to: '0x0000000000000000000000000000000000000000', @@ -48,7 +48,7 @@ const createUnsignedTransaction = () => { data: '0x', nonce: 0, type: 2, - chainId: 4, + chainId, }; }; @@ -262,6 +262,34 @@ const ethereumChainIdDec = parseInt(CHAIN_IDS.ETHEREUM, 16); const trackMetaMetricsEventSpy = jest.fn(); const getNetworkClientByIdSpy = jest.fn(); +const defaultState = { + smartTransactionsState: { + smartTransactions: { + [CHAIN_IDS.ETHEREUM]: [], + }, + userOptIn: undefined, + fees: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + feesByChainId: { + [CHAIN_IDS.ETHEREUM]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + [CHAIN_IDS.GOERLI]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + }, + liveness: true, + livenessByChainId: { + [CHAIN_IDS.ETHEREUM]: true, + [CHAIN_IDS.GOERLI]: true, + }, + }, +}; + describe('SmartTransactionsController', () => { let smartTransactionsController: SmartTransactionsController; let networkListener: (networkState: NetworkState) => void; @@ -301,33 +329,7 @@ describe('SmartTransactionsController', () => { }); it('initializes with default state', () => { - expect(smartTransactionsController.state).toStrictEqual({ - smartTransactionsState: { - smartTransactions: { - [CHAIN_IDS.ETHEREUM]: [], - }, - userOptIn: undefined, - fees: { - approvalTxFees: undefined, - tradeTxFees: undefined, - }, - feesByChainId: { - [CHAIN_IDS.ETHEREUM]: { - approvalTxFees: undefined, - tradeTxFees: undefined, - }, - [CHAIN_IDS.GOERLI]: { - approvalTxFees: undefined, - tradeTxFees: undefined, - }, - }, - liveness: true, - livenessByChainId: { - [CHAIN_IDS.ETHEREUM]: true, - [CHAIN_IDS.GOERLI]: true, - }, - }, - }); + expect(smartTransactionsController.state).toStrictEqual(defaultState); }); describe('onNetworkChange', () => { @@ -471,8 +473,8 @@ describe('SmartTransactionsController', () => { describe('getFees', () => { it('gets unsigned transactions and estimates based on an unsigned transaction', async () => { - const tradeTx = createUnsignedTransaction(); - const approvalTx = createUnsignedTransaction(); + const tradeTx = createUnsignedTransaction(ethereumChainIdDec); + const approvalTx = createUnsignedTransaction(ethereumChainIdDec); const getFeesApiResponse = createGetFeesApiResponse(); nock(API_BASE_URL) .post(`/networks/${ethereumChainIdDec}/getFees`) @@ -486,6 +488,54 @@ describe('SmartTransactionsController', () => { tradeTxFees: getFeesApiResponse.txs[1], }); }); + + it('should add fee data to feesByChainId state using the networkClientId passed in to identify the appropriate chain', async () => { + const goerliChainIdDec = parseInt(CHAIN_IDS.GOERLI, 16); + getNetworkClientByIdSpy.mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + configuration: { + chainId: CHAIN_IDS.ETHEREUM, + }, + }; + case 'goerli': + return { + configuration: { + chainId: CHAIN_IDS.GOERLI, + }, + }; + default: + throw new Error('Invalid network client id'); + } + }); + + const tradeTx = createUnsignedTransaction(goerliChainIdDec); + const approvalTx = createUnsignedTransaction(goerliChainIdDec); + const getFeesApiResponse = createGetFeesApiResponse(); + nock(API_BASE_URL) + .post(`/networks/${goerliChainIdDec}/getFees`) + .reply(200, getFeesApiResponse); + + expect( + smartTransactionsController.state.smartTransactionsState.feesByChainId, + ).toStrictEqual(defaultState.smartTransactionsState.feesByChainId); + + await smartTransactionsController.getFees(tradeTx, approvalTx, 'goerli'); + + expect( + smartTransactionsController.state.smartTransactionsState.feesByChainId, + ).toMatchObject({ + [CHAIN_IDS.ETHEREUM]: { + approvalTxFees: undefined, + tradeTxFees: undefined, + }, + [CHAIN_IDS.GOERLI]: { + approvalTxFees: getFeesApiResponse.txs[0], + tradeTxFees: getFeesApiResponse.txs[1], + }, + }); + }); }); describe('submitSignedTransactions', () => { @@ -823,36 +873,10 @@ describe('SmartTransactionsController', () => { .spyOn(smartTransactionsController, 'checkPoll') .mockImplementation(() => undefined); - const defaultState = { - smartTransactions: { - [CHAIN_IDS.ETHEREUM]: [], - }, - userOptIn: undefined, - fees: { - approvalTxFees: undefined, - tradeTxFees: undefined, - }, - feesByChainId: { - [CHAIN_IDS.ETHEREUM]: { - approvalTxFees: undefined, - tradeTxFees: undefined, - }, - [CHAIN_IDS.GOERLI]: { - approvalTxFees: undefined, - tradeTxFees: undefined, - }, - }, - liveness: true, - livenessByChainId: { - [CHAIN_IDS.ETHEREUM]: true, - [CHAIN_IDS.GOERLI]: true, - }, - }; - // pending transactions in state are required to test polling smartTransactionsController.update({ smartTransactionsState: { - ...defaultState, + ...defaultState.smartTransactionsState, smartTransactions: { '0x1': [ { @@ -964,11 +988,7 @@ describe('SmartTransactionsController', () => { ); // cleanup - smartTransactionsController.update({ - smartTransactionsState: { - ...defaultState, - }, - }); + smartTransactionsController.update(defaultState); smartTransactionsController.stopAllPolling(); jest.clearAllTimers(); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index ae6f802..1e1b6ca 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -322,8 +322,8 @@ export default class SmartTransactionsController extends PollingControllerV1< cancelledNonceIndex > -1 ? currentSmartTransactions .slice(0, cancelledNonceIndex) - ?.concat(currentSmartTransactions.slice(cancelledNonceIndex + 1)) - ?.concat(historifiedSmartTransaction) + .concat(currentSmartTransactions.slice(cancelledNonceIndex + 1)) + .concat(historifiedSmartTransaction) : currentSmartTransactions?.concat(historifiedSmartTransaction); this.update({ smartTransactionsState: { From 1a1bcbf0984f7b992ab123617bc7f45dfada529a Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 16 Oct 2023 10:58:39 -0500 Subject: [PATCH 09/25] add a test + bump test coverage --- jest.config.js | 8 +- src/SmartTransactionsController.test.ts | 106 ++++++++++++++---------- 2 files changed, 68 insertions(+), 46 deletions(-) diff --git a/jest.config.js b/jest.config.js index 432cbfa..57477b3 100644 --- a/jest.config.js +++ b/jest.config.js @@ -6,10 +6,10 @@ module.exports = { coverageReporters: ['text', 'html'], coverageThreshold: { global: { - branches: 77, - functions: 89, - lines: 92, - statements: 91, + branches: 78, + functions: 92.5, + lines: 94, + statements: 93.5, }, }, moduleFileExtensions: ['js', 'json', 'jsx', 'ts', 'tsx', 'node'], diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index b9a6af4..142bd2c 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -258,10 +258,9 @@ const testHistory = [ ]; const ethereumChainIdDec = parseInt(CHAIN_IDS.ETHEREUM, 16); +const goerliChainIdDec = parseInt(CHAIN_IDS.GOERLI, 16); const trackMetaMetricsEventSpy = jest.fn(); -const getNetworkClientByIdSpy = jest.fn(); - const defaultState = { smartTransactionsState: { smartTransactions: { @@ -307,7 +306,24 @@ describe('SmartTransactionsController', () => { provider: jest.fn(), confirmExternalTransaction: confirmExternalMock, trackMetaMetricsEvent: trackMetaMetricsEventSpy, - getNetworkClientById: getNetworkClientByIdSpy, + getNetworkClientById: jest.fn().mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + configuration: { + chainId: CHAIN_IDS.ETHEREUM, + }, + }; + case 'goerli': + return { + configuration: { + chainId: CHAIN_IDS.GOERLI, + }, + }; + default: + throw new Error('Invalid network client id'); + } + }), }); // eslint-disable-next-line jest/prefer-spy-on smartTransactionsController.subscribe = jest.fn(); @@ -402,6 +418,8 @@ describe('SmartTransactionsController', () => { }); describe('updateSmartTransactions', () => { + // TODO rewrite this test... updateSmartTransactions is getting called via the checkPoll method which is called whenever state is updated. + // this test should be more isolated to the updateSmartTransactions method. it('calls fetchSmartTransactionsStatus if there are pending transactions', () => { const fetchSmartTransactionsStatusSpy = jest .spyOn(smartTransactionsController, 'fetchSmartTransactionsStatus') @@ -490,25 +508,24 @@ describe('SmartTransactionsController', () => { }); it('should add fee data to feesByChainId state using the networkClientId passed in to identify the appropriate chain', async () => { - const goerliChainIdDec = parseInt(CHAIN_IDS.GOERLI, 16); - getNetworkClientByIdSpy.mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - configuration: { - chainId: CHAIN_IDS.ETHEREUM, - }, - }; - case 'goerli': - return { - configuration: { - chainId: CHAIN_IDS.GOERLI, - }, - }; - default: - throw new Error('Invalid network client id'); - } - }); + // getNetworkClientByIdSpy.mockImplementation((networkClientId) => { + // switch (networkClientId) { + // case 'mainnet': + // return { + // configuration: { + // chainId: CHAIN_IDS.ETHEREUM, + // }, + // }; + // case 'goerli': + // return { + // configuration: { + // chainId: CHAIN_IDS.GOERLI, + // }, + // }; + // default: + // throw new Error('Invalid network client id'); + // } + // }); const tradeTx = createUnsignedTransaction(goerliChainIdDec); const approvalTx = createUnsignedTransaction(goerliChainIdDec); @@ -681,6 +698,30 @@ describe('SmartTransactionsController', () => { const liveness = await smartTransactionsController.fetchLiveness(); expect(liveness).toBe(true); }); + + it('fetches liveness and sets in feesByChainId state for the Smart Transactions API for the chainId of the networkClientId passed in', async () => { + nock(API_BASE_URL) + .get(`/networks/${goerliChainIdDec}/health`) + .replyWithError('random error'); + + expect( + smartTransactionsController.state.smartTransactionsState + .livenessByChainId, + ).toStrictEqual({ + [CHAIN_IDS.ETHEREUM]: true, + [CHAIN_IDS.GOERLI]: true, + }); + + await smartTransactionsController.fetchLiveness('goerli'); + + expect( + smartTransactionsController.state.smartTransactionsState + .livenessByChainId, + ).toStrictEqual({ + [CHAIN_IDS.ETHEREUM]: true, + [CHAIN_IDS.GOERLI]: false, + }); + }); }); describe('updateSmartTransaction', () => { @@ -867,7 +908,7 @@ describe('SmartTransactionsController', () => { }); describe('startPollingByNetworkClientId', () => { - it('starts and stops calling smart transactions batch status api endpoint with the correct chainId at the interval passed via the constructor', async () => { + it('starts and stops calling smart transactions batch status api endpoint with the correct chainId at the polling interval', async () => { // mock this to a noop because it causes an extra fetch call to the API upon state changes jest .spyOn(smartTransactionsController, 'checkPoll') @@ -898,25 +939,6 @@ describe('SmartTransactionsController', () => { }, }); - getNetworkClientByIdSpy.mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - configuration: { - chainId: CHAIN_IDS.ETHEREUM, - }, - }; - case 'goerli': - return { - configuration: { - chainId: CHAIN_IDS.GOERLI, - }, - }; - default: - throw new Error('Invalid network client id'); - } - }); - jest.useFakeTimers(); const handleFetchSpy = jest.spyOn(utils, 'handleFetch'); From 80b6a6a745b80b19470a182ea3206d849aeed26c Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 16 Oct 2023 11:24:29 -0500 Subject: [PATCH 10/25] update type --- src/SmartTransactionsController.ts | 21 +++++++++++---------- src/types.ts | 2 ++ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 1e1b6ca..106622f 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -1,5 +1,5 @@ import { BaseConfig, BaseState } from '@metamask/base-controller'; -import { safelyExecute } from '@metamask/controller-utils'; +import { ChainId, safelyExecute } from '@metamask/controller-utils'; import { NetworkState, NetworkController, @@ -22,6 +22,7 @@ import { SmartTransactionStatuses, Fees, IndividualTxFees, + Hex, } from './types'; import { getAPIRequestURL, @@ -43,7 +44,7 @@ export const DEFAULT_INTERVAL = SECOND * 5; export type SmartTransactionsControllerConfig = BaseConfig & { interval: number; clientId: string; - chainId: string; + chainId: Hex; supportedChainIds: string[]; }; @@ -54,12 +55,12 @@ type FeeEstimates = { export type SmartTransactionsControllerState = BaseState & { smartTransactionsState: { - smartTransactions: Record; + smartTransactions: Record; userOptIn: boolean | undefined; liveness: boolean | undefined; fees: FeeEstimates; - feesByChainId: Record; - livenessByChainId: Record; + feesByChainId: Record; + livenessByChainId: Record; }; }; @@ -118,7 +119,7 @@ export default class SmartTransactionsController extends PollingControllerV1< this.defaultConfig = { interval: DEFAULT_INTERVAL, - chainId: CHAIN_IDS.ETHEREUM, + chainId: ChainId.mainnet, clientId: 'default', supportedChainIds: [CHAIN_IDS.ETHEREUM, CHAIN_IDS.GOERLI], }; @@ -290,7 +291,7 @@ export default class SmartTransactionsController extends PollingControllerV1< updateSmartTransaction( smartTransaction: SmartTransaction, - chainId: string, + chainId: Hex, ): void { const { smartTransactionsState } = this.state; const { smartTransactions } = smartTransactionsState; @@ -387,7 +388,7 @@ export default class SmartTransactionsController extends PollingControllerV1< // TODO make this a private method? async confirmSmartTransaction( smartTransaction: SmartTransaction, - chainId: string, + chainId: Hex, ) { const txHash = smartTransaction.statusMetadata?.minedHash; try { @@ -466,7 +467,7 @@ export default class SmartTransactionsController extends PollingControllerV1< // ! Ask backend API to accept list of uuids as params async fetchSmartTransactionsStatus( uuids: string[], - chainId: string, + chainId: Hex, ): Promise { const params = new URLSearchParams({ uuids: uuids.join(','), @@ -650,7 +651,7 @@ export default class SmartTransactionsController extends PollingControllerV1< return data; } - getChainId(networkClientId?: NetworkClientId): string { + getChainId(networkClientId?: NetworkClientId): Hex { if (!networkClientId) { return this.config.chainId; } diff --git a/src/types.ts b/src/types.ts index 045463f..b1032c1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -117,3 +117,5 @@ export type SignedTransaction = any; // TODO export type SignedCanceledTransaction = any; + +export type Hex = `0x${string}`; From b5910462c5089a5e293b4575a3873f12450fd45a Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 16 Oct 2023 11:29:37 -0500 Subject: [PATCH 11/25] fix type issue --- src/constants.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/constants.ts b/src/constants.ts index 3b8266c..00d8399 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,5 +1,12 @@ +import { Hex } from './types'; + export const API_BASE_URL = 'https://transaction.metaswap.codefi.network'; -export const CHAIN_IDS = { +export const CHAIN_IDS: { + ETHEREUM: Hex; + GOERLI: Hex; + RINKEBY: Hex; + BSC: Hex; +} = { ETHEREUM: '0x1', GOERLI: '0x5', RINKEBY: '0x4', From d644c5ca75e23be8e43c3a59869f7ad381db4a29 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 16 Oct 2023 13:35:10 -0500 Subject: [PATCH 12/25] address feedback --- src/constants.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/constants.ts b/src/constants.ts index 00d8399..693df74 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,14 +1,7 @@ -import { Hex } from './types'; - export const API_BASE_URL = 'https://transaction.metaswap.codefi.network'; -export const CHAIN_IDS: { - ETHEREUM: Hex; - GOERLI: Hex; - RINKEBY: Hex; - BSC: Hex; -} = { +export const CHAIN_IDS = { ETHEREUM: '0x1', GOERLI: '0x5', RINKEBY: '0x4', BSC: '0x38', -}; +} as const; From 311c991128085a4f00f6203d32c027b4becb5820 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 17 Oct 2023 15:41:38 -0500 Subject: [PATCH 13/25] fix provider logic --- src/SmartTransactionsController.test.ts | 37 ++++++++----- src/SmartTransactionsController.ts | 72 +++++++++++++++++-------- 2 files changed, 74 insertions(+), 35 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 142bd2c..6d8e51c 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -1,6 +1,7 @@ import nock from 'nock'; import { NetworkState } from '@metamask/network-controller'; import { convertHexToDecimal } from '@metamask/controller-utils'; +import { Web3Provider } from '@ethersproject/providers'; import SmartTransactionsController, { DEFAULT_INTERVAL, } from './SmartTransactionsController'; @@ -24,7 +25,7 @@ jest.mock('@ethersproject/bytes', () => ({ })); jest.mock('@ethersproject/providers', () => ({ - Web3Provider: class Web3Provider { + Web3Provider: class FakeProvider { getBalance = () => ({ toHexString: () => '0x1000' }); getTransactionReceipt = jest.fn(() => ({ blockNumber: '123' })); @@ -597,10 +598,9 @@ describe('SmartTransactionsController', () => { .get(`/networks/${ethereumChainIdDec}/batchStatus?uuids=uuid1`) .reply(200, pendingBatchStatusApiResponse); - await smartTransactionsController.fetchSmartTransactionsStatus( - uuids, - CHAIN_IDS.ETHEREUM, - ); + await smartTransactionsController.fetchSmartTransactionsStatus(uuids, { + networkClientId: 'mainnet', + }); const pendingState = createStateAfterPending()[0]; const pendingTransaction = { ...pendingState, history: [pendingState] }; expect(smartTransactionsController.state).toStrictEqual({ @@ -650,10 +650,9 @@ describe('SmartTransactionsController', () => { .get(`/networks/${ethereumChainIdDec}/batchStatus?uuids=uuid2`) .reply(200, successBatchStatusApiResponse); - await smartTransactionsController.fetchSmartTransactionsStatus( - uuids, - CHAIN_IDS.ETHEREUM, - ); + await smartTransactionsController.fetchSmartTransactionsStatus(uuids, { + networkClientId: 'mainnet', + }); const successState = createStateAfterSuccess()[0]; const successTransaction = { ...successState, history: [successState] }; expect(smartTransactionsController.state).toStrictEqual({ @@ -750,7 +749,10 @@ describe('SmartTransactionsController', () => { }; smartTransactionsController.updateSmartTransaction( updateTransaction as SmartTransaction, - CHAIN_IDS.ETHEREUM, + { + chainId: CHAIN_IDS.ETHEREUM, + ethersProvider: new Web3Provider(jest.fn()), + }, ); expect( @@ -783,7 +785,10 @@ describe('SmartTransactionsController', () => { }; smartTransactionsController.updateSmartTransaction( updateTransaction as SmartTransaction, - CHAIN_IDS.ETHEREUM, + { + chainId: CHAIN_IDS.ETHEREUM, + ethersProvider: new Web3Provider(jest.fn()), + }, ); expect(confirmSpy).toHaveBeenCalled(); }); @@ -802,7 +807,10 @@ describe('SmartTransactionsController', () => { }; await smartTransactionsController.confirmSmartTransaction( successfulStx as SmartTransaction, - CHAIN_IDS.ETHEREUM, + { + chainId: CHAIN_IDS.ETHEREUM, + ethersProvider: new Web3Provider(jest.fn()), + }, ); expect(confirmExternalMock).toHaveBeenCalled(); }); @@ -817,7 +825,10 @@ describe('SmartTransactionsController', () => { }; await smartTransactionsController.confirmSmartTransaction( successfulStx as SmartTransaction, - CHAIN_IDS.ETHEREUM, + { + chainId: CHAIN_IDS.ETHEREUM, + ethersProvider: new Web3Provider(jest.fn()), + }, ); expect(trackMetaMetricsEventSpy).toHaveBeenCalled(); }); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 106622f..4985603 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -179,7 +179,7 @@ export default class SmartTransactionsController extends PollingControllerV1< if (!this.config.supportedChainIds.includes(chainId)) { return Promise.resolve(); } - return this.updateSmartTransactions(networkClientId); + return this.updateSmartTransactions({ networkClientId }); } checkPoll(state: any) { @@ -291,7 +291,13 @@ export default class SmartTransactionsController extends PollingControllerV1< updateSmartTransaction( smartTransaction: SmartTransaction, - chainId: Hex, + { + chainId = this.config.chainId, + ethersProvider = this.ethersProvider, + }: { + chainId: Hex; + ethersProvider: Web3Provider; + }, ): void { const { smartTransactionsState } = this.state; const { smartTransactions } = smartTransactionsState; @@ -349,7 +355,10 @@ export default class SmartTransactionsController extends PollingControllerV1< ...currentSmartTransaction, ...smartTransaction, }; - this.confirmSmartTransaction(nextSmartTransaction, chainId); + this.confirmSmartTransaction(nextSmartTransaction, { + chainId, + ethersProvider, + }); } this.update({ @@ -369,37 +378,48 @@ export default class SmartTransactionsController extends PollingControllerV1< }); } - async updateSmartTransactions( - networkClientId?: NetworkClientId, - ): Promise { - const chainId = this.getChainId(networkClientId); + async updateSmartTransactions({ + networkClientId, + }: { + networkClientId?: NetworkClientId; + } = {}): Promise { const { smartTransactions } = this.state.smartTransactionsState; + const chainId = this.getChainId(networkClientId); + const smartTransactionsForChainId = smartTransactions?.[chainId]; - const currentSmartTransactions = smartTransactions?.[chainId]; - const transactionsToUpdate: string[] = currentSmartTransactions + const transactionsToUpdate: string[] = smartTransactionsForChainId .filter(isSmartTransactionPending) .map((smartTransaction) => smartTransaction.uuid); if (transactionsToUpdate.length > 0) { - this.fetchSmartTransactionsStatus(transactionsToUpdate, chainId); + this.fetchSmartTransactionsStatus(transactionsToUpdate, { + networkClientId, + }); } } // TODO make this a private method? async confirmSmartTransaction( smartTransaction: SmartTransaction, - chainId: Hex, + { + chainId, + ethersProvider, + }: { + chainId: Hex; + ethersProvider: any; + }, ) { const txHash = smartTransaction.statusMetadata?.minedHash; try { - const transactionReceipt = - await this.ethersProvider.getTransactionReceipt(txHash); - const transaction = await this.ethersProvider.getTransaction(txHash); + const transactionReceipt = await ethersProvider.getTransactionReceipt( + txHash, + ); + const transaction = await ethersProvider.getTransaction(txHash); const maxFeePerGas = transaction.maxFeePerGas?.toHexString(); const maxPriorityFeePerGas = transaction.maxPriorityFeePerGas?.toHexString(); if (transactionReceipt?.blockNumber) { - const blockData = await this.ethersProvider.getBlock( + const blockData = await ethersProvider.getBlock( transactionReceipt?.blockNumber, false, ); @@ -452,7 +472,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ...smartTransaction, confirmed: true, }, - chainId, + { chainId, ethersProvider }, ); } } catch (e) { @@ -467,11 +487,13 @@ export default class SmartTransactionsController extends PollingControllerV1< // ! Ask backend API to accept list of uuids as params async fetchSmartTransactionsStatus( uuids: string[], - chainId: Hex, + { networkClientId }: { networkClientId?: NetworkClientId } = {}, ): Promise { const params = new URLSearchParams({ uuids: uuids.join(','), }); + const chainId = this.getChainId(networkClientId); + const ethersProvider = this.getProvider(networkClientId); const url = `${getAPIRequestURL( APIType.BATCH_STATUS, chainId, @@ -488,7 +510,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ), uuid, }, - chainId, + { chainId, ethersProvider }, ); }); @@ -604,6 +626,7 @@ export default class SmartTransactionsController extends PollingControllerV1< networkClientId?: NetworkClientId; }) { const chainId = this.getChainId(networkClientId); + const ethersProvider = this.getProvider(networkClientId); const data = await this.fetch( getAPIRequestURL(APIType.SUBMIT_TRANSACTIONS, chainId), { @@ -617,9 +640,7 @@ export default class SmartTransactionsController extends PollingControllerV1< const time = Date.now(); let preTxBalance; try { - const preTxBalanceBN = await this.ethersProvider.getBalance( - txParams?.from, - ); + const preTxBalanceBN = await ethersProvider.getBalance(txParams?.from); preTxBalance = new BigNumber(preTxBalanceBN.toHexString()).toString(16); } catch (e) { console.error('ethers error', e); @@ -642,7 +663,7 @@ export default class SmartTransactionsController extends PollingControllerV1< uuid: data.uuid, cancellable: true, }, - chainId, + { chainId, ethersProvider }, ); } finally { nonceLock.releaseLock(); @@ -658,6 +679,13 @@ export default class SmartTransactionsController extends PollingControllerV1< return this.getNetworkClientById(networkClientId).configuration.chainId; } + getProvider(networkClientId?: NetworkClientId): any { + if (!networkClientId) { + return this.ethersProvider; + } + return this.getNetworkClientById(networkClientId).provider; + } + // TODO: This should return if the cancellation was on chain or not (for nonce management) // After this successful call client must update nonce representative // in transaction controller external transactions list From 38434164128471074560ff01b3391fd0e1d27339 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 17 Oct 2023 15:43:59 -0500 Subject: [PATCH 14/25] update test coverage threshholds --- jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jest.config.js b/jest.config.js index 57477b3..198fe80 100644 --- a/jest.config.js +++ b/jest.config.js @@ -6,10 +6,10 @@ module.exports = { coverageReporters: ['text', 'html'], coverageThreshold: { global: { - branches: 78, + branches: 77.5, functions: 92.5, - lines: 94, - statements: 93.5, + lines: 93.35, + statements: 93.35, }, }, moduleFileExtensions: ['js', 'json', 'jsx', 'ts', 'tsx', 'node'], From 684bc5923e2e105473626208e2b1ebe71e181928 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 17 Oct 2023 15:54:41 -0500 Subject: [PATCH 15/25] make a private and public version of updateSmartTransaction --- src/SmartTransactionsController.test.ts | 6 ++---- src/SmartTransactionsController.ts | 23 ++++++++++++++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 6d8e51c..453813c 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -750,8 +750,7 @@ describe('SmartTransactionsController', () => { smartTransactionsController.updateSmartTransaction( updateTransaction as SmartTransaction, { - chainId: CHAIN_IDS.ETHEREUM, - ethersProvider: new Web3Provider(jest.fn()), + networkClientId: 'mainnet', }, ); @@ -786,8 +785,7 @@ describe('SmartTransactionsController', () => { smartTransactionsController.updateSmartTransaction( updateTransaction as SmartTransaction, { - chainId: CHAIN_IDS.ETHEREUM, - ethersProvider: new Web3Provider(jest.fn()), + networkClientId: 'mainnet', }, ); expect(confirmSpy).toHaveBeenCalled(); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 4985603..3d39cbe 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -290,6 +290,23 @@ export default class SmartTransactionsController extends PollingControllerV1< } updateSmartTransaction( + SmartTransaction: SmartTransaction, + { networkClientId }: { networkClientId?: NetworkClientId } = {}, + ) { + let chainId = this.config.chainId; + let ethersProvider = this.ethersProvider; + if (networkClientId) { + const networkClient = this.getNetworkClientById(networkClientId); + chainId = networkClient.configuration.chainId; + ethersProvider = networkClient.provider; + } + this.#updateSmartTransaction(SmartTransaction, { + chainId, + ethersProvider, + }); + } + + #updateSmartTransaction( smartTransaction: SmartTransaction, { chainId = this.config.chainId, @@ -467,7 +484,7 @@ export default class SmartTransactionsController extends PollingControllerV1< category: 'swaps', }); - this.updateSmartTransaction( + this.#updateSmartTransaction( { ...smartTransaction, confirmed: true, @@ -501,7 +518,7 @@ export default class SmartTransactionsController extends PollingControllerV1< const data = await this.fetch(url); Object.entries(data).forEach(([uuid, stxStatus]) => { - this.updateSmartTransaction( + this.#updateSmartTransaction( { statusMetadata: stxStatus as SmartTransactionsStatus, status: calculateStatus(stxStatus as SmartTransactionsStatus), @@ -653,7 +670,7 @@ export default class SmartTransactionsController extends PollingControllerV1< } const { nonceDetails } = nonceLock; - this.updateSmartTransaction( + this.#updateSmartTransaction( { nonceDetails, preTxBalance, From 02af694d230682c012b0ced4c0bdd0348b698161 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 17 Oct 2023 15:57:16 -0500 Subject: [PATCH 16/25] fix capitalization" --- jest.config.js | 2 +- src/SmartTransactionsController.ts | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/jest.config.js b/jest.config.js index 198fe80..df6fc45 100644 --- a/jest.config.js +++ b/jest.config.js @@ -6,7 +6,7 @@ module.exports = { coverageReporters: ['text', 'html'], coverageThreshold: { global: { - branches: 77.5, + branches: 77.4, functions: 92.5, lines: 93.35, statements: 93.35, diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 3d39cbe..b4dfa4e 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -290,17 +290,18 @@ export default class SmartTransactionsController extends PollingControllerV1< } updateSmartTransaction( - SmartTransaction: SmartTransaction, + smartTransaction: SmartTransaction, { networkClientId }: { networkClientId?: NetworkClientId } = {}, ) { - let chainId = this.config.chainId; - let ethersProvider = this.ethersProvider; + let { chainId } = this.config; + let { ethersProvider } = this; if (networkClientId) { const networkClient = this.getNetworkClientById(networkClientId); chainId = networkClient.configuration.chainId; ethersProvider = networkClient.provider; } - this.#updateSmartTransaction(SmartTransaction, { + + this.#updateSmartTransaction(smartTransaction, { chainId, ethersProvider, }); From ec52211f7d3c0837bbbd71db0da2d6bb3ddb44b8 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Oct 2023 09:43:18 -0500 Subject: [PATCH 17/25] addressing feedback --- jest.config.js | 2 +- src/SmartTransactionsController.ts | 46 +++++++++++------------------- 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/jest.config.js b/jest.config.js index df6fc45..d273f83 100644 --- a/jest.config.js +++ b/jest.config.js @@ -6,7 +6,7 @@ module.exports = { coverageReporters: ['text', 'html'], coverageThreshold: { global: { - branches: 77.4, + branches: 77.35, functions: 92.5, lines: 93.35, statements: 93.35, diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index b4dfa4e..46f7446 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -60,7 +60,7 @@ export type SmartTransactionsControllerState = BaseState & { liveness: boolean | undefined; fees: FeeEstimates; feesByChainId: Record; - livenessByChainId: Record; + livenessByChainId: Record; }; }; @@ -606,23 +606,15 @@ export default class SmartTransactionsController extends PollingControllerV1< approvalTxFees, tradeTxFees, }, - }, - }); - - if (networkClientId) { - this.update({ - smartTransactionsState: { - ...this.state.smartTransactionsState, - feesByChainId: { - ...this.state.smartTransactionsState.feesByChainId, - [chainId]: { - approvalTxFees, - tradeTxFees, - }, + feesByChainId: { + ...this.state.smartTransactionsState.feesByChainId, + [chainId]: { + approvalTxFees, + tradeTxFees, }, }, - }); - } + }, + }); return { approvalTxFees, @@ -709,7 +701,11 @@ export default class SmartTransactionsController extends PollingControllerV1< // in transaction controller external transactions list async cancelSmartTransaction( uuid: string, - networkClientId?: NetworkClientId, + { + networkClientId, + }: { + networkClientId?: NetworkClientId; + } = {}, ): Promise { const chainId = this.getChainId(networkClientId); await this.fetch(getAPIRequestURL(APIType.CANCEL, chainId), { @@ -734,21 +730,13 @@ export default class SmartTransactionsController extends PollingControllerV1< smartTransactionsState: { ...this.state.smartTransactionsState, liveness, + livenessByChainId: { + ...this.state.smartTransactionsState.livenessByChainId, + [chainId]: liveness, + }, }, }); - if (networkClientId) { - this.update({ - smartTransactionsState: { - ...this.state.smartTransactionsState, - livenessByChainId: { - ...this.state.smartTransactionsState.livenessByChainId, - [chainId]: liveness, - }, - }, - }); - } - return liveness; } From fe9732ccf000b24a670fca8681edc998f45771c2 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Oct 2023 12:45:23 -0500 Subject: [PATCH 18/25] replace @ethersproject/providers with @metamask/eth-query --- jest.config.js | 2 +- package.json | 2 +- src/SmartTransactionsController.test.ts | 59 ++++-- src/SmartTransactionsController.ts | 95 +++++---- yarn.lock | 250 +----------------------- 5 files changed, 101 insertions(+), 307 deletions(-) diff --git a/jest.config.js b/jest.config.js index d273f83..153f417 100644 --- a/jest.config.js +++ b/jest.config.js @@ -6,7 +6,7 @@ module.exports = { coverageReporters: ['text', 'html'], coverageThreshold: { global: { - branches: 77.35, + branches: 76.5, functions: 92.5, lines: 93.35, statements: 93.35, diff --git a/package.json b/package.json index 179c294..1500d8c 100644 --- a/package.json +++ b/package.json @@ -27,9 +27,9 @@ "dependencies": { "@ethersproject/bignumber": "^5.7.0", "@ethersproject/bytes": "^5.7.0", - "@ethersproject/providers": "^5.7.0", "@metamask/base-controller": "^3.2.1", "@metamask/controller-utils": "^5.0.0", + "@metamask/eth-query": "^3.0.1", "@metamask/network-controller": "^15.0.0", "@metamask/polling-controller": "^0.2.0", "bignumber.js": "^9.0.1", diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 453813c..89d901c 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -1,7 +1,7 @@ import nock from 'nock'; import { NetworkState } from '@metamask/network-controller'; import { convertHexToDecimal } from '@metamask/controller-utils'; -import { Web3Provider } from '@ethersproject/providers'; +import EthQuery from '@metamask/eth-query'; import SmartTransactionsController, { DEFAULT_INTERVAL, } from './SmartTransactionsController'; @@ -24,20 +24,40 @@ jest.mock('@ethersproject/bytes', () => ({ hexlify: (str: string) => `0x${str}`, })); -jest.mock('@ethersproject/providers', () => ({ - Web3Provider: class FakeProvider { - getBalance = () => ({ toHexString: () => '0x1000' }); +jest.mock('@metamask/eth-query', () => { + return class FakeEthQuery { + sendAsync = jest.fn(({ method }, callback) => { + switch (method) { + case 'getBalance': { + callback(null, { toHexString: () => '0x1000' }); + break; + } - getTransactionReceipt = jest.fn(() => ({ blockNumber: '123' })); + case 'getTransactionReceipt': { + callback(null, { blockNumber: '123' }); + break; + } - getTransaction = jest.fn(() => ({ - maxFeePerGas: { toHexString: () => '0x123' }, - maxPriorityFeePerGas: { toHexString: () => '0x123' }, - })); + case 'getBlock': { + callback(null, { baseFeePerGas: { toHexString: () => '0x123' } }); + break; + } - getBlock = jest.fn(); - }, -})); + case 'getTransaction': { + callback(null, { + maxFeePerGas: { toHexString: () => '0x123' }, + maxPriorityFeePerGas: { toHexString: () => '0x123' }, + }); + break; + } + + default: { + throw new Error('Invalid method'); + } + } + }); + }; +}); const addressFrom = '0x268392a24B6b093127E8581eAfbD1DA228bAdAe3'; @@ -394,7 +414,7 @@ describe('SmartTransactionsController', () => { it('calls stop if there is a timeoutHandle and no pending transactions', () => { const stopSpy = jest.spyOn(smartTransactionsController, 'stop'); - smartTransactionsController.timeoutHandle = setInterval(() => ({})); + smartTransactionsController.timeoutHandle = setTimeout(() => ({})); smartTransactionsController.checkPoll(smartTransactionsController.state); expect(stopSpy).toHaveBeenCalled(); clearInterval(smartTransactionsController.timeoutHandle); @@ -807,16 +827,19 @@ describe('SmartTransactionsController', () => { successfulStx as SmartTransaction, { chainId: CHAIN_IDS.ETHEREUM, - ethersProvider: new Web3Provider(jest.fn()), + ethQuery: new EthQuery({ sendAsync: jest.fn() }), }, ); expect(confirmExternalMock).toHaveBeenCalled(); }); it('throws an error if ethersProvider fails', async () => { - smartTransactionsController.ethersProvider.getTransactionReceipt.mockRejectedValueOnce( - 'random error' as never, - ); + // smartTransactionsController.ethQuery.getTransactionReceipt.mockRejectedValueOnce( + // 'random error' as never, + // ); + // jest.spyOn('@metmask/eth-query', 'sendAsync')(() => { + // throw new Error('random error'); + // }); const successfulStx = { ...createStateAfterSuccess()[0], history: testHistory, @@ -825,7 +848,7 @@ describe('SmartTransactionsController', () => { successfulStx as SmartTransaction, { chainId: CHAIN_IDS.ETHEREUM, - ethersProvider: new Web3Provider(jest.fn()), + ethQuery: new EthQuery({ sendAsync: jest.fn() }), }, ); expect(trackMetaMetricsEventSpy).toHaveBeenCalled(); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 46f7446..84a98e9 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -1,5 +1,5 @@ import { BaseConfig, BaseState } from '@metamask/base-controller'; -import { ChainId, safelyExecute } from '@metamask/controller-utils'; +import { ChainId, safelyExecute, query } from '@metamask/controller-utils'; import { NetworkState, NetworkController, @@ -8,7 +8,7 @@ import { import { PollingControllerV1 } from '@metamask/polling-controller'; import { BigNumber } from 'bignumber.js'; import { BigNumber as ethersBigNumber } from '@ethersproject/bignumber'; -import { Web3Provider } from '@ethersproject/providers'; +import EthQuery from '@metamask/eth-query'; import { hexlify } from '@ethersproject/bytes'; import mapValues from 'lodash/mapValues'; import cloneDeep from 'lodash/cloneDeep'; @@ -72,7 +72,7 @@ export default class SmartTransactionsController extends PollingControllerV1< private getNonceLock: any; - public ethersProvider: any; + public ethQuery: EthQuery; public confirmExternalTransaction: any; @@ -151,7 +151,7 @@ export default class SmartTransactionsController extends PollingControllerV1< }; this.setIntervalLength(this.config.interval || DEFAULT_INTERVAL); this.getNonceLock = getNonceLock; - this.ethersProvider = new Web3Provider(provider); + this.ethQuery = new EthQuery(provider); this.confirmExternalTransaction = confirmExternalTransaction; this.trackMetaMetricsEvent = trackMetaMetricsEvent; this.getNetworkClientById = getNetworkClientById; @@ -164,7 +164,7 @@ export default class SmartTransactionsController extends PollingControllerV1< this.configure({ chainId }); this.initializeSmartTransactionsForChainId(); this.checkPoll(this.state); - this.ethersProvider = new Web3Provider(provider); + this.ethQuery = new EthQuery(provider); }); this.subscribe((currentState: any) => this.checkPoll(currentState)); @@ -175,7 +175,7 @@ export default class SmartTransactionsController extends PollingControllerV1< // with a networkClientId that is not supported, but for now I'll add a check in case // wondering if we should add some kind of predicate to the polling controller to check whether // we should poll or not - const chainId = this.getChainId(networkClientId); + const chainId = this.getChainId({ networkClientId }); if (!this.config.supportedChainIds.includes(chainId)) { return Promise.resolve(); } @@ -294,16 +294,17 @@ export default class SmartTransactionsController extends PollingControllerV1< { networkClientId }: { networkClientId?: NetworkClientId } = {}, ) { let { chainId } = this.config; - let { ethersProvider } = this; + let { ethQuery } = this; if (networkClientId) { const networkClient = this.getNetworkClientById(networkClientId); chainId = networkClient.configuration.chainId; - ethersProvider = networkClient.provider; + // @ts-expect-error TODO: Provider type alignment + ethQuery = new EthQuery(networkClient.provider); } this.#updateSmartTransaction(smartTransaction, { chainId, - ethersProvider, + ethQuery, }); } @@ -311,10 +312,10 @@ export default class SmartTransactionsController extends PollingControllerV1< smartTransaction: SmartTransaction, { chainId = this.config.chainId, - ethersProvider = this.ethersProvider, + ethQuery = this.ethQuery, }: { chainId: Hex; - ethersProvider: Web3Provider; + ethQuery: EthQuery; }, ): void { const { smartTransactionsState } = this.state; @@ -375,7 +376,7 @@ export default class SmartTransactionsController extends PollingControllerV1< }; this.confirmSmartTransaction(nextSmartTransaction, { chainId, - ethersProvider, + ethQuery, }); } @@ -402,7 +403,7 @@ export default class SmartTransactionsController extends PollingControllerV1< networkClientId?: NetworkClientId; } = {}): Promise { const { smartTransactions } = this.state.smartTransactionsState; - const chainId = this.getChainId(networkClientId); + const chainId = this.getChainId({ networkClientId }); const smartTransactionsForChainId = smartTransactions?.[chainId]; const transactionsToUpdate: string[] = smartTransactionsForChainId @@ -421,26 +422,29 @@ export default class SmartTransactionsController extends PollingControllerV1< smartTransaction: SmartTransaction, { chainId, - ethersProvider, + ethQuery, }: { chainId: Hex; - ethersProvider: any; + ethQuery: EthQuery; }, ) { const txHash = smartTransaction.statusMetadata?.minedHash; try { - const transactionReceipt = await ethersProvider.getTransactionReceipt( - txHash, + const transactionReceipt = await query( + ethQuery, + 'getTransactionReceipt', + [txHash], ); - const transaction = await ethersProvider.getTransaction(txHash); + const transaction = await query(ethQuery, 'getTransaction', [txHash]); + const maxFeePerGas = transaction.maxFeePerGas?.toHexString(); const maxPriorityFeePerGas = transaction.maxPriorityFeePerGas?.toHexString(); if (transactionReceipt?.blockNumber) { - const blockData = await ethersProvider.getBlock( + const blockData = await query(ethQuery, 'getBlock', [ transactionReceipt?.blockNumber, false, - ); + ]); const baseFeePerGas = blockData?.baseFeePerGas.toHexString(); const txReceipt = mapValues(transactionReceipt, (value) => { if (value instanceof ethersBigNumber) { @@ -478,6 +482,7 @@ export default class SmartTransactionsController extends PollingControllerV1< history: originalTxMeta.history.concat(entry), } : originalTxMeta; + this.confirmExternalTransaction(txMeta, txReceipt, baseFeePerGas); this.trackMetaMetricsEvent({ @@ -490,7 +495,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ...smartTransaction, confirmed: true, }, - { chainId, ethersProvider }, + { chainId, ethQuery }, ); } } catch (e) { @@ -510,8 +515,8 @@ export default class SmartTransactionsController extends PollingControllerV1< const params = new URLSearchParams({ uuids: uuids.join(','), }); - const chainId = this.getChainId(networkClientId); - const ethersProvider = this.getProvider(networkClientId); + const chainId = this.getChainId({ networkClientId }); + const ethQuery = this.getEthQuery({ networkClientId }); const url = `${getAPIRequestURL( APIType.BATCH_STATUS, chainId, @@ -528,7 +533,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ), uuid, }, - { chainId, ethersProvider }, + { chainId, ethQuery }, ); }); @@ -566,7 +571,7 @@ export default class SmartTransactionsController extends PollingControllerV1< approvalTx: UnsignedTransaction, networkClientId?: NetworkClientId, ): Promise { - const chainId = this.getChainId(networkClientId); + const chainId = this.getChainId({ networkClientId }); const transactions = []; let unsignedTradeTransactionWithNonce; if (approvalTx) { @@ -635,8 +640,8 @@ export default class SmartTransactionsController extends PollingControllerV1< txParams?: any; networkClientId?: NetworkClientId; }) { - const chainId = this.getChainId(networkClientId); - const ethersProvider = this.getProvider(networkClientId); + const chainId = this.getChainId({ networkClientId }); + const ethQuery = this.getEthQuery({ networkClientId }); const data = await this.fetch( getAPIRequestURL(APIType.SUBMIT_TRANSACTIONS, chainId), { @@ -650,7 +655,10 @@ export default class SmartTransactionsController extends PollingControllerV1< const time = Date.now(); let preTxBalance; try { - const preTxBalanceBN = await ethersProvider.getBalance(txParams?.from); + const preTxBalanceBN = await query(ethQuery, 'getBalance', [ + txParams?.from, + ]); + console.log('preTxBalanceBN', preTxBalanceBN); preTxBalance = new BigNumber(preTxBalanceBN.toHexString()).toString(16); } catch (e) { console.error('ethers error', e); @@ -673,7 +681,7 @@ export default class SmartTransactionsController extends PollingControllerV1< uuid: data.uuid, cancellable: true, }, - { chainId, ethersProvider }, + { chainId, ethQuery }, ); } finally { nonceLock.releaseLock(); @@ -682,18 +690,23 @@ export default class SmartTransactionsController extends PollingControllerV1< return data; } - getChainId(networkClientId?: NetworkClientId): Hex { - if (!networkClientId) { - return this.config.chainId; - } - return this.getNetworkClientById(networkClientId).configuration.chainId; + getChainId({ + networkClientId, + }: { networkClientId?: NetworkClientId } = {}): Hex { + return networkClientId + ? this.getNetworkClientById(networkClientId).configuration.chainId + : this.config.chainId; } - getProvider(networkClientId?: NetworkClientId): any { - if (!networkClientId) { - return this.ethersProvider; - } - return this.getNetworkClientById(networkClientId).provider; + getEthQuery({ + networkClientId, + }: { + networkClientId?: NetworkClientId; + }): EthQuery { + return networkClientId + ? // @ts-expect-error TODO: Provider type alignment + new EthQuery(this.getNetworkClientById(networkClientId).provider) + : this.ethQuery; } // TODO: This should return if the cancellation was on chain or not (for nonce management) @@ -707,7 +720,7 @@ export default class SmartTransactionsController extends PollingControllerV1< networkClientId?: NetworkClientId; } = {}, ): Promise { - const chainId = this.getChainId(networkClientId); + const chainId = this.getChainId({ networkClientId }); await this.fetch(getAPIRequestURL(APIType.CANCEL, chainId), { method: 'POST', body: JSON.stringify({ uuid }), @@ -715,7 +728,7 @@ export default class SmartTransactionsController extends PollingControllerV1< } async fetchLiveness(networkClientId?: NetworkClientId): Promise { - const chainId = this.getChainId(networkClientId); + const chainId = this.getChainId({ networkClientId }); let liveness = false; try { const response = await this.fetch( diff --git a/yarn.lock b/yarn.lock index acf5d3b..aa734ae 100644 --- a/yarn.lock +++ b/yarn.lock @@ -528,66 +528,6 @@ __metadata: languageName: node linkType: hard -"@ethersproject/abstract-provider@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/abstract-provider@npm:5.7.0" - dependencies: - "@ethersproject/bignumber": ^5.7.0 - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - "@ethersproject/networks": ^5.7.0 - "@ethersproject/properties": ^5.7.0 - "@ethersproject/transactions": ^5.7.0 - "@ethersproject/web": ^5.7.0 - checksum: 74cf4696245cf03bb7cc5b6cbf7b4b89dd9a79a1c4688126d214153a938126d4972d42c93182198653ce1de35f2a2cad68be40337d4774b3698a39b28f0228a8 - languageName: node - linkType: hard - -"@ethersproject/abstract-signer@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/abstract-signer@npm:5.7.0" - dependencies: - "@ethersproject/abstract-provider": ^5.7.0 - "@ethersproject/bignumber": ^5.7.0 - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - "@ethersproject/properties": ^5.7.0 - checksum: a823dac9cfb761e009851050ebebd5b229d1b1cc4a75b125c2da130ff37e8218208f7f9d1386f77407705b889b23d4a230ad67185f8872f083143e0073cbfbe3 - languageName: node - linkType: hard - -"@ethersproject/address@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/address@npm:5.7.0" - dependencies: - "@ethersproject/bignumber": ^5.7.0 - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/keccak256": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - "@ethersproject/rlp": ^5.7.0 - checksum: 64ea5ebea9cc0e845c413e6cb1e54e157dd9fc0dffb98e239d3a3efc8177f2ff798cd4e3206cf3660ee8faeb7bef1a47dc0ebef0d7b132c32e61e550c7d4c843 - languageName: node - linkType: hard - -"@ethersproject/base64@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/base64@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - checksum: 7dd5d734d623582f08f665434f53685041a3d3b334a0e96c0c8afa8bbcaab934d50e5b6b980e826a8fde8d353e0b18f11e61faf17468177274b8e7c69cd9742b - languageName: node - linkType: hard - -"@ethersproject/basex@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/basex@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/properties": ^5.7.0 - checksum: 326087b7e1f3787b5fe6cd1cf2b4b5abfafbc355a45e88e22e5e9d6c845b613ffc5301d629b28d5c4d5e2bfe9ec424e6782c804956dff79be05f0098cb5817de - languageName: node - linkType: hard - "@ethersproject/bignumber@npm:^5.7.0": version: 5.7.0 resolution: "@ethersproject/bignumber@npm:5.7.0" @@ -608,42 +548,6 @@ __metadata: languageName: node linkType: hard -"@ethersproject/constants@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/constants@npm:5.7.0" - dependencies: - "@ethersproject/bignumber": ^5.7.0 - checksum: 6d4b1355747cce837b3e76ec3bde70e4732736f23b04f196f706ebfa5d4d9c2be50904a390d4d40ce77803b98d03d16a9b6898418e04ba63491933ce08c4ba8a - languageName: node - linkType: hard - -"@ethersproject/hash@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/hash@npm:5.7.0" - dependencies: - "@ethersproject/abstract-signer": ^5.7.0 - "@ethersproject/address": ^5.7.0 - "@ethersproject/base64": ^5.7.0 - "@ethersproject/bignumber": ^5.7.0 - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/keccak256": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - "@ethersproject/properties": ^5.7.0 - "@ethersproject/strings": ^5.7.0 - checksum: 6e9fa8d14eb08171cd32f17f98cc108ec2aeca74a427655f0d689c550fee0b22a83b3b400fad7fb3f41cf14d4111f87f170aa7905bcbcd1173a55f21b06262ef - languageName: node - linkType: hard - -"@ethersproject/keccak256@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/keccak256@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - js-sha3: 0.8.0 - checksum: ff70950d82203aab29ccda2553422cbac2e7a0c15c986bd20a69b13606ed8bb6e4fdd7b67b8d3b27d4f841e8222cbaccd33ed34be29f866fec7308f96ed244c6 - languageName: node - linkType: hard - "@ethersproject/logger@npm:^5.7.0": version: 5.7.0 resolution: "@ethersproject/logger@npm:5.7.0" @@ -651,138 +555,6 @@ __metadata: languageName: node linkType: hard -"@ethersproject/networks@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/networks@npm:5.7.0" - dependencies: - "@ethersproject/logger": ^5.7.0 - checksum: 4f4d77e7c59e79cfcba616315a5d0e634a7653acbd11bb06a0028f4bd009b19f9a31556148a1e38f7308f55d1a1d170eb9f065290de9f9cf104b34e91cc348b8 - languageName: node - linkType: hard - -"@ethersproject/properties@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/properties@npm:5.7.0" - dependencies: - "@ethersproject/logger": ^5.7.0 - checksum: 6ab0ccf0c3aadc9221e0cdc5306ce6cd0df7f89f77d77bccdd1277182c9ead0202cd7521329ba3acde130820bf8af299e17cf567d0d497c736ee918207bbf59f - languageName: node - linkType: hard - -"@ethersproject/providers@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/providers@npm:5.7.0" - dependencies: - "@ethersproject/abstract-provider": ^5.7.0 - "@ethersproject/abstract-signer": ^5.7.0 - "@ethersproject/address": ^5.7.0 - "@ethersproject/base64": ^5.7.0 - "@ethersproject/basex": ^5.7.0 - "@ethersproject/bignumber": ^5.7.0 - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/constants": ^5.7.0 - "@ethersproject/hash": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - "@ethersproject/networks": ^5.7.0 - "@ethersproject/properties": ^5.7.0 - "@ethersproject/random": ^5.7.0 - "@ethersproject/rlp": ^5.7.0 - "@ethersproject/sha2": ^5.7.0 - "@ethersproject/strings": ^5.7.0 - "@ethersproject/transactions": ^5.7.0 - "@ethersproject/web": ^5.7.0 - bech32: 1.1.4 - ws: 7.4.6 - checksum: a6f80cea838424ceb367ff8e0f004f9fd6b43a87505da9d6aef33eb2bbc77cdb03ab51709ae83b7aa07d038fadf00634e08d8683fe6ae8b17b9351e3b30b26cb - languageName: node - linkType: hard - -"@ethersproject/random@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/random@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - checksum: 017829c91cff6c76470852855108115b0b52c611b6be817ed1948d56ba42d6677803ec2012aa5ae298a7660024156a64c11fcf544e235e239ab3f89f0fff7345 - languageName: node - linkType: hard - -"@ethersproject/rlp@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/rlp@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - checksum: bce165b0f7e68e4d091c9d3cf47b247cac33252df77a095ca4281d32d5eeaaa3695d9bc06b2b057c5015353a68df89f13a4a54a72e888e4beeabbe56b15dda6e - languageName: node - linkType: hard - -"@ethersproject/sha2@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/sha2@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - hash.js: 1.1.7 - checksum: 09321057c022effbff4cc2d9b9558228690b5dd916329d75c4b1ffe32ba3d24b480a367a7cc92d0f0c0b1c896814d03351ae4630e2f1f7160be2bcfbde435dbc - languageName: node - linkType: hard - -"@ethersproject/signing-key@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/signing-key@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - "@ethersproject/properties": ^5.7.0 - bn.js: ^5.2.1 - elliptic: 6.5.4 - hash.js: 1.1.7 - checksum: 8f8de09b0aac709683bbb49339bc0a4cd2f95598f3546436c65d6f3c3a847ffa98e06d35e9ed2b17d8030bd2f02db9b7bd2e11c5cf8a71aad4537487ab4cf03a - languageName: node - linkType: hard - -"@ethersproject/strings@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/strings@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/constants": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - checksum: 5ff78693ae3fdf3cf23e1f6dc047a61e44c8197d2408c42719fef8cb7b7b3613a4eec88ac0ed1f9f5558c74fe0de7ae3195a29ca91a239c74b9f444d8e8b50df - languageName: node - linkType: hard - -"@ethersproject/transactions@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/transactions@npm:5.7.0" - dependencies: - "@ethersproject/address": ^5.7.0 - "@ethersproject/bignumber": ^5.7.0 - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/constants": ^5.7.0 - "@ethersproject/keccak256": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - "@ethersproject/properties": ^5.7.0 - "@ethersproject/rlp": ^5.7.0 - "@ethersproject/signing-key": ^5.7.0 - checksum: a31b71996d2b283f68486241bff0d3ea3f1ba0e8f1322a8fffc239ccc4f4a7eb2ea9994b8fd2f093283fd75f87bae68171e01b6265261f821369aca319884a79 - languageName: node - linkType: hard - -"@ethersproject/web@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/web@npm:5.7.0" - dependencies: - "@ethersproject/base64": ^5.7.0 - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - "@ethersproject/properties": ^5.7.0 - "@ethersproject/strings": ^5.7.0 - checksum: 9d4ca82f8b1295bbc1c59d58cb351641802d2f70f4b7d523fc726f51b0615296da6d6585dee5749b4d5e4a6a9af6d6650d46fe562d5b04f43a0af5c7f7f4a77e - languageName: node - linkType: hard - "@gar/promisify@npm:^1.1.3": version: 1.1.3 resolution: "@gar/promisify@npm:1.1.3" @@ -1343,7 +1115,6 @@ __metadata: dependencies: "@ethersproject/bignumber": ^5.7.0 "@ethersproject/bytes": ^5.7.0 - "@ethersproject/providers": ^5.7.0 "@lavamoat/allow-scripts": ^2.3.1 "@metamask/auto-changelog": ^3.1.0 "@metamask/base-controller": ^3.2.1 @@ -1352,6 +1123,7 @@ __metadata: "@metamask/eslint-config-jest": ^10.0.0 "@metamask/eslint-config-nodejs": ^10.0.0 "@metamask/eslint-config-typescript": ^10.0.0 + "@metamask/eth-query": ^3.0.1 "@metamask/network-controller": ^15.0.0 "@metamask/polling-controller": ^0.2.0 "@types/jest": ^26.0.24 @@ -2380,13 +2152,6 @@ __metadata: languageName: node linkType: hard -"bech32@npm:1.1.4": - version: 1.1.4 - resolution: "bech32@npm:1.1.4" - checksum: 0e98db619191548390d6f09ff68b0253ba7ae6a55db93dfdbb070ba234c1fd3308c0606fbcc95fad50437227b10011e2698b89f0181f6e7f845c499bd14d0f4b - languageName: node - linkType: hard - "big-integer@npm:^1.6.44": version: 1.6.51 resolution: "big-integer@npm:1.6.51" @@ -3272,7 +3037,7 @@ __metadata: languageName: node linkType: hard -"elliptic@npm:6.5.4, elliptic@npm:^6.5.2": +"elliptic@npm:^6.5.2": version: 6.5.4 resolution: "elliptic@npm:6.5.4" dependencies: @@ -4632,7 +4397,7 @@ __metadata: languageName: node linkType: hard -"hash.js@npm:1.1.7, hash.js@npm:^1.0.0, hash.js@npm:^1.0.3, hash.js@npm:^1.1.7": +"hash.js@npm:^1.0.0, hash.js@npm:^1.0.3, hash.js@npm:^1.1.7": version: 1.1.7 resolution: "hash.js@npm:1.1.7" dependencies: @@ -5814,13 +5579,6 @@ __metadata: languageName: node linkType: hard -"js-sha3@npm:0.8.0": - version: 0.8.0 - resolution: "js-sha3@npm:0.8.0" - checksum: 75df77c1fc266973f06cce8309ce010e9e9f07ec35ab12022ed29b7f0d9c8757f5a73e1b35aa24840dced0dea7059085aa143d817aea9e188e2a80d569d9adce - languageName: node - linkType: hard - "js-sha3@npm:^0.5.7": version: 0.5.7 resolution: "js-sha3@npm:0.5.7" @@ -8924,7 +8682,7 @@ __metadata: languageName: node linkType: hard -"ws@npm:7.4.6, ws@npm:^7.4.4": +"ws@npm:^7.4.4": version: 7.4.6 resolution: "ws@npm:7.4.6" peerDependencies: From 76faad0be45d51e4fdedfaad53d8f2f06c8cbc76 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Oct 2023 12:48:34 -0500 Subject: [PATCH 19/25] cleanup --- src/SmartTransactionsController.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 84a98e9..d31b5ba 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -658,7 +658,6 @@ export default class SmartTransactionsController extends PollingControllerV1< const preTxBalanceBN = await query(ethQuery, 'getBalance', [ txParams?.from, ]); - console.log('preTxBalanceBN', preTxBalanceBN); preTxBalance = new BigNumber(preTxBalanceBN.toHexString()).toString(16); } catch (e) { console.error('ethers error', e); From 94e2fac1cfdfedacd31b038a72815abb013dc19c Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Oct 2023 14:01:59 -0500 Subject: [PATCH 20/25] address more feedback --- src/SmartTransactionsController.test.ts | 20 ++++------ src/SmartTransactionsController.ts | 51 +++++++++++++------------ 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 89d901c..72acf9d 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -29,7 +29,7 @@ jest.mock('@metamask/eth-query', () => { sendAsync = jest.fn(({ method }, callback) => { switch (method) { case 'getBalance': { - callback(null, { toHexString: () => '0x1000' }); + callback(null, '0x1000'); break; } @@ -38,15 +38,15 @@ jest.mock('@metamask/eth-query', () => { break; } - case 'getBlock': { - callback(null, { baseFeePerGas: { toHexString: () => '0x123' } }); + case 'getBlockByNumber': { + callback(null, { baseFeePerGas: '0x123' }); break; } case 'getTransaction': { callback(null, { - maxFeePerGas: { toHexString: () => '0x123' }, - maxPriorityFeePerGas: { toHexString: () => '0x123' }, + maxFeePerGas: '0x123', + maxPriorityFeePerGas: '0x123', }); break; } @@ -731,7 +731,9 @@ describe('SmartTransactionsController', () => { [CHAIN_IDS.GOERLI]: true, }); - await smartTransactionsController.fetchLiveness('goerli'); + await smartTransactionsController.fetchLiveness({ + networkClientId: 'goerli', + }); expect( smartTransactionsController.state.smartTransactionsState @@ -834,12 +836,6 @@ describe('SmartTransactionsController', () => { }); it('throws an error if ethersProvider fails', async () => { - // smartTransactionsController.ethQuery.getTransactionReceipt.mockRejectedValueOnce( - // 'random error' as never, - // ); - // jest.spyOn('@metmask/eth-query', 'sendAsync')(() => { - // throw new Error('random error'); - // }); const successfulStx = { ...createStateAfterSuccess()[0], history: testHistory, diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index d31b5ba..4765a4a 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -7,10 +7,8 @@ import { } from '@metamask/network-controller'; import { PollingControllerV1 } from '@metamask/polling-controller'; import { BigNumber } from 'bignumber.js'; -import { BigNumber as ethersBigNumber } from '@ethersproject/bignumber'; import EthQuery from '@metamask/eth-query'; import { hexlify } from '@ethersproject/bytes'; -import mapValues from 'lodash/mapValues'; import cloneDeep from 'lodash/cloneDeep'; import { APIType, @@ -430,28 +428,27 @@ export default class SmartTransactionsController extends PollingControllerV1< ) { const txHash = smartTransaction.statusMetadata?.minedHash; try { - const transactionReceipt = await query( - ethQuery, - 'getTransactionReceipt', - [txHash], - ); - const transaction = await query(ethQuery, 'getTransaction', [txHash]); - - const maxFeePerGas = transaction.maxFeePerGas?.toHexString(); - const maxPriorityFeePerGas = - transaction.maxPriorityFeePerGas?.toHexString(); + const transactionReceipt: { + maxFeePerGas?: string; + maxPriorityFeePerGas?: string; + blockNumber: string; + } | null = await query(ethQuery, 'getTransactionReceipt', [txHash]); + + const transaction: { + maxFeePerGas?: string; + maxPriorityFeePerGas?: string; + } = await query(ethQuery, 'getTransaction', [txHash]); + + const maxFeePerGas = transaction?.maxFeePerGas; + const maxPriorityFeePerGas = transaction?.maxPriorityFeePerGas; if (transactionReceipt?.blockNumber) { - const blockData = await query(ethQuery, 'getBlock', [ - transactionReceipt?.blockNumber, - false, - ]); - const baseFeePerGas = blockData?.baseFeePerGas.toHexString(); - const txReceipt = mapValues(transactionReceipt, (value) => { - if (value instanceof ethersBigNumber) { - return value.toHexString(); - } - return value; - }); + const blockData: { baseFeePerGas?: string } | null = await query( + ethQuery, + 'getBlockByNumber', + [transactionReceipt?.blockNumber, false], + ); + const baseFeePerGas = blockData?.baseFeePerGas; + const txReceipt = Object.values(transactionReceipt); const updatedTxParams = { ...smartTransaction.txParams, maxFeePerGas, @@ -658,7 +655,7 @@ export default class SmartTransactionsController extends PollingControllerV1< const preTxBalanceBN = await query(ethQuery, 'getBalance', [ txParams?.from, ]); - preTxBalance = new BigNumber(preTxBalanceBN.toHexString()).toString(16); + preTxBalance = new BigNumber(preTxBalanceBN).toString(16); } catch (e) { console.error('ethers error', e); } @@ -726,7 +723,11 @@ export default class SmartTransactionsController extends PollingControllerV1< }); } - async fetchLiveness(networkClientId?: NetworkClientId): Promise { + async fetchLiveness({ + networkClientId, + }: { + networkClientId?: NetworkClientId; + } = {}): Promise { const chainId = this.getChainId({ networkClientId }); let liveness = false; try { From 5fddb8d227ef1cb31acd4cecc28a2d691ecdf5ef Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Oct 2023 14:04:58 -0500 Subject: [PATCH 21/25] remove @ethersproject/bignumber --- package.json | 1 - yarn.lock | 14 +------------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/package.json b/package.json index 1500d8c..6f7b981 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,6 @@ "test:watch": "jest --watchAll" }, "dependencies": { - "@ethersproject/bignumber": "^5.7.0", "@ethersproject/bytes": "^5.7.0", "@metamask/base-controller": "^3.2.1", "@metamask/controller-utils": "^5.0.0", diff --git a/yarn.lock b/yarn.lock index aa734ae..b62c65c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -528,17 +528,6 @@ __metadata: languageName: node linkType: hard -"@ethersproject/bignumber@npm:^5.7.0": - version: 5.7.0 - resolution: "@ethersproject/bignumber@npm:5.7.0" - dependencies: - "@ethersproject/bytes": ^5.7.0 - "@ethersproject/logger": ^5.7.0 - bn.js: ^5.2.1 - checksum: 8c9a134b76f3feb4ec26a5a27379efb4e156b8fb2de0678a67788a91c7f4e30abe9d948638458e4b20f2e42380da0adacc7c9389d05fce070692edc6ae9b4904 - languageName: node - linkType: hard - "@ethersproject/bytes@npm:^5.7.0": version: 5.7.0 resolution: "@ethersproject/bytes@npm:5.7.0" @@ -1113,7 +1102,6 @@ __metadata: version: 0.0.0-use.local resolution: "@metamask/smart-transactions-controller@workspace:." dependencies: - "@ethersproject/bignumber": ^5.7.0 "@ethersproject/bytes": ^5.7.0 "@lavamoat/allow-scripts": ^2.3.1 "@metamask/auto-changelog": ^3.1.0 @@ -2199,7 +2187,7 @@ __metadata: languageName: node linkType: hard -"bn.js@npm:^5.1.2, bn.js@npm:^5.2.1": +"bn.js@npm:^5.1.2": version: 5.2.1 resolution: "bn.js@npm:5.2.1" checksum: 3dd8c8d38055fedfa95c1d5fc3c99f8dd547b36287b37768db0abab3c239711f88ff58d18d155dd8ad902b0b0cee973747b7ae20ea12a09473272b0201c9edd3 From 99fc2cc69f1b6a520d23b3eb77f092c883305a78 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Oct 2023 17:47:43 -0500 Subject: [PATCH 22/25] make confirmSmartTransaction a private method --- src/SmartTransactionsController.test.ts | 58 +++++++------------------ src/SmartTransactionsController.ts | 9 ++-- 2 files changed, 20 insertions(+), 47 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 72acf9d..e6825dd 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -1,7 +1,6 @@ import nock from 'nock'; import { NetworkState } from '@metamask/network-controller'; import { convertHexToDecimal } from '@metamask/controller-utils'; -import EthQuery from '@metamask/eth-query'; import SmartTransactionsController, { DEFAULT_INTERVAL, } from './SmartTransactionsController'; @@ -17,7 +16,7 @@ import * as utils from './utils'; function flushPromises(): Promise { return new Promise(jest.requireActual('timers').setImmediate); } -const confirmExternalMock = jest.fn(); +// const confirmExternalMock = jest.fn(); jest.mock('@ethersproject/bytes', () => ({ ...jest.requireActual('@ethersproject/bytes'), @@ -325,7 +324,7 @@ describe('SmartTransactionsController', () => { }; }), provider: jest.fn(), - confirmExternalTransaction: confirmExternalMock, + confirmExternalTransaction: jest.fn(), trackMetaMetricsEvent: trackMetaMetricsEventSpy, getNetworkClientById: jest.fn().mockImplementation((networkClientId) => { switch (networkClientId) { @@ -559,7 +558,9 @@ describe('SmartTransactionsController', () => { smartTransactionsController.state.smartTransactionsState.feesByChainId, ).toStrictEqual(defaultState.smartTransactionsState.feesByChainId); - await smartTransactionsController.getFees(tradeTx, approvalTx, 'goerli'); + await smartTransactionsController.getFees(tradeTx, approvalTx, { + networkClientId: 'goerli', + }); expect( smartTransactionsController.state.smartTransactionsState.feesByChainId, @@ -784,10 +785,6 @@ describe('SmartTransactionsController', () => { it('confirms a smart transaction that has status success', async () => { const { smartTransactionsState } = smartTransactionsController.state; - const confirmSpy = jest.spyOn( - smartTransactionsController, - 'confirmSmartTransaction', - ); const pendingStx = { ...createStateAfterPending()[0], history: testHistory, @@ -802,52 +799,27 @@ describe('SmartTransactionsController', () => { }); const updateTransaction = { ...pendingStx, - status: 'success', + status: SmartTransactionStatuses.SUCCESS, }; + smartTransactionsController.updateSmartTransaction( updateTransaction as SmartTransaction, { networkClientId: 'mainnet', }, ); - expect(confirmSpy).toHaveBeenCalled(); - }); - }); - describe('confirmSmartTransaction', () => { - beforeEach(() => { - // eslint-disable-next-line jest/prefer-spy-on - smartTransactionsController.checkPoll = jest.fn(() => ({})); - }); + await flushPromises(); - it('calls confirm external transaction', async () => { - const successfulStx = { - ...createStateAfterSuccess()[0], - history: testHistory, - }; - await smartTransactionsController.confirmSmartTransaction( - successfulStx as SmartTransaction, - { - chainId: CHAIN_IDS.ETHEREUM, - ethQuery: new EthQuery({ sendAsync: jest.fn() }), - }, - ); - expect(confirmExternalMock).toHaveBeenCalled(); - }); - - it('throws an error if ethersProvider fails', async () => { - const successfulStx = { - ...createStateAfterSuccess()[0], - history: testHistory, - }; - await smartTransactionsController.confirmSmartTransaction( - successfulStx as SmartTransaction, + expect( + smartTransactionsController.state.smartTransactionsState + .smartTransactions[CHAIN_IDS.ETHEREUM], + ).toStrictEqual([ { - chainId: CHAIN_IDS.ETHEREUM, - ethQuery: new EthQuery({ sendAsync: jest.fn() }), + ...updateTransaction, + confirmed: true, }, - ); - expect(trackMetaMetricsEventSpy).toHaveBeenCalled(); + ]); }); }); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 4765a4a..9780049 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -325,6 +325,7 @@ export default class SmartTransactionsController extends PollingControllerV1< const isNewSmartTransaction = this.isNewSmartTransaction( smartTransaction.uuid, ); + this.trackStxStatusChange( smartTransaction, isNewSmartTransaction @@ -372,7 +373,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ...currentSmartTransaction, ...smartTransaction, }; - this.confirmSmartTransaction(nextSmartTransaction, { + this.#confirmSmartTransaction(nextSmartTransaction, { chainId, ethQuery, }); @@ -416,7 +417,7 @@ export default class SmartTransactionsController extends PollingControllerV1< } // TODO make this a private method? - async confirmSmartTransaction( + async #confirmSmartTransaction( smartTransaction: SmartTransaction, { chainId, @@ -566,7 +567,7 @@ export default class SmartTransactionsController extends PollingControllerV1< async getFees( tradeTx: UnsignedTransaction, approvalTx: UnsignedTransaction, - networkClientId?: NetworkClientId, + { networkClientId }: { networkClientId?: NetworkClientId } = {}, ): Promise { const chainId = this.getChainId({ networkClientId }); const transactions = []; @@ -657,7 +658,7 @@ export default class SmartTransactionsController extends PollingControllerV1< ]); preTxBalance = new BigNumber(preTxBalanceBN).toString(16); } catch (e) { - console.error('ethers error', e); + console.error('provider error', e); } const nonceLock = await this.getNonceLock(txParams?.from); try { From 2ee91e5c072a20d37990b8981ede7c6a984c8fd3 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Oct 2023 17:49:10 -0500 Subject: [PATCH 23/25] remove comment --- src/SmartTransactionsController.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 9780049..2e3a94e 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -416,7 +416,6 @@ export default class SmartTransactionsController extends PollingControllerV1< } } - // TODO make this a private method? async #confirmSmartTransaction( smartTransaction: SmartTransaction, { From 9c4a772cc5dbff42e11bdad85d1076b94f751c9d Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Oct 2023 09:43:09 -0500 Subject: [PATCH 24/25] getTransaction -> getTransactionByHash --- src/SmartTransactionsController.test.ts | 2 +- src/SmartTransactionsController.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index e6825dd..5c80d64 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -42,7 +42,7 @@ jest.mock('@metamask/eth-query', () => { break; } - case 'getTransaction': { + case 'getTransactionByHash': { callback(null, { maxFeePerGas: '0x123', maxPriorityFeePerGas: '0x123', diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 2e3a94e..08aacd7 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -437,7 +437,7 @@ export default class SmartTransactionsController extends PollingControllerV1< const transaction: { maxFeePerGas?: string; maxPriorityFeePerGas?: string; - } = await query(ethQuery, 'getTransaction', [txHash]); + } | null = await query(ethQuery, 'getTransactionByHash', [txHash]); const maxFeePerGas = transaction?.maxFeePerGas; const maxPriorityFeePerGas = transaction?.maxPriorityFeePerGas; From c74619c6f4c284dd07b14b00a690ea8e65aca922 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Oct 2023 15:03:09 -0500 Subject: [PATCH 25/25] fix ethquery mock --- src/SmartTransactionsController.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index 5c80d64..aa22f87 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -24,25 +24,26 @@ jest.mock('@ethersproject/bytes', () => ({ })); jest.mock('@metamask/eth-query', () => { - return class FakeEthQuery { + const EthQuery = jest.requireActual('@metamask/eth-query'); + return class FakeEthQuery extends EthQuery { sendAsync = jest.fn(({ method }, callback) => { switch (method) { - case 'getBalance': { + case 'eth_getBalance': { callback(null, '0x1000'); break; } - case 'getTransactionReceipt': { + case 'eth_getTransactionReceipt': { callback(null, { blockNumber: '123' }); break; } - case 'getBlockByNumber': { + case 'eth_getBlockByNumber': { callback(null, { baseFeePerGas: '0x123' }); break; } - case 'getTransactionByHash': { + case 'eth_getTransactionByHash': { callback(null, { maxFeePerGas: '0x123', maxPriorityFeePerGas: '0x123',