From 9a37a30d6fefd3c6d6c968ca2fe27b84dad47f9a Mon Sep 17 00:00:00 2001 From: luu-alex Date: Thu, 21 Sep 2023 20:57:42 -0400 Subject: [PATCH] address feedback --- packages/web3-core/CHANGELOG.md | 6 +-- packages/web3-core/src/types.ts | 5 +-- packages/web3-core/src/web3_config.ts | 1 - packages/web3-core/src/web3_context.ts | 1 - .../src/utils/detect_transaction_type.ts | 19 ++++------ .../src/utils/get_transaction_gas_pricing.ts | 2 +- .../web3-eth/src/utils/transaction_builder.ts | 6 +-- .../test/fixtures/detect_transaction_type.ts | 14 ------- .../test/integration/defaults.test.ts | 18 ++++----- .../test/unit/detect_transaction_type.test.ts | 38 +++---------------- .../unit/utils/get_transaction_type.test.ts | 18 +-------- 11 files changed, 28 insertions(+), 100 deletions(-) diff --git a/packages/web3-core/CHANGELOG.md b/packages/web3-core/CHANGELOG.md index 06d8854da3e..b1837d57b90 100644 --- a/packages/web3-core/CHANGELOG.md +++ b/packages/web3-core/CHANGELOG.md @@ -179,8 +179,4 @@ Documentation: - Added to `Web3Config` property `contractDataInputFill` allowing users to have the choice using property `data`, `input` or `both` for contract methods to be sent to the RPC provider when creating contracts. (#6377) (#6400) -## [Unreleased] - -### Changed - -- type `TransactionTypeParser` now accepts `web3Context` and returns a `Promise` (#6282) \ No newline at end of file +## [Unreleased] \ No newline at end of file diff --git a/packages/web3-core/src/types.ts b/packages/web3-core/src/types.ts index 04de0941511..3f4e66d1f52 100644 --- a/packages/web3-core/src/types.ts +++ b/packages/web3-core/src/types.ts @@ -16,13 +16,10 @@ along with web3.js. If not, see . */ import { HexString, Transaction } from 'web3-types'; -// eslint-disable-next-line import/no-cycle -import { Web3Context } from './web3_context.js'; export type TransactionTypeParser = ( transaction: Transaction, - web3Context: Web3Context, -) => Promise; +) => HexString | undefined; export interface Method { name: string; diff --git a/packages/web3-core/src/web3_config.ts b/packages/web3-core/src/web3_config.ts index 9e52991f9c5..a8dffebab03 100644 --- a/packages/web3-core/src/web3_config.ts +++ b/packages/web3-core/src/web3_config.ts @@ -18,7 +18,6 @@ along with web3.js. If not, see . import { Numbers, HexString, BlockNumberOrTag, Common } from 'web3-types'; import { ConfigHardforkMismatchError, ConfigChainMismatchError } from 'web3-errors'; import { isNullish, toHex } from 'web3-utils'; -// eslint-disable-next-line import/no-cycle import { TransactionTypeParser } from './types.js'; // eslint-disable-next-line import/no-cycle import { TransactionBuilder } from './web3_context.js'; diff --git a/packages/web3-core/src/web3_context.ts b/packages/web3-core/src/web3_context.ts index 4899608ac2d..f653b581456 100644 --- a/packages/web3-core/src/web3_context.ts +++ b/packages/web3-core/src/web3_context.ts @@ -36,7 +36,6 @@ import { Web3RequestManager } from './web3_request_manager.js'; import { Web3SubscriptionConstructor } from './web3_subscriptions.js'; import { Web3SubscriptionManager } from './web3_subscription_manager.js'; import { Web3BatchRequest } from './web3_batch_request.js'; -// eslint-disable-next-line import/no-cycle import { ExtensionObject } from './types.js'; // To avoid circular dependencies, we need to export type from here. diff --git a/packages/web3-eth/src/utils/detect_transaction_type.ts b/packages/web3-eth/src/utils/detect_transaction_type.ts index 57e9f515d5e..ec5539b375a 100644 --- a/packages/web3-eth/src/utils/detect_transaction_type.ts +++ b/packages/web3-eth/src/utils/detect_transaction_type.ts @@ -21,8 +21,6 @@ import { EthExecutionAPI, HardforksOrdered, Transaction, ETH_DATA_FORMAT } from import { Web3ValidatorError, isNullish, validator } from 'web3-validator'; import { InvalidPropertiesForTransactionTypeError } from 'web3-errors'; -// eslint-disable-next-line import/no-cycle -import { getBlock } from '../rpc_method_wrappers.js'; import { InternalTransaction } from '../types.js'; // undefined is treated as null for JSON schema validator @@ -77,9 +75,8 @@ const validateTxTypeAndHandleErrors = ( } }; -export const defaultTransactionTypeParser: TransactionTypeParser = async ( - transaction, - web3Context, +export const defaultTransactionTypeParser: TransactionTypeParser = ( + transaction ) => { const tx = transaction as unknown as Transaction; if (!isNullish(tx.type)) { @@ -127,9 +124,8 @@ export const defaultTransactionTypeParser: TransactionTypeParser = async ( if (hardforkIndex === Object.keys(HardforksOrdered).indexOf('berlin')) return '0x0'; } - const block = await getBlock(web3Context, web3Context.defaultBlock, false, ETH_DATA_FORMAT); - // gasprice is defined or eip 1559 is not supported - if (!isNullish(tx.gasPrice) || isNullish(block.baseFeePerGas)) { + // gasprice is defined + if (!isNullish(tx.gasPrice)) { validateTxTypeAndHandleErrors(transactionType0x0Schema, tx, '0x0'); return '0x0'; } @@ -138,13 +134,12 @@ export const defaultTransactionTypeParser: TransactionTypeParser = async ( return undefined; }; -export const detectTransactionType = async ( +export const detectTransactionType = ( transaction: InternalTransaction, - web3Context: Web3Context, + web3Context?: Web3Context, ) => (web3Context?.transactionTypeParser ?? defaultTransactionTypeParser)( - transaction as unknown as Record, - web3Context, + transaction as unknown as Record ); export const detectRawTransactionType = (transaction: Uint8Array) => diff --git a/packages/web3-eth/src/utils/get_transaction_gas_pricing.ts b/packages/web3-eth/src/utils/get_transaction_gas_pricing.ts index b4d005e0ca7..035d217065d 100644 --- a/packages/web3-eth/src/utils/get_transaction_gas_pricing.ts +++ b/packages/web3-eth/src/utils/get_transaction_gas_pricing.ts @@ -83,7 +83,7 @@ export async function getTransactionGasPricing( > | undefined > { - const transactionType = await getTransactionType(transaction, web3Context); + const transactionType = getTransactionType(transaction, web3Context); if (!isNullish(transactionType)) { if (transactionType.startsWith('-')) throw new UnsupportedTransactionTypeError(transactionType); diff --git a/packages/web3-eth/src/utils/transaction_builder.ts b/packages/web3-eth/src/utils/transaction_builder.ts index 952405a63f7..17692d5c0a1 100644 --- a/packages/web3-eth/src/utils/transaction_builder.ts +++ b/packages/web3-eth/src/utils/transaction_builder.ts @@ -110,11 +110,11 @@ export const getTransactionNonce = async ( return getTransactionCount(web3Context, address, web3Context.defaultBlock, returnFormat); }; -export const getTransactionType = async ( +export const getTransactionType = ( transaction: FormatType, web3Context: Web3Context, ) => { - const inferredType = await detectTransactionType(transaction, web3Context); + const inferredType = detectTransactionType(transaction, web3Context); if (!isNullish(inferredType)) return inferredType; if (!isNullish(web3Context.defaultTransactionType)) return format({ format: 'uint' }, web3Context.defaultTransactionType, ETH_DATA_FORMAT); @@ -215,7 +215,7 @@ export async function defaultTransactionBuilder(option populatedTransaction.gasLimit = populatedTransaction.gas; } - populatedTransaction.type = await getTransactionType(populatedTransaction, options.web3Context); + populatedTransaction.type = getTransactionType(populatedTransaction, options.web3Context); if ( isNullish(populatedTransaction.accessList) && (populatedTransaction.type === '0x1' || populatedTransaction.type === '0x2') diff --git a/packages/web3-eth/test/fixtures/detect_transaction_type.ts b/packages/web3-eth/test/fixtures/detect_transaction_type.ts index 8091b3ed01b..bd957462cfb 100644 --- a/packages/web3-eth/test/fixtures/detect_transaction_type.ts +++ b/packages/web3-eth/test/fixtures/detect_transaction_type.ts @@ -209,20 +209,6 @@ export const transactionType0x2: FormatType }, ]; -export const transactionType0PostEIP1559: FormatType[] = [ - { - from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', - to: '0x3535353535353535353535353535353535353535', - value: '0x174876e800', - gas: '0x5208', - gasPrice: '0x4a817c800', - data: '0x', - nonce: '0x4', - chainId: '0x1', - gasLimit: '0x5208', - }, -] - export const transactionTypeUndefined: FormatType[] = [ { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', diff --git a/packages/web3-eth/test/integration/defaults.test.ts b/packages/web3-eth/test/integration/defaults.test.ts index bca979fdfbb..f66cf9ad88d 100644 --- a/packages/web3-eth/test/integration/defaults.test.ts +++ b/packages/web3-eth/test/integration/defaults.test.ts @@ -780,7 +780,7 @@ describe('defaults', () => { expect(eth2.defaultTransactionType).toBe('0x4444'); - const res = await getTransactionType( + const res = getTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', @@ -800,7 +800,7 @@ describe('defaults', () => { // tx.maxPriorityFeePerGas !== undefined || // tx.hardfork === 'london' || // tx.common?.hardfork === 'london' - const maxFeePerGasOverride = await getTransactionType( + const maxFeePerGasOverride = getTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', @@ -815,7 +815,7 @@ describe('defaults', () => { eth2, ); expect(maxFeePerGasOverride).toBe('0x2'); - const maxPriorityFeePerGasOverride = await getTransactionType( + const maxPriorityFeePerGasOverride = getTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', @@ -830,7 +830,7 @@ describe('defaults', () => { eth2, ); expect(maxPriorityFeePerGasOverride).toBe('0x2'); - const hardforkOverride = await getTransactionType( + const hardforkOverride = getTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', @@ -845,7 +845,7 @@ describe('defaults', () => { eth2, ); expect(hardforkOverride).toBe('0x2'); - const commonOverride = await getTransactionType( + const commonOverride = getTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', @@ -867,7 +867,7 @@ describe('defaults', () => { // override to 0x1 if: // tx.accessList !== undefined || tx.hardfork === 'berlin' || tx.common?.hardfork === 'berlin' - const accessListOverride = await getTransactionType( + const accessListOverride = getTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', @@ -888,7 +888,7 @@ describe('defaults', () => { ); expect(accessListOverride).toBe('0x1'); - const hardforkBerlinOverride = await getTransactionType( + const hardforkBerlinOverride = getTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', @@ -904,7 +904,7 @@ describe('defaults', () => { ); expect(hardforkBerlinOverride).toBe('0x0'); - const commonBerlinOverride = await getTransactionType( + const commonBerlinOverride = getTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', @@ -1033,7 +1033,7 @@ describe('defaults', () => { }, }); expect(eth2.transactionTypeParser).toBe(newParserMock); - await detectTransactionType( + detectTransactionType( { from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0', to: '0x3535353535353535353535353535353535353535', diff --git a/packages/web3-eth/test/unit/detect_transaction_type.test.ts b/packages/web3-eth/test/unit/detect_transaction_type.test.ts index 510f4ac60f1..4d583bbad17 100644 --- a/packages/web3-eth/test/unit/detect_transaction_type.test.ts +++ b/packages/web3-eth/test/unit/detect_transaction_type.test.ts @@ -24,15 +24,9 @@ import { transactionType0x0, transactionType0x1, transactionType0x2, - transactionType0PostEIP1559, transactionTypeUndefined, transactionTypeValidationError, } from '../fixtures/detect_transaction_type'; -import { - preEip1559Block, - postEip1559Block, -} from '../fixtures/prepare_transaction_for_signing' -import * as rpcMethodWrappers from '../../src/rpc_method_wrappers'; jest.mock('../../src/rpc_method_wrappers'); @@ -41,34 +35,13 @@ describe('detectTransactionType', () => { afterAll(() => { jest.resetAllMocks(); }); - describe('should override detectTransactionType method', () => { - const web3Context = new Web3Context({ - provider: new HttpProvider('http://127.0.0.1:80'), - }); - it.skip('should call override method', async () => { - const overrideFunction = jest.fn(); - await detectTransactionType(transactionTypeUndefined[0], web3Context); - expect(overrideFunction).toHaveBeenCalledWith(transactionTypeUndefined[0]); - }); - }); describe('should detect transaction type 0x0', () => { - jest.spyOn(rpcMethodWrappers, 'getBlock').mockResolvedValue(preEip1559Block) const web3Context = new Web3Context({ provider: new HttpProvider('http://127.0.0.1:80'), }); it.each(transactionType0x0)('%s', async transaction => { - expect(await detectTransactionType(transaction, web3Context)).toBe('0x0'); - }); - }); - - describe('should detect type 0x0 transaction post eip1559 block', () => { - jest.spyOn(rpcMethodWrappers, 'getBlock').mockResolvedValue(postEip1559Block) - const web3Context = new Web3Context({ - provider: new HttpProvider('http://127.0.0.1:80'), - }); - it.each(transactionType0PostEIP1559)('%s', async transaction => { - expect(await detectTransactionType(transaction, web3Context)).toBe('0x0'); + expect(detectTransactionType(transaction, web3Context)).toBe('0x0'); }); }); @@ -77,7 +50,7 @@ describe('detectTransactionType', () => { provider: new HttpProvider('http://127.0.0.1:80'), }); it.each(transactionType0x1)('%s', async transaction => { - expect(await detectTransactionType(transaction, web3Context)).toBe('0x1'); + expect(detectTransactionType(transaction, web3Context)).toBe('0x1'); }); }); @@ -86,17 +59,16 @@ describe('detectTransactionType', () => { provider: new HttpProvider('http://127.0.0.1:80'), }); it.each(transactionType0x2)('%s', async transaction => { - expect(await detectTransactionType(transaction, web3Context)).toBe('0x2'); + expect(detectTransactionType(transaction, web3Context)).toBe('0x2'); }); }); describe('should not be able to detect transaction type, returning undefined', () => { - jest.spyOn(rpcMethodWrappers, 'getBlock').mockResolvedValue(postEip1559Block) const web3Context = new Web3Context({ provider: new HttpProvider('http://127.0.0.1:80'), }); it.each(transactionTypeUndefined)('%s', async transaction => { - expect(await detectTransactionType(transaction, web3Context)).toBeUndefined(); + expect(detectTransactionType(transaction, web3Context)).toBeUndefined(); }); }); @@ -107,7 +79,7 @@ describe('detectTransactionType', () => { it.each(transactionTypeValidationError)('%s', async transaction => { let err; try { - await detectTransactionType(transaction, web3Context); + detectTransactionType(transaction, web3Context); } catch (error) { err = error; } diff --git a/packages/web3-eth/test/unit/utils/get_transaction_type.test.ts b/packages/web3-eth/test/unit/utils/get_transaction_type.test.ts index 58f4d01f201..73111b42067 100644 --- a/packages/web3-eth/test/unit/utils/get_transaction_type.test.ts +++ b/packages/web3-eth/test/unit/utils/get_transaction_type.test.ts @@ -22,10 +22,7 @@ import { format } from 'web3-utils'; import { transactionSchema } from '../../../src/schemas'; import { getTransactionType } from '../../../src/utils/transaction_builder'; -import * as rpcMethodWrappers from '../../../src/rpc_method_wrappers'; -import { preEip1559Block, postEip1559Block } from '../../fixtures/prepare_transaction_for_signing'; -jest.mock('../../../src/rpc_method_wrappers'); describe('getTransactionType', () => { const expectedFrom = '0xb8CE9ab6943e0eCED004cDe8e3bBed6568B2Fa01'; @@ -50,22 +47,9 @@ describe('getTransactionType', () => { }, }); - afterAll(() => { - jest.resetAllMocks(); - }); - - it('getTransactionType should return transaction type 0 when provider does not support type 2 transactions (baseFeePerGas is undefined)', async () => { - - jest.spyOn(rpcMethodWrappers, 'getBlock').mockResolvedValue(preEip1559Block) - - const transactionType = await getTransactionType(formattedTransaction, web3Context); - expect(transactionType).toBe('0x0'); - }); - it('should default to 0x2 when transaction type cannot be inferred and use default transaction type', async () => { - jest.spyOn(rpcMethodWrappers, 'getBlock').mockResolvedValue(postEip1559Block) - const transactionType = await getTransactionType(formattedTransaction, web3Context); + const transactionType = getTransactionType(formattedTransaction, web3Context); expect(transactionType).toBe('0x2'); }); });