Skip to content

Commit 9f0f984

Browse files
quiet-nodequiet-node
authored
chore: cherry picked PR#2730 (#2731)
fix: captured transactionFee in metrics and HBAR limiter class in executeTransaction and deleteFile (#2714) * fix: captured transaction fee in metrics and hbar limiter class for executeTransaction * fix: captured transaction fee in hbar limiter class for deleteFile * fix: improved the logic of capturing transaction fees to metrics in executeTransaction * fix: improved error handling logic for sendRawTransaction fix: added polling logic to retrieve updated account nonce for WRONG_NONCE error handler * fix: removed non-existed NEXT_STORAGE_CONTRACT_UPDATE singature hash * fix: added logic to throw WRONG_NONCE error * fix: tweaked and reused executeGetTransactionRecord * fix: fixed http rate limiter failing test * fix: reverted shouldLimit check --------- Signed-off-by: Logan Nguyen <[email protected]>
1 parent 6c1418d commit 9f0f984

File tree

5 files changed

+126
-87
lines changed

5 files changed

+126
-87
lines changed

‎packages/relay/src/lib/clients/mirrorNodeClient.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,9 @@ export class MirrorNodeClient {
12341234
public getMirrorNodeWeb3Instance() {
12351235
return this.web3Client;
12361236
}
1237+
public getMirrorNodeRetryDelay() {
1238+
return this.MIRROR_NODE_RETRY_DELAY;
1239+
}
12371240

12381241
/**
12391242
* This method is intended to be used in cases when the default axios-retry settings do not provide

‎packages/relay/src/lib/clients/sdkClient.ts‎

Lines changed: 81 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -469,18 +469,28 @@ export class SDKClient {
469469
const requestIdPrefix = formatRequestIdMessage(requestId);
470470
const currentDateNow = Date.now();
471471
try {
472-
const shouldLimit = this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.transactionMode, callerName);
473-
if (shouldLimit) {
472+
// check hbar limit before executing transaction
473+
if (this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.recordMode, callerName)) {
474474
throw predefined.HBAR_RATE_LIMIT_EXCEEDED;
475475
}
476476

477+
// execute transaction
477478
this.logger.info(`${requestIdPrefix} Execute ${transactionType} transaction`);
478-
const resp = await transaction.execute(this.clientMain);
479+
const transactionResponse = await transaction.execute(this.clientMain);
480+
481+
// retrieve and capture transaction fee in metrics and rate limiter class
482+
await this.executeGetTransactionRecord(
483+
transactionResponse,
484+
callerName,
485+
interactingEntity,
486+
transaction.constructor.name,
487+
requestId,
488+
);
479489

480490
this.logger.info(
481-
`${requestIdPrefix} ${resp.transactionId} ${callerName} ${transactionType} status: ${Status.Success} (${Status.Success._code})`,
491+
`${requestIdPrefix} ${transactionResponse.transactionId} ${callerName} ${transactionType} status: ${Status.Success} (${Status.Success._code})`,
482492
);
483-
return resp;
493+
return transactionResponse;
484494
} catch (e: any) {
485495
const sdkClientError = new SDKClientError(e, e.message);
486496
let transactionFee: number | Hbar = 0;
@@ -526,87 +536,88 @@ export class SDKClient {
526536
};
527537

528538
async executeGetTransactionRecord(
529-
resp: TransactionResponse,
530-
transactionName: string,
539+
transactionResponse: TransactionResponse,
531540
callerName: string,
532541
interactingEntity: string,
542+
txConstructorName: string,
533543
requestId?: string,
534-
): Promise<TransactionRecord> {
544+
) {
535545
const requestIdPrefix = formatRequestIdMessage(requestId);
536546
const currentDateNow = Date.now();
547+
let gasUsed: any = 0;
548+
let transactionFee: number = 0;
549+
const transactionId: string = transactionResponse.transactionId.toString();
550+
537551
try {
538-
if (!resp.getRecord) {
552+
if (!transactionResponse.getRecord) {
539553
throw new SDKClientError(
540554
{},
541-
`${requestIdPrefix} Invalid response format, expected record availability: ${JSON.stringify(resp)}`,
555+
`${requestIdPrefix} Invalid response format, expected record availability: ${JSON.stringify(
556+
transactionResponse,
557+
)}`,
542558
);
543559
}
544-
const shouldLimit = this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.recordMode, transactionName);
545-
if (shouldLimit) {
560+
if (this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.recordMode, callerName)) {
546561
throw predefined.HBAR_RATE_LIMIT_EXCEEDED;
547562
}
548563

549-
const transactionRecord: TransactionRecord = await resp.getRecord(this.clientMain);
550-
const cost = transactionRecord.transactionFee.toTinybars().toNumber();
551-
this.hbarLimiter.addExpense(cost, currentDateNow);
552-
this.logger.info(
553-
`${requestIdPrefix} ${resp.transactionId} ${callerName} ${transactionName} record status: ${Status.Success} (${Status.Success._code}), cost: ${transactionRecord.transactionFee}`,
554-
);
555-
this.captureMetrics(
556-
SDKClient.transactionMode,
557-
transactionName,
558-
transactionRecord.receipt.status,
559-
cost,
560-
transactionRecord?.contractFunctionResult?.gasUsed,
561-
callerName,
562-
interactingEntity,
563-
);
564-
565-
this.hbarLimiter.addExpense(cost, currentDateNow);
566-
567-
return transactionRecord;
564+
// get transactionRecord
565+
const transactionRecord: TransactionRecord = await transactionResponse.getRecord(this.clientMain);
566+
567+
// get transactionFee and gasUsed for metrics
568+
/**
569+
* @todo: Determine how to separate the fee charged exclusively by the operator because
570+
* the transactionFee below includes the entire charges of the transaction,
571+
* with some portions paid by tx.from, not the operator.
572+
*/
573+
transactionFee = transactionRecord.transactionFee.toTinybars().toNumber();
574+
gasUsed = transactionRecord?.contractFunctionResult?.gasUsed.toNumber();
568575
} catch (e: any) {
569-
// capture sdk record retrieval errors and shorten familiar stack trace
570-
const sdkClientError = new SDKClientError(e, e.message);
571-
let transactionFee: number | Hbar = 0;
572-
if (sdkClientError.isValidNetworkError()) {
573-
try {
574-
// pull transaction record for fee
575-
const transactionRecord = await new TransactionRecordQuery()
576-
.setTransactionId(resp.transactionId!)
577-
.setNodeAccountIds([resp.nodeId])
578-
.setValidateReceiptStatus(false)
579-
.execute(this.clientMain);
580-
transactionFee = transactionRecord.transactionFee;
581-
582-
this.captureMetrics(
583-
SDKClient.transactionMode,
584-
transactionName,
585-
sdkClientError.status,
586-
transactionFee.toTinybars().toNumber(),
587-
transactionRecord?.contractFunctionResult?.gasUsed,
588-
callerName,
589-
interactingEntity,
590-
);
591-
592-
this.hbarLimiter.addExpense(transactionFee.toTinybars().toNumber(), currentDateNow);
593-
} catch (err: any) {
594-
const recordQueryError = new SDKClientError(err, err.message);
595-
this.logger.error(
596-
recordQueryError,
597-
`${requestIdPrefix} Error raised during TransactionRecordQuery for ${resp.transactionId}`,
598-
);
599-
}
576+
try {
577+
// get transactionFee and gasUsed for metrics
578+
// Only utilize SDK query when .getRecord throws an error. This can limit the number of calls to the SDK.
579+
const transactionRecord = await new TransactionRecordQuery()
580+
.setTransactionId(transactionId)
581+
.setNodeAccountIds([transactionResponse.nodeId])
582+
.setValidateReceiptStatus(false)
583+
.execute(this.clientMain);
584+
transactionFee = transactionRecord.transactionFee.toTinybars().toNumber();
585+
gasUsed = transactionRecord?.contractFunctionResult?.gasUsed.toNumber();
586+
} catch (err: any) {
587+
const recordQueryError = new SDKClientError(err, err.message);
588+
this.logger.error(
589+
recordQueryError,
590+
`${requestIdPrefix} Error raised during TransactionRecordQuery for ${transactionId}`,
591+
);
600592
}
601593

594+
// log error from getRecord
595+
const sdkClientError = new SDKClientError(e, e.message);
602596
this.logger.debug(
603-
`${requestIdPrefix} ${resp.transactionId} ${callerName} ${transactionName} record status: ${sdkClientError.status} (${sdkClientError.status._code}), cost: ${transactionFee}`,
597+
`${requestIdPrefix} ${transactionId} ${callerName} record status: ${sdkClientError.status} (${sdkClientError.status._code}), cost: ${transactionFee}`,
604598
);
605599

606-
if (e instanceof JsonRpcError) {
607-
throw predefined.HBAR_RATE_LIMIT_EXCEEDED;
608-
}
609-
throw sdkClientError;
600+
// Throw WRONG_NONCE error as more error handling logic for WRONG_NONCE is awaited in eth.sendRawTransactionErrorHandler(). Otherwise, move on and return transactionResponse eventually.
601+
if (e.status && e.status.toString() === constants.TRANSACTION_RESULT_STATUS.WRONG_NONCE) throw sdkClientError;
602+
} finally {
603+
/**
604+
* @note Retrieving and capturing the charged transaction fees at the end of the flow
605+
* ensures these fees are eventually captured in the metrics and rate limiter class,
606+
* even if SDK transactions fail at any point.
607+
*/
608+
this.logger.trace(
609+
`${requestId} Capturing HBAR charged transaction fee: transactionId=${transactionId}, txConstructorName=${txConstructorName}, callerName=${callerName}, txChargedFee=${transactionFee} tinybars`,
610+
);
611+
this.hbarLimiter.addExpense(transactionFee, currentDateNow);
612+
this.captureMetrics(
613+
SDKClient.transactionMode,
614+
txConstructorName,
615+
Status.Success,
616+
transactionFee,
617+
gasUsed,
618+
callerName,
619+
interactingEntity,
620+
);
610621
}
611622
}
612623

@@ -786,6 +797,7 @@ export class SDKClient {
786797
*/
787798
public deleteFile = async (fileId: FileId, requestId?: string, callerName?: string, interactingEntity?: string) => {
788799
// format request ID msg
800+
const currentDateNow = Date.now();
789801
const requestIdPrefix = formatRequestIdMessage(requestId);
790802

791803
try {
@@ -801,7 +813,8 @@ export class SDKClient {
801813
// get fileDeleteTx's record
802814
const deleteFileRecord = await fileDeleteTxResponse.getRecord(this.clientMain);
803815

804-
// capture metrics
816+
// capture transactionFee in metrics and HBAR limiter class
817+
this.hbarLimiter.addExpense(deleteFileRecord.transactionFee.toTinybars().toNumber(), currentDateNow);
805818
this.captureMetrics(
806819
SDKClient.transactionMode,
807820
fileDeleteTx.constructor.name,

‎packages/relay/src/lib/eth.ts‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1379,6 +1379,7 @@ export class EthImpl implements Eth {
13791379
transaction,
13801380
transactionBuffer,
13811381
txSubmitted,
1382+
parsedTx,
13821383
requestIdPrefix,
13831384
): Promise<string | JsonRpcError> {
13841385
this.logger.error(
@@ -1391,6 +1392,36 @@ export class EthImpl implements Eth {
13911392

13921393
if (e instanceof SDKClientError) {
13931394
this.hapiService.decrementErrorCounter(e.statusCode);
1395+
if (e.status.toString() === constants.TRANSACTION_RESULT_STATUS.WRONG_NONCE) {
1396+
// note: because this is a WRONG_NONCE error handler, the nonce of the account is expected to be different from the nonce of the parsedTx
1397+
// running a polling loop to give mirror node enough time to update account nonce
1398+
let accountNonce: number | null = null;
1399+
for (let i = 0; i < this.MirrorNodeGetContractResultRetries; i++) {
1400+
const accountInfo = await this.mirrorNodeClient.getAccount(parsedTx.from!, requestIdPrefix);
1401+
if (accountInfo.ethereum_nonce !== parsedTx.nonce) {
1402+
accountNonce = accountInfo.ethereum_nonce;
1403+
break;
1404+
}
1405+
1406+
this.logger.trace(
1407+
`${requestIdPrefix} Repeating retry to poll for updated account nonce. Count ${i} of ${
1408+
this.MirrorNodeGetContractResultRetries
1409+
}. Waiting ${this.mirrorNodeClient.getMirrorNodeRetryDelay()} ms before initiating a new request`,
1410+
);
1411+
await new Promise((r) => setTimeout(r, this.mirrorNodeClient.getMirrorNodeRetryDelay()));
1412+
}
1413+
1414+
if (!accountNonce) {
1415+
this.logger.warn(`${requestIdPrefix} Cannot find updated account nonce.`);
1416+
throw predefined.INTERNAL_ERROR(`Cannot find updated account nonce for WRONT_NONCE error.`);
1417+
}
1418+
1419+
if (parsedTx.nonce > accountNonce) {
1420+
return predefined.NONCE_TOO_HIGH(parsedTx.nonce, accountNonce);
1421+
} else {
1422+
return predefined.NONCE_TOO_LOW(parsedTx.nonce, accountNonce);
1423+
}
1424+
}
13941425
}
13951426

13961427
if (!txSubmitted) {
@@ -1462,20 +1493,6 @@ export class EthImpl implements Eth {
14621493

14631494
if (!contractResult) {
14641495
this.logger.warn(`${requestIdPrefix} No record retrieved`);
1465-
const tx = await this.mirrorNodeClient.getTransactionById(txId, 0, requestIdPrefix);
1466-
1467-
if (tx?.transactions?.length) {
1468-
const result = tx.transactions[0].result;
1469-
if (result === constants.TRANSACTION_RESULT_STATUS.WRONG_NONCE) {
1470-
const accountInfo = await this.mirrorNodeClient.getAccount(parsedTx.from!, requestIdPrefix);
1471-
const accountNonce = accountInfo.ethereum_nonce;
1472-
if (parsedTx.nonce > accountNonce) {
1473-
throw predefined.NONCE_TOO_HIGH(parsedTx.nonce, accountNonce);
1474-
}
1475-
1476-
throw predefined.NONCE_TOO_LOW(parsedTx.nonce, accountNonce);
1477-
}
1478-
}
14791496
throw predefined.INTERNAL_ERROR(`No matching record found for transaction id ${txId}`);
14801497
}
14811498

@@ -1488,7 +1505,14 @@ export class EthImpl implements Eth {
14881505

14891506
return contractResult.hash;
14901507
} catch (e: any) {
1491-
return this.sendRawTransactionErrorHandler(e, transaction, transactionBuffer, txSubmitted, requestIdPrefix);
1508+
return this.sendRawTransactionErrorHandler(
1509+
e,
1510+
transaction,
1511+
transactionBuffer,
1512+
txSubmitted,
1513+
parsedTx,
1514+
requestIdPrefix,
1515+
);
14921516
} finally {
14931517
/**
14941518
* For transactions of type CONTRACT_CREATE, if the contract's bytecode (calldata) exceeds 5120 bytes, HFS is employed to temporarily store the bytecode on the network.

‎packages/server/tests/acceptance/rateLimiter.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ describe('@ratelimiter Rate Limiters Acceptance Tests', function () {
166166
await expect(relay.call(testConstants.ETH_ENDPOINTS.ETH_SEND_RAW_TRANSACTION, [signedTx], requestId)).to.be
167167
.fulfilled;
168168
const remainingHbarsAfter = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT));
169-
expect(remainingHbarsAfter).to.be.eq(remainingHbarsBefore);
169+
expect(remainingHbarsAfter).to.be.lt(remainingHbarsBefore);
170170
});
171171

172172
it('should deploy a large contract and decrease remaining HBAR in limiter when transaction data is large', async function () {

‎packages/server/tests/acceptance/rpc_batch2.spec.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,6 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
812812
let storageContract: ethers.Contract;
813813
let storageContractAddress: string;
814814
const STORAGE_CONTRACT_UPDATE = '0x2de4e884';
815-
const NEXT_STORAGE_CONTRACT_UPDATE = '0x160D6484';
816815

817816
this.beforeEach(async () => {
818817
storageContract = await Utils.deployContract(
@@ -985,7 +984,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
985984
const transaction1 = {
986985
...transaction,
987986
nonce: await relay.getAccountNonce(accounts[1].address),
988-
data: NEXT_STORAGE_CONTRACT_UPDATE,
987+
data: STORAGE_CONTRACT_UPDATE,
989988
};
990989

991990
const signedTx1 = await accounts[1].wallet.signTransaction(transaction1);
@@ -1030,7 +1029,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
10301029
const transaction1 = {
10311030
...transaction,
10321031
nonce: await relay.getAccountNonce(accounts[1].address),
1033-
data: NEXT_STORAGE_CONTRACT_UPDATE,
1032+
data: STORAGE_CONTRACT_UPDATE,
10341033
};
10351034

10361035
const signedTx1 = await accounts[1].wallet.signTransaction(transaction1);

0 commit comments

Comments
 (0)