Skip to content

Commit

Permalink
fix: Precision loss for gas calculation of HTS system contracts v2 (#…
Browse files Browse the repository at this point in the history
…15446)

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
  • Loading branch information
stoyanov-st authored Sep 13, 2024
1 parent 68fd768 commit 1a31fa6
Show file tree
Hide file tree
Showing 29 changed files with 894 additions and 128 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ public record ContractsConfig(
boolean systemContractUpdateCustomFeesEnabled,
@ConfigProperty(value = "systemContract.tokenInfo.v2.enabled", defaultValue = "false") @NetworkProperty
boolean systemContractTokenInfoV2Enabled,
@ConfigProperty(value = "systemContract.precisionLossFixForGas.enabled", defaultValue = "true") @NetworkProperty
boolean isGasPrecisionLossFixEnabled,
@ConfigProperty(value = "systemContract.canonicalViewGas.enabled", defaultValue = "true") @NetworkProperty
boolean isCanonicalViewGasEnabled,
@ConfigProperty(value = "evm.version.dynamic", defaultValue = "false") @NetworkProperty
boolean evmVersionDynamic,
@ConfigProperty(value = "evm.allowCallsToNonContractAccounts", defaultValue = "true") @NetworkProperty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.hedera.node.app.service.contract.impl.state.ProxyWorldUpdater;
import com.hedera.node.app.service.contract.impl.state.ScopedEvmFrameStateFactory;
import com.hedera.node.app.spi.workflows.QueryContext;
import com.hedera.node.config.data.ContractsConfig;
import com.hedera.node.config.data.HederaConfig;
import dagger.Binds;
import dagger.Module;
Expand All @@ -57,8 +58,9 @@ static HederaConfig provideHederaConfig(@NonNull final QueryContext context) {

@Provides
@QueryScope
static TinybarValues provideTinybarValues(@NonNull final ExchangeRate exchangeRate) {
return TinybarValues.forQueryWith(exchangeRate);
static TinybarValues provideTinybarValues(
@NonNull final ExchangeRate exchangeRate, @NonNull final QueryContext context) {
return TinybarValues.forQueryWith(exchangeRate, context.configuration().getConfigData(ContractsConfig.class));
}

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,13 @@ static FeatureFlags provideFeatureFlags(@NonNull final TransactionProcessor proc
static TinybarValues provideTinybarValues(
@TopLevelResourcePrices @NonNull final FunctionalityResourcePrices topLevelResourcePrices,
@ChildTransactionResourcePrices @NonNull final FunctionalityResourcePrices childTransactionResourcePrices,
@NonNull final ExchangeRate exchangeRate) {
return TinybarValues.forTransactionWith(exchangeRate, topLevelResourcePrices, childTransactionResourcePrices);
@NonNull final ExchangeRate exchangeRate,
@NonNull final HandleContext context) {
return TinybarValues.forTransactionWith(
exchangeRate,
context.configuration().getConfigData(ContractsConfig.class),
topLevelResourcePrices,
childTransactionResourcePrices);
}

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ public long logOperationGasCost(
final var hevmGasCost = gasCostOfStoring(
logSize(numTopics, dataLength),
lifetime,
tinybarValues.topLevelTinybarRbhPrice(),
tinybarValues.topLevelTinybarGasPrice());
tinybarValues.topLevelTinycentRbhPrice(),
tinybarValues.topLevelTinycentGasPrice());

return Math.max(evmGasCost, hevmGasCost);
}
Expand All @@ -104,15 +104,15 @@ public long getEdSignatureVerificationSystemContractGasCost() {

/**
* Logically, would return the gas cost of storing the given number of bytes for the given number of seconds,
* given the relative prices of a byte-hour and a gas unit in tinybar.
* given the relative prices of a byte-hour and a gas unit in tinycent.
*
* <p>But for differential testing, ignores the {@code numBytes} and returns the gas cost of storing just a
* single byte for the given number of seconds.
*
* @param numBytes ignored
* @param lifetime the number of seconds to store a single byte
* @param rbhPrice the price of a byte-hour in tinybar
* @param gasPrice the price of a gas unit in tinybar
* @param rbhPrice the price of a byte-hour in tinycent
* @param gasPrice the price of a gas unit in tinycent
* @return the gas cost of storing a single byte for the given number of seconds
*/
private static long gasCostOfStoring(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public enum DispatchType {
WIPE_NFT(HederaFunctionality.TOKEN_ACCOUNT_WIPE, TOKEN_NON_FUNGIBLE_UNIQUE),
UPDATE(HederaFunctionality.TOKEN_UPDATE, DEFAULT),
UTIL_PRNG(HederaFunctionality.UTIL_PRNG, DEFAULT),
TOKEN_INFO(HederaFunctionality.TOKEN_GET_INFO, DEFAULT),
UPDATE_TOKEN_CUSTOM_FEES(HederaFunctionality.TOKEN_FEE_SCHEDULE_UPDATE, DEFAULT);

private final HederaFunctionality functionality;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.hedera.node.app.service.contract.impl.exec.gas;

import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.FEE_SCHEDULE_UNITS_PER_TINYCENT;
import static java.util.Objects.requireNonNull;

import com.hedera.hapi.node.base.AccountID;
Expand All @@ -29,6 +30,12 @@
public class SystemContractGasCalculator {
private static final long FIXED_VIEW_GAS_COST = 100L;

// This represents the predefined gas price which is $0.000_000_0852 per unit of gas.
// 852_000 = $0.000_000_0852 * 100(cents per dollars) * 100_000_000 (tiny cents per cents) * 1000 (Fee schedule
// units
// per tiny cents). For more info -> https://hedera.com/blog/rolling-smart-contract-hedera-api-fees-into-gas-fees
private static final long FIXED_TINY_CENT_GAS_PRICE_COST = 852_000L;

private final TinybarValues tinybarValues;
private final CanonicalDispatchPrices dispatchPrices;
private final ToLongBiFunction<TransactionBody, AccountID> feeCalculator;
Expand Down Expand Up @@ -57,68 +64,85 @@ public long gasRequirement(
@NonNull final AccountID payer) {
requireNonNull(body);
requireNonNull(dispatchType);
return gasRequirement(body, payer, canonicalPriceInTinybars(dispatchType));
requireNonNull(payer);
// isGasPrecisionLossFixEnabled is a temporary feature flag that will be removed in the future.
if (!tinybarValues.isGasPrecisionLossFixEnabled()) {
return gasRequirementOldWithPrecisionLoss(body, payer, canonicalPriceInTinybars(dispatchType));
}
return gasRequirementWithTinycents(body, payer, dispatchPrices.canonicalPriceInTinycents(dispatchType));
}

/**
* Given a transaction body whose dispatch gas requirement must be at least equivalent to a given minimum
* amount of tinybars, returns the gas requirement for the transaction to be dispatched.
*
* @param body the transaction body to be dispatched
* Compares the canonical price and the feeCalculator's calculated price and uses the maximum of the two to
* calculate the gas requirement and returns it.
* @param body the transaction body
* @param payer the payer of the transaction
* @param minimumPriceInTinybars the minimum equivalent tinybar cost of the dispatch
* @return the gas requirement for the transaction to be dispatched
* @param minimumPriceInTinycents the minimum price in tiny cents
* @return the gas requirement for the transaction
*/
public long gasRequirement(
@NonNull final TransactionBody body, @NonNull final AccountID payer, final long minimumPriceInTinybars) {
final var nominalPriceInTinybars = feeCalculator.applyAsLong(body, payer);
final var priceInTinybars = Math.max(minimumPriceInTinybars, nominalPriceInTinybars);
return asGasRequirement(priceInTinybars);
public long gasRequirementWithTinycents(
@NonNull final TransactionBody body, @NonNull final AccountID payer, final long minimumPriceInTinycents) {
// If not enabled, make the calculation using the old method.
if (!tinybarValues.isGasPrecisionLossFixEnabled()) {
return gasRequirementOldWithPrecisionLoss(body, payer, minimumPriceInTinycents);
}
final var computedPriceInTinybars = feeCalculator.applyAsLong(body, payer);
final var priceInTinycents =
Math.max(minimumPriceInTinycents, tinybarValues.asTinycents(computedPriceInTinybars));
// For the rare cases where computedPrice > minimumPriceInTinycents:
// Precision loss may occur as we convert between tinyBars and tinycents, but it is typically negligible.
// The minimal computed price is > 1e6 tinycents, ensuring enough precision.
// In most cases, the gas difference is zero.
// In scenarios where we compare significant price fluctuations (200x, 100x), the gas difference should still be
// unlikely to exceed 0 gas.
return gasRequirementFromTinycents(priceInTinycents, tinybarValues.childTransactionTinycentGasPrice());
}

/**
* Given a transaction body whose, returns the gas requirement for the transaction to be
* dispatched using the gas price of the top-level HAPI operation and no markup.
* Returns the gas price for the top-level HAPI operation.
*
* @param body the transaction body to be dispatched
* @param payer the payer of the transaction
* @return the gas requirement for the transaction to be dispatched
* @return the gas price for the top-level HAPI operation in tinyBars.
*/
public long topLevelGasRequirement(@NonNull final TransactionBody body, @NonNull final AccountID payer) {
return feeCalculator.applyAsLong(body, payer) / tinybarValues.topLevelTinybarGasPrice();
public long topLevelGasPriceInTinyBars() {
return tinybarValues.topLevelTinybarGasPriceFullPrecision();
}

/**
* Returns the gas price for the top-level HAPI operation.
* Estimates the gas requirement for a view operation.
* The minimum gas requirement is 100 gas.
* For all view operations, the gas requirement is determined using the canonical gas value
* for the TOKEN_INFO dispatch type, as specified in the canonical-prices.json.
* The TOKEN_INFO operation is representative of view operations.
*
* @return the gas price for the top-level HAPI operation
* @return the gas requirement for a view operation
*/
public long topLevelGasPrice() {
return tinybarValues.topLevelTinybarGasPrice();
public long viewGasRequirement() {
// isCanonicalViewGasEnabled is a temporary feature flag that will be removed in the future.
if (!tinybarValues.isCanonicalViewGasEnabled()) {
return FIXED_VIEW_GAS_COST;
}
final var gasRequirement = gasRequirementFromTinycents(
dispatchPrices.canonicalPriceInTinycents(DispatchType.TOKEN_INFO), FIXED_TINY_CENT_GAS_PRICE_COST);
return Math.max(FIXED_VIEW_GAS_COST, gasRequirement);
}

/**
* Given a dispatch type, returns the canonical gas requirement for that dispatch type.
* Useful when providing a ballpark gas requirement in the absence of a valid
* transaction body for the dispatch type.
* Used for non-query operations.
*
* @param dispatchType the dispatch type
* @return the canonical gas requirement for that dispatch type
*/
public long canonicalGasRequirement(@NonNull final DispatchType dispatchType) {
return asGasRequirement(canonicalPriceInTinybars(dispatchType));
}

/**
* Although mono-service compares the fixed {@code 100 gas} cost to the implied gas requirement of a
* stand-in {@link com.hedera.hapi.node.transaction.TransactionGetRecordQuery}, this stand-in query's
* cost will be less than {@code 100 gas} at any exchange rate that sustains the existence of the network.
* So for simplicity, we drop that comparison and just return the fixed {@code 100 gas} cost.
*
* @return the minimum gas requirement for a view query
*/
public long viewGasRequirement() {
return FIXED_VIEW_GAS_COST;
// isGasPrecisionLossFixEnabled is a temporary feature flag that will be removed in the future.
if (!tinybarValues.isGasPrecisionLossFixEnabled()) {
return asGasRequirement(canonicalPriceInTinybars(dispatchType));
}
return gasRequirementFromTinycents(
dispatchPrices.canonicalPriceInTinycents(dispatchType),
tinybarValues.childTransactionTinycentGasPrice());
}

/**
Expand All @@ -127,9 +151,14 @@ public long viewGasRequirement() {
* @param dispatchType the dispatch type
* @return the canonical price for that dispatch type
*/
public long canonicalPriceInTinybars(@NonNull final DispatchType dispatchType) {
public long canonicalPriceInTinycents(@NonNull final DispatchType dispatchType) {
requireNonNull(dispatchType);
return tinybarValues.asTinybars(dispatchPrices.canonicalPriceInTinycents(dispatchType));
// If not enabled, return the price in TinyBars.
// This is directly used only in ClassicTransfersCall. However, it is easier to place the feature flag here.
if (!tinybarValues.isGasPrecisionLossFixEnabled()) {
return canonicalPriceInTinybars(dispatchType);
}
return dispatchPrices.canonicalPriceInTinycents(dispatchType);
}

/**
Expand All @@ -139,7 +168,7 @@ public long canonicalPriceInTinybars(@NonNull final DispatchType dispatchType) {
* @param payer the payer account
* @return the canonical price for that dispatch
*/
public long canonicalPriceInTinybars(@NonNull final TransactionBody body, @NonNull final AccountID payer) {
public long feeCalculatorPriceInTinyBars(@NonNull final TransactionBody body, @NonNull final AccountID payer) {
return feeCalculator.applyAsLong(body, payer);
}

Expand All @@ -154,15 +183,65 @@ public long gasCostInTinybars(final long gas) {
}

/**
* Given a tinybar price, returns the equivalent gas requirement at the current gas price.
* Calculates the gas requirement for an operation based on the provided tinycents price and gas price.
* The calculation rounds up the result to the nearest gas unit and then adds 20% to the computed gas
* requirement to account for the premium of executing a HAPI operation within the EVM.
*
* @param tinybarPrice the tinybar price
* @return the equivalent gas requirement at the current gas price
* @param tinycentsPrice the price of the operation in tinycents
* @param gasPriceInCents the current gas price in cents
* @return the computed gas requirement for the operation
*/
private long gasRequirementFromTinycents(long tinycentsPrice, final long gasPriceInCents) {
final var gasRequirement =
(tinycentsPrice + gasPriceInCents - 1) * FEE_SCHEDULE_UNITS_PER_TINYCENT / gasPriceInCents;
return gasRequirement + (gasRequirement / 5);
}

/**
* After feature flag isGasPrecisionLossFixEnabled is removed, this method should be removed.
* @deprecated
*/
@Deprecated(since = "Precision loss fix was implemented in PR #14842", forRemoval = true)
public long topLevelGasPrice() {
return tinybarValues.topLevelTinybarGasPrice();
}

/**
* After feature flag isGasPrecisionLossFixEnabled is removed, this method should be removed.
* @deprecated
*/
@Deprecated(since = "Precision loss fix was implemented in PR #14842", forRemoval = true)
public long canonicalPriceInTinybars(@NonNull final DispatchType dispatchType) {
requireNonNull(dispatchType);
return tinybarValues.asTinybars(dispatchPrices.canonicalPriceInTinycents(dispatchType));
}

/**
* After feature flag isGasPrecisionLossFixEnabled is removed, this method should be removed.
* @deprecated
*/
@Deprecated(since = "Precision loss fix was implemented in PR #14842", forRemoval = true)
public long gasRequirementOldWithPrecisionLoss(
@NonNull final TransactionBody body, @NonNull final AccountID payer, final long minimumPriceInTinybars) {
final var nominalPriceInTinybars = feeCalculator.applyAsLong(body, payer);
final var priceInTinybars = Math.max(minimumPriceInTinybars, nominalPriceInTinybars);
return asGasRequirement(priceInTinybars);
}

/**
* After feature flag isGasPrecisionLossFixEnabled is removed, this method should be removed.
* @deprecated
*/
@Deprecated(since = "Precision loss fix was implemented in PR #14842", forRemoval = true)
private long asGasRequirement(final long tinybarPrice) {
return asGasRequirement(tinybarPrice, tinybarValues.childTransactionTinybarGasPrice());
}

/**
* After feature flag isGasPrecisionLossFixEnabled is removed, this method should be removed.
* @deprecated
*/
@Deprecated(since = "Precision loss fix was implemented in PR #14842", forRemoval = true)
private long asGasRequirement(final long tinybarPrice, final long gasPrice) {
// We round up to the nearest gas unit, and then add 20% to account for the premium
// of doing a HAPI operation from inside the EVM
Expand Down
Loading

0 comments on commit 1a31fa6

Please sign in to comment.