Skip to content

fix(pxe): warn when block header unavailable for proven/finalized events#22050

Merged
benesjan merged 2 commits into
nextfrom
jb/pxe-warn-missing-block-header
Mar 27, 2026
Merged

fix(pxe): warn when block header unavailable for proven/finalized events#22050
benesjan merged 2 commits into
nextfrom
jb/pxe-warn-missing-block-header

Conversation

@benesjan

@benesjan benesjan commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Improved logging based on audit finding 114 in #21500 + random improvements that I noticed could be done

Summary

  • Adds warning logs in PXE block synchronizer when getBlockHeader returns null for chain-proven and chain-finalized events, instead of silently skipping the anchor update.
  • Aligns with the chain-pruned handler which already throws on missing headers.

Closes #21500 .

@benesjan benesjan requested a review from nchamo March 26, 2026 13:15
@benesjan benesjan requested a review from nventuro as a code owner March 26, 2026 13:56
@benesjan benesjan force-pushed the jb/pxe-warn-missing-block-header branch from d9f299c to 8640ae3 Compare March 26, 2026 13:57

benesjan commented Mar 26, 2026

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Comment on lines -33 to -39
if f.has_named_attribute("noinitcheck") {
true
} else {
// #[only_self] functions automatically skip the initialization check as the check is assumed to be done by the
// calling external function or explicitly skipped. See only_self function docs for more details.
is_fn_only_self(f)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this code yesterday and then I recalled I could simplify it and it kept on bothering me


this.log.debug(`Syncing PXE with the node`);
// Capture the promise locally so we always await the exact promise we created, even if this.isSyncing is modified
// between assignment and await (e.g. due to future refactors introducing a yield point).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not clear to me and AI helped me here to explain it so I added there a comment.

@benesjan benesjan removed the request for review from nventuro March 26, 2026 14:10

@nchamo nchamo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warnings are always appreciated 👐

Comment on lines +33 to +35
// #[only_self] functions automatically skip the initialization check as the check is assumed to be done by the
// calling external function or explicitly skipped. See only_self function docs for more details.
f.has_named_attribute("noinitcheck") | is_fn_only_self(f)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about why this is necessary. only_self adds the noinitcheck attribute automatically. I assumed this meant that it would pass the f.has_named_attribute("noinitcheck") check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because we don't want to force people to slap #[noinitcheck] on #[only_self] - we essentially want that to be implicit.

@benesjan benesjan enabled auto-merge (squash) March 26, 2026 14:41
Base automatically changed from merge-train/fairies to next March 26, 2026 17:56
benesjan and others added 2 commits March 27, 2026 02:18
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@benesjan benesjan force-pushed the jb/pxe-warn-missing-block-header branch from 8640ae3 to 7472aa9 Compare March 27, 2026 02:18
@benesjan benesjan requested a review from charlielye as a code owner March 27, 2026 02:18
@benesjan benesjan changed the base branch from next to merge-train/fairies March 27, 2026 02:19
Base automatically changed from merge-train/fairies to next March 27, 2026 07:58
@benesjan benesjan added this pull request to the merge queue Mar 27, 2026
Merged via the queue into next with commit 847ce95 Mar 27, 2026
23 of 32 checks passed
@benesjan benesjan deleted the jb/pxe-warn-missing-block-header branch March 27, 2026 08:43
@benesjan

Copy link
Copy Markdown
Contributor Author

Sorry, forgot to squash this before merging

@AztecBot

Copy link
Copy Markdown
Collaborator

❌ Failed to cherry-pick to v4-next due to conflicts. (🤖) View backport run.

AztecBot pushed a commit that referenced this pull request Apr 15, 2026
…nts (#22050)

Improved logging based on audit finding 114 in #21500 + random
improvements that I noticed could be done

## Summary
- Adds warning logs in PXE block synchronizer when `getBlockHeader`
returns null for `chain-proven` and `chain-finalized` events, instead of
silently skipping the anchor update.
- Aligns with the `chain-pruned` handler which already throws on missing
headers.

Closes #21500 .
benesjan added a commit that referenced this pull request Apr 15, 2026
…nts (backport #22050) (#22547)

## Summary
Backport of #22050
to v4-next.

Cherry-pick applied cleanly. The noir-projects change from the original
PR was already present on v4-next, so only the PXE block_synchronizer
changes were needed.

### Changes
- Adds warning logs in PXE block synchronizer when `getBlockHeader`
returns null for `chain-proven` and `chain-finalized` events, instead of
silently skipping the anchor update
- Adds a comment clarifying the local promise capture pattern in
`sync()`

ClaudeBox log: https://claudebox.work/s/017ec0a44d097055?run=1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PXE sync, note discovery, and reorg resilience

3 participants