fix(notifier): keep gloas exec-block pre-gloas-shaped#19
Conversation
Adds a `prev-payload:` row to the node notifier that surfaces the execution payload status of the block the next proposer would build on top of — the parent of the current head. This is the payload that will be referenced by the next block's parent hash. - Pre-Gloas: parent's single variant is always FULL, shows block number + hash. - Gloas: parent may be FULL (payload revealed) or EMPTY (PTC voted not-present). 🤖 Generated with AI assistance
|
please fix the diff, it shows commits from unstable branch |
|
Fixed — fork's |
| const headRow = `head: ${headDiffInfo}${prettyBytes(headInfo.blockRoot)}`; | ||
|
|
||
| const executionInfo = getHeadExecutionInfo(config, clockEpoch, headState, headInfo); | ||
| const prevPayloadInfo = getPrevSlotPayloadInfo(chain.forkChoice, headInfo, clockEpoch, config); |
There was a problem hiding this comment.
why do you make this so complicated? once we have the block in gloas, we know the parent hash from the bid in the block, and the only way to import that block is if we have imported the parent hash execution payload, so there isn't ever the case where this can be empty/full/pending, this is fork choice internal details that should not matter here
can't we just log this the same way as pre-gloas as exec-block?
There was a problem hiding this comment.
Fair point — I over-engineered this. The multi-variant getBlockHex(root, FULL|EMPTY) lookup is internal fork-choice bookkeeping that the notifier user doesn't care about.
Will simplify to mirror pre-gloas exec-block format:
const parent = forkChoice.getBlockHexDefaultStatus(headInfo.parentRoot);
if (parent === null || parent.executionStatus === ExecutionStatus.PreMerge) return [];
const statusStr = parent.executionStatus.toLowerCase();
if (parent.executionPayloadBlockHash !== null) {
return [`prev-payload: ${statusStr}(${parent.executionPayloadNumber} ${prettyBytesShort(parent.executionPayloadBlockHash)})`];
}
return [`prev-payload: ${statusStr}`];That gets us prev-payload: valid(19876543 0x123456…) using executionStatus (valid/invalid/syncing) instead of my invented full/empty/pending.
One clarification for the Gloas case: when the parent's payload was missed (bid never revealed, inherited parent hash from grandparent), the default variant's executionPayloadBlockHash may be null. Should I:
(a) emit prev-payload: ${statusStr} (no hash/number — would hide the fact that a payload was missed), or
(b) drop the row entirely in that case?
Leaning (a) for visibility. LMK if you'd prefer something else.
There was a problem hiding this comment.
Simplified per your feedback — pushed ab55d4b.
exec-block:is back to the same operator-facing shape as pre-Gloas.- No
prev-payload:row and noempty-parent/ FULL / EMPTY / PENDING annotation in the log output. - For Gloas imported heads I only keep the minimal normalization
PayloadSeparated -> valid, so we don't print an internal status string while still logging the inherited execution anchor already carried onheadInfo.executionPayloadBlockHash/executionPayloadNumber. - Also dropped the unrelated envelope/import provenance test drift from this branch and replaced it with one focused notifier unit test file (
3/3passing locally).
So the branch is back to the notifier-only scope your comment was asking for.
|
Design plan, converged after first-principles pass + two review rounds (gpt-advisor, devils-advocate, codex defender fallback): What changes vs. the current PR
Example outputImplementation shape (single PR, only
|
Nico's feedback on PR #19: once a gloas block is imported, its parent execution payload is guaranteed imported+validated, so there is no operator-facing reason to resolve the parent variant and print FULL/ EMPTY/PENDING. Fork-choice already stores the bid's parent_block_hash and the parent variant's block number on the head ProtoBlock at import time, so exec-block: <status>(<number> <hash>) renders the correct parent anchor for gloas PENDING/EMPTY heads with no lookup. The only remaining display issue was that executionStatus for gloas non-FULL heads is PayloadSeparated, which .toLowerCase() would render as "payloadseparated" - ugly and misleading. Map it to "valid" because the parent anchor's payload was validated before the block could be imported. Result: pre-gloas notifier format unchanged, and gloas output matches that format across PENDING/EMPTY/FULL states with the parent anchor or head's own payload in the (number hash) tuple as appropriate. Missed payloads are visible via EL-block-number stagnation across slots. Removes getPrevSlotPayloadInfo and getResolvedParentBlock. 🤖 Generated with AI assistance
Replace the PayloadSeparated->"valid" mapping with a lookup of the parent variant the head built on, using head.parentBlockHash (the bid's parent_block_hash). Render the exec-block status as "empty" when the parent's payloadStatus was EMPTY, "full" otherwise. This preserves missed-payload visibility in the notifier: two consecutive logs with the same exec-block hash and "empty" label signal a string of missed payloads, since a block building on an EMPTY parent inherits the grandparent's anchor as its bid.parent_block_hash. Pre-gloas behavior is unchanged (head.parentBlockHash is null pre-gloas, so the existing executionStatus.toLowerCase() path is preserved). 🤖 Generated with AI assistance
…loas empty-parent heads Combine the pre-Gloas valid/syncing label (EL validation signal) with the Gloas parent-variant signal. PayloadSeparated heads render as 'valid' since the displayed exec-block is the parent's already-imported anchor. When the head built on an EMPTY parent variant, suffix '/empty' so two consecutive logs with the same hash and '/empty' reveal a string of missed payloads. 🤖 Generated with AI assistance
|
Iteration 3 pushed as Final output shapeDiff vs iteration 2Iteration 2 overrode the EL validation signal with let executionStatusStr =
headInfo.executionStatus === ExecutionStatus.PayloadSeparated ? "valid" : headInfo.executionStatus.toLowerCase();
if (headInfo.parentBlockHash !== null) {
const parentVariant = forkChoice.getBlockHexAndBlockHash(headInfo.parentRoot, headInfo.parentBlockHash);
if (parentVariant?.payloadStatus === PayloadStatus.EMPTY) {
executionStatusStr = `${executionStatusStr}/empty`;
}
}PR body + title updated to match. Lint clean. Let me know if |
…label suffix Move the EMPTY-parent signal out of the status label into a trailing 'empty-parent' annotation after the tuple. Keeps the historical valid(...)/syncing(...)/invalid(...) grammar intact, avoids conflating EL validation state with the orthogonal fork-choice parent variant, and preserves existing prefix-based tooling. Output: Pre-Gloas / Gloas FULL-parent: exec-block: valid(N hash) Gloas EMPTY-parent: exec-block: valid(N hash) empty-parent 🤖 Generated with AI assistance
|
Ran another advisor cycle on the encoding. Both advisors (codex-advisor xhigh + devil's-advocate pass) converged on swapping the Why not the slash-suffix
New outputGrammar stays Codeconst executionStatusStr =
headInfo.executionStatus === ExecutionStatus.PayloadSeparated ? "valid" : headInfo.executionStatus.toLowerCase();
let emptyParentSuffix = "";
if (headInfo.parentBlockHash !== null) {
const parentVariant = forkChoice.getBlockHexAndBlockHash(headInfo.parentRoot, headInfo.parentBlockHash);
if (parentVariant?.payloadStatus === PayloadStatus.EMPTY) {
emptyParentSuffix = " empty-parent";
}
}
// …
return [`exec-block: ${status}(${num} ${hashShort})${emptyParentSuffix}`];PR title + body updated to match. Lint clean. |
Simplify PR #19 back to the notifier behavior Nico asked for: - keep a single exec-block row shaped like pre-Gloas output - normalize PayloadSeparated -> valid instead of surfacing an internal status string - drop the empty-parent annotation and the unrelated envelope/import test drift - add a focused notifier unit test covering pre-Gloas, Gloas PayloadSeparated, and Gloas FULL output 🤖 Generated with AI assistance
9f1b2c3 to
ab55d4b
Compare
|
Closing this per Nico's request; treating it as done. |
Summary
Keeps the
exec-block:notifier row pre-Gloas-shaped on Gloas heads. Only fix: normalize the internalPayloadSeparatedfork-choice status tovalidin the printed label, so the log never surfaces raw enum strings.No new full/empty marker for now — we discussed a few encodings (
valid/full|empty, trailing annotations, separate warning log) and none felt right on the notifier. Operators can already infer an empty-parent streak implicitly:exec-block:number+hash stay flat whileslot:/head:advance.Output
Pre-Gloas (unchanged):
Gloas (same shape,
PayloadSeparatednormalized tovalid):Implementation
Single file change in
packages/beacon-node/src/node/notifier.ts: ingetHeadExecutionInfo, collapseExecutionStatus.PayloadSeparated → "valid"before lowercasing. Plus a focused unit test covering pre-GloasValid, GloasPayloadSeparated, and GloasValidheads all rendering the same pre-Gloas-shaped row.We can revisit surfacing full/empty later if the implicit signal turns out to be insufficient in practice.
Test plan
unstable.exec-block:row renders asvalid(<number> <hash>)under normal flow, never aspayloadseparated(...).