diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java index 666ca545831..bc58ca284c3 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java @@ -536,7 +536,6 @@ public TransactionProcessingResult processTransaction( .stateGasUsed(initialFrame.getStateGasUsed()) .initialFrameStateGasSpill(initialFrameStateGasSpill) .stateGasSpillBurned(initialFrame.getStateGasSpillBurned()) - .regularGasCollisionBurned(initialFrame.getRegularGasCollisionBurned()) .refundedGas(refundedGas) .floorCost(floorCost) .regularGasLimitExceeded(regularGasLimitExceeded) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionGasAccounting.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionGasAccounting.java index c99be511830..08e4722c6c7 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionGasAccounting.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionGasAccounting.java @@ -55,9 +55,6 @@ public record GasResult(long effectiveStateGas, long gasUsedByTransaction, long /** Total state gas spilled into gasRemaining from reverted frames. */ public abstract long stateGasSpillBurned(); - /** Gas burned by CREATE children that halted before executing code (address collision). */ - public abstract long regularGasCollisionBurned(); - /** Gas refunded to the sender. */ public abstract long refundedGas(); @@ -95,32 +92,26 @@ public GasResult calculate() { // EIP-8037: Include leftover reservoir in remaining gas for execution gas calculation final long executionGas = txGasLimit() - remainingGas() - stateGasReservoir(); // EIP-8037: Floor applies to regular gas only, not total gas. - // Pre-Amsterdam: stateGasUsed=0, spillBurned=0, collisionBurned=0 — identical. + // Pre-Amsterdam: stateGasUsed=0, spillBurned=0 — identical. // stateGasSpillBurned: state gas that spilled into gasRemaining from reverted frames. // For child frame reverts: not metered as regular or state gas (invisible to block). // For initial frame revert/halt: counts as state gas for block accounting, because the // transaction's state gas consumption is final regardless of execution outcome. - // regularGasCollisionBurned: gas burned by CREATE children that halted before executing - // code (address collision); excluded from block regular gas but still charged as fees. final long stateGas = stateGasUsed() + initialFrameStateGasSpill(); // initialFrameStateGasSpill is already included in spillBurned AND stateGas, // so subtract it from spillBurned to avoid double-counting. final long regularGas = - executionGas - - stateGas - - (stateGasSpillBurned() - initialFrameStateGasSpill()) - - regularGasCollisionBurned(); + executionGas - stateGas - (stateGasSpillBurned() - initialFrameStateGasSpill()); if (regularGas < 0) { // This should not happen under normal circumstances. A negative regularGas indicates a // bug in gas accounting — log at error level to ensure visibility. LOG.error( - "Negative regularGas={} (executionGas={}, stateGas={}, spillBurned={}, initialSpill={}, collisionBurned={})", + "Negative regularGas={} (executionGas={}, stateGas={}, spillBurned={}, initialSpill={})", regularGas, executionGas, stateGas, stateGasSpillBurned(), - initialFrameStateGasSpill(), - regularGasCollisionBurned()); + initialFrameStateGasSpill()); } final long gasUsedByTransaction = Math.max(regularGas, floorCost()) + stateGas; final long usedGas = txGasLimit() - refundedGas(); diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/TransactionGasAccountingTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/TransactionGasAccountingTest.java index ff021908f53..99082dd2408 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/TransactionGasAccountingTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/TransactionGasAccountingTest.java @@ -31,7 +31,6 @@ private static ImmutableTransactionGasAccounting.Builder baseBuilder() { .stateGasUsed(0L) .initialFrameStateGasSpill(0L) .stateGasSpillBurned(0L) - .regularGasCollisionBurned(0L) .refundedGas(0L) .floorCost(0L) .regularGasLimitExceeded(false); @@ -143,7 +142,7 @@ public void stateGasSpill_doubleCountingAvoided() { @Test public void zeroStateGas_preAmsterdamEquivalent() { - // Pre-Amsterdam: stateGasUsed=0, spillBurned=0, collisionBurned=0 + // Pre-Amsterdam: stateGasUsed=0, spillBurned=0 // Should behave identically to pre-8037 gas accounting final var result = baseBuilder() diff --git a/ethereum/referencetests/build.gradle b/ethereum/referencetests/build.gradle index 655f4ba774f..bd92b0b625e 100644 --- a/ethereum/referencetests/build.gradle +++ b/ethereum/referencetests/build.gradle @@ -314,7 +314,7 @@ dependencies { ) devnetTarConfig( group: 'ethereum', name: 'execution-spec-tests', - version: 'bal@v5.3.0', classifier: 'fixtures_bal', ext: 'tar.gz' + version: 'bal@v5.4.0', classifier: 'fixtures_bal', ext: 'tar.gz' ) referenceTestImplementation 'com.fasterxml.jackson.core:jackson-databind' referenceTestImplementation 'com.google.guava:guava' diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java index 29f2c597a22..bdce7e34c13 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java @@ -919,26 +919,6 @@ public long getStateGasSpillBurned() { return txValues.stateGasSpillBurned()[0]; } - /** - * Accumulates regular gas burned by a CREATE child frame that halted before executing code - * (address collision). NOT undone on revert — excluded from block gas accounting but still - * charged for fee purposes. - * - * @param amount the collision gas amount to accumulate - */ - public void accumulateRegularGasCollisionBurned(final long amount) { - txValues.regularGasCollisionBurned()[0] += amount; - } - - /** - * Returns accumulated regular gas burned by pre-execution CREATE collision halts. - * - * @return accumulated collision gas burned - */ - public long getRegularGasCollisionBurned() { - return txValues.regularGasCollisionBurned()[0]; - } - /** * Add recipient to the self-destruct set if not already present. * diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/TxValues.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/TxValues.java index 5fbe6b878fd..679402f2315 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/frame/TxValues.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/frame/TxValues.java @@ -56,9 +56,6 @@ * undone on revert * @param stateGasSpillBurned EIP-8037 accumulated state gas that spilled from reverted child * frames; NOT undone on revert (permanent burn counter for block accounting) - * @param regularGasCollisionBurned EIP-8037 accumulated regular gas burned by CREATE child frames - * that halted before executing any code (address collision); NOT undone on revert. Excluded - * from block regular gas accounting but still counts toward fee deduction. */ public record TxValues( BlockHashLookup blockHashLookup, @@ -78,8 +75,7 @@ public record TxValues( UndoScalar gasRefunds, UndoScalar stateGasUsed, UndoScalar stateGasReservoir, - long[] stateGasSpillBurned, - long[] regularGasCollisionBurned) { + long[] stateGasSpillBurned) { /** * Creates a new TxValues for the initial (depth-0) frame of a transaction. EIP-8037 gas tracking @@ -124,7 +120,6 @@ public static TxValues forTransaction( new UndoScalar<>(0L), new UndoScalar<>(0L), new UndoScalar<>(0L), - new long[] {0L}, new long[] {0L}); } diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java index 8e7791c9e47..b3a4d50e9a3 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java @@ -89,9 +89,7 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) { frame.clearReturnData(); - // EIP-8037: Charge state gas for CREATE operation (new account creation: 112 * cpsb) - // Charged before balance/depth/initcode-size checks — state gas is consumed even on failure. - // For oversized initcode, the spill mechanism tracks the burned state gas via spillBurned. + // EIP-8037: Charge state gas for CREATE operation. if (!gasCalculator().stateGasCostCalculator().chargeCreateStateGas(frame)) { return new OperationResult(cost, ExceptionalHaltReason.INSUFFICIENT_GAS); } @@ -103,7 +101,9 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) { return new OperationResult(cost, ExceptionalHaltReason.INSUFFICIENT_GAS); } - Code code = codeSupplier.get(); + // Resolve initcode after state gas charge to avoid unnecessary work (e.g. memory read, + // Code object creation) on paths where state gas is insufficient. + final Code code = codeSupplier.get(); if (code != null && code.getSize() > evm.getMaxInitcodeSize()) { frame.popStackItems(getStackItemsConsumed()); diff --git a/evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java b/evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java index 5e6677c6a34..b56dad19c1f 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java @@ -167,21 +167,9 @@ private void handleStateGasSpill(final MessageFrame frame) { * Gets called when the message frame encounters an exceptional halt. * * @param frame The message frame - * @param preExecutionHalt true if the halt occurred before any code was executed (e.g. address - * collision in CONTRACT_CREATION detected in start()) */ - private void exceptionalHalt(final MessageFrame frame, final boolean preExecutionHalt) { + private void exceptionalHalt(final MessageFrame frame) { handleStateGasSpill(frame); - - // EIP-8037: Gas burned by a CREATE child that halted before executing any code (address - // collision) is excluded from block regular gas accounting. It still counts toward fees. - if (preExecutionHalt && frame.getType() == MessageFrame.Type.CONTRACT_CREATION) { - final long collisionGas = frame.getRemainingGas(); - if (collisionGas > 0) { - frame.accumulateRegularGasCollisionBurned(collisionGas); - } - } - frame.clearGasRemaining(); frame.clearOutputData(); frame.setState(MessageFrame.State.COMPLETED_FAILED); @@ -262,7 +250,7 @@ public void process(final MessageFrame frame, final OperationTracer operationTra } if (frame.getState() == MessageFrame.State.EXCEPTIONAL_HALT) { - exceptionalHalt(frame, !wasCodeExecuting); + exceptionalHalt(frame); } if (frame.getState() == MessageFrame.State.REVERT) { diff --git a/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java b/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java index d264c91e9d6..cb67c029fb4 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java @@ -169,8 +169,26 @@ public void codeSuccess(final MessageFrame frame, final OperationTracer operatio final Bytes contractCode = frame.getCreatedCode() == null ? frame.getOutputData() : frame.getCreatedCode().getBytes(); - final long depositFee = evm.getGasCalculator().codeDepositGasCost(contractCode.size()); + // Oversized contracts must fail without charging code deposit gas or state gas. + // We must check this first. + final Optional firstValidationFailure = + contractValidationRules.stream() + .map(rule -> rule.validate(contractCode, frame, evm)) + .flatMap(Optional::stream) + .findFirst(); + if (firstValidationFailure.isPresent()) { + if (frame.getDepth() == 0) { + failCodeDepositWithoutRollback(frame, operationTracer, firstValidationFailure); + } else { + frame.setExceptionalHaltReason(firstValidationFailure); + frame.setState(MessageFrame.State.EXCEPTIONAL_HALT); + operationTracer.traceAccountCreationResult(frame, firstValidationFailure); + } + return; + } + // Check and charge code deposit gas (regular gas) before state gas + final long depositFee = evm.getGasCalculator().codeDepositGasCost(contractCode.size()); if (frame.getRemainingGas() < depositFee) { LOG.trace( "Not enough gas to pay the code deposit fee for {}: " @@ -187,80 +205,41 @@ public void codeSuccess(final MessageFrame frame, final OperationTracer operatio } else { frame.setState(MessageFrame.State.COMPLETED_SUCCESS); } - } else { - final var invalidReason = - contractValidationRules.stream() - .map(rule -> rule.validate(contractCode, frame, evm)) - .filter(Optional::isPresent) - .findFirst(); - if (invalidReason.isEmpty()) { - frame.decrementRemainingGas(depositFee); - - // EIP-8037: Charge state gas for code deposit (cpsb * codeSize) - if (!evm.getGasCalculator() - .stateGasCostCalculator() - .chargeCodeDepositStateGas(frame, contractCode.size())) { - LOG.trace("Contract creation error: insufficient state gas for code deposit"); - // EIP-8037: For depth-0 (initial tx) frames, use forced charge + no-rollback failure so - // that stateGasUsed is preserved for block gas accounting (e.g. short_one_gas test). - if (frame.getDepth() == 0) { - final long stateGasAmount = - evm.getGasCalculator() - .stateGasCostCalculator() - .codeDepositStateGas(contractCode.size(), frame.getBlockValues().getGasLimit()); - if (stateGasAmount > 0) { - frame.consumeStateGasForced(stateGasAmount); - } - failCodeDepositWithoutRollback( - frame, operationTracer, Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS)); - } else { - frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS)); - frame.setState(MessageFrame.State.EXCEPTIONAL_HALT); - operationTracer.traceAccountCreationResult( - frame, Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS)); - } - return; - } + return; + } + frame.decrementRemainingGas(depositFee); - // Finalize contract creation, setting the contract code. - final MutableAccount contract = - frame.getWorldUpdater().getOrCreate(frame.getContractAddress()); - contract.setCode(contractCode); - LOG.trace( - "Successful creation of contract {} with code of size {} (Gas remaining: {})", - frame.getContractAddress(), - contractCode.size(), - frame.getRemainingGas()); - frame.setState(MessageFrame.State.COMPLETED_SUCCESS); - if (operationTracer.isExtendedTracing()) { - operationTracer.traceAccountCreationResult(frame, Optional.empty()); - } + // Only now charge state gas for code deposit (cpsb * codeSize). + if (!evm.getGasCalculator() + .stateGasCostCalculator() + .chargeCodeDepositStateGas(frame, contractCode.size())) { + LOG.trace("Contract creation error: insufficient state gas for code deposit"); + if (frame.getDepth() == 0) { + // Do NOT force-charge state gas here. The spec's charge_state_gas raises + // OutOfGasError without modifying anything (no reservoir drain, no stateGasUsed + // increment). failCodeDepositWithoutRollback clears remaining gas, matching + // the spec's exception handler behavior of burning gas_left. + failCodeDepositWithoutRollback( + frame, operationTracer, Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS)); } else { - // EIP-8037: For depth-0 (initial tx) frames with code that fails validation (e.g. - // CODE_TOO_LARGE), charge the code deposit state gas before failing so the charge is - // preserved in stateGasUsed for block gas accounting (e.g. over_max test). - // Use consumeStateGas (not forced) — it returns false without modifying on failure, so - // we can safely fall through to EXCEPTIONAL_HALT if gas is insufficient. - if (frame.getDepth() == 0) { - final long stateGasAmount = - evm.getGasCalculator() - .stateGasCostCalculator() - .codeDepositStateGas(contractCode.size(), frame.getBlockValues().getGasLimit()); - if (stateGasAmount > 0 && frame.consumeStateGas(stateGasAmount)) { - // Sufficient state gas: charge succeeded, fail without rollback so stateGasUsed - // is preserved for block accounting. - final Optional haltReason = invalidReason.get(); - failCodeDepositWithoutRollback(frame, operationTracer, haltReason); - return; - } - // Insufficient state gas or no state gas (non-EIP-8037): fall through to - // EXCEPTIONAL_HALT. - } - final Optional exceptionalHaltReason = invalidReason.get(); - frame.setExceptionalHaltReason(exceptionalHaltReason); + frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS)); frame.setState(MessageFrame.State.EXCEPTIONAL_HALT); - operationTracer.traceAccountCreationResult(frame, exceptionalHaltReason); + operationTracer.traceAccountCreationResult( + frame, Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS)); } + return; + } + + final MutableAccount contract = frame.getWorldUpdater().getOrCreate(frame.getContractAddress()); + contract.setCode(contractCode); + LOG.trace( + "Successful creation of contract {} with code of size {} (Gas remaining: {})", + frame.getContractAddress(), + contractCode.size(), + frame.getRemainingGas()); + frame.setState(MessageFrame.State.COMPLETED_SUCCESS); + if (operationTracer.isExtendedTracing()) { + operationTracer.traceAccountCreationResult(frame, Optional.empty()); } } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/processor/ContractCreationProcessorTest.java b/evm/src/test/java/org/hyperledger/besu/evm/processor/ContractCreationProcessorTest.java index e173146b489..4ac2f704866 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/processor/ContractCreationProcessorTest.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/processor/ContractCreationProcessorTest.java @@ -15,15 +15,14 @@ package org.hyperledger.besu.evm.processor; import static org.assertj.core.api.Assertions.assertThat; +import static org.hyperledger.besu.evm.frame.MessageFrame.State.COMPLETED_FAILED; import static org.hyperledger.besu.evm.frame.MessageFrame.State.COMPLETED_SUCCESS; -import static org.hyperledger.besu.evm.frame.MessageFrame.State.EXCEPTIONAL_HALT; import org.hyperledger.besu.evm.EVM; import org.hyperledger.besu.evm.EvmSpecVersion; import org.hyperledger.besu.evm.MainnetEVMs; import org.hyperledger.besu.evm.contractvalidation.MaxCodeSizeRule; import org.hyperledger.besu.evm.contractvalidation.PrefixCodeRule; -import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; import org.hyperledger.besu.evm.frame.MessageFrame; import org.hyperledger.besu.evm.internal.EvmConfiguration; import org.hyperledger.besu.evm.testutils.TestMessageFrameBuilder; @@ -56,9 +55,10 @@ void shouldThrowAnExceptionWhenCodeContractFormatInvalidPreEOF() { messageFrame.setGasRemaining(10600L); processor.codeSuccess(messageFrame, OperationTracer.NO_TRACING); - assertThat(messageFrame.getState()).isEqualTo(EXCEPTIONAL_HALT); - assertThat(messageFrame.getExceptionalHaltReason()) - .contains(ExceptionalHaltReason.INVALID_CODE); + // At depth 0, validation failures use COMPLETED_FAILED to preserve state gas reservoir + assertThat(messageFrame.getState()).isEqualTo(COMPLETED_FAILED); + // Gas should be cleared — failCodeDepositWithoutRollback burns all remaining gas + assertThat(messageFrame.getRemainingGas()).isEqualTo(0L); } @Test @@ -102,9 +102,10 @@ void shouldThrowAnExceptionWhenCodeContractTooLarge() { messageFrame.setGasRemaining(10_000_000L); processor.codeSuccess(messageFrame, OperationTracer.NO_TRACING); - assertThat(messageFrame.getState()).isEqualTo(EXCEPTIONAL_HALT); - assertThat(messageFrame.getExceptionalHaltReason()) - .contains(ExceptionalHaltReason.CODE_TOO_LARGE); + // At depth 0, validation failures use COMPLETED_FAILED to preserve state gas reservoir + assertThat(messageFrame.getState()).isEqualTo(COMPLETED_FAILED); + // Gas should be cleared — failCodeDepositWithoutRollback burns all remaining gas + assertThat(messageFrame.getRemainingGas()).isEqualTo(0L); } @Test @@ -142,9 +143,10 @@ void shouldRejectDeployedCodeAboveAmsterdamLimit() { messageFrame.setGasRemaining(10_000_000L); processor.codeSuccess(messageFrame, OperationTracer.NO_TRACING); - assertThat(messageFrame.getState()).isEqualTo(EXCEPTIONAL_HALT); - assertThat(messageFrame.getExceptionalHaltReason()) - .contains(ExceptionalHaltReason.CODE_TOO_LARGE); + // At depth 0, validation failures use COMPLETED_FAILED to preserve state gas reservoir + assertThat(messageFrame.getState()).isEqualTo(COMPLETED_FAILED); + // Gas should be cleared — failCodeDepositWithoutRollback burns all remaining gas + assertThat(messageFrame.getRemainingGas()).isEqualTo(0L); } @Test diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index e3c72034b1e..eea257e5128 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -1721,9 +1721,9 @@ - - - + + +