Skip to content

Commit 7a8743a

Browse files
Cherry pick PR #2698 (#2715)
fix: Hbar usage tracking when transaction is larger than FILE_APPEND_CHUNK_SIZE (#2698) * fix: use executeAll for fileAppend * chore: code cleanup * chore: fix typo * nit: code smell * fix: set lower hbar limit for acceptance tests * fix: addressing comments * fix: set operator key * test: check if switching operator in relay restart breaks sdkClient * chore: remove .only * fix: change operator for all tests * chore: reverted an unwanted change --------- Signed-off-by: Ivo Yankov <[email protected]> Co-authored-by: Ivo Yankov <[email protected]>
1 parent 8f380a2 commit 7a8743a

File tree

8 files changed

+243
-102
lines changed

8 files changed

+243
-102
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"acceptancetest:api_batch2": "ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@api-batch-2' --exit",
3434
"acceptancetest:api_batch3": "ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@api-batch-3' --exit",
3535
"acceptancetest:erc20": "ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@erc20' --exit",
36-
"acceptancetest:ratelimiter": "ts-mocha packages/ws-server/tests/acceptance/index.spec.ts -g '@web-socket-ratelimiter' --exit && ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@ratelimiter' --exit",
36+
"acceptancetest:ratelimiter": "ts-mocha packages/ws-server/tests/acceptance/index.spec.ts -g '@web-socket-ratelimiter' --exit && HBAR_RATE_LIMIT_TINYBAR=3000000000 ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@ratelimiter' --exit",
3737
"acceptancetest:tokencreate": "ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@tokencreate' --exit",
3838
"acceptancetest:tokenmanagement": "ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@tokenmanagement' --exit",
3939
"acceptancetest:htsprecompilev1": "ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@htsprecompilev1' --exit",

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

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -630,17 +630,6 @@ export class SDKClient {
630630
return balance.hbars.to(HbarUnit.Tinybar).multipliedBy(constants.TINYBAR_TO_WEIBAR_COEF);
631631
}
632632

633-
private async calculateFileAppendTxTotalTinybarsCost(fileAppendTx: FileAppendTransaction): Promise<number> {
634-
// @ts-ignore
635-
const fileAppendTxs = fileAppendTx._transactionIds.list.map((txId) =>
636-
new TransactionRecordQuery().setTransactionId(txId).execute(this.clientMain),
637-
);
638-
639-
return (await Promise.all(fileAppendTxs)).reduce((base, record) => {
640-
return base + record.transactionFee.toTinybars().toNumber();
641-
}, 0);
642-
}
643-
644633
private createFile = async (
645634
callData: Uint8Array,
646635
client: Client,
@@ -687,22 +676,24 @@ export class SDKClient {
687676
.setContents(hexedCallData.substring(this.fileAppendChunkSize, hexedCallData.length))
688677
.setChunkSize(this.fileAppendChunkSize)
689678
.setMaxChunks(this.maxChunks);
690-
const fileAppendTxResponse = await fileAppendTx.execute(client);
691-
692-
// get transaction fee and add expense to limiter
693-
const appendFileRecord = await fileAppendTxResponse.getRecord(this.clientMain);
694-
transactionFee = appendFileRecord.transactionFee;
695-
this.hbarLimiter.addExpense(transactionFee.toTinybars().toNumber(), currentDateNow);
696-
697-
this.captureMetrics(
698-
SDKClient.transactionMode,
699-
fileAppendTx.constructor.name,
700-
Status.Success,
701-
await this.calculateFileAppendTxTotalTinybarsCost(fileAppendTx),
702-
0,
703-
callerName,
704-
interactingEntity,
705-
);
679+
const fileAppendTxResponses = await fileAppendTx.executeAll(client);
680+
681+
for (let fileAppendTxResponse of fileAppendTxResponses) {
682+
// get transaction fee and add expense to limiter
683+
const appendFileRecord = await fileAppendTxResponse.getRecord(this.clientMain);
684+
const tinybarsCost = appendFileRecord.transactionFee.toTinybars().toNumber();
685+
686+
this.captureMetrics(
687+
SDKClient.transactionMode,
688+
fileAppendTx.constructor.name,
689+
Status.Success,
690+
tinybarsCost,
691+
0,
692+
callerName,
693+
interactingEntity,
694+
);
695+
this.hbarLimiter.addExpense(tinybarsCost, currentDateNow);
696+
}
706697
}
707698

708699
// Ensure that the calldata file is not empty

packages/server/tests/acceptance/index.spec.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import fs from 'fs';
3131
import ServicesClient from '../clients/servicesClient';
3232
import MirrorClient from '../clients/mirrorClient';
3333
import RelayClient from '../clients/relayClient';
34+
import MetricsClient from '../clients/metricsClient';
3435

3536
// Server related
3637
import app from '../../dist/server';
@@ -86,12 +87,22 @@ describe('RPC Server Acceptance Tests', function () {
8687
logger.child({ name: `services-test-client` }),
8788
);
8889
global.mirrorNode = new MirrorClient(MIRROR_NODE_URL, logger.child({ name: `mirror-node-test-client` }));
90+
global.metrics = new MetricsClient(RELAY_URL, logger.child({ name: `metrics-test-client` }));
8991
global.relay = new RelayClient(RELAY_URL, logger.child({ name: `relay-test-client` }));
9092
global.relayServer = relayServer;
9193
global.socketServer = socketServer;
9294
global.logger = logger;
9395
global.initialBalance = INITIAL_BALANCE;
9496

97+
global.restartLocalRelay = async function () {
98+
if (global.relayIsLocal) {
99+
stopRelay();
100+
await new Promise((r) => setTimeout(r, 5000)); // wait for server to shutdown
101+
102+
runLocalRelay();
103+
}
104+
};
105+
95106
before(async () => {
96107
// configuration details
97108
logger.info('Acceptance Tests Configurations successfully loaded');
@@ -152,15 +163,7 @@ describe('RPC Server Acceptance Tests', function () {
152163
const cost = startOperatorBalance.toTinybars().subtract(endOperatorBalance.toTinybars());
153164
logger.info(`Acceptance Tests spent ${Hbar.fromTinybars(cost)}`);
154165

155-
//stop relay
156-
logger.info('Stop relay');
157-
if (relayServer !== undefined) {
158-
relayServer.close();
159-
}
160-
161-
if (process.env.TEST_WS_SERVER === 'true' && socketServer !== undefined) {
162-
socketServer.close();
163-
}
166+
stopRelay();
164167
});
165168

166169
describe('Acceptance tests', async () => {
@@ -181,6 +184,18 @@ describe('RPC Server Acceptance Tests', function () {
181184
}
182185
}
183186

187+
function stopRelay() {
188+
//stop relay
189+
logger.info('Stop relay');
190+
if (relayServer !== undefined) {
191+
relayServer.close();
192+
}
193+
194+
if (process.env.TEST_WS_SERVER === 'true' && global.socketServer !== undefined) {
195+
global.socketServer.close();
196+
}
197+
}
198+
184199
function runLocalRelay() {
185200
// start local relay, relay instance in local should not be running
186201

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

Lines changed: 113 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,17 @@ import relayConstants from '../../../../packages/relay/src/lib/constants';
3030

3131
// Local resources
3232
import parentContractJson from '../contracts/Parent.json';
33+
import largeContractJson from '../contracts/EstimatePrecompileContract.json';
3334
import { Utils } from '../helpers/utils';
35+
import { predefined } from '@hashgraph/json-rpc-relay';
3436

3537
describe('@ratelimiter Rate Limiters Acceptance Tests', function () {
3638
this.timeout(480 * 1000); // 480 seconds
3739

3840
const accounts: AliasAccount[] = [];
3941

4042
// @ts-ignore
41-
const { mirrorNode, relay, logger, initialBalance } = global;
43+
const { mirrorNode, relay, logger, initialBalance, metrics } = global;
4244

4345
// cached entities
4446
let parentContractAddress: string;
@@ -90,72 +92,121 @@ describe('@ratelimiter Rate Limiters Acceptance Tests', function () {
9092
});
9193
});
9294

93-
describe('HBAR Limiter Acceptance Tests', function () {
94-
this.timeout(480 * 1000); // 480 seconds
95-
96-
this.beforeAll(async () => {
97-
requestId = Utils.generateRequestId();
98-
const requestIdPrefix = Utils.formatRequestIdMessage(requestId);
99-
100-
logger.info(`${requestIdPrefix} Creating accounts`);
101-
logger.info(`${requestIdPrefix} HBAR_RATE_LIMIT_TINYBAR: ${process.env.HBAR_RATE_LIMIT_TINYBAR}`);
102-
103-
const initialAccount: AliasAccount = global.accounts[0];
104-
105-
const neededAccounts: number = 2;
106-
accounts.push(
107-
...(await Utils.createMultipleAliasAccounts(
108-
mirrorNode,
109-
initialAccount,
110-
neededAccounts,
111-
initialBalance,
112-
requestId,
113-
)),
114-
);
115-
global.accounts.push(...accounts);
116-
117-
const parentContract = await Utils.deployContract(
118-
parentContractJson.abi,
119-
parentContractJson.bytecode,
120-
accounts[0].wallet,
121-
);
122-
123-
parentContractAddress = parentContract.target as string;
124-
global.logger.trace(`${requestIdPrefix} Deploy parent contract on address ${parentContractAddress}`);
125-
});
126-
127-
this.beforeEach(async () => {
128-
requestId = Utils.generateRequestId();
129-
});
95+
// The following tests exhaust the hbar limit, so they should only be run against a local relay
96+
if (global.relayIsLocal) {
97+
describe('HBAR Limiter Acceptance Tests', function () {
98+
before(async () => {
99+
// Restart the relay to reset the limits
100+
await global.restartLocalRelay();
101+
});
130102

131-
describe('HBAR Rate Limit Tests', () => {
132-
const defaultGasPrice = Assertions.defaultGasPrice;
133-
const defaultGasLimit = 3_000_000;
134-
135-
const defaultLondonTransactionData = {
136-
value: ONE_TINYBAR,
137-
chainId: Number(CHAIN_ID),
138-
maxPriorityFeePerGas: defaultGasPrice,
139-
maxFeePerGas: defaultGasPrice,
140-
gasLimit: defaultGasLimit,
141-
type: 2,
142-
};
103+
this.timeout(480 * 1000); // 480 seconds
104+
105+
this.beforeAll(async () => {
106+
requestId = Utils.generateRequestId();
107+
const requestIdPrefix = Utils.formatRequestIdMessage(requestId);
108+
109+
logger.info(`${requestIdPrefix} Creating accounts`);
110+
logger.info(`${requestIdPrefix} HBAR_RATE_LIMIT_TINYBAR: ${process.env.HBAR_RATE_LIMIT_TINYBAR}`);
111+
112+
const initialAccount: AliasAccount = global.accounts[0];
113+
114+
const neededAccounts: number = 2;
115+
accounts.push(
116+
...(await Utils.createMultipleAliasAccounts(
117+
mirrorNode,
118+
initialAccount,
119+
neededAccounts,
120+
initialBalance,
121+
requestId,
122+
)),
123+
);
124+
global.accounts.push(...accounts);
125+
126+
const parentContract = await Utils.deployContract(
127+
parentContractJson.abi,
128+
parentContractJson.bytecode,
129+
accounts[0].wallet,
130+
);
131+
132+
parentContractAddress = parentContract.target as string;
133+
global.logger.trace(`${requestIdPrefix} Deploy parent contract on address ${parentContractAddress}`);
134+
});
143135

144-
it('should execute "eth_sendRawTransaction" without triggering HBAR rate limit exceeded ', async function () {
145-
const gasPrice = await relay.gasPrice(requestId);
136+
this.beforeEach(async () => {
137+
requestId = Utils.generateRequestId();
138+
});
146139

147-
const transaction = {
148-
...defaultLondonTransactionData,
149-
to: parentContractAddress,
150-
nonce: await relay.getAccountNonce(accounts[1].address, requestId),
151-
maxPriorityFeePerGas: gasPrice,
152-
maxFeePerGas: gasPrice,
140+
describe('HBAR Rate Limit Tests', () => {
141+
const defaultGasPrice = Assertions.defaultGasPrice;
142+
const defaultGasLimit = 3_000_000;
143+
144+
const defaultLondonTransactionData = {
145+
value: ONE_TINYBAR,
146+
chainId: Number(CHAIN_ID),
147+
maxPriorityFeePerGas: defaultGasPrice,
148+
maxFeePerGas: defaultGasPrice,
149+
gasLimit: defaultGasLimit,
150+
type: 2,
153151
};
154-
const signedTx = await accounts[1].wallet.signTransaction(transaction);
155152

156-
await expect(relay.call(testConstants.ETH_ENDPOINTS.ETH_SEND_RAW_TRANSACTION, [signedTx], requestId)).to.be
157-
.fulfilled;
153+
it('should execute "eth_sendRawTransaction" without triggering HBAR rate limit exceeded', async function () {
154+
const gasPrice = await relay.gasPrice(requestId);
155+
const remainingHbarsBefore = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT));
156+
157+
const transaction = {
158+
...defaultLondonTransactionData,
159+
to: parentContractAddress,
160+
nonce: await relay.getAccountNonce(accounts[1].address, requestId),
161+
maxPriorityFeePerGas: gasPrice,
162+
maxFeePerGas: gasPrice,
163+
};
164+
const signedTx = await accounts[1].wallet.signTransaction(transaction);
165+
166+
await expect(relay.call(testConstants.ETH_ENDPOINTS.ETH_SEND_RAW_TRANSACTION, [signedTx], requestId)).to.be
167+
.fulfilled;
168+
const remainingHbarsAfter = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT));
169+
expect(remainingHbarsAfter).to.be.eq(remainingHbarsBefore);
170+
});
171+
172+
it('should deploy a large contract and decrease remaining HBAR in limiter when transaction data is large', async function () {
173+
const remainingHbarsBefore = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT));
174+
expect(remainingHbarsBefore).to.be.gt(0);
175+
176+
const largeContract = await Utils.deployContract(
177+
largeContractJson.abi,
178+
largeContractJson.bytecode,
179+
accounts[0].wallet,
180+
);
181+
await largeContract.waitForDeployment();
182+
const remainingHbarsAfter = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT));
183+
expect(largeContract.target).to.not.be.null;
184+
expect(remainingHbarsAfter).to.be.lt(remainingHbarsBefore);
185+
});
186+
187+
it('multiple deployments of large contracts should eventually exhaust the remaining hbar limit', async function () {
188+
const remainingHbarsBefore = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT));
189+
expect(remainingHbarsBefore).to.be.gt(0);
190+
try {
191+
for (let i = 0; i < 50; i++) {
192+
const largeContract = await Utils.deployContract(
193+
largeContractJson.abi,
194+
largeContractJson.bytecode,
195+
accounts[0].wallet,
196+
);
197+
await largeContract.waitForDeployment();
198+
expect(largeContract.target).to.not.be.null;
199+
}
200+
201+
expect(true).to.be.false;
202+
} catch (e: any) {
203+
expect(e.message).to.contain(predefined.HBAR_RATE_LIMIT_EXCEEDED.message);
204+
}
205+
206+
const remainingHbarsAfter = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT));
207+
expect(remainingHbarsAfter).to.be.lte(0);
208+
});
158209
});
159210
});
160-
});
211+
}
161212
});

0 commit comments

Comments
 (0)