eip-7547 #1
eip-7547 #1
Conversation
Remove `inclusion_list_exclusions` and simplify logic
| nonce: U64 | ||
|
|
||
| @dataclass | ||
| class InclusionListSummary: | ||
| """ | ||
| Full inclusion list summary. | ||
| """ | ||
| slot: U64 | ||
| proposer_index: U64 | ||
| parent_hash: Hash32 |
There was a problem hiding this comment.
Do we need to have nonce or only address list should suffice as inclusion entries?
I don't see any use on EL side for slot, proposer_index nor parent_hash, why are they supplied?
There was a problem hiding this comment.
nonce is used to avoid having to fetch which entries were satisfied by the parent block. danny prefers to not have cross block dependencies (e.g., need to fetch n-1 block to validate block n).
the slot and proposer will be checked by the CL but to make the type the same i just pass them in. for parent_hash, we need to make sure the parent hash matches. let me try to add that somewhere
There was a problem hiding this comment.
I probably don't understand something, but this seems overly complicated to me.
Currently IL requires:
- That a block is built on specified parent block
- That a block has transactions for address X up to at leas nonce N.
Why can't it be simplified to only be:
- Block has transactions for address X?
|
as for shanghai, since this is just the PoC spec, it felt ok to directly modify. can migrate once it stabilizes on the CL end more if that's ok? that is where most of the complexity is IMO. see ethereum/consensus-specs#3617 |
| @@ -66,6 +67,7 @@ | |||
| ELASTICITY_MULTIPLIER = 2 | |||
| GAS_LIMIT_ADJUSTMENT_FACTOR = 1024 | |||
| GAS_LIMIT_MINIMUM = 5000 | |||
| INCLUSION_LIST_GAS = 4194304 # 2^22 gas for the IL (to start) | |||
There was a problem hiding this comment.
I don't think we should make this a constant, this should be dynamic.
There was a problem hiding this comment.
this is the maximum. there needs to be a max!
| # If any inclusion list addresses are not satisfied, the block is invalid. | ||
| for addr in inclusion_list_nonces: | ||
| sender_account = get_account(env.state, addr) | ||
| if inclusion_list_nonces[addr] < sender_account.nonce: |
There was a problem hiding this comment.
It seems to me that two txs from the same sender in an IL is invalid, is this correct?
There was a problem hiding this comment.
should be fine! b/c the IL commits to the nonce and the address, so they can both be included. we just check that the nonce of the account is higher than what was specified in the IL
| @@ -411,7 +413,8 @@ def apply_body( | |||
| transactions: Tuple[Union[LegacyTransaction, Bytes], ...], | |||
| chain_id: U64, | |||
| withdrawals: Tuple[Withdrawal, ...], | |||
| ) -> Tuple[Uint, Root, Root, Bloom, State, Root]: | |||
| inclusion_list_summary: InclusionListSummary, | |||
| ) -> Tuple[Uint, Root, Root, Bloom, State, Root, Root, Root]: | |||
There was a problem hiding this comment.
I see two extra Root fields appended to the retval here, but no update to the Returns section of the comment below, nor to the return at the end of the function.
Overview: Update
state_transitionandapply_bodyto make IL checks part of block validation.Note some pulled from ethereum/execution-specs@master...fradamt:execution-specs:eip-7547.
Itemized:
fork_types.pyInclusionListSummaryEntryandInclusionListSummaryHeader(ExecutionPayloadHeaderin the CL spec) andBlock(ExecutionPayloadin the CL spec) types.state_transitionto pass the IL summary toapply_body.apply_body(called fromstate_transition) to calculate the added gas from the inclusion list and assert that the inclusion list entries are satisfied.