Skip to content

Commit d62e9c2

Browse files
authored
refactor: optimize precheckSendRawTransactionCheck for improved efficiency and code cleanliness (#4360)
Signed-off-by: Logan Nguyen <[email protected]>
1 parent 5a20300 commit d62e9c2

File tree

7 files changed

+98
-189
lines changed

7 files changed

+98
-189
lines changed

packages/relay/src/lib/errors/JsonRpcError.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,9 @@ export const predefined = {
155155
code: -32601,
156156
message: 'Not yet implemented',
157157
}),
158-
UNSUPPORTED_TRANSACTION_TYPE: new JsonRpcError({
158+
UNSUPPORTED_TRANSACTION_TYPE_3: new JsonRpcError({
159159
code: -32611,
160-
message: 'Unsupported transaction type',
160+
message: `Unsupported transaction type: txType=3`,
161161
}),
162162
VALUE_TOO_LOW: new JsonRpcError({
163163
code: -32602,

packages/relay/src/lib/precheck.ts

Lines changed: 48 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import { Logger } from 'pino';
77
import { prepend0x } from '../formatters';
88
import { MirrorNodeClient } from './clients';
99
import constants from './constants';
10-
import { JsonRpcError, predefined } from './errors/JsonRpcError';
10+
import { predefined } from './errors/JsonRpcError';
1111
import { CommonService } from './services';
1212
import { RequestDetails } from './types';
13+
import { IAccountBalance } from './types/mirrorNode';
1314

1415
/**
1516
* Precheck class for handling various prechecks before sending a raw transaction.
@@ -21,9 +22,9 @@ export class Precheck {
2122

2223
/**
2324
* Creates an instance of Precheck.
24-
* @param {MirrorNodeClient} mirrorNodeClient - The MirrorNodeClient instance.
25-
* @param {Logger} logger - The logger instance.
26-
* @param {string} chainId - The chain ID.
25+
* @param mirrorNodeClient - The MirrorNodeClient instance.
26+
* @param logger - The logger instance.
27+
* @param chainId - The chain ID.
2728
*/
2829
constructor(mirrorNodeClient: MirrorNodeClient, logger: Logger, chainId: string) {
2930
this.mirrorNodeClient = mirrorNodeClient;
@@ -33,7 +34,7 @@ export class Precheck {
3334

3435
/**
3536
* Parses the transaction if needed.
36-
* @param {string | Transaction} transaction - The transaction to parse.
37+
* @param transaction - The transaction to parse.
3738
* @returns {Transaction} The parsed transaction.
3839
*/
3940
public static parseRawTransaction(transaction: string | Transaction): Transaction {
@@ -46,7 +47,7 @@ export class Precheck {
4647

4748
/**
4849
* Checks if the value of the transaction is valid.
49-
* @param {Transaction} tx - The transaction.
50+
* @param tx - The transaction.
5051
*/
5152
value(tx: Transaction): void {
5253
if ((tx.value > 0 && tx.value < constants.TINYBAR_TO_WEIBAR_COEF) || tx.value < 0) {
@@ -56,9 +57,9 @@ export class Precheck {
5657

5758
/**
5859
* Sends a raw transaction after performing various prechecks.
59-
* @param {ethers.Transaction} parsedTx - The parsed transaction.
60-
* @param {number} networkGasPriceInWeiBars - The predefined gas price of the network in weibar.
61-
* @param {RequestDetails} requestDetails - The request details for logging and tracking.
60+
* @param parsedTx - The parsed transaction.
61+
* @param networkGasPriceInWeiBars - The predefined gas price of the network in weibar.
62+
* @param requestDetails - The request details for logging and tracking.
6263
*/
6364
async sendRawTransactionCheck(
6465
parsedTx: ethers.Transaction,
@@ -69,33 +70,23 @@ export class Precheck {
6970
this.transactionSize(parsedTx);
7071
this.transactionType(parsedTx);
7172
this.gasLimit(parsedTx);
72-
const mirrorAccountInfo = await this.verifyAccount(parsedTx, requestDetails);
73-
this.nonce(parsedTx, mirrorAccountInfo.ethereum_nonce);
7473
this.chainId(parsedTx);
7574
this.value(parsedTx);
7675
this.gasPrice(parsedTx, networkGasPriceInWeiBars);
77-
this.balance(parsedTx, mirrorAccountInfo);
76+
const signerAccountInfo = await this.verifyAccount(parsedTx, requestDetails);
77+
this.nonce(parsedTx, signerAccountInfo.ethereum_nonce);
78+
this.balance(parsedTx, signerAccountInfo.balance);
7879
await this.receiverAccount(parsedTx, requestDetails);
7980
}
8081

8182
/**
8283
* Verifies the account.
83-
* @param {Transaction} tx - The transaction.
84-
* @param {RequestDetails} requestDetails - The request details for logging and tracking.
85-
* @returns {Promise<any>} A Promise.
84+
* @param tx - The transaction.
85+
* @param requestDetails - The request details for logging and tracking.
8686
*/
8787
async verifyAccount(tx: Transaction, requestDetails: RequestDetails): Promise<any> {
8888
const accountInfo = await this.mirrorNodeClient.getAccount(tx.from!, requestDetails);
8989
if (accountInfo == null) {
90-
if (this.logger.isLevelEnabled('trace')) {
91-
this.logger.trace(
92-
`Failed to retrieve address '${
93-
tx.from
94-
}' account details from mirror node on verify account precheck for sendRawTransaction(transaction=${JSON.stringify(
95-
tx,
96-
)})`,
97-
);
98-
}
9990
throw predefined.RESOURCE_NOT_FOUND(`address '${tx.from}'.`);
10091
}
10192

@@ -105,35 +96,29 @@ export class Precheck {
10596
/**
10697
* Checks the nonce of the transaction.
10798
* @param tx - The transaction.
108-
* @param accountInfoNonce - The nonce of the account.
99+
* @param accountNonce - The nonce of the account.
109100
*/
110-
nonce(tx: Transaction, accountInfoNonce: number): void {
111-
if (this.logger.isLevelEnabled('trace')) {
112-
this.logger.trace(
113-
`Nonce precheck for sendRawTransaction(tx.nonce=${tx.nonce}, accountInfoNonce=${accountInfoNonce})`,
114-
);
101+
nonce(tx: Transaction, accountNonce: number | undefined): void {
102+
if (accountNonce == undefined) {
103+
throw predefined.RESOURCE_NOT_FOUND(`Account nonce unavailable for address: ${tx.from}.`);
115104
}
116105

117-
if (accountInfoNonce > tx.nonce) {
118-
throw predefined.NONCE_TOO_LOW(tx.nonce, accountInfoNonce);
106+
if (accountNonce > tx.nonce) {
107+
throw predefined.NONCE_TOO_LOW(tx.nonce, accountNonce);
119108
}
120109
}
121110

122111
/**
123-
* Checks the chain ID of the transaction.
124-
* @param tx - The transaction.
112+
* Validates that the transaction's chain ID matches the network's chain ID.
113+
* Legacy unprotected transactions (pre-EIP155) are exempt from this check.
114+
*
115+
* @param tx - The transaction to validate.
116+
* @throws {JsonRpcError} If the transaction's chain ID doesn't match the network's chain ID.
125117
*/
126118
chainId(tx: Transaction): void {
127119
const txChainId = prepend0x(Number(tx.chainId).toString(16));
128120
const passes = this.isLegacyUnprotectedEtx(tx) || txChainId === this.chain;
129121
if (!passes) {
130-
if (this.logger.isLevelEnabled('trace')) {
131-
this.logger.trace(
132-
`Failed chainId precheck for sendRawTransaction(transaction=%s, chainId=%s)`,
133-
JSON.stringify(tx),
134-
txChainId,
135-
);
136-
}
137122
throw predefined.UNSUPPORTED_CHAIN_ID(txChainId, this.chain);
138123
}
139124
}
@@ -181,22 +166,14 @@ export class Precheck {
181166
}
182167
}
183168

184-
if (this.logger.isLevelEnabled('trace')) {
185-
this.logger.trace(
186-
`Failed gas price precheck for sendRawTransaction(transaction=%s, gasPrice=%s, requiredGasPrice=%s)`,
187-
JSON.stringify(tx),
188-
txGasPrice,
189-
networkGasPrice,
190-
);
191-
}
192169
throw predefined.GAS_PRICE_TOO_LOW(txGasPrice, networkGasPrice);
193170
}
194171
}
195172

196173
/**
197174
* Checks if a transaction is the deterministic deployment transaction.
198-
* @param {Transaction} tx - The transaction to check.
199-
* @returns {boolean} Returns true if the transaction is the deterministic deployment transaction, otherwise false.
175+
* @param tx - The transaction to check.
176+
* @returns Returns true if the transaction is the deterministic deployment transaction, otherwise false.
200177
*/
201178
static isDeterministicDeploymentTransaction(tx: Transaction): boolean {
202179
return tx.serialized === constants.DETERMINISTIC_DEPLOYER_TRANSACTION;
@@ -205,58 +182,18 @@ export class Precheck {
205182
/**
206183
* Checks the balance of the sender account.
207184
* @param tx - The transaction.
208-
* @param account - The account information.
185+
* @param accountBalance - The account balance information.
209186
*/
210-
balance(tx: Transaction, account: any): void {
211-
const result = {
212-
passes: false,
213-
error: predefined.INSUFFICIENT_ACCOUNT_BALANCE,
214-
};
187+
balance(tx: Transaction, accountBalance: IAccountBalance | undefined): void {
188+
if (accountBalance?.balance == undefined) {
189+
throw predefined.RESOURCE_NOT_FOUND(`Account balance unavailable for address: ${tx.from}.`);
190+
}
215191

216192
const txGasPrice = BigInt(tx.gasPrice || tx.maxFeePerGas! + tx.maxPriorityFeePerGas!);
217193
const txTotalValue = tx.value + txGasPrice * tx.gasLimit;
194+
const accountBalanceInWeiBars = BigInt(accountBalance.balance) * BigInt(constants.TINYBAR_TO_WEIBAR_COEF);
218195

219-
if (account == null) {
220-
if (this.logger.isLevelEnabled('trace')) {
221-
this.logger.trace(
222-
`Failed to retrieve account details from mirror node on balance precheck for sendRawTransaction(transaction=${JSON.stringify(
223-
tx,
224-
)}, totalValue=${txTotalValue})`,
225-
);
226-
}
227-
throw predefined.RESOURCE_NOT_FOUND(`tx.from '${tx.from}'.`);
228-
}
229-
230-
let tinybars: bigint;
231-
try {
232-
tinybars = BigInt(account.balance.balance.toString()) * BigInt(constants.TINYBAR_TO_WEIBAR_COEF);
233-
result.passes = tinybars >= txTotalValue;
234-
} catch (error: any) {
235-
if (this.logger.isLevelEnabled('trace')) {
236-
this.logger.trace(
237-
`Error on balance precheck for sendRawTransaction(transaction=%s, totalValue=%s, error=%s)`,
238-
JSON.stringify(tx),
239-
txTotalValue,
240-
error.message,
241-
);
242-
}
243-
if (error instanceof JsonRpcError) {
244-
// preserve original error
245-
throw error;
246-
} else {
247-
throw predefined.INTERNAL_ERROR(`balance precheck: ${error.message}`);
248-
}
249-
}
250-
251-
if (!result.passes) {
252-
if (this.logger.isLevelEnabled('trace')) {
253-
this.logger.trace(
254-
`Failed balance precheck for sendRawTransaction(transaction=%s, totalValue=%s, accountTinyBarBalance=%s)`,
255-
JSON.stringify(tx),
256-
txTotalValue,
257-
tinybars,
258-
);
259-
}
196+
if (accountBalanceInWeiBars < txTotalValue) {
260197
throw predefined.INSUFFICIENT_ACCOUNT_BALANCE;
261198
}
262199
}
@@ -267,29 +204,11 @@ export class Precheck {
267204
*/
268205
gasLimit(tx: Transaction): void {
269206
const gasLimit = Number(tx.gasLimit);
270-
const failBaseLog = 'Failed gasLimit precheck for sendRawTransaction(transaction=%s).';
271-
272207
const intrinsicGasCost = Precheck.transactionIntrinsicGasCost(tx.data);
273208

274209
if (gasLimit > constants.MAX_TRANSACTION_FEE_THRESHOLD) {
275-
if (this.logger.isLevelEnabled('trace')) {
276-
this.logger.trace(
277-
`${failBaseLog} Gas Limit was too high: %s, block gas limit: %s`,
278-
JSON.stringify(tx),
279-
gasLimit,
280-
constants.MAX_TRANSACTION_FEE_THRESHOLD,
281-
);
282-
}
283210
throw predefined.GAS_LIMIT_TOO_HIGH(gasLimit, constants.MAX_TRANSACTION_FEE_THRESHOLD);
284211
} else if (gasLimit < intrinsicGasCost) {
285-
if (this.logger.isLevelEnabled('trace')) {
286-
this.logger.trace(
287-
`${failBaseLog} Gas Limit was too low: %s, intrinsic gas cost: %s`,
288-
JSON.stringify(tx),
289-
gasLimit,
290-
intrinsicGasCost,
291-
);
292-
}
293212
throw predefined.GAS_LIMIT_TOO_LOW(gasLimit, intrinsicGasCost);
294213
}
295214
}
@@ -298,8 +217,8 @@ export class Precheck {
298217
* Calculates the intrinsic gas cost based on the number of bytes in the data field.
299218
* Using a loop that goes through every two characters in the string it counts the zero and non-zero bytes.
300219
* Every two characters that are packed together and are both zero counts towards zero bytes.
301-
* @param {string} data - The data with the bytes to be calculated
302-
* @returns {number} The intrinsic gas cost.
220+
* @param data - The data with the bytes to be calculated
221+
* @returns The intrinsic gas cost.
303222
* @private
304223
*/
305224
public static transactionIntrinsicGasCost(data: string): number {
@@ -326,7 +245,7 @@ export class Precheck {
326245
* The serialized transaction length is converted from hex string length to byte count
327246
* by subtracting the '0x' prefix (2 characters) and dividing by 2 (since each byte is represented by 2 hex characters).
328247
*
329-
* @param {Transaction} tx - The transaction to validate.
248+
* @param tx - The transaction to validate.
330249
* @throws {JsonRpcError} If the transaction size exceeds the configured limit.
331250
*/
332251
transactionSize(tx: Transaction): void {
@@ -342,7 +261,7 @@ export class Precheck {
342261
* The data field length is converted from hex string length to byte count
343262
* by subtracting the '0x' prefix (2 characters) and dividing by 2 (since each byte is represented by 2 hex characters).
344263
*
345-
* @param {Transaction} tx - The transaction to validate.
264+
* @param tx - The transaction to validate.
346265
* @throws {JsonRpcError} If the call data size exceeds the configured limit.
347266
*/
348267
callDataSize(tx: Transaction): void {
@@ -353,22 +272,23 @@ export class Precheck {
353272
}
354273
}
355274

275+
/**
276+
* Validates the transaction type and throws an error if the transaction is unsupported.
277+
* Specifically, blob transactions (type 3) are not supported as per HIP 866.
278+
* @param tx The transaction object to validate.
279+
* @throws {Error} Throws a predefined error if the transaction type is unsupported.
280+
*/
356281
transactionType(tx: Transaction) {
357282
// Blob transactions are not supported as per HIP 866
358283
if (tx.type === 3) {
359-
if (this.logger.isLevelEnabled('trace')) {
360-
this.logger.trace(
361-
`Transaction with type=${tx.type} is unsupported for sendRawTransaction(transaction=${JSON.stringify(tx)})`,
362-
);
363-
}
364-
throw predefined.UNSUPPORTED_TRANSACTION_TYPE;
284+
throw predefined.UNSUPPORTED_TRANSACTION_TYPE_3;
365285
}
366286
}
367287

368288
/**
369289
* Checks if the receiver account exists and has receiver_sig_required set to true.
370-
* @param {Transaction} tx - The transaction.
371-
* @param {RequestDetails} requestDetails - The request details for logging and tracking.
290+
* @param tx - The transaction.
291+
* @param requestDetails - The request details for logging and tracking.
372292
*/
373293
async receiverAccount(tx: Transaction, requestDetails: RequestDetails) {
374294
if (tx.to) {

packages/relay/src/lib/services/ethService/transactionService/TransactionService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ export class TransactionService implements ITransactionService {
441441

442442
await this.precheck.sendRawTransactionCheck(parsedTx, networkGasPriceInWeiBars, requestDetails);
443443
} catch (e: any) {
444-
this.logger.error(`Precheck failed: transaction=${JSON.stringify(parsedTx)}`);
444+
this.logger.error(e, `Precheck failed: errorMessage=${e.message}`);
445445
throw this.common.genericErrorHandler(e);
446446
}
447447
}

packages/relay/tests/lib/eth/eth_sendRawTransaction.spec.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,14 @@ describe('@ethSendRawTransaction eth_sendRawTransaction spec', async function ()
102102
balance: {
103103
balance: Hbar.from(100_000_000_000, HbarUnit.Hbar).to(HbarUnit.Tinybar),
104104
},
105+
ethereum_nonce: 0,
105106
};
106107
const RECEIVER_ACCOUNT_RES = {
107108
account: ACCOUNT_ADDRESS_1,
108109
balance: {
109110
balance: Hbar.from(1, HbarUnit.Hbar).to(HbarUnit.Tinybar),
110111
},
112+
ethereum_nonce: 0,
111113
receiver_sig_required: false,
112114
};
113115
const useAsyncTxProcessing = ConfigService.get('USE_ASYNC_TX_PROCESSING');
@@ -280,7 +282,7 @@ describe('@ethSendRawTransaction eth_sendRawTransaction spec', async function ()
280282
const signed = await signTransaction(type3tx);
281283

282284
await RelayAssertions.assertRejection(
283-
predefined.UNSUPPORTED_TRANSACTION_TYPE,
285+
predefined.UNSUPPORTED_TRANSACTION_TYPE_3,
284286
ethImpl.sendRawTransaction,
285287
false,
286288
ethImpl,

packages/relay/tests/lib/openrpc.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ describe('Open RPC Specification', function () {
193193
balance: {
194194
balance: 100000000000,
195195
},
196+
ethereum_nonce: 0,
196197
}),
197198
);
198199
mock

0 commit comments

Comments
 (0)