Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Added rateLimiter to fileCreate. #2632

Closed
wants to merge 3 commits into from
Closed

fix: Added rateLimiter to fileCreate. #2632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c062b0b
fix: Added rateLimiter to fileCreate.
ebadiere Jun 27, 2024
01b8b2c
fix: Added the fileAppend to the HBar expence to the HBar rate limiter
ebadiere Jun 28, 2024
231c8c3
fix: Apply transaction fees in the createFile to the HBar rate limiter.
ebadiere Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 121 additions & 38 deletions packages/relay/src/lib/clients/sdkClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ export class SDKClient {
async submitEthereumTransaction(
transactionBuffer: Uint8Array,
callerName: string,
requestId?: string,
requestId: string,
): Promise<{ txResponse: TransactionResponse; fileId: FileId | null }> {
const ethereumTransactionData: EthereumTransactionData = EthereumTransactionData.fromBytes(transactionBuffer);
const ethereumTransaction = new EthereumTransaction();
Expand Down Expand Up @@ -476,6 +476,7 @@ export class SDKClient {

this.logger.info(`${requestIdPrefix} Execute ${transactionType} transaction`);
const resp = await transaction.execute(this.clientMain);

this.logger.info(
`${requestIdPrefix} ${resp.transactionId} ${callerName} ${transactionType} status: ${Status.Success} (${Status.Success._code})`,
);
Expand Down Expand Up @@ -643,60 +644,142 @@ export class SDKClient {
private createFile = async (
callData: Uint8Array,
client: Client,
requestId?: string,
callerName?: string,
interactingEntity?: string,
requestId: string,
callerName: string,
interactingEntity: string,
) => {
const requestIdPrefix = formatRequestIdMessage(requestId);
const hexedCallData = Buffer.from(callData).toString('hex');
const currentDateNow = Date.now();
let createFileId, fileCreateTx, fileAppendTx;

const fileCreateTx = await new FileCreateTransaction()
.setContents(hexedCallData.substring(0, this.fileAppendChunkSize))
.setKeys(client.operatorPublicKey ? [client.operatorPublicKey] : []);
const fileCreateTxResponse = await fileCreateTx.execute(client);
const { fileId } = await fileCreateTxResponse.getReceipt(client);

const createFileRecord = await fileCreateTxResponse.getRecord(this.clientMain);
this.captureMetrics(
SDKClient.transactionMode,
fileCreateTx.constructor.name,
Status.Success,
createFileRecord.transactionFee.toTinybars().toNumber(),
createFileRecord?.contractFunctionResult?.gasUsed,
callerName,
interactingEntity,
);
try {
const shouldLimit = this.hbarLimiter.shouldLimit(currentDateNow, SDKClient.transactionMode, callerName);
if (shouldLimit) {
throw predefined.HBAR_RATE_LIMIT_EXCEEDED;
}

if (fileId && callData.length > this.fileAppendChunkSize) {
const fileAppendTx = await new FileAppendTransaction()
.setFileId(fileId)
.setContents(hexedCallData.substring(this.fileAppendChunkSize, hexedCallData.length))
.setChunkSize(this.fileAppendChunkSize)
.setMaxChunks(this.maxChunks);
await fileAppendTx.execute(client);
const fileCreateTx = await new FileCreateTransaction()
.setContents(hexedCallData.substring(0, this.fileAppendChunkSize))
.setKeys(client.operatorPublicKey ? [client.operatorPublicKey] : []);

const fileCreateTxResponse = await fileCreateTx.execute(client);
const { fileId } = await fileCreateTxResponse.getReceipt(client);

// get transaction fee and add expense to limiter
const createFileRecord = await fileCreateTxResponse.getRecord(this.clientMain);
let transactionFee = createFileRecord.transactionFee;
this.hbarLimiter.addExpense(transactionFee.toTinybars().toNumber(), currentDateNow);

this.captureMetrics(
SDKClient.transactionMode,
fileAppendTx.constructor.name,
fileCreateTx.constructor.name,
Status.Success,
await this.calculateFileAppendTxTotalTinybarsCost(fileAppendTx),
0,
createFileRecord.transactionFee.toTinybars().toNumber(),
createFileRecord?.contractFunctionResult?.gasUsed,
callerName,
interactingEntity,
);
}

// Ensure that the calldata file is not empty
if (fileId) {
const fileSize = await (await new FileInfoQuery().setFileId(fileId).execute(client)).size;
if (fileId && callData.length > this.fileAppendChunkSize) {
const fileAppendTx = await new FileAppendTransaction()
.setFileId(fileId)
.setContents(hexedCallData.substring(this.fileAppendChunkSize, hexedCallData.length))
.setChunkSize(this.fileAppendChunkSize)
.setMaxChunks(this.maxChunks);
await fileAppendTx.execute(client);

const fileAppendTxResponse = await fileAppendTx.execute(client);

// get transaction fee and add expense to limiter
const appendFileRecord = await fileAppendTxResponse.getRecord(this.clientMain);
transactionFee = appendFileRecord.transactionFee;
this.hbarLimiter.addExpense(transactionFee.toTinybars().toNumber(), currentDateNow);

this.captureMetrics(
SDKClient.transactionMode,
fileAppendTx.constructor.name,
Status.Success,
await this.calculateFileAppendTxTotalTinybarsCost(fileAppendTx),
0,
callerName,
interactingEntity,
);
}

// Ensure that the calldata file is not empty
if (fileId) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Where's the actual logic to add the costs of the FileCreate and FileAppend to the hbar rate limiter?

const fileSize = await (await new FileInfoQuery().setFileId(fileId).execute(client)).size;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: does this have a cost associated with it?
If so can you open a ticket so we come back and see if this is needed still and more so also capture its cost in the metric and rate limits


if (callData.length > 0 && fileSize.isZero()) {
throw new SDKClientError({}, `${requestIdPrefix} Created file is empty. `);
if (callData.length > 0 && fileSize.isZero()) {
throw new SDKClientError({}, `${requestIdPrefix} Created file is empty. `);
}
this.logger.trace(`${requestIdPrefix} Created file with fileId: ${fileId} and file size ${fileSize}`);
}
this.logger.trace(`${requestIdPrefix} Created file with fileId: ${fileId} and file size ${fileSize}`);
} catch (error: any) {
const sdkClientError = new SDKClientError(error, error.message);
let transactionFee: number | Hbar = 0;

// if valid network error utilize transaction id
if (sdkClientError.isValidNetworkError()) {
try {
const transactionCreateRecord = await new TransactionRecordQuery()
.setTransactionId(fileCreateTx.transactionId!)
.setNodeAccountIds(fileCreateTx.nodeAccountIds!)
.setValidateReceiptStatus(false)
.execute(this.clientMain);
transactionFee = transactionCreateRecord.transactionFee;
this.hbarLimiter.addExpense(transactionFee.toTinybars().toNumber(), currentDateNow);

this.captureMetrics(
SDKClient.transactionMode,
fileCreateTx.constructor.name,
sdkClientError.status,
transactionFee.toTinybars().toNumber(),
transactionCreateRecord?.contractFunctionResult?.gasUsed,
callerName,
interactingEntity,
);

const transactionAppendRecord = await new TransactionRecordQuery()
.setTransactionId(fileAppendTx.transactionId!)
.setNodeAccountIds(fileAppendTx.nodeAccountIds!)
.setValidateReceiptStatus(false)
.execute(this.clientMain);
transactionFee = transactionAppendRecord.transactionFee;
this.hbarLimiter.addExpense(transactionFee.toTinybars().toNumber(), currentDateNow);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only adds the file create transaction cost.
What about the fileAppend transaction(s)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.


this.captureMetrics(
SDKClient.transactionMode,
fileCreateTx.constructor.name,
sdkClientError.status,
transactionFee.toTinybars().toNumber(),
transactionCreateRecord?.contractFunctionResult?.gasUsed,
callerName,
interactingEntity,
);

this.logger.info(
`${requestIdPrefix} ${fileCreateTx.transactionId} ${callerName} ${fileCreateTx.constructor.name} status: ${sdkClientError.status} (${sdkClientError.status._code}), cost: ${transactionFee}`,
);
} catch (err: any) {
const recordQueryError = new SDKClientError(err, err.message);
this.logger.error(
recordQueryError,
`${requestIdPrefix} Error raised during TransactionRecordQuery for ${fileCreateTx.transactionId}`,
);
}
}

this.logger.info(`${requestIdPrefix} HBAR_RATE_LIMIT_EXCEEDED cost: ${transactionFee}`);

if (error instanceof JsonRpcError) {
throw predefined.HBAR_RATE_LIMIT_EXCEEDED;
}
throw sdkClientError;
}

return fileId;
return createFileId;
};

/**
Expand Down
Loading
Loading