Skip to content

feat: add MAX_GAS_ALLOWANCE_HBAR configuration #3865

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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions .env.http.example
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ OPERATOR_KEY_MAIN= # Operator private key used to sign transaction
# FEE_HISTORY_MAX_RESULTS=10 # Maximum number of results to return for eth_feeHistory
# TX_DEFAULT_GAS=400000 # Default gas for transactions that don't specify gas
# MAX_TRANSACTION_FEE_THRESHOLD=15000000 # Max transaction fee paid by relay operator account
# MAX_GAS_ALLOWANCE_HBAR=0 # The maximum amount, in hbars, that the relay is willing to pay to complete the transaction in case the senders don't provide enough funds.

# ========== CACHE CONFIGURATION ==========
# CACHE_MAX=1000 # Maximum items in cache
Expand Down
1 change: 1 addition & 0 deletions charts/hedera-json-rpc-relay/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ config:
# FEE_HISTORY_MAX_RESULTS:
# TX_DEFAULT_GAS:
# MAX_TRANSACTION_FEE_THRESHOLD:
# MAX_GAS_ALLOWANCE_HBAR:

# ========== CACHE CONFIGURATION ==========
# CACHE_MAX:
Expand Down
1 change: 1 addition & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ Unless you need to set a non-default value, it is recommended to only populate o
| `TX_DEFAULT_GAS` | "400000" | Default gas for transactions that do not specify gas. |
| `USE_ASYNC_TX_PROCESSING` | "true" | Set to `true` to enable `eth_sendRawTransaction` to return the transaction hash immediately after passing all prechecks, while processing the transaction asynchronously in the background. |
| `MAX_TRANSACTION_FEE_THRESHOLD` | "15000000" | Used to set the max transaction fee. This is the HAPI fee which is paid by the relay operator account. |
| `MAX_GAS_ALLOWANCE_HBAR` | "0" | The maximum amount, in hbars, that the relay is willing to pay to complete the transaction in case the senders don't provide enough funds to pay for their transactions. Please note, in case of full subsidized transactions, the sender must set the gas price to `0` and the relay must configure both the `GAS_PRICE_TINY_BAR_BUFFER`, to a number big enough to cover the discrepancy between the base gas price and `0`, and the `MAX_GAS_ALLOWANCE_HBAR` parameters, with the max amount of hbars the relay is willing to spend to execute the transaction. |
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about setting GAS_PRICE_TINY_BAR_BUFFER in a fully subsidized JSON-RPC Relay. It seems GAS_PRICE_TINY_BAR_BUFFER has a different purpose. We might need to change the precheck to account for the MAX_GAS_ALLOWANCE_HBAR variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The MAX_GAS_ALLOWANCE_HBAR is set by the operator, so I'm not sure it needs a dedicated pre-check.

About GAS_PRICE_TINY_BAR_BUFFER, you are right, that serve a different purpose, and that's fine.

That parameter is mentioned here only as a reference, because in case the user set gasPrice=0, a low buffer value (that is probably what you want in a classic JSON-RPC relay configuration) will block those transactions during the pre-check.

So, in case an operator want to support gasPrice=0 transactions, it must update the GAS_PRICE_TINY_BAR_BUFFER parameter also accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, in case an operator want to support gasPrice=0 transactions, it must update the GAS_PRICE_TINY_BAR_BUFFER parameter also accordingly.

Exactly. I believe this is not the best UX for operators, as we are requiring them to update the unrelated GAS_PRICE_TINY_BAR_BUFFER variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can think about improving the operators UX in general, but probably just providing a preset of config values for the different use cases they want to achieve is enough.

The GAS_PRICE_TINY_BAR_BUFFER is mentioned in the comments because it's involved in the process, but it's not a constraint to set it to a specific value. Operators can still decide to keep that value small as they prefer, and pay only for txs which costs is in that specific cost range.

BTW, the scope of this PR is very limited: it's just enable operators setting the MAX_GAS_ALLOWANCE_HBAR value as they prefer, like they do for other parameters, so I would suggest to postpone further improvements for another PR.

| `USE_MIRROR_NODE_MODULARIZED_SERVICES` | null | Controls routing of Mirror Node traffic through modularized services. When set to `true`, enables routing a percentage of traffic to modularized services. When set to `false`, ensures traffic follows the traditional non-modularized flow. When not set (i.e. `null` by default), no specific routing preference is applied. As Mirror Node gradually transitions to a fully modularized architecture across all networks, this setting will eventually default to `true`. |

## Server
Expand Down
6 changes: 6 additions & 0 deletions packages/config-service/src/services/globalConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,12 @@ const _CONFIG = {
required: false,
defaultValue: 15_000_000,
},
MAX_GAS_ALLOWANCE_HBAR: {
envName: 'MAX_GAS_ALLOWANCE_HBAR',
type: 'number',
required: false,
defaultValue: 0,
},
MEMWATCH_ENABLED: {
envName: 'MEMWATCH_ENABLED',
type: 'boolean',
Expand Down
2 changes: 2 additions & 0 deletions packages/relay/src/lib/clients/sdkClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ export class SDKClient {
),
);

ethereumTransaction.setMaxGasAllowanceHbar(ConfigService.get('MAX_GAS_ALLOWANCE_HBAR'));

return {
fileId,
txResponse: await this.executeTransaction(
Expand Down
5 changes: 3 additions & 2 deletions packages/relay/src/lib/precheck.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: Apache-2.0

import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
import { ethers, Transaction } from 'ethers';
import { Logger } from 'pino';

Expand Down Expand Up @@ -169,10 +170,10 @@
const passes = txGasPrice >= networkGasPrice || Precheck.isDeterministicDeploymentTransaction(tx);

if (!passes) {
if (constants.GAS_PRICE_TINY_BAR_BUFFER) {
if (ConfigService.get('GAS_PRICE_TINY_BAR_BUFFER')) {
// Check if failure is within buffer range (Often it's by 1 tinybar) as network gasprice calculation can change slightly.
// e.g gasPrice=1450000000000, requiredGasPrice=1460000000000, in which case we should allow users to go through and let the network check
const txGasPriceWithBuffer = txGasPrice + BigInt(constants.GAS_PRICE_TINY_BAR_BUFFER);
const txGasPriceWithBuffer = txGasPrice + BigInt(ConfigService.get('GAS_PRICE_TINY_BAR_BUFFER'));

Check warning on line 176 in packages/relay/src/lib/precheck.ts

View check run for this annotation

Codecov / codecov/patch

packages/relay/src/lib/precheck.ts#L176

Added line #L176 was not covered by tests
if (txGasPriceWithBuffer >= networkGasPrice) {
return;
}
Expand Down
52 changes: 52 additions & 0 deletions packages/server/tests/acceptance/rpc_batch1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { expect } from 'chai';
import { ethers } from 'ethers';

import { ConfigServiceTestHelper } from '../../../config-service/tests/configServiceTestHelper';
import { withOverriddenEnvsInMochaTest } from '../../../relay/tests/helpers';
import basicContract from '../../tests/contracts/Basic.json';
import RelayCalls from '../../tests/helpers/constants';
import MirrorClient from '../clients/mirrorClient';
Expand Down Expand Up @@ -1511,6 +1512,57 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () {
);
});

withOverriddenEnvsInMochaTest(
{
GAS_PRICE_TINY_BAR_BUFFER: 2000000000000,
MAX_GAS_ALLOWANCE_HBAR: 100,
},
() => {
it('should execute "eth_sendRawTransaction" and pays the total amount of the fees on behalf of the sender', async function () {
const balanceBefore = await relay.getBalance(accounts[2].wallet.address, 'latest', requestId);

const transaction = {
type: 1,
chainId: Number(CHAIN_ID),
nonce: await relay.getAccountNonce(accounts[2].wallet.address, requestId),
gasPrice: 0,
gasLimit: Constants.MAX_TRANSACTION_FEE_THRESHOLD,
data: '0x' + '00'.repeat(Constants.CONTRACT_CODE_SIZE_LIMIT),
};
const signedTx = await accounts[2].wallet.signTransaction(transaction);
const transactionHash = await relay.sendRawTransaction(signedTx, requestId);
const info = await mirrorNode.get(`/contracts/results/${transactionHash}`, requestId);
expect(info).to.have.property('contract_id');
expect(info.contract_id).to.not.be.null;
expect(info).to.have.property('created_contract_ids');
expect(info.created_contract_ids.length).to.be.equal(1);

const balanceAfter = await relay.getBalance(accounts[2].wallet.address, 'latest', requestId);
expect(balanceAfter).to.be.equal(balanceBefore);

const transaction2 = {
type: 2,
chainId: Number(CHAIN_ID),
nonce: await relay.getAccountNonce(accounts[2].wallet.address, requestId),
maxPriorityFeePerGas: 0,
maxFeePerGas: 0,
gasLimit: Constants.MAX_TRANSACTION_FEE_THRESHOLD,
data: '0x' + '00'.repeat(Constants.CONTRACT_CODE_SIZE_LIMIT),
};
const signedTx2 = await accounts[2].wallet.signTransaction(transaction2);
const transactionHash2 = await relay.sendRawTransaction(signedTx2, requestId);
const info2 = await mirrorNode.get(`/contracts/results/${transactionHash2}`, requestId);
expect(info2).to.have.property('contract_id');
expect(info2.contract_id).to.not.be.null;
expect(info2).to.have.property('created_contract_ids');
expect(info2.created_contract_ids.length).to.be.equal(1);

const balanceAfter2 = await relay.getBalance(accounts[2].wallet.address, 'latest', requestId);
expect(balanceAfter2).to.be.equal(balanceBefore);
});
},
);

describe('Prechecks', async function () {
it('should fail "eth_sendRawTransaction" for transaction with incorrect chain_id', async function () {
const transaction = {
Expand Down
Loading