Fix sync protocol next_sync_committee_branch verification and remove active header

[hwwhww]

Discussed with @vbuterin today:

1. Bugfixes
   1. In the update.next_sync_committee_branch Merkle branch verification, it should be on attested_header rather than using active_header that may be finalized_header.
   2. update_period should be from attested_header rather than using active_header.
2. Largely simplified by removing "active header"
3. Extract apply_light_client_update into update_sync_committees_from_update and update_new_finalized_header.
4. For the LightClientUpdate builder, no need to fill empty next_sync_committee_branch.

---

From the perspective of a LightClientUpdate builder:

• Always fill attested_header with the recent block header
  • Note: if it's a finality update, the attested_header state should point to the finalized_checkpoint
• next_sync_committee and next_sync_committee_branch
  • Set the updated next_sync_committee and next_sync_committee_branch corresponding to attested_header
• Finality update
  • If it's a finality update
    • Fill finalized_header != BeaconBlockHeader()
    • Fill finality_branch corresponding to the state of the attested_header
  • Else
    • Fill finalized_header with BeaconBlockHeader()
    • Fill finality_branch with [Bytes32() for _ in range(floorlog2(FINALIZED_ROOT_INDEX))]
• sync_aggregate is always the SyncAggregate of attested_header
• fork_version is for sync_aggregate

/cc @etan-status @jinfwhuang @dapplion if you have time to review :)

[etan-status]

The changes seem a bit rushed to me.

1. This could be an oversimplification. The previous approach ensured that new sync committees would only be accepted once they have finalized (or, after extended period of non-finality). The new approach optimistically locks in the next_sync_committee as soon as a majority among current_sync_committee signed anything with any finalized checkpoint later than the current one, even when the new sync committee assignments are not yet final (i.e., the new finalized checkpoint is still in the old sync committee period). This could be a problem if there is forking at the beginning of a sync committee period, and the sync committee produces multiple majority signatures for different next_sync_committee branches. The previous approach did not have this problem.

2. While simplifications are nice, I think that it may lower the overall security properties. The previous sync protocol could also be reliably used to accelerate full node syncing by following a series of finalized_header from a weak-subjectivity checkpoint to the latest finalized block root, then start syncing from there. The new proposal depends on following the correct chain of signatures without any way to ensure that only finalized information is being followed. Overall, while having the two headers was initially a bit difficult to understand, the implementation of it is actually quite straightforward.

3. I'm not sure if it is correct when there are multiple sync committee periods of non-finality. Like, applying a couple updates using the timeout mechanism follows along their attested_header, then applying a new majority update with a newer update.attested_header.slot but a super old finalized_checkpoint resets store.finalized_header to update.finalized_header which may be several sync committee periods in the past, which desyncs store.current/next_sync_committee from store.finalized_header, failing all future validation of updates.

4. Yeah, that assert was quite annoying to work with, so it's good to see it gone. However, I think a new assert could be appropriate to ensure that the new update is following the same sync committee chain, if the store has already finalized on one.

Re perspective of builder:

• Always fill attested_header with the recent block header: Same as in old approach?
• next_sync_committee and next_sync_committee_branch: See (1) above.
• Finality update: Same as in old approach?
• sync_aggregate is always the SyncAggregate of attested_header: That data is unavailable on the blockchain. The sync committee signs the parent block of the previous slot. So, it could be that an attested_header is signed by next_sync_committee (instead of current_sync_committee), and also with a new fork_digest if there were any forks. If there were missed slots around a sync committee period change, it could also become difficult to determine the signing sync committee without trying to verify using both committee keys (sig verify potentially expensive on weak hardware).
• fork_version is for sync_aggregate: Same as in old approach?

Other related PRs (building on top of the previous approach, but the edge cases are discussed there as well):

• Allow light client to verify signatures at period boundary #2805
• Revise participation tracking in light client #2810
• Allow light client updates to only improve optimistic header #2811
• Define libp2p protocol for light client sync #2802

[dapplion]

Agree with @etan-status on most points. In regular mainnet conditions we can always wait for a finalized update which gives you the highest security guarantee at no extra cost

[etan-status]

The work here is covered by #2932

[hwwhww]

closing in favor of #2932