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

Open
wants to merge 2 commits into
base: verkle
Choose a base branch
from

Conversation

lu-pinto
Copy link
Contributor

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

PR description

This fixes gas charges when creating contract where other clients are charging the contract account creation when nonce is incremented on the contract account and not when the code is completed. An issues was spotted where if the init code does not complete we were not applying any charges.

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

Signed-off-by: Luis Pinto <[email protected]>
@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 <[email protected]>
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