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 EIP-4844: Rename "data gas" to "blob gas" #7354

Merged
merged 1 commit into from
Jul 24, 2023

Conversation

lightclient
Copy link
Member

I think "data gas" is problematic because we already have the concept of "data" in transaction -> calldata. To uninitiated, things like excess_data_gas seems to refer to calldata. Of course this isn't the case though. We should improve the naming to be clear what "data gas" is and that it actually refers to "blob data".

Ultimately I decided to drop "data" almost entirely and refer to gas for blobs as "blob gas". The idea here is that it is gas for paying for blobs. One argument might be that this conflates the idea that a blob might cost a lot (2**17) "blob gas". I think this isn't the case. The "blob" adjective is just a discriminator to note that this gas is typed for blob payment. GAS_PER_BLOB further clarifies the unit of gas being a fraction of the total cost of a blob. This is the cleanest approach IMO.

@github-actions github-actions bot added c-update Modifies an existing proposal s-review This EIP is in Review t-core labels Jul 18, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Jul 18, 2023

✅ All reviewers have approved.

@eth-bot eth-bot changed the title Rename "data gas" to "blob gas" Update EIP-4844: Rename "data gas" to "blob gas" Jul 18, 2023
@djrtwo
Copy link
Contributor

djrtwo commented Jul 18, 2023

@adietrichs wanted this term to be more general to the abstract notion of transient data -- in whatever form it might come. Thus the name and how it has it's own min/max and independently a price.

I sympathize with this generality -- blobs are a very specific construct in this pricing regime and not necessarily all types of transient data constructs. If we need to change to disambiguate, I might be in favor of something like DA_GAS

@fjl
Copy link
Contributor

fjl commented Jul 18, 2023

So you are arguing for dataAvailabilityGas? Related field names would change to dataAvailabilityGasUsed and excessDataAvailabilityGas.

@lightclient
Copy link
Member Author

I don't think we should be worried too much about a post-blob world with the naming if it actively harms the understandability of the protocol. Blobs are designed specifically to be our "end game" scaling form factor (thus all the work on getting the crypto right). If something else with a slightly different form factor shows up, we should tackle the disambiguation then IMO.

@hwwhww
Copy link
Contributor

hwwhww commented Jul 24, 2023

Note: if we accept it, it requires a corresponding CL specs update.

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

imo its good since we also renamed opcode from data hash => blob hash 👍

@lightclient
Copy link
Member Author

Approved in 4844 breakout call 27.

@lightclient lightclient marked this pull request as ready for review July 24, 2023 17:02
@lightclient lightclient reopened this Jul 24, 2023
@eth-bot eth-bot enabled auto-merge (squash) July 24, 2023 19:39
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

github-merge-queue bot pushed a commit to ledgerwatch/erigon-lib that referenced this pull request Jul 28, 2023
AskAlexSharov pushed a commit to ledgerwatch/erigon-lib that referenced this pull request Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal s-review This EIP is in Review t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants