Skip to content

fix(validator): vote for the justified checkpoint when the target walk lands behind it - #1207

Open
adust09 wants to merge 1 commit into
leanEthereum:mainfrom
adust09:fix/clamp-attestation-target-to-source
Open

adust09 wants to merge 1 commit into
leanEthereum:mainfrom
adust09:fix/clamp-attestation-target-to-source

Conversation

@adust09

@adust09 adust09 commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Description

produce_attestation_data asserted source.slot <= target.slot. The assert is reachable: the target walk is bounded by the safe target and the finalized checkpoint, never by the head's justified one, so on a sparse chain the justifiability walk can select a block behind the source.

#1206 has the head chain from a three-node devnet that does it (62 -> 59 -> 51 -> 50 -> 49 -> 45 -> 42, finalized 42, justified 49: the walk lands on 45).

Every lean client already clamps the target up to the justified checkpoint in this case, outside the spec (survey in #1206). This makes the spec say so: when the source is after the target, the vote's target is the justified checkpoint itself. It is on the head chain and justifiable by construction, and the vote keeps its weight in fork choice instead of being dropped.

The clamp sits in produce_attestation_data, not in get_attestation_target, so the target walk and the fixtures that check it through attestation_target_slot are unchanged.

Tests: tests/spec/forks/lstar/test_validator_duties.py builds that chain on a genesis store. The first test fails on main at the assert and passes here; the second checks a target at or after the source is left as the walk selected it.

Closes #1206

…k lands behind it

produce_attestation_data asserted source.slot <= target.slot. The target
walk is bounded by the safe target and the finalized checkpoint, never by
the head's justified one, so on a sparse chain the justifiability walk can
select a block behind the source and the assert fires. A three-node devnet
produced the head chain 62 -> 59 -> 51 -> 50 -> 49 -> 45 -> 42 with
finalized 42 and justified 49: the lookback reaches 50, neither 50 nor 49
is justifiable after 42, and the walk lands on 45.

Every lean client already clamps the target up to the justified checkpoint
here, outside the spec. Make the spec say so: when the source is after the
target, the vote's target is the justified checkpoint itself. The target
walk and the fixtures that check it are unchanged.

Closes leanEthereum#1206

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@gopikannappan

Copy link
Copy Markdown

Another pair of eyes from someone new here, so weigh it accordingly. I read this after #1204 and #1206, ran it, and poked at what happens to the clamped vote downstream.

It does what it says. Both tests pass on 08e1481a, the walk still picks 45, and the duty votes source = target = 49 instead of asserting. I checked the fork-choice side rather than assuming: with the store clock set to slot 63, validate_attestation accepts the clamped vote, so it really does keep its head weight. That is a clear improvement on crashing, and it matches what the clients in #1206 already do.

Two things I noticed, one worth a comment and one that is really a question about older code.

The clamped vote can never contribute to justification. process_attestations drops it twice over:

  • if justified_slots.is_slot_justified(finalized_slot, target.slot): continue — the target is the justified checkpoint, so this fires by construction;
  • if target.slot <= source.slot: continue — source and target are now equal.

I confirmed both predicates on the produced data. So the vote counts for head weight and never for justification, which I think is exactly what you intended, but the docstring's "it is justifiable" reads as though it could still help justify. A line saying the vote is deliberately fork-choice-only would save the next reader the trip I just made.

It also has a cost worth naming: those votes still get aggregated, and xmss/aggregation.py's recursive proof is roughly constant-cost whatever goes into it (that is the measurement in #747). So a sparse chain spends real proving work on votes that cannot move justification. Not an argument against this PR, which is strictly better than asserting, but it makes me wonder whether the longer-term fix is to bound the target walk by the head's justified checkpoint so the target lands after the source and the vote stays useful. Your call, and I would not hold this up for it.

A question about the two lines above your change. The placeholder branch pairs the head's root with the justified checkpoint's slot:

if justified_source.root == Bytes32.zero():
    justified_source = Checkpoint(root=store.head, slot=justified_source.slot)

At genesis those agree, because the head is the genesis block at slot 0. Once the head advances while nothing has justified yet, they do not. I built that case, a head at slot 5 with latest_justified still the genesis placeholder, and the produced attestation is rejected by fork choice:

source root is the head block at slot 5, but source.slot = 0
fork choice rejects: Source checkpoint slot mismatch

That is pre-existing and not yours, so I am not asking you to fix it here. I mention it because it sits two lines above your change and the clamp can now copy that same checkpoint into the target as well, so the same inconsistency would reach one more field. Mostly I would like to know whether that state is reachable on a real node, or whether something justifies early enough that the placeholder is always gone by the time the head moves. You would know far better than I would, and if it is reachable it probably deserves its own issue.

Neither point blocks merging as far as I can see.

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.

produce_attestation_data asserts source <= target, but the target walk can land behind the head's justified checkpoint

2 participants