Skip to content

Commit

Permalink
fix: reduce hbar limit only by operator fees (#2755)
Browse files Browse the repository at this point in the history
* fix: reduce hbar limit only by operator fees

Signed-off-by: Ivo Yankov <[email protected]>

* chore: update expected values

Signed-off-by: Ivo Yankov <[email protected]>

* chore: fix code smells

Signed-off-by: Ivo Yankov <[email protected]>

* nit: re-trigger ci

Signed-off-by: Ivo Yankov <[email protected]>

* chore: addressing comments

Signed-off-by: Ivo Yankov <[email protected]>

* fix: hbar limit error handle

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* nit: update comment

Signed-off-by: Ivo Yankov <[email protected]>

* fix: unit test mocks

Signed-off-by: Ivo Yankov <[email protected]>

* fix: update assertions and mocks

Signed-off-by: Ivo Yankov <[email protected]>

---------

Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Co-authored-by: Eric Badiere <[email protected]>
Signed-off-by: ebadiere <[email protected]>
  • Loading branch information
Ivo-Yankov and ebadiere committed Aug 1, 2024
1 parent 78e6653 commit 54fea0c
Show file tree
Hide file tree
Showing 8 changed files with 234 additions and 19 deletions.
11 changes: 11 additions & 0 deletions packages/relay/src/formatters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,16 @@ const isHex = (value: string): boolean => {
return hexRegex.test(value);
};

// Returns the sum of all transfer amounts for the specified account. The amount is negative if the account is charged,
// it is positive if the account is receiving it, thus the amount is first negated and then added to the sum.
const getTransferAmountSumForAccount = (transactionRecord, accountId: string): number => {
return transactionRecord.transfers
.filter((transfer) => transfer.accountId.toString() === accountId)
.reduce((acc, transfer) => {
return BN.sum(acc, transfer.amount.toTinybars().negate()).toNumber();
}, 0);
};

export {
hashNumber,
formatRequestIdMessage,
Expand All @@ -317,4 +327,5 @@ export {
isValidEthereumAddress,
isHex,
ASCIIToHex,
getTransferAmountSumForAccount,
};
6 changes: 4 additions & 2 deletions packages/relay/src/lib/clients/sdkClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import {
} from '@hashgraph/sdk';
import { BigNumber } from '@hashgraph/sdk/lib/Transfer';
import { Logger } from 'pino';
import { formatRequestIdMessage } from '../../formatters';
import { formatRequestIdMessage, getTransferAmountSumForAccount } from '../../formatters';
import HbarLimit from '../hbarlimiter';
import constants from './../constants';
import { SDKClientError } from './../errors/SDKClientError';
Expand Down Expand Up @@ -562,6 +562,8 @@ export class SDKClient {
// 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;
} else if (e instanceof JsonRpcError) {
throw e;
} else {
if (!transactionResponse) {
throw predefined.INTERNAL_ERROR(
Expand Down Expand Up @@ -700,7 +702,7 @@ export class SDKClient {
const transactionRecord: TransactionRecord = await transactionResponse.getRecord(this.clientMain);

// get transactionFee and gasUsed for metrics
transactionFee = transactionRecord.transactionFee.toTinybars().toNumber();
transactionFee = getTransferAmountSumForAccount(transactionRecord, this.clientMain.operatorAccountId!.toString());
gasUsed = transactionRecord.contractFunctionResult
? transactionRecord.contractFunctionResult.gasUsed.toNumber()
: 0;
Expand Down
5 changes: 3 additions & 2 deletions packages/relay/tests/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ export const contractId1 = '0.0.5001';
export const contractId2 = '0.0.5002';
export const signedTransactionHash =
'0x02f87482012a0485a7a358200085a7a3582000832dc6c09400000000000000000000000000000000000003f78502540be40080c001a006f4cd8e6f84b76a05a5c1542a08682c928108ef7163d9c1bf1f3b636b1cd1fba032097cbf2dda17a2dcc40f62c97964d9d930cdce2e8a9df9a8ba023cda28e4ad';
export const LONG_ZERO_ADDRESS = '0x0000000000000000000000000000000000000557';

export const defaultBlock = {
count: blockTransactionCount,
Expand Down Expand Up @@ -435,7 +436,7 @@ export const defaultContractResults = {
contract_id: '0.0.1375',
created_contract_ids: ['0.0.1375'],
error_message: null,
from: '0x0000000000000000000000000000000000000557',
from: LONG_ZERO_ADDRESS,
function_parameters: '0x',
gas_limit: maxGasLimit,
gas_used: gasUsed1,
Expand Down Expand Up @@ -467,7 +468,7 @@ export const defaultContractResults = {
contract_id: '0.0.1374',
created_contract_ids: [],
error_message: null,
from: '0x0000000000000000000000000000000000000557',
from: LONG_ZERO_ADDRESS,
function_parameters:
'0x2b6adf430000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000084865792c204d6121000000000000000000000000000000000000000000000000',
gas_limit: maxGasLimit - 1000,
Expand Down
46 changes: 44 additions & 2 deletions packages/relay/tests/lib/eth/eth-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,15 @@
* limitations under the License.
*
*/
import { defaultEvmAddress, defaultLogs1, defaultLogs2, defaultLogs3, mockData, toHex } from '../../helpers';
import {
defaultEvmAddress,
defaultLogs1,
defaultLogs2,
defaultLogs3,
mockData,
toHex,
LONG_ZERO_ADDRESS,
} from '../../helpers';
import { numberTo0x } from '../../../dist/formatters';
import constants from '../../../src/lib/constants';

Expand Down Expand Up @@ -436,7 +444,7 @@ export const DEFAULT_CONTRACT_RES_REVERT = {
created_contract_ids: [],
error_message:
'0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000',
from: '0x0000000000000000000000000000000000000557',
from: LONG_ZERO_ADDRESS,
function_parameters: '0x',
gas_limit: MAX_GAS_LIMIT,
gas_used: GAS_USED_1,
Expand Down Expand Up @@ -682,6 +690,40 @@ export const CONTRACTS_LOGS_WITH_FILTER = `contracts/${CONTRACT_ADDRESS_1}/resul
export const CONTRACT_RESULTS_LOGS_WITH_FILTER_URL = `contracts/results/logs?timestamp=gte:${DEFAULT_BLOCK.timestamp.from}&timestamp=lte:${DEFAULT_BLOCK.timestamp.to}&limit=100&order=asc`;
export const BLOCKS_LIMIT_ORDER_URL = 'blocks?limit=1&order=desc';
export const CONTRACTS_RESULTS_NEXT_URL = `contracts/results?timestamp=lte:${DEFAULT_BLOCK.timestamp.to}&timestamp=gte:${DEFAULT_BLOCK.timestamp.from}&limit=100&order=asc`; // just flip the timestamp parameters for simplicity
export const ACCOUNT_WITHOUT_TRANSACTIONS = `accounts/${LONG_ZERO_ADDRESS}?transactions=false`;
export const contractByEvmAddress = (evmAddress: string) => `contracts/${evmAddress}`;

export const MOCK_ACCOUNT_WITHOUT_TRANSACTIONS = {
account: '0.0.1367',
alias: null,
auto_renew_period: 105825166,
balance: {
balance: 350074689935,
timestamp: '1722499895.340270625',
tokens: [],
},
created_timestamp: '1706812520.644859499',
decline_reward: false,
deleted: false,
ethereum_nonce: 0,
evm_address: LONG_ZERO_ADDRESS,
expiry_timestamp: '1812637686.644859499',
key: {
_type: 'ED25519',
key: 'e06b22e0966108fa5d63fc6ae53f9824319b891cd4d6050dbf2b242be7e13344',
},
max_automatic_token_associations: 0,
memo: '',
pending_reward: 0,
receiver_sig_required: false,
staked_account_id: null,
staked_node_id: null,
stake_period_start: null,
transactions: [],
links: {
next: null,
},
};

//responce objects
export const MOCK_BLOCK_NUMBER_1000_RES = {
Expand Down
13 changes: 12 additions & 1 deletion packages/relay/tests/lib/eth/eth_getBlockByHash.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { SDKClient } from '../../../src/lib/clients';
import RelayAssertions from '../../assertions';
import { numberTo0x } from '../../../dist/formatters';
import {
ACCOUNT_WITHOUT_TRANSACTIONS,
BLOCK_HASH,
BLOCK_HASH_PREV_TRIMMED,
BLOCK_HASH_TRIMMED,
Expand All @@ -45,10 +46,13 @@ import {
CONTRACT_TIMESTAMP_1,
CONTRACT_TIMESTAMP_2,
DEFAULT_BLOCK,
DEFAULT_CONTRACT,
DEFAULT_ETH_GET_BLOCK_BY_LOGS,
DEFAULT_NETWORK_FEES,
LINKS_NEXT_RES,
MOCK_ACCOUNT_WITHOUT_TRANSACTIONS,
NO_SUCH_BLOCK_EXISTS_RES,
contractByEvmAddress,
} from './eth-config';
import { generateEthTestEnv } from './eth-helpers';

Expand All @@ -75,6 +79,13 @@ describe('@ethGetBlockByHash using MirrorNode', async function () {
sdkClientStub = sinon.createStubInstance(SDKClient);
getSdkClientStub = sinon.stub(hapiServiceInstance, 'getSDKClient').returns(sdkClientStub);
restMock.onGet('network/fees').reply(200, DEFAULT_NETWORK_FEES);
restMock.onGet(ACCOUNT_WITHOUT_TRANSACTIONS).reply(200, MOCK_ACCOUNT_WITHOUT_TRANSACTIONS);
restMock
.onGet(contractByEvmAddress(CONTRACT_ADDRESS_1))
.reply(200, { ...DEFAULT_CONTRACT, evmAddress: CONTRACT_ADDRESS_1 });
restMock
.onGet(contractByEvmAddress(CONTRACT_ADDRESS_2))
.reply(200, { ...DEFAULT_CONTRACT, evmAddress: CONTRACT_ADDRESS_2 });

currentMaxBlockRange = Number(process.env.ETH_GET_TRANSACTION_COUNT_MAX_BLOCK_RANGE);
process.env.ETH_GET_TRANSACTION_COUNT_MAX_BLOCK_RANGE = '1';
Expand Down Expand Up @@ -265,7 +276,7 @@ describe('@ethGetBlockByHash using MirrorNode', async function () {
.onGet(
`contracts/results?timestamp=gte:${randomBlock.timestamp.from}&timestamp=lte:${randomBlock.timestamp.to}&limit=100&order=asc`,
)
.abortRequestOnce();
.abortRequest();
await RelayAssertions.assertRejection(predefined.INTERNAL_ERROR(), ethImpl.getBlockByHash, false, ethImpl, [
BLOCK_HASH,
false,
Expand Down
43 changes: 41 additions & 2 deletions packages/relay/tests/lib/sdkClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2087,18 +2087,57 @@ describe('SdkClient', async function () {
getReceipt: (_client: NodeClient) => Promise.resolve(transactionReceipt),
getRecord: (_client: NodeClient) => {
let transactionFee: number;
let transfers: any = [];
switch (transactionType) {
case 'FileCreateTransaction':
transactionFee = fileCreateFee;
transfers = [
{
accountId: process.env.OPERATOR_ID_MAIN,
amount: Hbar.fromTinybars(-1 * fileCreateFee),
is_approval: false,
},
];
break;
case 'FileAppendTransaction':
transactionFee = fileAppendFee;
transfers = [
{
accountId: process.env.OPERATOR_ID_MAIN,
amount: Hbar.fromTinybars(-1 * fileAppendFee),
is_approval: false,
},
];
break;
case 'FileDeleteTransaction':
transactionFee = fileDeleteFee;
transfers = [
{
accountId: process.env.OPERATOR_ID_MAIN,
amount: Hbar.fromTinybars(-1 * fileDeleteFee),
is_approval: false,
},
];
break;
default:
transactionFee = defaultTransactionFee;
transfers = [
{
accountId: '0.0.800',
amount: Hbar.fromTinybars(defaultTransactionFee),
is_approval: false,
},
{
accountId: process.env.OPERATOR_ID_MAIN,
amount: Hbar.fromTinybars(-1 * defaultTransactionFee),
is_approval: false,
},
{
accountId: accountId.toString(),
amount: Hbar.fromTinybars(-1 * defaultTransactionFee),
is_approval: false,
},
];
break;
}
return Promise.resolve({
Expand All @@ -2107,6 +2146,7 @@ describe('SdkClient', async function () {
contractFunctionResult: {
gasUsed: Long.fromNumber(10000),
},
transfers,
});
},
}) as unknown as TransactionResponse;
Expand Down Expand Up @@ -2154,8 +2194,7 @@ describe('SdkClient', async function () {

it('should rate limit before creating file and add expenses to limiter for large transaction data', async () => {
const fileAppendChunks = Math.min(MAX_CHUNKS, Math.ceil(transactionBuffer.length / FILE_APPEND_CHUNK_SIZE));

const queryStub = sinon.stub(Query.prototype, 'execute').resolves(fileInfo);
const queryStub = sinon.stub(FileInfoQuery.prototype, 'execute').resolves(fileInfo);
const transactionStub = sinon
.stub(EthereumTransaction.prototype, 'execute')
.resolves(getTransactionResponse('EthereumTransaction'));
Expand Down
Loading

0 comments on commit 54fea0c

Please sign in to comment.