Skip to content

Commit

Permalink
address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
luu-alex committed Sep 22, 2023
1 parent 9d9ca4e commit 9a37a30
Show file tree
Hide file tree
Showing 11 changed files with 28 additions and 100 deletions.
6 changes: 1 addition & 5 deletions packages/web3-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
## [Unreleased]
5 changes: 1 addition & 4 deletions packages/web3-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@ along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/

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>;
) => HexString | undefined;

export interface Method {
name: string;
Expand Down
1 change: 0 additions & 1 deletion packages/web3-core/src/web3_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ along with web3.js. If not, see <http://www.gnu.org/licenses/>.
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';
Expand Down
1 change: 0 additions & 1 deletion packages/web3-core/src/web3_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
19 changes: 7 additions & 12 deletions packages/web3-eth/src/utils/detect_transaction_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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';
}
Expand All @@ -138,13 +134,12 @@ export const defaultTransactionTypeParser: TransactionTypeParser = async (
return undefined;
};

export const detectTransactionType = async (
export const detectTransactionType = (
transaction: InternalTransaction,
web3Context: Web3Context<EthExecutionAPI>,
web3Context?: Web3Context<EthExecutionAPI>,
) =>
(web3Context?.transactionTypeParser ?? defaultTransactionTypeParser)(
transaction as unknown as Record<string, unknown>,
web3Context,
transaction as unknown as Record<string, unknown>
);

export const detectRawTransactionType = (transaction: Uint8Array) =>
Expand Down
2 changes: 1 addition & 1 deletion packages/web3-eth/src/utils/get_transaction_gas_pricing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export async function getTransactionGasPricing<ReturnFormat extends DataFormat>(
>
| undefined
> {
const transactionType = await getTransactionType(transaction, web3Context);
const transactionType = getTransactionType(transaction, web3Context);
if (!isNullish(transactionType)) {
if (transactionType.startsWith('-'))
throw new UnsupportedTransactionTypeError(transactionType);
Expand Down
6 changes: 3 additions & 3 deletions packages/web3-eth/src/utils/transaction_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ export const getTransactionNonce = async <ReturnFormat extends DataFormat>(
return getTransactionCount(web3Context, address, web3Context.defaultBlock, returnFormat);
};

export const getTransactionType = async (
export const getTransactionType = (
transaction: FormatType<Transaction, typeof ETH_DATA_FORMAT>,
web3Context: Web3Context<EthExecutionAPI>,
) => {
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);
Expand Down Expand Up @@ -215,7 +215,7 @@ export async function defaultTransactionBuilder<ReturnType = Transaction>(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')
Expand Down
14 changes: 0 additions & 14 deletions packages/web3-eth/test/fixtures/detect_transaction_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,20 +209,6 @@ export const transactionType0x2: FormatType<Transaction, typeof ETH_DATA_FORMAT>
},
];

export const transactionType0PostEIP1559: FormatType<Transaction, typeof ETH_DATA_FORMAT>[] = [
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
to: '0x3535353535353535353535353535353535353535',
value: '0x174876e800',
gas: '0x5208',
gasPrice: '0x4a817c800',
data: '0x',
nonce: '0x4',
chainId: '0x1',
gasLimit: '0x5208',
},
]

export const transactionTypeUndefined: FormatType<Transaction, typeof ETH_DATA_FORMAT>[] = [
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
Expand Down
18 changes: 9 additions & 9 deletions packages/web3-eth/test/integration/defaults.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ describe('defaults', () => {

expect(eth2.defaultTransactionType).toBe('0x4444');

const res = await getTransactionType(
const res = getTransactionType(
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
to: '0x3535353535353535353535353535353535353535',
Expand All @@ -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',
Expand All @@ -815,7 +815,7 @@ describe('defaults', () => {
eth2,
);
expect(maxFeePerGasOverride).toBe('0x2');
const maxPriorityFeePerGasOverride = await getTransactionType(
const maxPriorityFeePerGasOverride = getTransactionType(
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
to: '0x3535353535353535353535353535353535353535',
Expand All @@ -830,7 +830,7 @@ describe('defaults', () => {
eth2,
);
expect(maxPriorityFeePerGasOverride).toBe('0x2');
const hardforkOverride = await getTransactionType(
const hardforkOverride = getTransactionType(
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
to: '0x3535353535353535353535353535353535353535',
Expand All @@ -845,7 +845,7 @@ describe('defaults', () => {
eth2,
);
expect(hardforkOverride).toBe('0x2');
const commonOverride = await getTransactionType(
const commonOverride = getTransactionType(
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
to: '0x3535353535353535353535353535353535353535',
Expand All @@ -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',
Expand All @@ -888,7 +888,7 @@ describe('defaults', () => {
);
expect(accessListOverride).toBe('0x1');

const hardforkBerlinOverride = await getTransactionType(
const hardforkBerlinOverride = getTransactionType(
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
to: '0x3535353535353535353535353535353535353535',
Expand All @@ -904,7 +904,7 @@ describe('defaults', () => {
);
expect(hardforkBerlinOverride).toBe('0x0');

const commonBerlinOverride = await getTransactionType(
const commonBerlinOverride = getTransactionType(
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
to: '0x3535353535353535353535353535353535353535',
Expand Down Expand Up @@ -1033,7 +1033,7 @@ describe('defaults', () => {
},
});
expect(eth2.transactionTypeParser).toBe(newParserMock);
await detectTransactionType(
detectTransactionType(
{
from: '0xEB014f8c8B418Db6b45774c326A0E64C78914dC0',
to: '0x3535353535353535353535353535353535353535',
Expand Down
38 changes: 5 additions & 33 deletions packages/web3-eth/test/unit/detect_transaction_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -41,34 +35,13 @@ describe('detectTransactionType', () => {
afterAll(() => {
jest.resetAllMocks();
});
describe('should override detectTransactionType method', () => {
const web3Context = new Web3Context<EthExecutionAPI>({
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<EthExecutionAPI>({
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<EthExecutionAPI>({
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');
});
});

Expand All @@ -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');
});
});

Expand All @@ -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<EthExecutionAPI>({
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();
});
});

Expand All @@ -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;
}
Expand Down
18 changes: 1 addition & 17 deletions packages/web3-eth/test/unit/utils/get_transaction_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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');
});
});

0 comments on commit 9a37a30

Please sign in to comment.