Conversation
…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>
|
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 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.
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 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 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. |
Description
produce_attestation_dataassertedsource.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 inget_attestation_target, so the target walk and the fixtures that check it throughattestation_target_slotare unchanged.Tests:
tests/spec/forks/lstar/test_validator_duties.pybuilds that chain on a genesis store. The first test fails onmainat the assert and passes here; the second checks a target at or after the source is left as the walk selected it.Closes #1206