Skip to content

Commit 89a6b33

Browse files
gkozyryatskyystoyanov-st
gkozyryatskyy
and
stoyanov-st
authored
fix: IntrinsicGas calculation fix (#22131)
Signed-off-by: Glib Kozyryatskyy <[email protected]> Co-authored-by: Stanimir Stoyanov <[email protected]>
1 parent 382c284 commit 89a6b33

File tree

7 files changed

+120
-66
lines changed

7 files changed

+120
-66
lines changed

‎hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/gas/CustomGasCalculator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public long transactionIntrinsicGasCost(
5757

5858
long cost = TX_BASE_COST + TX_DATA_ZERO_COST * zeros + ISTANBUL_TX_DATA_NON_ZERO_COST * nonZeros;
5959

60-
return isContractCreate ? (cost + txCreateExtraGasCost()) : cost;
60+
return isContractCreate ? (cost + contractCreationCost(payload.size())) : cost;
6161
}
6262

6363
@Override

‎hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCallHandler.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,13 @@ public void pureChecks(@NonNull final PureChecksContext context) throws PreCheck
8383
if (op.contractID().hasEvmAddress()) {
8484
final var evmAddress = op.contractID().evmAddressOrThrow();
8585
validateTruePreCheck(evmAddress.length() == EVM_ADDRESS_LENGTH_AS_INT, INVALID_CONTRACT_ID);
86-
// In the EVM, call to zero address means contract deploy and we are not supporting contract deploy from
87-
// contract call
86+
// call to zero address make no sense, and we are not supporting it
8887
validateFalsePreCheck(
8988
Arrays.equals(ConstantUtils.ZERO_ADDRESS_BYTE_ARRAY, evmAddress.toByteArray()),
9089
INVALID_CONTRACT_ID);
9190
} else if (op.contractID().hasContractNum()) {
9291
final var contractId = op.contractID();
93-
// In the EVM, call to zero address means contract deploy and we are not supporting contract deploy from
94-
// contract call
92+
// call to zero address make no sense, and we are not supporting it
9593
validateFalsePreCheck(ConstantUtils.ZERO_CONTRACT_ID.equals(contractId), INVALID_CONTRACT_ID);
9694
}
9795

‎hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/EthereumTransactionHandler.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,6 @@ public void pureChecks(@NonNull final PureChecksContext context) throws PreCheck
9898
requireNonNull(txn.ethereumTransactionOrThrow().ethereumData())
9999
.toByteArray());
100100
validateTruePreCheck(nonNull(ethTxData), INVALID_ETHEREUM_TRANSACTION);
101-
final byte[] callData = ethTxData.hasCallData() ? ethTxData.callData() : new byte[0];
102-
// TODO: Revisit baselineGas with Pectra support epic
103-
final var intrinsicGas =
104-
gasCalculator.transactionIntrinsicGasCost(org.apache.tuweni.bytes.Bytes.wrap(callData), false, 0L);
105-
validateTruePreCheck(ethTxData.gasLimit() >= intrinsicGas, INSUFFICIENT_GAS);
106101
// Do not allow sending HBars to Burn Address
107102
if (ethTxData.value().compareTo(BigInteger.ZERO) > 0) {
108103
validateFalsePreCheck(Arrays.equals(ethTxData.to(), EMPTY_ADDRESS), INVALID_SOLIDITY_ADDRESS);
@@ -111,6 +106,13 @@ public void pureChecks(@NonNull final PureChecksContext context) throws PreCheck
111106
if (ethTxData.hasToAddress()) {
112107
validateTruePreCheck(ethTxData.to().length == EVM_ADDRESS_LENGTH_AS_INT, INVALID_CONTRACT_ID);
113108
}
109+
// gas requirements check
110+
final byte[] callData = ethTxData.hasCallData() ? ethTxData.callData() : new byte[0];
111+
final var isContractCreate = !ethTxData.hasToAddress();
112+
// TODO: Revisit baselineGas with Pectra support epic
113+
final var intrinsicGas = gasCalculator.transactionIntrinsicGasCost(
114+
org.apache.tuweni.bytes.Bytes.wrap(callData), isContractCreate, 0L);
115+
validateTruePreCheck(ethTxData.gasLimit() >= intrinsicGas, INSUFFICIENT_GAS);
114116
} catch (@NonNull final Exception e) {
115117
bumpExceptionMetrics(ETHEREUM_TRANSACTION, e);
116118
if (e instanceof NullPointerException) {

‎hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/gas/CustomGasCalculatorTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ void transactionIntrinsicGasCost() {
4141
4 * 3 + // zero byte cost
4242
16 * 2 + // non-zero byte cost
4343
21_000L + // base TX cost
44-
32_000L, // contract creation base cost
44+
32_000L + // contract creation base cost
45+
2, // contract creation 1 word cost
4546
subject.transactionIntrinsicGasCost(Bytes.of(0, 1, 0, 3, 0), true, 0L));
4647
}
4748
}

‎hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/EthereumTransactionHandlerTest.java‎

Lines changed: 71 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,13 @@
22
package com.hedera.node.app.service.contract.impl.test.handlers;
33

44
import static com.hedera.hapi.node.base.HederaFunctionality.ETHEREUM_TRANSACTION;
5-
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ETHEREUM_TRANSACTION;
6-
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.CALLED_CONTRACT_ID;
7-
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.DEFAULT_CONFIG;
8-
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.ETH_DATA_WITHOUT_TO_ADDRESS;
9-
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.ETH_DATA_WITH_TO_ADDRESS;
10-
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.HEVM_CREATION;
11-
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.SENDER_ID;
12-
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.SIGNER_NONCE;
13-
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.SUCCESS_RESULT_WITH_SIGNER_NONCE;
5+
import static com.hedera.hapi.node.base.ResponseCodeEnum.*;
6+
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.*;
147
import static com.hedera.node.app.service.contract.impl.test.handlers.ContractCallHandlerTest.INTRINSIC_GAS_FOR_0_ARG_METHOD;
158
import static com.hedera.node.app.spi.fixtures.Assertions.assertThrowsPreCheck;
169
import static com.hedera.node.app.spi.workflows.HandleContext.DispatchMetadata.EMPTY_METADATA;
1710
import static com.hedera.node.app.spi.workflows.HandleContext.DispatchMetadata.Type.ETHEREUM_NONCE_INCREMENT_CALLBACK;
18-
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
19-
import static org.junit.jupiter.api.Assertions.assertThrows;
11+
import static org.junit.jupiter.api.Assertions.*;
2012
import static org.mockito.ArgumentMatchers.any;
2113
import static org.mockito.ArgumentMatchers.notNull;
2214
import static org.mockito.BDDMockito.given;
@@ -407,33 +399,94 @@ void testCalculateFeesWithZeroHapiFeesConfigDisabled() {
407399
}
408400

409401
@Test
410-
void validatePureChecks() {
402+
void validatePureChecksHappyPath() {
403+
// check bad to evm address
404+
try (MockedStatic<EthTxData> ethTxData = Mockito.mockStatic(EthTxData.class)) {
405+
ethTxData.when(() -> EthTxData.populateEthTxData(any())).thenReturn(ethTxDataReturned);
406+
given(pureChecksContext.body()).willReturn(ethTxWithTx());
407+
given(ethTxDataReturned.value()).willReturn(BigInteger.ONE);
408+
given(ethTxDataReturned.hasToAddress()).willReturn(true);
409+
final var toAddress = RECEIVER_ADDRESS.toByteArray();
410+
given(ethTxDataReturned.to()).willReturn(toAddress);
411+
given(gasCalculator.transactionIntrinsicGasCost(org.apache.tuweni.bytes.Bytes.wrap(new byte[0]), false, 0L))
412+
.willReturn(INTRINSIC_GAS_FOR_0_ARG_METHOD);
413+
given(ethTxDataReturned.gasLimit()).willReturn(INTRINSIC_GAS_FOR_0_ARG_METHOD);
414+
assertDoesNotThrow(() -> subject.pureChecks(pureChecksContext));
415+
}
416+
}
417+
418+
@Test
419+
void validatePureChecksCheckBadEthTxnBody() {
411420
// check bad eth txn body
412421
final var txn1 = ethTxWithNoTx();
413422
given(pureChecksContext.body()).willReturn(txn1);
414423
assertThrows(PreCheckException.class, () -> subject.pureChecks(pureChecksContext));
424+
PreCheckException exception =
425+
assertThrows(PreCheckException.class, () -> subject.pureChecks(pureChecksContext));
426+
assertEquals(INVALID_ETHEREUM_TRANSACTION, exception.responseCode());
427+
}
428+
429+
@Test
430+
void validatePureChecksHbarsToBurnAddress() {
431+
// check bad to evm address
432+
try (MockedStatic<EthTxData> ethTxData = Mockito.mockStatic(EthTxData.class)) {
433+
ethTxData.when(() -> EthTxData.populateEthTxData(any())).thenReturn(ethTxDataReturned);
434+
given(pureChecksContext.body()).willReturn(ethTxWithTx());
435+
given(ethTxDataReturned.value()).willReturn(BigInteger.ONE);
436+
given(ethTxDataReturned.to()).willReturn(new byte[20]);
437+
PreCheckException exception =
438+
assertThrows(PreCheckException.class, () -> subject.pureChecks(pureChecksContext));
439+
assertEquals(INVALID_SOLIDITY_ADDRESS, exception.responseCode());
440+
}
441+
}
415442

443+
@Test
444+
void validatePureChecksBadToEvmAddress() {
416445
// check bad to evm address
417446
try (MockedStatic<EthTxData> ethTxData = Mockito.mockStatic(EthTxData.class)) {
418447
final var toAddress = new byte[] {1, 0, 1, 0};
419448
ethTxData.when(() -> EthTxData.populateEthTxData(any())).thenReturn(ethTxDataReturned);
420-
given(ethTxDataReturned.gasLimit()).willReturn(INTRINSIC_GAS_FOR_0_ARG_METHOD + 1);
449+
given(pureChecksContext.body()).willReturn(ethTxWithTx());
421450
given(ethTxDataReturned.value()).willReturn(BigInteger.ZERO);
422451
given(ethTxDataReturned.hasToAddress()).willReturn(true);
423-
given(gasCalculator.transactionIntrinsicGasCost(org.apache.tuweni.bytes.Bytes.wrap(new byte[0]), false, 0L))
424-
.willReturn(INTRINSIC_GAS_FOR_0_ARG_METHOD);
425452
given(ethTxDataReturned.to()).willReturn(toAddress);
426-
given(pureChecksContext.body()).willReturn(ethTxWithTx());
427-
assertThrows(PreCheckException.class, () -> subject.pureChecks(pureChecksContext));
453+
PreCheckException exception =
454+
assertThrows(PreCheckException.class, () -> subject.pureChecks(pureChecksContext));
455+
assertEquals(INVALID_CONTRACT_ID, exception.responseCode());
428456
}
457+
}
429458

459+
@Test
460+
void validatePureChecksAtLeastIntrinsicGas() {
430461
// check at least intrinsic gas
431462
try (MockedStatic<EthTxData> ethTxData = Mockito.mockStatic(EthTxData.class)) {
432463
ethTxData.when(() -> EthTxData.populateEthTxData(any())).thenReturn(ethTxDataReturned);
464+
given(pureChecksContext.body()).willReturn(ethTxWithTx());
465+
given(ethTxDataReturned.value()).willReturn(BigInteger.ZERO);
466+
given(ethTxDataReturned.hasToAddress()).willReturn(true);
467+
final var toAddress = RECEIVER_ADDRESS.toByteArray();
468+
given(ethTxDataReturned.to()).willReturn(toAddress);
433469
given(gasCalculator.transactionIntrinsicGasCost(org.apache.tuweni.bytes.Bytes.wrap(new byte[0]), false, 0L))
434470
.willReturn(INTRINSIC_GAS_FOR_0_ARG_METHOD);
471+
PreCheckException exception =
472+
assertThrows(PreCheckException.class, () -> subject.pureChecks(pureChecksContext));
473+
assertEquals(INSUFFICIENT_GAS, exception.responseCode());
474+
}
475+
}
476+
477+
@Test
478+
void validatePureChecksAtLeastIntrinsicGasForContractCreate() {
479+
// check at least intrinsic gas for contract create (hasToAddress() == false)
480+
try (MockedStatic<EthTxData> ethTxData = Mockito.mockStatic(EthTxData.class)) {
481+
ethTxData.when(() -> EthTxData.populateEthTxData(any())).thenReturn(ethTxDataReturned);
435482
given(pureChecksContext.body()).willReturn(ethTxWithTx());
436-
assertThrows(PreCheckException.class, () -> subject.pureChecks(pureChecksContext));
483+
given(ethTxDataReturned.value()).willReturn(BigInteger.ZERO);
484+
given(ethTxDataReturned.hasToAddress()).willReturn(false);
485+
given(gasCalculator.transactionIntrinsicGasCost(org.apache.tuweni.bytes.Bytes.wrap(new byte[0]), true, 0L))
486+
.willReturn(INTRINSIC_GAS_FOR_0_ARG_METHOD);
487+
PreCheckException exception =
488+
assertThrows(PreCheckException.class, () -> subject.pureChecks(pureChecksContext));
489+
assertEquals(INSUFFICIENT_GAS, exception.responseCode());
437490
}
438491
}
439492

‎hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/precompile/SigningReqsSuite.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ final Stream<DynamicTest> autoRenewAccountCanUseLegacySigActivationIfConfigured(
6868
uploadInitCode(MINIMAL_CREATIONS_CONTRACT),
6969
contractCreate(MINIMAL_CREATIONS_CONTRACT)
7070
.exposingContractIdTo(contractId::set)
71-
.gas(5_000_000L)
71+
.gas(6_000_000L)
7272
.refusingEthConversion(),
7373
cryptoCreate(autoRenew)
7474
.keyShape(origKey.signedWith(sigs(ON, MINIMAL_CREATIONS_CONTRACT)))

0 commit comments

Comments
 (0)