Skip to content

Add Fast Confirmation Rule (FCR) implementation#656

Open
bomanaps wants to merge 6 commits into
grandinetech:developfrom
bomanaps:feature/FCR
Open

Add Fast Confirmation Rule (FCR) implementation#656
bomanaps wants to merge 6 commits into
grandinetech:developfrom
bomanaps:feature/FCR

Conversation

@bomanaps

@bomanaps bomanaps commented Apr 3, 2026

Copy link
Copy Markdown

Implements the Fast Confirmation Rule (FCR) for the fork choice store, including the confirmation logic, chain safety checks, FFG data computation, and HTTP API integration for the safe block ID. Adds FCR-specific tests covering confirmation advancement, staleness reversion, and the disabled fallback path.

ethereum/consensus-specs#4747

Comment thread features/src/lib.rs Outdated
Comment thread fork_choice_control/src/extra_tests.rs Outdated
Comment thread fork_choice_store/src/fast_confirmation.rs Outdated

@ArtiomTr ArtiomTr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Haven't reviewed everything yet, but some places need major refactoring. Specifically, the biggest issue is that FCR lives in forkchoice store, but probably would be better living on its own.

Also, I don't see spec tests updated - those are required to be wired & passing, before merging

UPD: looks like formatting check was failing - all lints also need to be passing, before merging. For convenience, you can run them locally, with ./scripts/ci/clippy.bash --deny warnings

Comment thread fork_choice_store/src/fast_confirmation.rs Outdated
Comment thread fork_choice_store/src/lib.rs Outdated
Comment thread runtime/src/grandine_args.rs Outdated
Comment thread fork_choice_store/src/store.rs Outdated
Comment thread fork_choice_store/src/store.rs Outdated
Comment thread fork_choice_store/src/store.rs Outdated
Comment thread fork_choice_store/src/store.rs Outdated
Comment thread fork_choice_store/src/store.rs Outdated
Comment thread fork_choice_store/src/store.rs Outdated
Comment thread fork_choice_store/src/store.rs Outdated
@bomanaps

Copy link
Copy Markdown
Author

While running the FCR spec-test vectors, I hit two divergences that appear to live in pre-existing in our code (not introduced by the FCR PR), and I would appreciate a sanity-check before assuming am misreading the spec (1) accessors::relative_epoch (helper_functions/src/accessors.rs:103-114) rejects epoch lookups more than ±1 from the state's current epoch, but the FCR spec note for get_slot_committee says implementations "MUST support committees of epochs starting from current_epoch − 2" is this an intentional limitation, or could the helper handle older epochs from the state's RANDAO history? (2) cl.unrealized_justified_checkpoint.epoch (computed via combined::process_justification_and_finalization at store.rs:1281-1292) appears to diverge from the spec's expected store.unrealized_justifications[block].epoch on multi-fork chains the test_fcr_previous_epoch_012 parameter block_vs_fresh=False implies the spec expects 0 for these prev-epoch blocks, but I seem to compute ≥1, which makes Loop 2's final gate over-advance confirmed_root could you point me at how the spec's compute_pulled_up_tip semantics should map onto our pulled-up path? Any pointers would be much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants