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.

Sign up for GitHub

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

Jump to bottom

Move EIP-4762 gas charging for contract account creation from code su… #7869

Merged
merged 3 commits into from
Nov 22, 2024
Merged

Move EIP-4762 gas charging for contract account creation from code su… #7869

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc7201a
Move EIP-4762 gas charging for contract account creation from code su…
lu-pinto Nov 7, 2024
0ecd16f
fixup! Move EIP-4762 gas charging for contract account creation from …
lu-pinto Nov 11, 2024
95a9f49
fixup! Move EIP-4762 gas charging for contract account creation from …
lu-pinto Nov 22, 2024
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
3 changes: 2 additions & 1 deletion evm/src/main/java/org/hyperledger/besu/evm/gascalculator/stateless/Eip4762AccessWitness.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ public long touchAddressAndChargeGas(
final int accessMode) {
final short accessEvents = touchAddress(address, treeIndex, subIndex, accessMode);
long gas = 0;
LOG.atDebug().log(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me it is useful for debugging because it tells me what has been charged in a single touch call, because we could have multiple charges in one call.
Maybe we can do a pretty print at the end to show this information at once instead. I'll have a look

if (AccessEvents.isBranchRead(accessEvents)) {
gas = clampedAdd(gas, WITNESS_BRANCH_COST);
final long gasView = gas;
Expand Down Expand Up @@ -368,7 +369,7 @@ public long touchAddressAndChargeGas(
+ " "
+ gasView);
}

LOG.atDebug().log("<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

return gas;
}

Expand Down
126 changes: 74 additions & 52 deletions evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.Supplier;

import org.apache.tuweni.bytes.Bytes;
import org.slf4j.Logger;
Expand Down Expand Up @@ -101,7 +102,17 @@ public void start(final MessageFrame frame, final OperationTracer operationTrace
final MutableAccount contract = frame.getWorldUpdater().getOrCreate(contractAddress);
long statelessGasCost =
evm.getGasCalculator().proofOfAbsenceCost(frame, contract.getAddress());
if (handleInsufficientGas(
frame,
statelessGasCost,
() ->
String.format(
"Not enough gas to cover proof of absence fee for %s: remaining gas = %d < %d = creation fee",
frame.getContractAddress(), frame.getRemainingGas(), statelessGasCost))) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to return or set the state of the frame as we do here ?
frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.ILLEGAL_STATE_CHANGE));
frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it Illegal state change? I think it is insufficient gas. It's already done inside handleInsufficientGas

}
frame.decrementRemainingGas(statelessGasCost);

if (accountExists(contract)) {
LOG.trace(
"Contract creation error: account has already been created for address {}",
Expand All @@ -111,6 +122,20 @@ public void start(final MessageFrame frame, final OperationTracer operationTrace
operationTracer.traceAccountCreationResult(
frame, Optional.of(ExceptionalHaltReason.ILLEGAL_STATE_CHANGE));
} else {
final long accountCreationFee =
evm.getGasCalculator().completedCreateContractGasCost(frame);
if (handleInsufficientGas(
frame,
accountCreationFee,
() ->
String.format(
"Not enough gas to pay the contract creation fee for %s: "
+ "remaining gas = %d < %d = creation fee",
frame.getContractAddress(), frame.getRemainingGas(), accountCreationFee))) {
return;
}
frame.decrementRemainingGas(accountCreationFee);

frame.addCreate(contractAddress);
contract.incrementBalance(frame.getValue());
contract.setNonce(initialContractNonce);
Expand All @@ -132,65 +157,62 @@ public void codeSuccess(final MessageFrame frame, final OperationTracer operatio

final long depositFee = evm.getGasCalculator().codeDepositGasCost(frame, contractCode.size());

if (frame.getRemainingGas() < depositFee) {
LOG.trace(
"Not enough gas to pay the code deposit fee for {}: "
+ "remaining gas = {} < {} = deposit fee",
frame.getContractAddress(),
frame.getRemainingGas(),
depositFee);
if (handleInsufficientGas(
frame,
depositFee,
() ->
String.format(
"Not enough gas to pay the code deposit fee for %s: "
+ "remaining gas = %d < %d = deposit fee",
frame.getContractAddress(), frame.getRemainingGas(), depositFee))) {

if (requireCodeDepositToSucceed) {
LOG.trace("Contract creation error: insufficient funds for code deposit");
frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
operationTracer.traceAccountCreationResult(
frame, Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
} 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);

final long statelessContractCompletionFee =
evm.getGasCalculator().completedCreateContractGasCost(frame);
if (frame.getRemainingGas() < statelessContractCompletionFee) {
LOG.trace(
"Not enough gas to pay the contract creation completion fee for {}: "
+ "remaining gas = {} < {} = deposit fee",
frame.getContractAddress(),
frame.getRemainingGas(),
statelessContractCompletionFee);
frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
} else {
frame.decrementRemainingGas(statelessContractCompletionFee);
// 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);
}
return;
}

if (operationTracer.isExtendedTracing()) {
operationTracer.traceAccountCreationResult(frame, Optional.empty());
}
} else {
final Optional<ExceptionalHaltReason> exceptionalHaltReason = invalidReason.get();
frame.setExceptionalHaltReason(exceptionalHaltReason);
frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
operationTracer.traceAccountCreationResult(frame, exceptionalHaltReason);
}
final var invalidReason =
contractValidationRules.stream()
.map(rule -> rule.validate(contractCode, frame, evm))
.filter(Optional::isPresent)
.findFirst();
if (invalidReason.isPresent()) {
final Optional<ExceptionalHaltReason> exceptionalHaltReason = invalidReason.get();
frame.setExceptionalHaltReason(exceptionalHaltReason);
frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
operationTracer.traceAccountCreationResult(frame, exceptionalHaltReason);
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());
}
}

private static boolean handleInsufficientGas(
final MessageFrame frame, final long gasFee, final Supplier<String> message) {
if (frame.getRemainingGas() < gasFee) {
LOG.trace(message.get());
frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
return true;
}
return false;
}
}
Loading