Skip to content

fix: use updated latest_finalized.slot for justifiability checks#203

Merged
pablodeymo merged 2 commits intolambdaclass:mainfrom
shariqnaiyer:fix/shariqnaiyer/justifiability
Mar 12, 2026
Merged

fix: use updated latest_finalized.slot for justifiability checks#203
pablodeymo merged 2 commits intolambdaclass:mainfrom
shariqnaiyer:fix/shariqnaiyer/justifiability

Conversation

@shariqnaiyer
Copy link
Contributor

leanEthereum/leanSpec#443

The above PR in leanSpec removed the use of the original_finalized_slot snapshot and changed it to using the updated latest_finalized.slot from state. A difference in this implementation within clients leads to some state root mismatches at certain blocks.

I am making this PR so we can get consistency across clients prior to the devnet run in about 6-8 hours. Please feel to request changes, merge or close this PR.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR aligns the client implementation with leanSpec PR #443 by removing the original_finalized_slot snapshot and having is_valid_vote and try_finalize read state.latest_finalized.slot live. This means that if finalization advances mid-loop during process_attestations, subsequent attestation validity checks and finalization candidate checks operate against the already-updated finalized slot rather than a stale snapshot, producing consistent state roots across clients.

Key changes:

  • Removed original_finalized_slot local variable from process_attestations.
  • is_valid_vote no longer accepts an original_finalized_slot parameter; it now reads state.latest_finalized.slot directly when calling slot_is_justifiable_after.
  • try_finalize no longer accepts an original_finalized_slot parameter; the justifiability guard also reads state.latest_finalized.slot live.
  • The behavioral implication is that once finalization advances within the attestation loop, all remaining attestations in the same block see the updated finalized slot — matching the spec semantics introduced in leanSpec PR #443.
  • No logic errors were identified; the implementation faithfully mirrors the described spec change. The existing concern around root_to_slot[root] panicking for roots not present in the map (in try_finalize's justifications.retain) is pre-existing and unrelated to this PR.

Confidence Score: 4/5

  • This PR is safe to merge; the change is a focused, well-scoped alignment with an upstream spec update and does not introduce new logic errors.
  • The diff is small, mechanical, and directly mirrors leanSpec PR #443. The removal of the snapshot simplifies the function signatures and the live reads are consistent throughout process_attestations. No new failure modes are introduced — the only pre-existing concern (potential panic in root_to_slot[root] inside try_finalize) already existed before this PR and is unaffected by the change.
  • No files require special attention; the single changed file has a straightforward, low-risk modification.

Important Files Changed

Filename Overview
crates/blockchain/state_transition/src/lib.rs Removes original_finalized_slot snapshot; is_valid_vote and try_finalize now read state.latest_finalized.slot live, allowing mid-loop finalization advances to affect subsequent attestation checks — aligning with leanSpec PR #443.

Sequence Diagram

sequenceDiagram
    participant PA as process_attestations
    participant IV as is_valid_vote
    participant TF as try_finalize
    participant S as state

    Note over PA,S: Before PR: original_finalized_slot snapshot taken once
    Note over PA,S: After PR: state.latest_finalized.slot read live each time

    loop For each attestation
        PA->>IV: is_valid_vote(state, source, target)
        IV->>S: read state.latest_finalized.slot (live)
        IV-->>PA: valid / invalid

        alt supermajority reached
            PA->>S: update state.latest_justified
            PA->>TF: try_finalize(state, source, target, ...)
            TF->>S: read state.latest_finalized.slot (live, may be updated)
            alt no justifiable slots between source and target
                TF->>S: state.latest_finalized = source  (advances finalized slot)
                Note over S: latest_finalized.slot updated — affects remaining loop iterations
            else justifiable slots exist
                TF-->>PA: return early (no finalization)
            end
        end
    end
Loading

Last reviewed commit: cf2866b

Copy link
Collaborator

@MegaRedHand MegaRedHand left a comment

Choose a reason for hiding this comment

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

Thanks!

@MegaRedHand MegaRedHand force-pushed the fix/shariqnaiyer/justifiability branch from af17e9c to fbea199 Compare March 12, 2026 12:49
@pablodeymo pablodeymo merged commit 9c7f5f6 into lambdaclass:main Mar 12, 2026
2 checks passed
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.

3 participants