Add executable gossip validation functions for gloas#5294
Conversation
github-actions
Bot
added
testing
| def validate_execution_payload_bid_gossip( | ||
| seen: Seen, | ||
| store: Store, | ||
| state: BeaconState, |
There was a problem hiding this comment.
what's the state here? eg. for bid validation we need to use the state of the parent block advanced to the bid slot via process_slots to make sure is_active_builder / can_builder_cover_bid are properly evaluated
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
The correct state is process_slots(head_state, bid.slot)
using the head state is not correct, the state needs to be the post-state of the parent block advanced to the bid slot
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
This PR ensures that we no longer just use the head state to verify bids
sigp/lighthouse#9337
We shouldn't use the head state by default, its definitely wrong. But I believe it's only a real issue in a non-finalized forky network
There was a problem hiding this comment.
The PR above also ensures we advance the state to the correct slot before making any checks
There was a problem hiding this comment.
This PR ensures that we no longer just use the head state to verify bids
sigp/lighthouse#9337
do you? maybe I am misreading this
There was a problem hiding this comment.
oh my bad that PR I linked was for preferences not bids, i got the two mixed up. we'll need to fix bids as well
| parent_state = store.block_states[bid.parent_block_root] | ||
| dependent_root = get_proposer_dependent_root(parent_state, proposal_epoch) |
There was a problem hiding this comment.
| parent_state = store.block_states[bid.parent_block_root] | |
| dependent_root = get_proposer_dependent_root(parent_state, proposal_epoch) | |
| dependent_root = get_dependent_root(store, bid.parent_block_root) |
Can we remove get_proposer_dependent_root and use get_dependent_root instead like this? Related to #5196 and #5306. cc @nflaig
There was a problem hiding this comment.
I haven't followed up on that other PR but it should be equivalent
There was a problem hiding this comment.
I'd suggest using get_dependent_root() then. The other PR introduces it for PB conditions check and I think we can use it here, too.
5 participants
This PR adds executable gossip message validation specs for Gloas.
Here are the reference tests for all Gloas gossip tests: