[fix][ethereum2][offchain] fix period-tail sync committee verification - #75
[fix][ethereum2][offchain] fix period-tail sync committee verification#75fengjy73 wants to merge 1 commit into
Conversation
zouxyan
left a comment
There was a problem hiding this comment.
Thanks for addressing the period-tail off-by-one and for carrying the actual signature_slot; both changes are directionally correct. However, I do not think the current implementation is consensus-correct yet. It can still reject valid Ethereum consensus states in three boundary cases:
-
The sync-committee fork domain is derived from the wrong slot.
In both
EthConsensusStateData.validateBlockandvalidateLightClientUpdateInternal, the code callsgetForkBySlot(signatureSlot). The Ethereum consensus spec derives the fork version frommax(signature_slot, 1) - 1, because the aggregate carried at slotSsigns the previous-slot block root. See: https://github.com/ethereum/consensus-specs/blob/master/specs/altair/light-client/sync-protocol.md#L464-L479 and https://github.com/ethereum/consensus-specs/blob/master/specs/altair/beacon-chain.md#L621-L654.With the current code, when
signature_slotis the first slot of a hard fork, verification selects the new fork domain while the valid aggregate was created under the previous-slot fork domain, so BLS verification fails. Both call sites should usemax(signatureSlot, 1) - 1for fork selection. -
A missed period-tail slot can require the next committee before it has been learned.
For a minimal period length of 64, consider: slot 62 has a block, slot 63 (the period tail) is missed, and slot 64 is the next block. The aggregate in slot 64 can attest to the root carried through missed slot 63, so the consensus state for slot 62 has
signature_slot = 64and must be verified with period 1's committee. But slot 62 is not the tail, sogetEthConsensusStateData(62)does not attach a light-client update, and the parent node info does not yet containnextSyncCommittee.EthereumHcdvsServicetherefore returnsmissing next sync committee for endorsementsfor a valid state.The producer/verifier needs to make the next committee available whenever the observed
signaturePeriodadvances beyond the header period (or maintain the verified next committee proactively), not only when the target header itself is exactly the period-tail slot. -
The anchor path still always verifies the block with the current committee.
verifyAnchorConsensusStatecallsethConsensusStateData.validate(currentSyncCommittee, ...)before extracting the next committee from the included light-client update. For an anchor at the period tail, its endorsement is normally carried at the first slot of the next period and therefore requires the next committee. Although the state data contains the update needed to authenticate that committee, block verification happens first and fails with the current committee.This path should mirror the split flow used by
verifyConsensusState: validate the light-client update with the trusted current committee, obtain the authenticated next committee, select the block-verification committee fromsignaturePeriod, verify the block, and only then rotate at the real boundary.
The added testPeriodTailBoundary only checks the isLastSlotForCurrentPeriod predicate. Please add end-to-end tests for (a) signature_slot at a fork activation slot, (b) a missed tail with the next produced block in the following period, and (c) anchor verification at the period tail.
The previous implementation treated
period * syncPeriodLengthas the last slot of the current sync committee period, but that slot is actually the first slot of the next period. The real period tail is(period + 1) * syncPeriodLength - 1.On minimal private networks this causes verification failures around
64n - 1slots, where HCDVS may verify a sync aggregate with the wrong committee or fork domain and reportsync committee signature is invalid.This PR also preserves the actual slot that produced the sync aggregate, so missed slots and period-boundary endorsements no longer rely on guessing
block slot + 1.signature_slotin consensus endorsements.lib/ptc/CommitteePtcVerifier.solfrom web3j wrapper generation.