Gloas spec v1.7.0-alpha.5 and beacon_chain tests#8998
Gloas spec v1.7.0-alpha.5 and beacon_chain tests#8998mergify[bot] merged 132 commits intosigp:unstablefrom
Conversation
michaelsproul
added
blocked
work-in-progress
michaelsproul
changed the title
|
There are a lot more failing tests here than I realised. Idk what I was smoking when I thought they were all passing :( |
eserilev
left a comment
eserilev
left a comment
There was a problem hiding this comment.
just the block replayer simplifications alone make me so happy that we aren't dealing with the dual-state transition architecture
looks really good. I still haven't finished reviewing but super stoked about this. Thanks for all your hard work here
dapplion
left a comment
dapplion
left a comment
There was a problem hiding this comment.
Initial pass, will continue today
| let suggested_fee_recipient = execution_layer | ||
| .get_suggested_fee_recipient(proposer_index) | ||
| .await; | ||
| let slot_number = if fork.gloas_enabled() { |
There was a problem hiding this comment.
Why this guard if this function is gloas only?
There was a problem hiding this comment.
Removed in
| Option<Hash256>, | ||
| PayloadStatus, | ||
| Option<Arc<SignedExecutionPayloadEnvelope<T::EthSpec>>>, | ||
| ), |
There was a problem hiding this comment.
Should this become a struct for clarity?
There was a problem hiding this comment.
Refactored in
| let bid = block.body().signed_execution_payload_bid()?.message.clone(); | ||
| let parent_bid = state.latest_execution_payload_bid()?.clone(); |
There was a problem hiding this comment.
Removed the one from the block, but to keep things looking like the spec I left the clone on parent_bid. We run into issues with multiple borrows when calling apply_parent_execution_payload otherwise (can't borrow &mut state and state.latest_execution_payload_bid simultaneously). Clarity probably most important here, as cloning the bid is likely trivial from a performance PoV.
michaelsproul
changed the title
|
@mergify queue |
Merge Queue Status🛑 Queue command has been cancelled |
Merge Queue Status
This pull request spent 29 minutes 43 seconds in the queue, including 28 minutes 16 seconds running CI. Required conditions to merge
|
|
Did a post-merge review and could not find any issues, thank you for the hard work here!! |
4 participants
Issue Addressed
Fix database pruning post-Gloas
Proposed Changes
beacon_chaintests running withFORK_NAME=gloas🎉Additional Info
EF tests are blocked on:
test_builder_payment_after_missed_epochsethereum/consensus-specs#5120Implements:
Builds on:
TODO:
BeaconState::is_parent_block_full(it is deleted from spec)queue_builder_payment