Skip to content

Commit

Permalink
fix: captured transactionFee in metrics and HBAR limiter class in exe…
Browse files Browse the repository at this point in the history 
…cuteTransaction and deleteFile (#2714)

* fix: captured transaction fee in metrics and hbar limiter class for executeTransaction

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

* fix: captured transaction fee in hbar limiter class for deleteFile

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

* fix: improved the logic of capturing transaction fees to metrics in executeTransaction

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

* fix: improved error handling logic for sendRawTransaction

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

fix: added polling logic to retrieve updated account nonce for WRONG_NONCE error handler

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

* fix: removed non-existed NEXT_STORAGE_CONTRACT_UPDATE singature hash

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

* fix: added logic to throw WRONG_NONCE error

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

* fix: tweaked and reused executeGetTransactionRecord

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

* fix: fixed http rate limiter failing test

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

* fix: reverted shouldLimit check

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>

---------

Signed-off-by: Logan Nguyen <logan.nguyen@swirldslabs.com>
  • Loading branch information
@quiet-node
quiet-node committed Jul 19, 2024
1 parent c9e17a3 commit 46c5a76
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 87 deletions.
3 changes: 3 additions & 0 deletions packages/relay/src/lib/clients/mirrorNodeClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
149 changes: 81 additions & 68 deletions packages/relay/src/lib/clients/sdkClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -526,87 +536,88 @@ export class SDKClient {
};

async executeGetTransactionRecord(
resp: TransactionResponse,
transactionName: string,
transactionResponse: TransactionResponse,
callerName: string,
interactingEntity: string,
txConstructorName: string,
requestId?: string,
): Promise<TransactionRecord> {
) {
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,
);
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
54 changes: 39 additions & 15 deletions packages/relay/src/lib/eth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1438,6 +1438,7 @@ export class EthImpl implements Eth {
transaction,
transactionBuffer,
txSubmitted,
parsedTx,
requestIdPrefix,
): Promise<string | JsonRpcError> {
this.logger.error(
Expand All @@ -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) {
Expand Down Expand Up @@ -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}`);
}

Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion packages/server/tests/acceptance/rateLimiter.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
5 changes: 2 additions & 3 deletions packages/server/tests/acceptance/rpc_batch2.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 46c5a76

Please sign in to comment.