-
Notifications
You must be signed in to change notification settings - Fork 750
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
evm: add blobgasfee eip 7516 #3035
Changes from all commits
f208388
8a25c22
2f8e578
bcfedc8
1e84dde
7665573
7aac40c
f8e3152
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -760,6 +760,18 @@ | |
return baseFee | ||
} | ||
|
||
/** | ||
* Returns the Blob Base Fee of the block as proposed in [EIP-7516](https;//eips.etheruem.org/EIPS/eip-7516) | ||
*/ | ||
getBlobBaseFee(): bigint { | ||
const blobBaseFee = this._env.block.header.getBlobGasPrice() | ||
if (blobBaseFee === undefined) { | ||
// Sanity check | ||
throw new Error('Block has no Blob Base Fee') | ||
} | ||
return blobBaseFee | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not so deep in the blob pricing topic: so is "blobBaseFee === blobGasPrice" here? If so: I think we then should consider still renaming all There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, the confusion is real- here is an actual discussion happening on it lol: https://discord.com/channels/595666850260713488/745077610685661265/1153600913932554270 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guess we might want to wait with merging here until this is settled? 🤔 Since this is the very last question on ACD channel and still un-answered? |
||
|
||
/** | ||
* Returns the chain ID for current chain. Introduced for the | ||
* CHAINID opcode proposed in [EIP-1344](https://eips.ethereum.org/EIPS/eip-1344). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually the opcode gas fee lookup in evm is:
so it has to be all lowercase i think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I loked across the
eips.ts
file and there's no consistency here. Just a silly thought but maybe a follow-up PR is to just make all of thegasPrices
fields incommon/src/eips
lowercase so we can drop thattoLowerCase
call here. That's certainly a small performance gain for every single time we get the base fee.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@acolytec3 Note that the line which @g11tech references to is in
codes.ts
of EVM (the opcode builder). The opcode builder expects all to be opcode names in common to be lowercase. Note: this method is only called if we get new opcodes (at EVM startup + fork transition) so this is not called each time for all opcodes if one is called.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't die on this hill but in most cases we do have the camel case and I do somewhat like it for readability, but it is also true that this is a lot more inconsistent than I anticipated when looking through Common naming. Atm we can't update anyhow since this would be a breaking change.
So for here I guess we can keep both ways.