Blobgas computation in CL #3813
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introducing new fields in the middle of an existing `Container` pointlessly breaks merkleization of all subsequent fields. In the case of `committee_bits`, it is also misleading, as `signature` only covers `data` inside `Attestation`.
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
potuz
reviewed
Jun 24, 2024
Co-authored-by: Potuz <potuz@prysmaticlabs.com>
mkalinin
reviewed
Jul 1, 2024
| @@ -423,10 +435,50 @@ class BeaconState(Container): | |||
| # [New in Electra:EIP7251] | |||
| pending_partial_withdrawals: List[PendingPartialWithdrawal, PENDING_PARTIAL_WITHDRAWALS_LIMIT] | |||
| pending_consolidations: List[PendingConsolidation, PENDING_CONSOLIDATIONS_LIMIT] # [New in Electra:EIP7251] | |||
| # [New in Electra: compute blob gas in CL] | |||
| excess_blob_gas: uint64 | |||
| base_fee_per_blob_gas: uint64 | |||
There was a problem hiding this comment.
do we need to keep this parameter to be in the state as it can be easily computed given the state?
| get_base_fee_per_blob_gas(state)): | ||
| return False | ||
|
|
||
| state.excess_blob_gas += execution_payload.blob_gas_used |
There was a problem hiding this comment.
I think this like should be a part of the process_execution_payload. The other thing is that there is no excess blob gas computation, I believe it should be computed as follows:
def compute_excess_blob_gas(excess_blob_gas: uint64, blob_gas_used: uint64) -> uint64:
if blob_gas_used > TARGET_BLOB_GAS_PER_BLOCK:
return excess_blob_gas + blob_gas_used - TARGET_BLOB_GAS_PER_BLOCK
else:
if excess_blob_gas < TARGET_BLOB_GAS_PER_BLOCK - blob_gas_used
return uint64(0)
else:
return excess_blob_gas - (TARGET_BLOB_GAS_PER_BLOCK - blob_gas_used)| @@ -332,7 +344,7 @@ class ExecutionPayload(Container): | |||
| transactions: List[Transaction, MAX_TRANSACTIONS_PER_PAYLOAD] | |||
| withdrawals: List[Withdrawal, MAX_WITHDRAWALS_PER_PAYLOAD] | |||
| blob_gas_used: uint64 | |||
| excess_blob_gas: uint64 | |||
| excess_blob_gas: uint64 # [Deprecated in Electra: compute blob gas in CL] | |||
There was a problem hiding this comment.
I believe this field can be repurposed and renamed to base_fee_per_blob_gas, this is to make EL block self-contained in terms of execution which is required for optimistic sync (in an edge case but still). Then, this parameter must be verified in the process_execution_payload:
assert payload.base_fee_per_blob_gas == get_base_fee_per_blob_gas(state)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an alternative version of #3800 where in addition to specifying the blob gas limit in CL, it lifts the whole basefee computation into the CL.
Potential advantages are: