hashgraph / hedera-services

Crypto, token, consensus, file, and smart contract services for the Hedera public ledger
Apache License 2.0
313 stars 136 forks source link

Gas calculation for precompiles incorrectly varies with HAPI exchange rate #10828

Closed lukelee-sl closed 1 month ago

lukelee-sl commented 10 months ago

Description

Gas calculation appears to vary with hbar exchange rate, compromising Hedera's policy of fixed fee gas costs.

Steps to reproduce

Run the following unit test in the build:

private static final long GAS_COST_IN_TINYCENT = 852;

@Test
void exchangeRateChangesCausesUnstableGasCost() {
    final var mintFeeTinycents = 10000123L;
    final var fixedHbarEquiv = 30000;
    final var pre0900CentEquiv = 269100;
    final var post0900CentEquiv = 269232;
    System.out.println("Pre-09:00 gas cost: " + gasCostGiven(mintFeeTinycents, fixedHbarEquiv, pre0900CentEquiv));
    System.out.println("Post-09:00 gas cost: " + gasCostGiven(mintFeeTinycents, fixedHbarEquiv, post0900CentEquiv));
}

private long gasCostGiven(final long tinycentFee, final int hbarEquiv, final int centEquiv) {
    final var rate = ExchangeRate.newBuilder().setHbarEquiv(hbarEquiv).setCentEquiv(centEquiv).build();
    final var gasPriceInTinybar = getTinybarsFromTinyCents(rate, GAS_COST_IN_TINYCENT);
    final var feeInTinbyar = getTinybarsFromTinyCents(rate, tinycentFee);
    final var baseGasCost = (feeInTinbyar + gasPriceInTinybar - 1L) / gasPriceInTinybar;
    return baseGasCost + (baseGasCost / 5L);
}

with the following exchange rates:

At 08:59 CST
curl -X 'GET' \
  'https://mainnet-public.mirrornode.hedera.com/api/v1/network/exchangerate?timestamp=1703343540' \
  -H 'accept: application/json'
and
At 09:01 CST
curl -X 'GET' \
  'https://mainnet-public.mirrornode.hedera.com/api/v1/network/exchangerate?timestamp=1703343660' \
  -H 'accept: application/json'

Additional context

Proof on mainnet for same function call at two different times (with two different exchange rates): fail https://hashscan.io/mainnet/transaction/1703343586.545013838 success https://hashscan.io/mainnet/transaction/1703344296.632845003 fail gas: 14,232 success gas: 14,226

Impact

dApps cannot be certain the gas consumed by a smart contract function call when the gas changes with HAPI exchange rate. This compromises Hedera's stated policy that fees are denominated in USD.

Hedera network

mainnet

Version

v0.45.0

Operating system

None

Nana-EC commented 9 months ago

We should ensure documentation around this occurrence is clear for community

MrValioBg commented 2 months ago

The following table represents values from before and after the fix for Precision Loss.

Since before, the gas was changing with the exchange rate, the table represents a test with a fixed exchange rate. Additionally, after the changes, the "Gas Now" value should be constant, no matter what the exchange rate is.

Exchange rate tested with: hbarEquiv = "1"; centEquiv = "12"; 1hbar = ~0.12$

Keep in mind that since the current behavior is unstable and increasing the price, for example to hbarEquiv = 1; centEquiv = 39 can lead to getting 1–25000 more gas units.

The following gas includes the intrinsic gas + cost for the contractCall to the contract that interacts with the HTS system contract. However, it should be pretty similar for every operation, so we can easily track the delta.

System Contract Previous Gas Gas now Diff %
approve() -> Redirect/ERC 733069 734267 0.16%
approve() 730664 731862 0.16%
LazyCreate via Constructor 97484 97484 0%
transferFrom() -> on the ERC20 Contract + lazy create 749543 750754 0.16%
createFungibleTokenWithCustomFees() 164204 164204 0%
createNonFungibleTokenWithCustomFees() 168742 168742 0%
createFungibleToken() 164965 164965 0%
createNonFungibleToken() 166112 166112 0%
cryptoTransfer() -> hBar only 32105 33304 3.7%
cryptoTransfer() -> Fungible - 47085
cryptoTransfer() -> Fungible + Internal Auto-Associate 750112 752509 0.31%
cryptoTransfer() -> Non-Fungible - 44666
cryptoTransfer() -> Non-Fungible + Internal Auto-Associate 747693 750090 0.32%
transferNFTs() -> With Internal Auto-associate 748151 750548 0.32%
transferNFTs() 43925 45124 2.72%
transferNFT() 41036 42235 2.92%
transferToken() 40970 42169 2.92%
transferToken() -> With Internal Auto-Associate 745196 747593 0.32%
transferFrom() 41065 42264 2.91%
transferFrom() -> ERC 43534 44733 2.75%
burn() 14085 15284 8.51%
mint() 14085 15284 8.51%
balanceOf() 27567 30074 9.09%
name() 27700 30207 9.05%
getTokenCustomFees() 28914 31421 8.67%
getTokenInfo() 76298 78805 3.28%
View Ops 100 2607 ~ $0.0001 2600%

Note: For view functions, we now calculate using the actual price from the canonical price, converted into gas, rather than just using a fixed 100 gas. All of the view functions currently use the BASE price for TokenGetInfo from canonical-prices.json.

This leads to an increase in the gas cost for the view operation from 100 to approximately 2600. ( In the table above we see higher values for view calls, because of the intrinsic/call gas cost.)