Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not return gas cost along with ExceptionalHaltReason in OperationResult unless OOG #7919

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,12 @@
"sub": null
},
{
"cost": 8,
"cost": 0,
"ex": {
"mem": null,
"push": [],
"store": null,
"used": 16756191
"used": 16756199
},
"pc": 2,
"sub": null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@
"sub": null
},
{
"cost": 8,
"cost": 0,
"ex": {
"mem": null,
"push": [],
"store": null,
"used": 16756191
"used": 16756199
},
"pc": 2,
"sub": null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@
"sub": null
},
{
"cost": 8,
"cost": 0,
"ex": {
"mem": null,
"push": [],
"store": null,
"used": 16756191
"used": 16756199
},
"pc": 2,
"sub": null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@
"pc" : 0,
"sub" : null
}, {
"cost" : 8,
"cost" : 0,
"ex" : {
"mem" : null,
"push" : [ ],
"store" : null,
"used" : 16756191
"used" : 16756199
},
"pc" : 2,
"sub" : null
Expand All @@ -47,4 +47,4 @@
"id" : 74
},
"statusCode" : 200
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,4 @@
"id" : 102
},
"statusCode" : 200
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,4 @@
"id" : 104
},
"statusCode" : 200
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@
"pc" : 0,
"sub" : null
}, {
"cost" : 8,
"cost" : 0,
"ex" : {
"mem" : null,
"push" : [ ],
"store" : null,
"used" : 16756191
"used" : 16756199
},
"pc" : 2,
"sub" : null
Expand All @@ -40,4 +40,4 @@
"id" : 127
},
"statusCode" : 200
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -269,4 +269,4 @@
"id" : 155
},
"statusCode" : 200
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,4 @@
"id" : 157
},
"statusCode" : 200
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.log.Log;
import org.hyperledger.besu.evm.operation.Operation;
import org.hyperledger.besu.evm.operation.OperationResult;
import org.hyperledger.besu.evm.worldstate.WorldView;
import org.hyperledger.besu.plugin.data.BlockBody;
import org.hyperledger.besu.plugin.data.BlockHeader;
Expand Down Expand Up @@ -67,8 +67,7 @@ public void tracePreExecution(final MessageFrame frame) {
}

@Override
public void tracePostExecution(
final MessageFrame frame, final Operation.OperationResult operationResult) {
public void tracePostExecution(final MessageFrame frame, final OperationResult operationResult) {
checkInterrupt();
delegate.tracePostExecution(frame, operationResult);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,7 @@ public PrecompileContractResult computePrecompile(
pmtHash, privacyGroupId, disposablePrivateState, privateMetadataUpdater, result);
}

return new PrecompileContractResult(
result.getOutput(), true, MessageFrame.State.CODE_EXECUTING, Optional.empty());
return PrecompileContractResult.executing(result.getOutput());
}

private void sendParticipantRemovedEvent(final PrivateTransaction privateTransaction) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ public PrecompileContractResult computePrecompile(
pmtHash, privacyGroupId, disposablePrivateState, privateMetadataUpdater, result);
}

return new PrecompileContractResult(
result.getOutput(), true, MessageFrame.State.CODE_EXECUTING, Optional.empty());
return PrecompileContractResult.executing(result.getOutput());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import org.hyperledger.besu.evm.worldstate.WorldUpdater;

import java.util.Base64;
import java.util.Optional;
import javax.annotation.Nonnull;

import org.apache.tuweni.bytes.Bytes;
Expand All @@ -68,9 +67,7 @@ public class PrivacyPrecompiledContract extends AbstractPrecompiledContract {

private static final Logger LOG = LoggerFactory.getLogger(PrivacyPrecompiledContract.class);

static final PrecompileContractResult NO_RESULT =
new PrecompileContractResult(
Bytes.EMPTY, true, MessageFrame.State.CODE_EXECUTING, Optional.empty());
static final PrecompileContractResult NO_RESULT = PrecompileContractResult.executing(Bytes.EMPTY);

public PrivacyPrecompiledContract(
final GasCalculator gasCalculator,
Expand Down Expand Up @@ -219,8 +216,7 @@ public PrecompileContractResult computePrecompile(
pmtHash, privacyGroupId, disposablePrivateState, privateMetadataUpdater, result);
}

return new PrecompileContractResult(
result.getOutput(), true, MessageFrame.State.CODE_EXECUTING, Optional.empty());
return PrecompileContractResult.executing(result.getOutput());
}

protected void maybeApplyGenesisToPrivateWorldState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.operation.AbstractCallOperation;
import org.hyperledger.besu.evm.operation.Operation;
import org.hyperledger.besu.evm.operation.Operation.OperationResult;
import org.hyperledger.besu.evm.operation.OperationResult;
import org.hyperledger.besu.evm.tracing.OperationTracer;
import org.hyperledger.besu.evm.worldstate.WorldUpdater;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.hyperledger.besu.evm.operation.AbstractOperation;
import org.hyperledger.besu.evm.operation.CallOperation;
import org.hyperledger.besu.evm.operation.Operation;
import org.hyperledger.besu.evm.operation.Operation.OperationResult;
import org.hyperledger.besu.evm.operation.OperationResult;
import org.hyperledger.besu.evm.worldstate.WorldUpdater;

import java.util.Map;
Expand Down Expand Up @@ -62,7 +62,7 @@ class DebugOperationTracerTest {
new AbstractOperation(0x02, "MUL", 2, 1, null) {
@Override
public OperationResult execute(final MessageFrame frame, final EVM evm) {
return new OperationResult(20L, null);
return new OperationResult(20L);
}
};

Expand Down Expand Up @@ -193,8 +193,7 @@ void shouldCaptureFrameWhenExceptionalHaltOccurs() {

final DebugOperationTracer tracer =
new DebugOperationTracer(new TraceOptions(true, true, true), false);
tracer.tracePostExecution(
frame, new OperationResult(50L, ExceptionalHaltReason.INSUFFICIENT_GAS));
tracer.tracePostExecution(frame, OperationResult.insufficientGas(50L));

final TraceFrame traceFrame = getOnlyTraceFrame(tracer);
assertThat(traceFrame.getExceptionalHaltReason())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
import org.hyperledger.besu.evm.operation.CallCodeOperation;
import org.hyperledger.besu.evm.operation.Operation.OperationResult;
import org.hyperledger.besu.evm.operation.OperationResult;
import org.hyperledger.besu.evm.operation.SStoreOperation;
import org.hyperledger.besu.evm.tracing.EstimateGasOperationTracer;

Expand All @@ -45,7 +45,7 @@ public void setUp() {
@Test
public void shouldDetectChangeInDepthDuringExecution() {

final OperationResult testResult = new OperationResult(6, null);
final OperationResult testResult = new OperationResult(6);

assertThat(operationTracer.getMaxDepth()).isZero();

Expand Down Expand Up @@ -76,7 +76,7 @@ public void shouldDetectChangeInDepthDuringExecution() {
@Test
public void shouldDetectMinimumGasRemainingForSStoreOperation() {

final OperationResult testResult = new OperationResult(6, null);
final OperationResult testResult = new OperationResult(6);
final long minimumGasRemaining = 2300L;

assertThat(operationTracer.getStipendNeeded()).isZero();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,4 @@
{"pc":24,"op":243,"gas":"0x6e","gasCost":"0x0","memSize":96,"stack":["0x1","0x40"],"depth":1,"refund":0,"opName":"RETURN"},
{"stateRoot":"0xcb5e8e232189003640b6f131ea2c09b1791ffd2e8357f64610f638e9a11ab2d2","output":"0x40","gasUsed":"0x619e","pass":true,"fork":"Cancun"}
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@
{"pc":24,"op":243,"gas":"0x6e","gasCost":"0x0","memSize":96,"stack":["0x1","0x40"],"depth":1,"refund":0,"opName":"RETURN"},
{"stateRoot":"0xcb5e8e232189003640b6f131ea2c09b1791ffd2e8357f64610f638e9a11ab2d2","output":"0x40","gasUsed":"0x619e","pass":true,"fork":"Cancun"}
]
}
}
19 changes: 5 additions & 14 deletions evm/src/main/java/org/hyperledger/besu/evm/EVM.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.hyperledger.besu.evm.operation.DupOperation;
import org.hyperledger.besu.evm.operation.ExpOperation;
import org.hyperledger.besu.evm.operation.GtOperation;
import org.hyperledger.besu.evm.operation.InvalidOperation;
import org.hyperledger.besu.evm.operation.IsZeroOperation;
import org.hyperledger.besu.evm.operation.JumpDestOperation;
import org.hyperledger.besu.evm.operation.JumpOperation;
Expand All @@ -49,8 +48,8 @@
import org.hyperledger.besu.evm.operation.MulOperation;
import org.hyperledger.besu.evm.operation.NotOperation;
import org.hyperledger.besu.evm.operation.Operation;
import org.hyperledger.besu.evm.operation.Operation.OperationResult;
import org.hyperledger.besu.evm.operation.OperationRegistry;
import org.hyperledger.besu.evm.operation.OperationResult;
import org.hyperledger.besu.evm.operation.OrOperation;
import org.hyperledger.besu.evm.operation.PopOperation;
import org.hyperledger.besu.evm.operation.Push0Operation;
Expand All @@ -77,14 +76,6 @@
public class EVM {
private static final Logger LOG = LoggerFactory.getLogger(EVM.class);

/** The constant OVERFLOW_RESPONSE. */
protected static final OperationResult OVERFLOW_RESPONSE =
new OperationResult(0L, ExceptionalHaltReason.TOO_MANY_STACK_ITEMS);

/** The constant UNDERFLOW_RESPONSE. */
protected static final OperationResult UNDERFLOW_RESPONSE =
new OperationResult(0L, ExceptionalHaltReason.INSUFFICIENT_STACK_ITEMS);

private final OperationRegistry operations;
private final GasCalculator gasCalculator;
private final Operation endOfScriptStop;
Expand Down Expand Up @@ -241,7 +232,7 @@ public void runToHalt(final MessageFrame frame, final OperationTracer tracing) {
case 0x09 -> MulModOperation.staticOperation(frame);
case 0x0a -> ExpOperation.staticOperation(frame, gasCalculator);
case 0x0b -> SignExtendOperation.staticOperation(frame);
case 0x0c, 0x0d, 0x0e, 0x0f -> InvalidOperation.INVALID_RESULT;
case 0x0c, 0x0d, 0x0e, 0x0f -> OperationResult.invalidOperation();
case 0x10 -> LtOperation.staticOperation(frame);
case 0x11 -> GtOperation.staticOperation(frame);
case 0x12 -> SLtOperation.staticOperation(frame);
Expand All @@ -259,7 +250,7 @@ public void runToHalt(final MessageFrame frame, final OperationTracer tracing) {
case 0x5f ->
enableShanghai
? Push0Operation.staticOperation(frame)
: InvalidOperation.INVALID_RESULT;
: OperationResult.invalidOperation();
case 0x60, // PUSH1-32
0x61,
0x62,
Expand Down Expand Up @@ -333,9 +324,9 @@ public void runToHalt(final MessageFrame frame, final OperationTracer tracing) {
}
};
} catch (final OverflowException oe) {
result = OVERFLOW_RESPONSE;
result = OperationResult.overFlow();
} catch (final UnderflowException ue) {
result = UNDERFLOW_RESPONSE;
result = OperationResult.underFlow();
}
final ExceptionalHaltReason haltReason = result.getHaltReason();
if (haltReason != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.hyperledger.besu.evm.EVM;
import org.hyperledger.besu.evm.account.Account;
import org.hyperledger.besu.evm.code.CodeV0;
import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.frame.MessageFrame.State;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
Expand All @@ -39,10 +38,6 @@
*/
public abstract class AbstractCallOperation extends AbstractOperation {

/** The constant UNDERFLOW_RESPONSE. */
protected static final OperationResult UNDERFLOW_RESPONSE =
new OperationResult(0L, ExceptionalHaltReason.INSUFFICIENT_STACK_ITEMS);

static final Bytes LEGACY_SUCCESS_STACK_ITEM = BYTES_ONE;
static final Bytes LEGACY_FAILURE_STACK_ITEM = Bytes.EMPTY;

Expand Down Expand Up @@ -177,14 +172,14 @@ protected boolean isDelegate() {
public OperationResult execute(final MessageFrame frame, final EVM evm) {
// manual check because some reads won't come until the "complete" step.
if (frame.stackSize() < getStackItemsConsumed()) {
return UNDERFLOW_RESPONSE;
return OperationResult.underFlow();
}

final Address to = to(frame);
final boolean accountIsWarm = frame.warmUpAddress(to) || gasCalculator().isPrecompile(to);
final long cost = cost(frame, accountIsWarm);
if (frame.getRemainingGas() < cost) {
return new OperationResult(cost, ExceptionalHaltReason.INSUFFICIENT_GAS);
return OperationResult.insufficientGas(cost);
}
frame.decrementRemainingGas(cost);

Expand All @@ -196,8 +191,7 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
final DelegatedCodeGasCostHelper.Result result =
deductDelegatedCodeGasCost(frame, gasCalculator(), contract);
if (result.status() != DelegatedCodeGasCostHelper.Status.SUCCESS) {
return new Operation.OperationResult(
result.gasCost(), ExceptionalHaltReason.INSUFFICIENT_GAS);
return OperationResult.insufficientGas(result.gasCost());
}
}

Expand All @@ -213,7 +207,7 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
frame.incrementRemainingGas(gasAvailableForChildCall(frame) + cost);
frame.popStackItems(getStackItemsConsumed());
frame.pushStackItem(LEGACY_FAILURE_STACK_ITEM);
return new OperationResult(cost, null);
return new OperationResult(cost);
}

final Bytes inputData = frame.readMutableMemory(inputDataOffset(frame), inputDataLength(frame));
Expand All @@ -225,7 +219,7 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {

// invalid code results in a quick exit
if (!code.isValid()) {
return new OperationResult(cost, ExceptionalHaltReason.INVALID_CODE, 0);
return OperationResult.invalidCode();
}

MessageFrame.builder()
Expand All @@ -246,7 +240,7 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
frame.incrementRemainingGas(cost);

frame.setState(MessageFrame.State.CODE_SUSPENDED);
return new OperationResult(cost, null, 0);
return new OperationResult(cost, 0);
}

/**
Expand Down
Loading