From 353baea83f756a6a213bab061e59144ab9ca505e Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Tue, 15 Aug 2023 12:38:24 +0200 Subject: [PATCH 1/4] limit public API of Tx controller --- .../src/TransactionController.test.ts | 71 ++-------------- .../src/TransactionController.ts | 80 +++++++++---------- 2 files changed, 47 insertions(+), 104 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 1f4649a808..ca113ac377 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1,6 +1,4 @@ /* eslint-disable jest/expect-expect */ - -import { Common } from '@ethereumjs/common'; import { ChainId, NetworkType, @@ -23,7 +21,7 @@ import type { TransactionControllerMessenger, TransactionConfig, } from './TransactionController'; -import { TransactionController, HARDFORK } from './TransactionController'; +import { TransactionController } from './TransactionController'; import type { TransactionMeta } from './types'; import { WalletDevice, TransactionStatus } from './types'; import { ESTIMATE_GAS_ERROR } from './utils'; @@ -211,8 +209,7 @@ function waitForTransactionFinished( ): Promise { return new Promise((resolve) => { controller.hub.once( - `${controller.state.transactions[0].id}:${ - confirmed ? 'confirmed' : 'finished' + `${controller.state.transactions[0].id}:${confirmed ? 'confirmed' : 'finished' }`, (txMeta) => { resolve(txMeta); @@ -436,15 +433,6 @@ describe('TransactionController', () => { ); } - /** - * Wait for a specified number of milliseconds. - * - * @param ms - The number of milliseconds to wait. - */ - async function wait(ms: number) { - await new Promise((resolve) => setTimeout(resolve, ms)); - } - beforeEach(() => { for (const key in mockFlags) { mockFlags[key] = null; @@ -1386,7 +1374,9 @@ describe('TransactionController', () => { describe('stopTransaction', () => { it('rejects result promise', async () => { - const controller = newController(); + const controller = newController({ + network: MOCK_LINEA_GOERLI_NETWORK, + }); const { result, transactionMeta } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1416,7 +1406,9 @@ describe('TransactionController', () => { describe('speedUpTransaction', () => { it('creates additional transaction with increased gas', async () => { - const controller = newController(); + const controller = newController({ + network: MOCK_LINEA_MAINNET_NETWORK, + }); const { transactionMeta } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1482,53 +1474,6 @@ describe('TransactionController', () => { }); }); - describe('getCommonConfiguration', () => { - it('should get the common network configuration for mainnet', async () => { - const controller = new TransactionController({ - getNetworkState: () => MOCK_MAINNET_NETWORK.state, - onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - provider: MOCK_MAINNET_NETWORK.provider, - blockTracker: MOCK_MAINNET_NETWORK.blockTracker, - messenger: messengerMock, - }); - - const config = await controller.getCommonConfiguration(); - expect(config).toStrictEqual( - new Common({ chain: 'mainnet', hardfork: HARDFORK }), - ); - }); - - it.each([ - ['linea-mainnet', MOCK_LINEA_MAINNET_NETWORK, 59144], - ['linea-goerli', MOCK_LINEA_GOERLI_NETWORK, 59140], - ])( - 'should get a custom network configuration for %s', - async ( - _, - { state, subscribe, provider, blockTracker }: MockNetwork, - chainId: number, - ) => { - const controller = new TransactionController({ - getNetworkState: () => state, - onNetworkStateChange: subscribe, - provider, - blockTracker, - messenger: messengerMock, - }); - - const config = controller.getCommonConfiguration(); - expect(config).toStrictEqual( - Common.custom({ - name: undefined, - chainId, - networkId: chainId, - defaultHardfork: HARDFORK, - }), - ); - }, - ); - }); - describe('initApprovals', () => { it('creates approvals for all unapproved transaction', async () => { const transaction = { diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 2fe455791b..f5c605c351 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -416,40 +416,6 @@ export class TransactionController extends BaseController< } } - /** - * `@ethereumjs/tx` uses `@ethereumjs/common` as a configuration tool for - * specifying which chain, network, hardfork and EIPs to support for - * a transaction. By referencing this configuration, and analyzing the fields - * specified in txParams, @ethereumjs/tx is able to determine which EIP-2718 - * transaction type to use. - * - * @returns {Common} common configuration object - */ - - getCommonConfiguration(): Common { - const { - networkId, - providerConfig: { type: chain, chainId, nickname: name }, - } = this.getNetworkState(); - - if ( - chain !== RPC && - chain !== NetworkType['linea-goerli'] && - chain !== NetworkType['linea-mainnet'] - ) { - return new Common({ chain, hardfork: HARDFORK }); - } - - const customChainParams: Partial = { - name, - chainId: parseInt(chainId, 16), - networkId: networkId === null ? NaN : parseInt(networkId, undefined), - defaultHardfork: HARDFORK, - }; - - return Common.custom(customChainParams); - } - /** * Attempts to cancel a transaction based on its ID by setting its status to "rejected" * and emitting a `:finished` hub event. @@ -1270,14 +1236,46 @@ export class TransactionController extends BaseController< return { meta: transaction, isCompleted }; } - private getChainAndNetworkId(): { - networkId: string | null; - chainId: Hex; - } { - const { networkId, providerConfig } = this.getNetworkState(); - const chainId = providerConfig?.chainId; + private prepareUnsignedEthTx( + txParams: Record, + ): TypedTransaction { + return TransactionFactory.fromTxData(txParams, { + common: this.getCommonConfiguration(), + freeze: false, + }); + } + + /** + * `@ethereumjs/tx` uses `@ethereumjs/common` as a configuration tool for + * specifying which chain, network, hardfork and EIPs to support for + * a transaction. By referencing this configuration, and analyzing the fields + * specified in txParams, @ethereumjs/tx is able to determine which EIP-2718 + * transaction type to use. + * + * @returns common configuration object + */ + private getCommonConfiguration(): Common { + const { + networkId, + providerConfig: { type: chain, chainId, nickname: name }, + } = this.getNetworkState(); - return { networkId, chainId }; + if ( + chain !== RPC && + chain !== NetworkType['linea-goerli'] && + chain !== NetworkType['linea-mainnet'] + ) { + return new Common({ chain, hardfork: HARDFORK }); + } + + const customChainParams: Partial = { + name, + chainId: parseInt(chainId, 16), + networkId: networkId === null ? NaN : parseInt(networkId, undefined), + defaultHardfork: HARDFORK, + }; + + return Common.custom(customChainParams); } } From 00291293d60d2213626b4f9923ae2e18b78fb50f Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Wed, 16 Aug 2023 09:31:04 +0200 Subject: [PATCH 2/4] fix test --- .../src/TransactionController.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index ca113ac377..1890ed3fef 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -433,6 +433,15 @@ describe('TransactionController', () => { ); } + /** + * Wait for a specified number of milliseconds. + * + * @param ms - The number of milliseconds to wait. + */ + async function wait(ms: number) { + await new Promise((resolve) => setTimeout(resolve, ms)); + } + beforeEach(() => { for (const key in mockFlags) { mockFlags[key] = null; From 78eab7388aa420bbf39d89223e7bfb4958c711bc Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Wed, 16 Aug 2023 11:12:18 +0200 Subject: [PATCH 3/4] fix lint --- .../transaction-controller/src/TransactionController.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 1890ed3fef..291f1150ff 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -209,7 +209,8 @@ function waitForTransactionFinished( ): Promise { return new Promise((resolve) => { controller.hub.once( - `${controller.state.transactions[0].id}:${confirmed ? 'confirmed' : 'finished' + `${controller.state.transactions[0].id}:${ + confirmed ? 'confirmed' : 'finished' }`, (txMeta) => { resolve(txMeta); From 28a290c0b3990558f6a9697db59bf6446b6325ba Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Wed, 16 Aug 2023 11:19:27 +0200 Subject: [PATCH 4/4] fix lint --- .../src/TransactionController.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index f5c605c351..4c29522648 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -388,13 +388,6 @@ export class TransactionController extends BaseController< }; } - prepareUnsignedEthTx(txParams: Record): TypedTransaction { - return TransactionFactory.fromTxData(txParams, { - common: this.getCommonConfiguration(), - freeze: false, - }); - } - /** * Creates approvals for all unapproved transactions persisted. */ @@ -1236,6 +1229,15 @@ export class TransactionController extends BaseController< return { meta: transaction, isCompleted }; } + private getChainAndNetworkId(): { + networkId: string | null; + chainId: Hex; + } { + const { networkId, providerConfig } = this.getNetworkState(); + const chainId = providerConfig?.chainId; + return { networkId, chainId }; + } + private prepareUnsignedEthTx( txParams: Record, ): TypedTransaction {