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

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

Merged

Conversation

lu-pinto
Copy link
Contributor

@lu-pinto lu-pinto commented Nov 7, 2024

PR description

This fixes gas charges when creating contracts. While other clients charge the contract account creation while writing the nonce, besu was charging when the contract code was written to storage. A mismatch occurs if the code fails to deploy but the nonce is still written. In that case BASIC_DATA and CODE_HASH (empty contract hashcode) are still written to, despite code not being deployed.

@lu-pinto lu-pinto requested a review from matkt November 7, 2024 12:46
…ccessfully created to transaction start

Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
@lu-pinto lu-pinto force-pushed the fix-eip-4762-gas-charge-contract-creation branch from a525c77 to cc7201a Compare November 7, 2024 12:47
@@ -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

@@ -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

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

…code successfully created to transaction start

 compact code for debug printing witness gas schedule

Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
…code successfully created to transaction start

 update verkle reference tests build

Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
@lu-pinto lu-pinto merged commit 445235d into hyperledger:verkle Nov 22, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants