refactor(consensus): Move Forkchoice Finalize Off Queue#2536
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
refcell
force-pushed
the
rf/refactor/direct-engine-forkchoice-finalize
branch
from
82dcdd9 to
b24924e
Compare
Move delegated forkchoice and finalized-head updates onto direct Engine methods. Route the engine processor through those methods and remove the old task wrappers and queue variants. Co-authored-by: Codex <noreply@openai.com>
refcell
force-pushed
the
rf/refactor/direct-engine-forkchoice-finalize
branch
from
b24924e to
e9568bc
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| //! [`Engine`]: crate::Engine | ||
|
|
||
| use std::cmp::Ordering; | ||
| //! Common task error traits used by direct engine operations. |
There was a problem hiding this comment.
Nit: two stale EngineTask references remain in this file:
- Line 15:
/// This is used to determine how to handle the error when draining the engine task queue. - Line 54:
/// An error that may occur during an [`EngineTask`]'s execution.
EngineTask was removed in this PR; these doc comments should be updated accordingly.
| pub use task::{ | ||
| EngineTask, EngineTaskError, EngineTaskErrorSeverity, EngineTaskErrors, EngineTaskExt, | ||
| }; | ||
| pub use task::{EngineTaskError, EngineTaskErrorSeverity, EngineTaskErrors, EngineTaskExt}; |
There was a problem hiding this comment.
Nit: EngineTask was removed but the module-level doc on line 1 (//! Tasks to update the engine state.) still says "Tasks". Consider rewording to reflect the new architecture (e.g., //! Error types and traits for direct engine operations.).
Merge duplicated finalize test match arms, refresh the SP1 ELF manifest, and use relative docs links so lychee does not check stale main-branch paths. Co-authored-by: Codex <noreply@openai.com>
Review SummaryThe refactoring correctly moves ObservationsCorrectness: The behavioral migration is sound. The processor still gets safe-head and EL-sync-complete notifications on the next loop iteration via Stale references (existing inline comments already cover
These are outside the diff so I couldn't leave inline comments. Consider a follow-up pass to update these doc strings. No blocking issues found. The refactoring is clean and the test updates properly exercise the new direct-call paths. |
Move delegated forkchoice and finalized-head updates onto direct Engine methods. Route the engine processor through those methods and remove the old task wrappers and queue variants. Merge duplicated finalize test match arms, preserve stale finalized-head no-op behavior in the direct path, and keep derived docs/manifest updates from the rebased base branch. Co-authored-by: Codex <noreply@openai.com>
1 participant
Summary
Move delegated forkchoice and finalized-head updates onto direct Engine methods. The engine processor now executes those paths directly and maps errors through the existing engine task severity handling. The old task wrappers remain as compatibility shells so the queue machinery can be deleted in the next stage without mixing behavior changes into this PR.