Finalized header not updated during sync committee change

[nflaig]

Based on the logs from #6861 (comment), it seems like the light client server is not updating the finality header if sync committee changes, i.e. during transition in new sync committee period.

Looking at logs from beacon-2024-07-01.log.gz

New sync committee period started

Jul-01 22:40:29.001[]                 info: New sync committee period 1150

It seems like we are storing new partial updates but all are not finalized

Jul-01 22:40:37.174[chain]           debug: Stored new PartialLightClientUpdate syncPeriod=1150, isFinalized=false, participation=0.900390625
Jul-01 22:40:50.201[chain]           debug: Stored new PartialLightClientUpdate syncPeriod=1150, isFinalized=false, participation=0.98828125
Jul-01 22:41:12.950[chain]           debug: Stored new PartialLightClientUpdate syncPeriod=1150, isFinalized=false, participation=0.990234375
Jul-01 22:41:49.108[chain]           debug: Stored new PartialLightClientUpdate syncPeriod=1150, isFinalized=false, participation=0.9921875
Jul-01 22:42:13.916[chain]           debug: Stored new PartialLightClientUpdate syncPeriod=1150, isFinalized=false, participation=0.99609375
Jul-01 22:42:24.880[chain]           debug: Stored new PartialLightClientUpdate syncPeriod=1150, isFinalized=false, participation=0.998046875

and next sync committee period

Jul-01 22:40:26.647[chain]           debug: Stored nextSyncCommittee period=1150, slot=9420800, dependentRoot=0x1b67d2012d73c2ff7cc81ed2500114c0d37378c586d075eb8db3ff66da24f82e

My best guess right now is that this is due to the check here

lodestar/packages/beacon-node/src/chain/lightClient/index.ts

Line 431 in dbaa46e

finalizedCheckpointPeriod === period &&

as sync period of finalized checkpoint would be 1149 while current period is 1150 which means we are storing it as isFinalized=false

[nflaig]

Confirmed this issue on our mainnet node as well, the finalized_header is not updated for 2 epochs after new sync committee period starts.

Those apis return different finalized slots

curl -s https://lodestar-mainnet.chainsafe.io/eth/v1/beacon/light_client/finality_update | jq -r .data.finalized_header.beacon.slot
9445280

curl -s https://lodestar-mainnet.chainsafe.io/eth/v2/beacon/blocks/finalized | jq -r .data.message.slot
9445343

The light client server is lacking behind.

We have two places in the code where we compare the sync committee period which I think is the reason for this

lodestar/packages/beacon-node/src/chain/lightClient/index.ts

Line 431 in dbaa46e

finalizedCheckpointPeriod === period &&

lodestar/packages/beacon-node/src/chain/lightClient/index.ts

Line 637 in b6e2077

computeSyncPeriodAtSlot(finalizedHeaderAttested.beacon.slot) == syncPeriod

From the light client spec it's not clear why this check is required.

There is a note about when to update

LightClientUpdate are only considered if compute_sync_committee_period_at_slot(update.attested_header.beacon.slot) == compute_sync_committee_period_at_slot(update.signature_slot)

But this compared the sync period of attested_header slot and not finalized_header.

There is no such check when setting finalized header

# Indicate finality whenever possible
    if finalized_block is not None:
        if finalized_block.message.slot != GENESIS_SLOT:
            update.finalized_header = block_to_light_client_header(finalized_block)
            assert hash_tree_root(update.finalized_header.beacon) == attested_state.finalized_checkpoint.root
        else:
            assert attested_state.finalized_checkpoint.root == Bytes32()
        update.finality_branch = FinalityBranch(
            compute_merkle_proof(attested_state, FINALIZED_ROOT_GINDEX))

Maybe I am missing something from the spec, but our implementation looks incorrect to me