merge queue: embarking unstable (91456fb) and #9382 together#9410
Closed
mergify[bot] wants to merge 15 commits into
Closed
merge queue: embarking unstable (91456fb) and #9382 together#9410mergify[bot] wants to merge 15 commits into
mergify[bot] wants to merge 15 commits into
Conversation
Ports the consensus lessons from the gloas-lookup-sync work to unstable: when deciding whether a block's parent is "imported", a post-Gloas FULL child (whose bid commits to the parent's execution payload) must also wait for the parent's payload to be imported, not just the parent block. - proto_array: add `Block::is_child_full(child_bid)` to test whether a post-Gloas child commits to this block's execution payload. - fork_choice: add `ParentImportedStatus` and `is_parent_imported`/`is_parent_imported_status`, which return `UnimportedPayload` for a FULL child whose parent payload has not been received yet. - block_verification: use these in `verify_parent_block_is_known`, the `ExecutionPendingBlock` parent-invalid/parent-unknown gate, and `load_parent` (replacing the plain `get_block`/`contains_block` checks), and drop the now-obsolete gloas TODO comment.
Remove redundant explanation about FULL child requirement.
Production fix: - fork_choice: `is_parent_imported_status` no longer gates a Gloas child on its parent's payload when the parent is pre-Gloas. A pre-Gloas parent's execution payload is embedded in the block and always present, so the first Gloas block at the fork-transition boundary must not be rejected with `ParentUnknown`. Test harness fixes (all under FORK_NAME=gloas): - block_verification: import each block's payload envelope into fork choice before importing its child (process_chain_segment imports blocks only); for the invalid-signature tests, pre-import ancestors and assert on the segment suffix to avoid re-processing finalization-pruned blocks. - store_tests: mark the weak-subjectivity checkpoint anchor's payload as received so the first forward-sync block (a FULL child of the anchor) is not rejected.
- Rename `ProtoBlock::is_child_full` -> `is_parent_full` (per review); the method is read from the parent's perspective. - Reword `ParentImportedStatus` variant docs and the `is_parent_imported_status` doc comment to describe the condition in terms of the child's bid. No behaviour change.
- Rename `verify_parent_block_is_known` -> `verify_parent_block_and_envelope_are_known` so the added parent-envelope check is visible from the call site (michaelsproul). - Rename `is_parent_imported_status` -> `get_parent_import_status` and the `ParentImportedStatus` enum -> `ParentImportStatus` (michaelsproul). - Drop the contested "in production payload envelopes are fetched separately" claim from the test helper doc; describe current behaviour only (eserilev).
Range sync does not import payload envelopes yet (coming in #9362), so a multi-block segment of consecutive post-Gloas FULL blocks cannot be imported by `process_chain_segment`. Skip the affected `block_verification` tests under `FORK_NAME=gloas` until then, and drop the `import_chain_segment_with_envelopes` placeholder helper and the per-test workarounds it required.
Reduce the weak-subjectivity forward-sync loop to the minimum needed to import post-Gloas blocks: persist the payload envelope and mark it received in fork choice. The data-column reconstruction was unnecessary (the WSS test chain has no blobs). Placeholder until range sync imports payload envelopes (#9362).
- Remove the parent-not-known explanation in the gate match (duplicated in load_parent) and the invalid-parent comment (error name is self-documenting). - Remove the stale spec-paraphrase in verify_parent_block_and_envelope_are_known. - Shorten the get_parent_import_status / verify_parent docs and the WSS anchor and TODO(gloas) comments.
|
|
1 similar comment
|
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 pull request has been checked successfully and will be merged soon. 🎉
Branch unstable (91456fb) and #9382 are embarked together for merge.
This pull request has been created by Mergify to check the mergeability of #9382.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
defaultfor merge:#approved-reviews-by >= 1[🛡 GitHub branch protection]check-success=local-testnet-successcheck-success=test-suite-successRequired conditions to stay in the queue:
#approved-reviews-by >= 1#approved-reviews-by >= 1[🛡 GitHub branch protection]check-success=license/clacheck-success=target-branch-checklabel!=do-not-merge