Gloas gossip handlers - #9924
pawanjay176 wants to merge 33 commits into
Conversation
|
Some required checks have failed. Could you please take a look @pawanjay176? 🙏 |
| } | ||
|
|
||
| let cached_head = ctx.canonical_head.cached_head(); | ||
| let current_slot = ctx |
There was a problem hiding this comment.
I'm not completely sure of this. I'd reckon some of the parent gas limit changes would affect this too, so happy to modify this once that's merged
There was a problem hiding this comment.
just a note for myself later, but it seems the spec has moved to head-compatible bid validation only, so the bid parent state will always be either the head or its parent.
| "gossip_execution_payload_envelope__ignore_pre_finalized", | ||
| ]; | ||
| const IGNORED_EXECUTION_PAYLOAD_BID_CASES: &[&str] = &[ | ||
| // Advancing the parent state across an epoch for every gossip bid would put epoch |
There was a problem hiding this comment.
I'm torn on this one.
To re-enable this test, we need to run epoch processing in gossip which kinda seems unnecessary.
We would still accept the bid if it gets accepted in the block, but ignore it over gossip.
Doing epoch processing in gossip during bad network conditions might be a lot of work, so I have opted for disabling the test and not running epoch processing, but open to change my mind on this.
There was a problem hiding this comment.
maybe we could run the tests and assert that we ignore (with no peer penalty?) rather than accept?
I agree that state advance inside bid validation seems pretty sketchy
There was a problem hiding this comment.
That would require adding a bit of machinery in the harness to special case this test which kinda seems weird. Our current behaviour is ignore + no penalty. If we start doing this for other tests, then the harness would get gnarly soon imo.
There was a problem hiding this comment.
…#10061) ## Issue Addressed Valid payload bids can be dropped with `EpochOutOfBounds` when the cached head is from the previous epoch. Bids on the head's parent can also be rejected with `InvalidPrevRandao` because validation uses the head's RANDAO mix. ## Proposed Changes Validate `prev_randao` against the bid's beacon parent using the existing `head_random` and `parent_random` helpers. Add a regression for a head in the previous epoch. Head-parent RANDAO spec coverage is tracked in [consensus-specs#5645](ethereum/consensus-specs#5645). ## Additional Info The [Gloas gossip rule](https://github.com/ethereum/consensus-specs/blob/a8475719ce77cb269191e327e1f4175c295851ee/specs/gloas/p2p-interface.md#execution_payload_bid) uses the parent post-state's current epoch. Related to #9924.
|
This pull request has merge conflicts. Could you please resolve them @pawanjay176? 🙏 |
|
Merged |
* Fix mock engine panic in bid gossip spec tests * Limit mock fork-choice responses to setup payloads
|
Some required checks have failed. Could you please take a look @pawanjay176? 🙏 |
|
This is ready for a re-review. I'm not super particular about getting this for 8.3.0 RC, but the main prod changes are
Rest are just test changes. |
|
Some required checks have failed. Could you please take a look @pawanjay176? 🙏 |
Issue Addressed
N/A
Proposed Changes
Enable all gloas gossip test handlers. Have intentionally skipped quite a few test vectors where the tests expects us to store invalid blocks or construct a history of invalid blocks.
Must re-enable gas limit tests after #9905
Must re-enable partial data column tests in #9325
Note to reviewer: better to review commit wise.
Used codex assistance for the harness building.