Skip to content

[fix][ethereum2][offchain] fix period-tail sync committee verification - #75

Open
fengjy73 wants to merge 1 commit into
AntChainOpenLabs:mainfrom
fengjy73:fix/ethereum2-period-tail-verification
Open

[fix][ethereum2][offchain] fix period-tail sync committee verification#75
fengjy73 wants to merge 1 commit into
AntChainOpenLabs:mainfrom
fengjy73:fix/ethereum2-period-tail-verification

Conversation

@fengjy73

@fengjy73 fengjy73 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

The previous implementation treated period * syncPeriodLength as 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 - 1 slots, where HCDVS may verify a sync aggregate with the wrong committee or fork domain and report sync 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.

  • Fix Ethereum2 sync committee period-tail calculation.
  • Carry the actual sync aggregate signature_slot in consensus endorsements.
  • Verify block signatures with the committee matching the real signature period.
  • Keep current and next sync committees separately, and rotate only at the real period boundary.
  • Split block verification from light client update verification.
  • Exclude the duplicate lib/ptc/CommitteePtcVerifier.sol from web3j wrapper generation.

@zouxyan zouxyan 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.

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:

  1. The sync-committee fork domain is derived from the wrong slot.

    In both EthConsensusStateData.validateBlock and validateLightClientUpdateInternal, the code calls getForkBySlot(signatureSlot). The Ethereum consensus spec derives the fork version from max(signature_slot, 1) - 1, because the aggregate carried at slot S signs 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_slot is 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 use max(signatureSlot, 1) - 1 for fork selection.

  2. 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 = 64 and must be verified with period 1's committee. But slot 62 is not the tail, so getEthConsensusStateData(62) does not attach a light-client update, and the parent node info does not yet contain nextSyncCommittee. EthereumHcdvsService therefore returns missing next sync committee for endorsements for a valid state.

    The producer/verifier needs to make the next committee available whenever the observed signaturePeriod advances beyond the header period (or maintain the verified next committee proactively), not only when the target header itself is exactly the period-tail slot.

  3. The anchor path still always verifies the block with the current committee.

    verifyAnchorConsensusState calls ethConsensusStateData.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 from signaturePeriod, 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.

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