Skip to content

Commit

Permalink
fix: Consider input bytes when calculating gas cost (#10379)
Browse files Browse the repository at this point in the history
Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>
  • Loading branch information
stoqnkpL committed Dec 11, 2023
1 parent ff96c7f commit 23a28ea
Show file tree
Hide file tree
Showing 23 changed files with 299 additions and 252 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.hedera.node.app.service.evm.contracts.execution;

import com.google.common.annotations.VisibleForTesting;
import com.hedera.node.app.service.evm.contracts.execution.traceability.HederaEvmOperationTracer;
import com.hedera.node.app.service.evm.store.contracts.HederaEvmMutableWorldState;
import com.hedera.node.app.service.evm.store.contracts.HederaEvmWorldUpdater;
Expand Down Expand Up @@ -48,7 +49,7 @@ public abstract class HederaEvmTxProcessor {
protected BlockMetaSource blockMetaSource;
protected HederaEvmMutableWorldState worldState;

protected final GasCalculator gasCalculator;
protected GasCalculator gasCalculator;
// FEATURE WORK to be covered by #3949
protected final PricesAndFeesProvider livePricesSource;
protected final Map<String, Provider<MessageCallProcessor>> mcps;
Expand Down Expand Up @@ -179,8 +180,8 @@ public HederaEvmTransactionProcessingResult execute(
}
}

public void setupFields(final boolean contractCreation) {
this.intrinsicGas = gasCalculator.transactionIntrinsicGasCost(Bytes.EMPTY, contractCreation);
public void setupFields(final Bytes payload, final boolean contractCreation) {
this.intrinsicGas = gasCalculator.transactionIntrinsicGasCost(payload, contractCreation);
this.updater = worldState.updater();
this.coinbase = dynamicProperties.fundingAccountAddress();
}
Expand Down Expand Up @@ -223,4 +224,9 @@ private AbstractMessageProcessor getMessageProcessor(final MessageFrame.Type typ
case CONTRACT_CREATION -> contractCreationProcessor;
};
}

@VisibleForTesting
public void setGasCalculator(GasCalculator gasCalculator) {
this.gasCalculator = gasCalculator;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.hedera.node.app.service.evm.contracts.execution.traceability.DefaultHederaTracer;
Expand Down Expand Up @@ -152,7 +155,7 @@ void assertSuccessExecution() {
givenValidMock(0L);
given(globalDynamicProperties.fundingAccountAddress()).willReturn(fundingAccount);

evmTxProcessor.setupFields(false);
evmTxProcessor.setupFields(Bytes.EMPTY, false);
final var result =
evmTxProcessor.execute(sender, receiver, 33_333L, 1234L, 1L, Bytes.EMPTY, true, mirrorReceiver);
assertTrue(result.isSuccessful());
Expand Down Expand Up @@ -184,7 +187,7 @@ void assertSuccessExecutionChargesCorrectMinimumGas() {

given(globalDynamicProperties.maxGasRefundPercentage()).willReturn(MAX_REFUND_PERCENT);
given(globalDynamicProperties.fundingAccountAddress()).willReturn(fundingAccount);
evmTxProcessor.setupFields(false);
evmTxProcessor.setupFields(Bytes.EMPTY, false);
final var result =
evmTxProcessor.execute(sender, receiver, 0L, GAS_LIMIT, 1234L, Bytes.EMPTY, false, mirrorReceiver);
assertTrue(result.isSuccessful());
Expand All @@ -200,7 +203,7 @@ void assertSuccessExecutionChargesCorrectGasWhenGasUsedIsLargerThanMinimum() {
given(gasCalculator.transactionIntrinsicGasCost(Bytes.EMPTY, false)).willReturn(intrinsicGasCost);
given(globalDynamicProperties.fundingAccountAddress()).willReturn(fundingAccount);

evmTxProcessor.setupFields(false);
evmTxProcessor.setupFields(Bytes.EMPTY, false);
final var result =
evmTxProcessor.execute(sender, receiver, 0L, GAS_LIMIT, 1234L, Bytes.EMPTY, false, mirrorReceiver);
assertTrue(result.isSuccessful());
Expand All @@ -224,7 +227,7 @@ void throwsWhenSenderCannotCoverUpfrontCost() {
given(stackedUpdater.getOrCreate(any())).willReturn(wrappedRecipientAccount);
given(updater.updater()).willReturn(stackedUpdater);

evmTxProcessor.setupFields(false);
evmTxProcessor.setupFields(Bytes.EMPTY, false);
final var result =
evmTxProcessor.execute(sender, receiver, 333_333L, 1234L, 1L, Bytes.EMPTY, true, mirrorReceiver);
assertEquals(INSUFFICIENT_GAS, result.getHaltReason().get().name());
Expand Down Expand Up @@ -252,7 +255,7 @@ void throwsWhenIntrinsicGasCostExceedsGasLimit() {

givenInvalidMock();

evmTxProcessor.setupFields(false);
evmTxProcessor.setupFields(Bytes.EMPTY, false);
final var result =
evmTxProcessor.execute(sender, receiver, 33_333L, 0L, 1234L, Bytes.EMPTY, true, mirrorReceiver);
assertEquals(INSUFFICIENT_GAS, result.getHaltReason().get().name());
Expand All @@ -265,7 +268,7 @@ void throwsWhenIntrinsicGasCostExceedsGasLimitAndGasLimitIsEqualToMaxGasLimit()
final int maxGasLimit = 10_000_000;
given(gasCalculator.transactionIntrinsicGasCost(Bytes.EMPTY, false)).willReturn(maxGasLimit + 1L);

evmTxProcessor.setupFields(false);
evmTxProcessor.setupFields(Bytes.EMPTY, false);
final var result =
evmTxProcessor.execute(sender, receiver, 0L, maxGasLimit, 1234L, Bytes.EMPTY, false, mirrorReceiver);
assertEquals(INSUFFICIENT_GAS, result.getHaltReason().get().name());
Expand Down Expand Up @@ -308,7 +311,7 @@ void assertSuccessExecutionWithRefund() {
given(globalDynamicProperties.maxGasRefundPercentage()).willReturn(100);
given(globalDynamicProperties.fundingAccountAddress()).willReturn(fundingAccount);

evmTxProcessor.setupFields(false);
evmTxProcessor.setupFields(Bytes.EMPTY, false);
final var result =
evmTxProcessor.execute(sender, receiver, GAS_LIMIT, 0L, 1234L, Bytes.EMPTY, true, mirrorReceiver);

Expand Down Expand Up @@ -348,7 +351,7 @@ void testEvmVersionLoading() {
given(globalDynamicProperties.fundingAccountAddress()).willReturn(fundingAccount);

// uses default setup
evmTxProcessor.setupFields(false);
evmTxProcessor.setupFields(Bytes.EMPTY, false);

evmTxProcessor.execute(sender, receiver, 33_333L, 1234L, 1L, Bytes.EMPTY, true, mirrorReceiver);
assertEquals(EVM_VERSION_0_30, mcpVersion);
Expand All @@ -369,4 +372,21 @@ void testEvmVersionLoading() {
NullPointerException.class,
() -> evmTxProcessor.execute(sender, receiver, 33_333L, 1234L, 1L, Bytes.EMPTY, true, mirrorReceiver));
}

@Test
void forwardsPayloadToGasCalculator() {
final var mockedGasCalculator = mock(GasCalculator.class);
evmTxProcessor.setGasCalculator(mockedGasCalculator);

Bytes empty = Bytes.EMPTY;
Bytes somePayload = Bytes.fromBase64String("9499rew9rwefdsfkad9cd09f0dscds0cds");

// with empty bytes
evmTxProcessor.setupFields(empty, true);
verify(mockedGasCalculator).transactionIntrinsicGasCost(argThat(x -> x.equals(empty)), eq(true));

// with actual payload
evmTxProcessor.setupFields(somePayload, true);
verify(mockedGasCalculator).transactionIntrinsicGasCost(argThat(x -> x.equals(somePayload)), eq(true));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ protected TransactionProcessingResult execute(
final Wei gasCost = Wei.of(Math.multiplyExact(gasLimit, gasPrice));
final Wei upfrontCost = gasCost.add(value);

super.setupFields(contractCreation);
super.setupFields(payload, contractCreation);

final var chargingResult = chargeForGas(
gasCost,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -339,7 +340,7 @@ private void givenValidMock(final long amount, boolean getOrCreateMocking) {
given(updater.getOrCreate(any())).willReturn(evmAccount);
}

given(gasCalculator.transactionIntrinsicGasCost(Bytes.EMPTY, true)).willReturn(0L);
given(gasCalculator.transactionIntrinsicGasCost(any(), eq(true))).willReturn(0L);

given(evmAccount.decrementBalance(any())).willReturn(Wei.of(1234L));
given(evmAccount.incrementBalance(any())).willReturn(Wei.of(1500L));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import edu.umd.cs.findbugs.annotations.Nullable;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.apache.tuweni.bytes.Bytes;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;

/**
Expand Down Expand Up @@ -114,7 +113,8 @@ public GasCharges chargeForGas(
if (context.isNoopGasContext()) {
return ZERO_CHARGES;
}
final var intrinsicGas = gasCalculator.transactionIntrinsicGasCost(Bytes.EMPTY, transaction.isCreate());
final var intrinsicGas =
gasCalculator.transactionIntrinsicGasCost(transaction.evmPayload(), transaction.isCreate());
validateTrue(transaction.gasLimit() >= intrinsicGas, INSUFFICIENT_GAS);
if (transaction.isEthereumTransaction()) {
final var allowanceUsed =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.wellKnownRelayedHapiCallWithGasLimit;
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.wellKnownRelayedHapiCallWithUserGasPriceAndMaxAllowance;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
Expand All @@ -43,7 +45,6 @@
import com.hedera.node.app.service.contract.impl.hevm.HederaWorldUpdater;
import com.hedera.node.app.service.contract.impl.state.HederaEvmAccount;
import com.hedera.node.app.service.contract.impl.test.TestHelpers;
import org.apache.tuweni.bytes.Bytes;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -365,7 +366,6 @@ private void givenWellKnownIntrinsicGasCost() {
}

private void givenWellKnownIntrinsicGasCost(boolean isCreation) {
given(gasCalculator.transactionIntrinsicGasCost(Bytes.EMPTY, isCreation))
.willReturn(TestHelpers.INTRINSIC_GAS);
given(gasCalculator.transactionIntrinsicGasCost(any(), eq(isCreation))).willReturn(TestHelpers.INTRINSIC_GAS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -733,9 +733,11 @@ private HapiSpec bitcarbonTestStillPasses() {
.getContractAccountID())))),
uploadInitCode(addressBook, jurisdictions),
contractCreate(addressBook)
.gas(1_000_000L)
.exposingNumTo(num -> addressBookMirror.set(asHexedSolidityAddress(0, 0, num)))
.payingWith(DEFAULT_CONTRACT_SENDER),
contractCreate(jurisdictions)
.gas(1_000_000L)
.exposingNumTo(num -> jurisdictionMirror.set(asHexedSolidityAddress(0, 0, num)))
.withExplicitParams(() -> EXPLICIT_JURISDICTION_CONS_PARAMS)
.payingWith(DEFAULT_CONTRACT_SENDER),
Expand All @@ -744,6 +746,7 @@ private HapiSpec bitcarbonTestStillPasses() {
minters,
bookInterpolated(literalInitcodeFor(minters).toByteArray(), addressBookMirror.get()))),
contractCreate(minters)
.gas(2_000_000L)
.withExplicitParams(
() -> String.format(EXPLICIT_MINTER_CONS_PARAMS_TPL, jurisdictionMirror.get()))
.payingWith(DEFAULT_CONTRACT_SENDER))
Expand Down Expand Up @@ -1399,7 +1402,7 @@ inlineTestContract, GET_CODE_SIZE, asHeadlongAddress(acctAddress))
private HapiSpec multipleSelfDestructsAreSafe() {
final var contract = "Fuse";
return defaultHapiSpec("MultipleSelfDestructsAreSafe", NONDETERMINISTIC_TRANSACTION_FEES)
.given(uploadInitCode(contract), contractCreate(contract).gas(300_000))
.given(uploadInitCode(contract), contractCreate(contract).gas(600_000))
.when(contractCall(contract, "light").via("lightTxn").scrambleTxnBody(tx -> tx))
.then(getTxnRecord("lightTxn").logged());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,8 @@ private HapiSpec lpFarmSimulation() {
BigInteger.valueOf(10000L),
BigInteger.valueOf(1000000000000000L),
BigInteger.valueOf(2500000000L))
.bytecode(initcode)),
.bytecode(initcode)
.gas(500_000L)),
tokenCreate(sauce)
.supplyType(TokenSupplyType.FINITE)
.initialSupply(300_000_000)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ private HapiSpec childCreationsHaveExpectedKeysWithOmittedAdminKey() {
return defaultHapiSpec("ChildCreationsHaveExpectedKeysWithOmittedAdminKey")
.given(
uploadInitCode(contract),
contractCreate(contract).omitAdminKey().gas(300_000).via(txn),
contractCreate(contract).omitAdminKey().gas(600_000).via(txn),
withOpContext((spec, opLog) -> {
final var op = getTxnRecord(txn);
allRunFor(spec, op);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ private HapiSpec canCallFinalizedContractViaHapi() {
newKeyNamed(SECP_256K1_SOURCE_KEY).shape(SECP_256K1_SHAPE),
cryptoTransfer(tinyBarsFromAccountToAlias(GENESIS, SECP_256K1_SOURCE_KEY, ONE_HUNDRED_HBARS)),
uploadInitCode(contract),
contractCreate(contract).payingWith(GENESIS),
contractCreate(contract).payingWith(GENESIS).gas(500_000L),
contractCallLocal(contract, "computeChildAddress", salt)
.exposingTypedResultsTo(results -> childAddress.set((Address) results[0])),
sourcing(() -> ethereumCryptoTransferToAddress(childAddress.get(), ONE_HBAR)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ private HapiSpec erc1155() {
uploadInitCode(CONTRACT))
.when()
.then(
contractCreate(CONTRACT).via("contractCreate").payingWith(ACCOUNT2),
contractCreate(CONTRACT)
.gas(500_000L)
.via("contractCreate")
.payingWith(ACCOUNT2),
getTxnRecord("contractCreate").logged(),
getAccountBalance(ACCOUNT2).logged(),
getAccountInfo(ACCOUNT1).savingSnapshot(ACCOUNT1 + "Info"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ private HapiSpec callsERC721ContractInteractions() {
contractCreate(CONTRACT)
.payingWith(DEFAULT_CONTRACT_SENDER)
.hasKnownStatus(SUCCESS)
.gas(500_000L)
.via(CREATE_TX),
cryptoTransfer(tinyBarsFromTo(
DEFAULT_CONTRACT_SENDER, DEFAULT_CONTRACT_RECEIVER, 10 * ONE_HBAR))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ private HapiSpec invalidSingleAbiCallConsumesAllProvidedGas() {
.getTransactionRecord(INVALID_SINGLE_ABI_CALL_TXN)
.getContractCallResult()
.getGasUsed();
assertEquals(99011, gasUsed);
assertEquals(99014, gasUsed);
}));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ private HapiSpecOperation someWellKnownTokensAndAccounts(
cryptoCreate(B_WELL_KNOWN_RECEIVER)
.exposingCreatedIdTo(id -> bReceiverAddr.set(idAsHeadlongAddress(id))),
uploadInitCode(WELL_KNOWN_TREASURY_CONTRACT),
contractCreate(WELL_KNOWN_TREASURY_CONTRACT),
contractCreate(WELL_KNOWN_TREASURY_CONTRACT).gas(500_000L),
tokenCreate(WELL_KNOWN_FUNGIBLE_TOKEN)
.exposingAddressTo(fungibleTokenMirrorAddr::set)
.tokenType(TokenType.FUNGIBLE_COMMON)
Expand Down Expand Up @@ -359,7 +359,7 @@ private HapiSpec canStillTransferByVirtueOfContractIdInEOAThreshold() {
// Create an immutable contract with a method
// transferViaThresholdContractKey()
// that tries to transfer token units from a spender to a receiver
contractCreate(managementContract).omitAdminKey(),
contractCreate(managementContract).gas(500_000L).omitAdminKey(),
// Setup a 1/2 threshold key with this contract's ID as the first key
newKeyNamed(controlledSpenderKey)
.shape(threshKeyShape.signedWith(sigs(managementContract, ON))),
Expand Down Expand Up @@ -407,9 +407,9 @@ private HapiSpec contractKeysStillHaveSpecificityNoMatterTopLevelSignatures() {
.given(
uploadInitCode(managementContract, PAY_RECEIVABLE_CONTRACT),
newKeyNamed(tmpAdminKey),
contractCreate(managementContract).adminKey(tmpAdminKey),
contractCreate(managementContract).gas(500_000L).adminKey(tmpAdminKey),
// Just create some other contract to be the real admin key
contractCreate(PAY_RECEIVABLE_CONTRACT),
contractCreate(PAY_RECEIVABLE_CONTRACT).gas(500_000L),
newKeyNamed(otherContractAsKey).shape(CONTRACT.signedWith(PAY_RECEIVABLE_CONTRACT)),
cryptoCreate(associatedAccount).keyShape(SECP256K1_ON).exposingEvmAddressTo(accountAddr::set),
tokenCreate(fungibleToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ private HapiSpec transferNftAfterNestedMint() {
.withStatus(SUCCESS)
.withTotalSupply(1L)
.withSerialNumbers(1L))
.gas(3_837_920L)
.gas(3_836_587L)
.amount(0L)
.functionParameters(functionParameters()
.forFunction(
Expand Down

0 comments on commit 23a28ea

Please sign in to comment.