diff --git a/packages/relay/src/lib/clients/mirrorNodeClient.ts b/packages/relay/src/lib/clients/mirrorNodeClient.ts index 8dd6bbe1e1..9bcaa1e819 100644 --- a/packages/relay/src/lib/clients/mirrorNodeClient.ts +++ b/packages/relay/src/lib/clients/mirrorNodeClient.ts @@ -1263,6 +1263,9 @@ export class MirrorNodeClient { public getMirrorNodeWeb3Instance() { return this.web3Client; } + public getMirrorNodeRetryDelay() { + return this.MIRROR_NODE_RETRY_DELAY; + } /** * This method is intended to be used in cases when the default axios-retry settings do not provide diff --git a/packages/relay/src/lib/clients/sdkClient.ts b/packages/relay/src/lib/clients/sdkClient.ts index f839166da8..15e6bf6b22 100644 --- a/packages/relay/src/lib/clients/sdkClient.ts +++ b/packages/relay/src/lib/clients/sdkClient.ts @@ -469,18 +469,28 @@ export class SDKClient { const requestIdPrefix = formatRequestIdMessage(requestId); const currentDateNow = Date.now(); try { - const shouldLimit = this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.transactionMode, callerName); - if (shouldLimit) { + // check hbar limit before executing transaction + if (this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.recordMode, callerName)) { throw predefined.HBAR_RATE_LIMIT_EXCEEDED; } + // execute transaction this.logger.info(`${requestIdPrefix} Execute ${transactionType} transaction`); - const resp = await transaction.execute(this.clientMain); + const transactionResponse = await transaction.execute(this.clientMain); + + // retrieve and capture transaction fee in metrics and rate limiter class + await this.executeGetTransactionRecord( + transactionResponse, + callerName, + interactingEntity, + transaction.constructor.name, + requestId, + ); this.logger.info( - `${requestIdPrefix} ${resp.transactionId} ${callerName} ${transactionType} status: ${Status.Success} (${Status.Success._code})`, + `${requestIdPrefix} ${transactionResponse.transactionId} ${callerName} ${transactionType} status: ${Status.Success} (${Status.Success._code})`, ); - return resp; + return transactionResponse; } catch (e: any) { const sdkClientError = new SDKClientError(e, e.message); let transactionFee: number | Hbar = 0; @@ -526,87 +536,88 @@ export class SDKClient { }; async executeGetTransactionRecord( - resp: TransactionResponse, - transactionName: string, + transactionResponse: TransactionResponse, callerName: string, interactingEntity: string, + txConstructorName: string, requestId?: string, - ): Promise { + ) { const requestIdPrefix = formatRequestIdMessage(requestId); const currentDateNow = Date.now(); + let gasUsed: any = 0; + let transactionFee: number = 0; + const transactionId: string = transactionResponse.transactionId.toString(); + try { - if (!resp.getRecord) { + if (!transactionResponse.getRecord) { throw new SDKClientError( {}, - `${requestIdPrefix} Invalid response format, expected record availability: ${JSON.stringify(resp)}`, + `${requestIdPrefix} Invalid response format, expected record availability: ${JSON.stringify( + transactionResponse, + )}`, ); } - const shouldLimit = this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.recordMode, transactionName); - if (shouldLimit) { + if (this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.recordMode, callerName)) { throw predefined.HBAR_RATE_LIMIT_EXCEEDED; } - const transactionRecord: TransactionRecord = await resp.getRecord(this.clientMain); - const cost = transactionRecord.transactionFee.toTinybars().toNumber(); - this.hbarLimiter.addExpense(cost, currentDateNow); - this.logger.info( - `${requestIdPrefix} ${resp.transactionId} ${callerName} ${transactionName} record status: ${Status.Success} (${Status.Success._code}), cost: ${transactionRecord.transactionFee}`, - ); - this.captureMetrics( - SDKClient.transactionMode, - transactionName, - transactionRecord.receipt.status, - cost, - transactionRecord?.contractFunctionResult?.gasUsed, - callerName, - interactingEntity, - ); - - this.hbarLimiter.addExpense(cost, currentDateNow); - - return transactionRecord; + // get transactionRecord + const transactionRecord: TransactionRecord = await transactionResponse.getRecord(this.clientMain); + + // get transactionFee and gasUsed for metrics + /** + * @todo: Determine how to separate the fee charged exclusively by the operator because + * the transactionFee below includes the entire charges of the transaction, + * with some portions paid by tx.from, not the operator. + */ + transactionFee = transactionRecord.transactionFee.toTinybars().toNumber(); + gasUsed = transactionRecord?.contractFunctionResult?.gasUsed.toNumber(); } catch (e: any) { - // capture sdk record retrieval errors and shorten familiar stack trace - const sdkClientError = new SDKClientError(e, e.message); - let transactionFee: number | Hbar = 0; - if (sdkClientError.isValidNetworkError()) { - try { - // pull transaction record for fee - const transactionRecord = await new TransactionRecordQuery() - .setTransactionId(resp.transactionId!) - .setNodeAccountIds([resp.nodeId]) - .setValidateReceiptStatus(false) - .execute(this.clientMain); - transactionFee = transactionRecord.transactionFee; - - this.captureMetrics( - SDKClient.transactionMode, - transactionName, - sdkClientError.status, - transactionFee.toTinybars().toNumber(), - transactionRecord?.contractFunctionResult?.gasUsed, - callerName, - interactingEntity, - ); - - this.hbarLimiter.addExpense(transactionFee.toTinybars().toNumber(), currentDateNow); - } catch (err: any) { - const recordQueryError = new SDKClientError(err, err.message); - this.logger.error( - recordQueryError, - `${requestIdPrefix} Error raised during TransactionRecordQuery for ${resp.transactionId}`, - ); - } + try { + // get transactionFee and gasUsed for metrics + // Only utilize SDK query when .getRecord throws an error. This can limit the number of calls to the SDK. + const transactionRecord = await new TransactionRecordQuery() + .setTransactionId(transactionId) + .setNodeAccountIds([transactionResponse.nodeId]) + .setValidateReceiptStatus(false) + .execute(this.clientMain); + transactionFee = transactionRecord.transactionFee.toTinybars().toNumber(); + gasUsed = transactionRecord?.contractFunctionResult?.gasUsed.toNumber(); + } catch (err: any) { + const recordQueryError = new SDKClientError(err, err.message); + this.logger.error( + recordQueryError, + `${requestIdPrefix} Error raised during TransactionRecordQuery for ${transactionId}`, + ); } + // log error from getRecord + const sdkClientError = new SDKClientError(e, e.message); this.logger.debug( - `${requestIdPrefix} ${resp.transactionId} ${callerName} ${transactionName} record status: ${sdkClientError.status} (${sdkClientError.status._code}), cost: ${transactionFee}`, + `${requestIdPrefix} ${transactionId} ${callerName} record status: ${sdkClientError.status} (${sdkClientError.status._code}), cost: ${transactionFee}`, ); - if (e instanceof JsonRpcError) { - throw predefined.HBAR_RATE_LIMIT_EXCEEDED; - } - throw sdkClientError; + // Throw WRONG_NONCE error as more error handling logic for WRONG_NONCE is awaited in eth.sendRawTransactionErrorHandler(). Otherwise, move on and return transactionResponse eventually. + if (e.status && e.status.toString() === constants.TRANSACTION_RESULT_STATUS.WRONG_NONCE) throw sdkClientError; + } finally { + /** + * @note Retrieving and capturing the charged transaction fees at the end of the flow + * ensures these fees are eventually captured in the metrics and rate limiter class, + * even if SDK transactions fail at any point. + */ + this.logger.trace( + `${requestId} Capturing HBAR charged transaction fee: transactionId=${transactionId}, txConstructorName=${txConstructorName}, callerName=${callerName}, txChargedFee=${transactionFee} tinybars`, + ); + this.hbarLimiter.addExpense(transactionFee, currentDateNow); + this.captureMetrics( + SDKClient.transactionMode, + txConstructorName, + Status.Success, + transactionFee, + gasUsed, + callerName, + interactingEntity, + ); } } @@ -786,6 +797,7 @@ export class SDKClient { */ public deleteFile = async (fileId: FileId, requestId?: string, callerName?: string, interactingEntity?: string) => { // format request ID msg + const currentDateNow = Date.now(); const requestIdPrefix = formatRequestIdMessage(requestId); try { @@ -801,7 +813,8 @@ export class SDKClient { // get fileDeleteTx's record const deleteFileRecord = await fileDeleteTxResponse.getRecord(this.clientMain); - // capture metrics + // capture transactionFee in metrics and HBAR limiter class + this.hbarLimiter.addExpense(deleteFileRecord.transactionFee.toTinybars().toNumber(), currentDateNow); this.captureMetrics( SDKClient.transactionMode, fileDeleteTx.constructor.name, diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts index 3bb6fa69cc..4e12ed0b9c 100644 --- a/packages/relay/src/lib/eth.ts +++ b/packages/relay/src/lib/eth.ts @@ -1438,6 +1438,7 @@ export class EthImpl implements Eth { transaction, transactionBuffer, txSubmitted, + parsedTx, requestIdPrefix, ): Promise { this.logger.error( @@ -1450,6 +1451,36 @@ export class EthImpl implements Eth { if (e instanceof SDKClientError) { this.hapiService.decrementErrorCounter(e.statusCode); + if (e.status.toString() === constants.TRANSACTION_RESULT_STATUS.WRONG_NONCE) { + // 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 + // running a polling loop to give mirror node enough time to update account nonce + let accountNonce: number | null = null; + for (let i = 0; i < this.MirrorNodeGetContractResultRetries; i++) { + const accountInfo = await this.mirrorNodeClient.getAccount(parsedTx.from!, requestIdPrefix); + if (accountInfo.ethereum_nonce !== parsedTx.nonce) { + accountNonce = accountInfo.ethereum_nonce; + break; + } + + this.logger.trace( + `${requestIdPrefix} Repeating retry to poll for updated account nonce. Count ${i} of ${ + this.MirrorNodeGetContractResultRetries + }. Waiting ${this.mirrorNodeClient.getMirrorNodeRetryDelay()} ms before initiating a new request`, + ); + await new Promise((r) => setTimeout(r, this.mirrorNodeClient.getMirrorNodeRetryDelay())); + } + + if (!accountNonce) { + this.logger.warn(`${requestIdPrefix} Cannot find updated account nonce.`); + throw predefined.INTERNAL_ERROR(`Cannot find updated account nonce for WRONT_NONCE error.`); + } + + if (parsedTx.nonce > accountNonce) { + return predefined.NONCE_TOO_HIGH(parsedTx.nonce, accountNonce); + } else { + return predefined.NONCE_TOO_LOW(parsedTx.nonce, accountNonce); + } + } } if (!txSubmitted) { @@ -1522,20 +1553,6 @@ export class EthImpl implements Eth { if (!contractResult) { this.logger.warn(`${requestIdPrefix} No record retrieved`); - const tx = await this.mirrorNodeClient.getTransactionById(txId, 0, requestIdPrefix); - - if (tx?.transactions?.length) { - const result = tx.transactions[0].result; - if (result === constants.TRANSACTION_RESULT_STATUS.WRONG_NONCE) { - const accountInfo = await this.mirrorNodeClient.getAccount(parsedTx.from!, requestIdPrefix); - const accountNonce = accountInfo.ethereum_nonce; - if (parsedTx.nonce > accountNonce) { - throw predefined.NONCE_TOO_HIGH(parsedTx.nonce, accountNonce); - } - - throw predefined.NONCE_TOO_LOW(parsedTx.nonce, accountNonce); - } - } throw predefined.INTERNAL_ERROR(`No matching record found for transaction id ${txId}`); } @@ -1548,7 +1565,14 @@ export class EthImpl implements Eth { return contractResult.hash; } catch (e: any) { - return this.sendRawTransactionErrorHandler(e, transaction, transactionBuffer, txSubmitted, requestIdPrefix); + return this.sendRawTransactionErrorHandler( + e, + transaction, + transactionBuffer, + txSubmitted, + parsedTx, + requestIdPrefix, + ); } finally { /** * 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. diff --git a/packages/server/tests/acceptance/rateLimiter.spec.ts b/packages/server/tests/acceptance/rateLimiter.spec.ts index 08ee9a4635..08e50e255d 100644 --- a/packages/server/tests/acceptance/rateLimiter.spec.ts +++ b/packages/server/tests/acceptance/rateLimiter.spec.ts @@ -166,7 +166,7 @@ describe('@ratelimiter Rate Limiters Acceptance Tests', function () { await expect(relay.call(testConstants.ETH_ENDPOINTS.ETH_SEND_RAW_TRANSACTION, [signedTx], requestId)).to.be .fulfilled; const remainingHbarsAfter = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT)); - expect(remainingHbarsAfter).to.be.eq(remainingHbarsBefore); + expect(remainingHbarsAfter).to.be.lt(remainingHbarsBefore); }); it('should deploy a large contract and decrease remaining HBAR in limiter when transaction data is large', async function () { diff --git a/packages/server/tests/acceptance/rpc_batch2.spec.ts b/packages/server/tests/acceptance/rpc_batch2.spec.ts index 87859b1940..2142a32a42 100644 --- a/packages/server/tests/acceptance/rpc_batch2.spec.ts +++ b/packages/server/tests/acceptance/rpc_batch2.spec.ts @@ -810,7 +810,6 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () { let storageContract: ethers.Contract; let storageContractAddress: string; const STORAGE_CONTRACT_UPDATE = '0x2de4e884'; - const NEXT_STORAGE_CONTRACT_UPDATE = '0x160D6484'; this.beforeEach(async () => { storageContract = await Utils.deployContract( @@ -983,7 +982,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () { const transaction1 = { ...transaction, nonce: await relay.getAccountNonce(accounts[1].address), - data: NEXT_STORAGE_CONTRACT_UPDATE, + data: STORAGE_CONTRACT_UPDATE, }; const signedTx1 = await accounts[1].wallet.signTransaction(transaction1); @@ -1028,7 +1027,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () { const transaction1 = { ...transaction, nonce: await relay.getAccountNonce(accounts[1].address), - data: NEXT_STORAGE_CONTRACT_UPDATE, + data: STORAGE_CONTRACT_UPDATE, }; const signedTx1 = await accounts[1].wallet.signTransaction(transaction1);