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

Update the type for blob_gas_used #1161

Merged
merged 2 commits into from
Apr 1, 2025

Conversation

Shashwat-Nautiyal
Copy link

@Shashwat-Nautiyal Shashwat-Nautiyal commented Mar 17, 2025

What was wrong?

The blob_gas_used variable had inconsistent types across the repo. Somewhere U64 and somewhere Uint.

Related to Issue #1125

How was it fixed?

I updated the type of blob_gas_used to U64 across all files. Ensured to also update the return types of functions that populated the blob_gas_used variable.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

tx: Transaction,
index: Uint,
) -> None:
env: vm.Environment, tx: Transaction
Copy link
Collaborator

Choose a reason for hiding this comment

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

The blob_gas_used was introduced in Cancun and as such should only affect the Cancun and the Prague fork folders. The code in the earlier forks folders should not change unless we are back-porting something.

Copy link
Author

Choose a reason for hiding this comment

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

umm, actually these changes are not made by me. Maybe they got included while I was rebasing the branch. Can you suggest what should i do ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm. Adding a commit with the relevant changes on top of the current forks/prague would be the cleanest way to handle this imo. Let me know if you need any further details.

Copy link
Author

Choose a reason for hiding this comment

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

@gurukamath Sorry but I am not exactly sure how to handle this. Should i create a new branch and cherry-pick only the feature commit or is there any other alternative?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That should work.

Copy link
Author

Choose a reason for hiding this comment

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

@gurukamath, I cherry-picked the commits into a new feature branch and then force pushed it to the current branch,

@gurukamath gurukamath changed the title Closes #1125 ( Rebased to Forks/Prague ) Update the type for blob_gas_used Mar 19, 2025
@gurukamath
Copy link
Collaborator

There are some linting issues with static code checks. You can find out the issues by running the command tox -e static.

I would also suggest you to run tox -e py3 locally to make sure all the tests are passing.

@gurukamath gurukamath merged commit 75635cd into ethereum:forks/prague Apr 1, 2025
3 of 7 checks passed
@Shashwat-Nautiyal Shashwat-Nautiyal deleted the blob_gas_used branch April 1, 2025 12:47
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