defaultTransactionTypeParser Refactor (#6102)

Author: spacesailor24

CHANGELOG.md

@@ -1475,6 +1475,12 @@ should use 4.0.1-alpha.0 for testing.
 #### web3-types
 
 -   Added `filters` param to the `Filter` type (#6010)
+-   Export for `HardforksOrdered` enum (#6102)
+-   Export for `Web3ValidationErrorObject` type (#6102)
+
+#### web3-errors
+
+-   `InvalidPropertiesForTransactionTypeError` with error code `429` (#6102)
 
 ### Fixed
 
@@ -1488,3 +1494,10 @@ should use 4.0.1-alpha.0 for testing.
 
 -   `formatTransaction` no longer throws a `TransactionDataAndInputError` if it's passed a transaction object with both `data` and `input` properties set (as long as they are the same value) (#6064)
 -   Refactored documentation for `rpc_method_wrappers` to point to the previously duplicated documentation found under the `Web3Eth` class documentation (#6054)
+-   Refactored `defaultTransactionTypeParser` to return correct EIP-2718 types, prior implementation was prioritizing `transaction.hardfork` and ignoring the use of `transaction.gasLimit`. `defaultTransactionTypeParser` will now throw `InvalidPropertiesForTransactionTypeError`s for properties are used that are incompatible with `transaction.type` (#6102)
+
+### Removed
+
+#### web3-validator
+
+-   `Web3ValidationErrorObject` type is now exported from `web3-types` package (#6102)

packages/web3-errors/CHANGELOG.md

@@ -99,6 +99,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+-   `InvalidPropertiesForTransactionTypeError` with error code `429` (#6102)
+
 ### Changed
 
 -   Nested Smart Contract error data is extracted at `Eip838ExecutionError` constructor and the nested error is set at `innerError` (#6045)

packages/web3-errors/src/error_codes.ts

@@ -88,6 +88,7 @@ export const ERR_TX_CHAIN_MISMATCH = 435;
 export const ERR_TX_HARDFORK_MISMATCH = 436;
 export const ERR_TX_INVALID_RECEIVER = 437;
 export const ERR_TX_REVERT_TRANSACTION_CUSTOM_ERROR = 438;
+export const ERR_TX_INVALID_PROPERTIES_FOR_TYPE = 439;
 
 // Connection error codes
 export const ERR_CONN = 500;

packages/web3-errors/src/errors/transaction_errors.ts

@@ -17,7 +17,13 @@ along with web3.js.  If not, see <http://www.gnu.org/licenses/>.
 
 /* eslint-disable max-classes-per-file */
 
-import { Bytes, HexString, Numbers, TransactionReceipt } from 'web3-types';
+import {
+	Bytes,
+	HexString,
+	Numbers,
+	TransactionReceipt,
+	Web3ValidationErrorObject,
+} from 'web3-types';
 import {
 	ERR_RAW_TX_UNDEFINED,
 	ERR_TX,
@@ -58,6 +64,7 @@ import {
 	ERR_TX_UNSUPPORTED_EIP_1559,
 	ERR_TX_UNSUPPORTED_TYPE,
 	ERR_TX_REVERT_TRANSACTION_CUSTOM_ERROR,
+	ERR_TX_INVALID_PROPERTIES_FOR_TYPE,
 } from '../error_codes';
 import { InvalidValueError, BaseWeb3Error } from '../web3_error_base';
 
@@ -540,3 +547,26 @@ export class LocalWalletNotAvailableError extends InvalidValueError {
 		);
 	}
 }
+export class InvalidPropertiesForTransactionTypeError extends BaseWeb3Error {
+	public code = ERR_TX_INVALID_PROPERTIES_FOR_TYPE;
+
+	public constructor(
+		validationError: Web3ValidationErrorObject[],
+		txType: '0x0' | '0x1' | '0x2',
+	) {
+		const invalidPropertyNames: string[] = [];
+		validationError.forEach(error =>
+			invalidPropertyNames.push(
+				// These errors are erroneously reported, error
+				// has type Web3ValidationErrorObject, but eslint doesn't recognize it
+				// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call
+				(error.keyword.match(/data.(.+)/) as string[])[1],
+			),
+		);
+		super(
+			`The following properties are invalid for the transaction type ${txType}: ${invalidPropertyNames.join(
+				', ',
+			)}`,
+		);
+	}
+}

packages/web3-errors/test/unit/__snapshots__/errors.test.ts.snap

@@ -166,6 +166,15 @@ Object {
 }
 `;
 
+exports[`errors InvalidPropertiesForTransactionTypeError should have valid json structure 1`] = `
+Object {
+  "code": 439,
+  "innerError": undefined,
+  "message": "The following properties are invalid for the transaction type 0x0: property",
+  "name": "InvalidPropertiesForTransactionTypeError",
+}
+`;
+
 exports[`errors InvalidProviderError should have valid json structure 1`] = `
 Object {
   "code": 601,

packages/web3-errors/test/unit/errors.test.ts

@@ -51,6 +51,7 @@ describe('errors', () => {
 			...transactionErrors,
 			...utilsErrors,
 		})) {
+			if (ErrorClass === transactionErrors.InvalidPropertiesForTransactionTypeError) break;
 			// To disable error for the abstract class
 
 			// eslint-disable-next-line @typescript-eslint/ban-ts-comment
@@ -191,6 +192,24 @@ describe('errors', () => {
 		});
 	});
 
+	describe('InvalidPropertiesForTransactionTypeError', () => {
+		it('should have valid json structure', () => {
+			expect(
+				new transactionErrors.InvalidPropertiesForTransactionTypeError(
+					[
+						{
+							keyword: 'data.property',
+							instancePath: '',
+							schemaPath: '',
+							params: {},
+						},
+					],
+					'0x0',
+				).toJSON(),
+			).toMatchSnapshot();
+		});
+	});
+
 	describe('NoContractAddressFoundError', () => {
 		it('should have valid json structure', () => {
 			expect(

packages/web3-eth-contract/src/contract.ts

@@ -69,6 +69,7 @@ import {
 	DataFormat,
 	DEFAULT_RETURN_FORMAT,
 	Numbers,
+	Web3ValidationErrorObject,
 } from 'web3-types';
 import { format, isDataFormat, toChecksumAddress } from 'web3-utils';
 import {
@@ -77,7 +78,6 @@ import {
 	utils as validatorUtils,
 	ValidationSchemaInput,
 	Web3ValidatorError,
-	Web3ValidationErrorObject,
 } from 'web3-validator';
 import { ALL_EVENTS_ABI } from './constants';
 import { decodeEventABI, decodeMethodReturn, encodeEventABI, encodeMethodABI } from './encoding';

packages/web3-eth/CHANGELOG.md

@@ -131,3 +131,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 -   `formatTransaction` no longer throws a `TransactionDataAndInputError` if it's passed a transaction object with both `data` and `input` properties set (as long as they are the same value) (#6064)
 -   Refactored documentation for `rpc_method_wrappers` to point to the previously duplicated documentation found under the `Web3Eth` class documentation (#6054)
 -   Replaced Buffer for Uint8Array (#6004)
+-   Refactored `defaultTransactionTypeParser` to return correct EIP-2718 types, prior implementation was prioritizing `transaction.hardfork` and ignoring the use of `transaction.gasLimit`. `defaultTransactionTypeParser` will now throw `InvalidPropertiesForTransactionTypeError`s for properties are used that are incompatible with `transaction.type` (#6102)

packages/web3-eth/src/utils/detect_transaction_type.ts

@@ -17,26 +17,127 @@ along with web3.js.  If not, see <http://www.gnu.org/licenses/>.
 
 import { format, toHex } from 'web3-utils';
 import { TransactionTypeParser, Web3Context } from 'web3-core';
-import { EthExecutionAPI, Transaction, ETH_DATA_FORMAT } from 'web3-types';
-import { isNullish } from 'web3-validator';
+import { EthExecutionAPI, HardforksOrdered, Transaction, ETH_DATA_FORMAT } from 'web3-types';
+import { Web3ValidatorError, isNullish, validator } from 'web3-validator';
+import { InvalidPropertiesForTransactionTypeError } from 'web3-errors';
+
 import { InternalTransaction } from '../types';
 
+// undefined is treated as null for JSON schema validator
+const transactionType0x0Schema = {
+	type: 'object',
+	properties: {
+		accessList: {
+			type: 'null',
+		},
+		maxFeePerGas: {
+			type: 'null',
+		},
+		maxPriorityFeePerGas: {
+			type: 'null',
+		},
+	},
+};
+const transactionType0x1Schema = {
+	type: 'object',
+	properties: {
+		maxFeePerGas: {
+			type: 'null',
+		},
+		maxPriorityFeePerGas: {
+			type: 'null',
+		},
+	},
+};
+const transactionType0x2Schema = {
+	type: 'object',
+	properties: {
+		gasPrice: {
+			type: 'null',
+		},
+	},
+};
+
+const validateTxTypeAndHandleErrors = (
+	txSchema: object,
+	tx: Transaction,
+	txType: '0x0' | '0x1' | '0x2',
+) => {
+	try {
+		validator.validateJSONSchema(txSchema, tx);
+	} catch (error) {
+		if (error instanceof Web3ValidatorError)
+			// Erroneously reported error
+			// eslint-disable-next-line @typescript-eslint/no-unsafe-call
+			throw new InvalidPropertiesForTransactionTypeError(error.errors, txType);
+
+		throw error;
+	}
+};
+
 export const defaultTransactionTypeParser: TransactionTypeParser = transaction => {
 	const tx = transaction as unknown as Transaction;
 
-	if (!isNullish(tx.type)) return format({ format: 'uint' }, tx.type, ETH_DATA_FORMAT);
+	if (!isNullish(tx.type)) {
+		let txSchema;
+		switch (tx.type) {
+			case '0x0':
+				txSchema = transactionType0x0Schema;
+				break;
+			case '0x1':
+				txSchema = transactionType0x1Schema;
+				break;
+			case '0x2':
+				txSchema = transactionType0x2Schema;
+				break;
+
+			default:
+				return format({ format: 'uint' }, tx.type, ETH_DATA_FORMAT);
+		}
 
-	if (
-		!isNullish(tx.maxFeePerGas) ||
-		!isNullish(tx.maxPriorityFeePerGas) ||
-		tx.hardfork === 'london' ||
-		tx.common?.hardfork === 'london'
-	)
+		validateTxTypeAndHandleErrors(txSchema, tx, tx.type);
+
+		return format({ format: 'uint' }, tx.type, ETH_DATA_FORMAT);
+	}
+
+	if (!isNullish(tx.maxFeePerGas) || !isNullish(tx.maxPriorityFeePerGas)) {
+		validateTxTypeAndHandleErrors(transactionType0x2Schema, tx, '0x2');
 		return '0x2';
+	}
 
-	if (!isNullish(tx.accessList) || tx.hardfork === 'berlin' || tx.common?.hardfork === 'berlin')
+	if (!isNullish(tx.accessList)) {
+		validateTxTypeAndHandleErrors(transactionType0x1Schema, tx, '0x1');
 		return '0x1';
+	}
+
+	// We don't return 0x0 here, because if gasPrice is not
+	// undefined, we still don't know if the network
+	// supports EIP-2718 (https://eips.ethereum.org/EIPS/eip-2718)
+	// and whether we should return undefined for legacy txs,
+	// or type 0x0 for legacy txs post EIP-2718
+	if (!isNullish(tx.gasPrice)) {
+		validateTxTypeAndHandleErrors(transactionType0x0Schema, tx, '0x0');
+	}
+
+	const givenHardfork = tx.hardfork ?? tx.common?.hardfork;
+	// If we don't have a hardfork, then we can't be sure we're post
+	// EIP-2718 where transaction types are available
+	if (givenHardfork === undefined) return undefined;
+
+	const hardforkIndex = Object.keys(HardforksOrdered).indexOf(givenHardfork);
+
+	// Unknown hardfork
+	if (hardforkIndex === undefined) return undefined;
+
+	// givenHardfork is London or later, so EIP-2718 is supported
+	if (hardforkIndex >= Object.keys(HardforksOrdered).indexOf('london'))
+		return !isNullish(tx.gasPrice) ? '0x0' : '0x2';
+
+	// givenHardfork is Berlin, tx.accessList is undefined, assume type is 0x0
+	if (hardforkIndex === Object.keys(HardforksOrdered).indexOf('berlin')) return '0x0';
 
+	// For all pre-Berlin hardforks, return undefined since EIP-2718
+	// isn't supported
 	return undefined;
 };