[altair-devnet-1] Invalid SyncCommitteeMessage

[twoeths]

Describe the bug

Adrian from teku informed us some invalid SyncCommitteeMessage

• SyncCommitteeMessage 1

0xffbf0000000000004e7486693bcc3fd5ec59638782a477efee8b4e746bb730ef2f0402a7dd067b29e401000000000000b64596cc72f57a3d036fe7609ef03fb8bc99e549a23ecadd8a3757b8907161e8616ddffa102fe7267550aea3ebe88ae112410ef75e631800e3806d3f1b877848be49749d492d410180c1c06d16c3b3a4e09f6803203ae548a58e33f6f869a3e6

slot: '49151'
beaconBlockRoot: '0x4e7486693bcc3fd5ec59638782a477efee8b4e746bb730ef2f0402a7dd067b29'
validatorIndex: '484'
signature: >-
  0xb64596cc72f57a3d036fe7609ef03fb8bc99e549a23ecadd8a3757b8907161e8616ddffa102fe7267550aea3ebe88ae112410ef75e631800e3806d3f1b877848be49749d492d410180c1c06d16c3b3a4e09f6803203ae548a58e33f6f869a3e6

• SyncCommitteeMessage 2

0xffbf0000000000004e7486693bcc3fd5ec59638782a477efee8b4e746bb730ef2f0402a7dd067b29ce00000000000000b210c73d5b751e537d815657e8f0f356c8142a7e5c37abc619f2f0dfec2e03218295f349daddadc3e081bba192ebb47e10ce5afde48fdca8f59771dd81f05da958541848250eac6da4fa82c2ce2afaab8232cdded6ff4d03f523775d9241db13

slot: '49151'
beaconBlockRoot: '0x4e7486693bcc3fd5ec59638782a477efee8b4e746bb730ef2f0402a7dd067b29'
validatorIndex: '206'
signature: >-
  0xb210c73d5b751e537d815657e8f0f356c8142a7e5c37abc619f2f0dfec2e03218295f349daddadc3e081bba192ebb47e10ce5afde48fdca8f59771dd81f05da958541848250eac6da4fa82c2ce2afaab8232cdded6ff4d03f523775d9241db13

Expected behavior

Need to investigate why these messages pass our gossip validation from api before being spread to peers through gossip

[twoeths]

some more logs for validator 206 (0xa294f3656425d539753f610fac0e435d938027170934bbe631ce9b57dccb92dbda87c853e343924786accb7c9e551135) at slot 49151:

1. It signs a SyncCommitteeMessage: Jul-22 07:50:16.013 [] ^[[34mdebug^[[39m: Signed SyncCommitteeMessage slot=49151, validator=0xa2 94…1135
2. Validator publish it to node Jul-22 07:50:16.023 [] ^[[32minfo^[[39m: Published SyncCommitteeMessage slot=49151, count=10
3. Node validates the signature of that SyncCommitteeMessage successfully (it should not if it's an invalid signature) and spread it to peers
4. Node produces SyncCommitteeContribution: Jul-22 07:50:20.004 [] ^[[36mverbose^[[39m: Producing SyncCommitteeContribution slot=49151, index= 0
5. Validator signs SyncCommitteeContribution: Jul-22 07:50:20.008 [] ^[[34mdebug^[[39m: Signed SyncCommitteeContribution slot=49151, index=0, validator=0x8fa0…a348
6. Only after this stage, node is not able to verify the aggregated signature of it Jul-22 07:50:20.064 [] ^[[31merror^[[39m: Error publishing SyncCommitteeContribution slot=49151, index=0 message=Internal Server Error: SYNC_COMMITTEE_INVALID_SIGNATURE. Node's log: Jul-22 07:50:20.053 [API] ^[[31merror^[[39m: Error on publishContributionAndProofs [0] slot=49151, subCommitteeIndex=0 code=SYNC_COMMITTEE_INVALID_SIGNATURE

why our node can't detect the invalid signature at step 3 of SyncCommitteeMessage but SignedContributionAndProof at step 6?

lodestar/packages/lodestar/src/api/impl/beacon/pool/index.ts

Line 127 in 7e00bf4

await validateSyncCommitteeSigOnly(chain, state, signature);

[twoeths]

I was able to resync from genesis to get state 49151 and inspect its current and next sync committee.
preState_49151.zip

• note that slot 49151 belongs to the 49152 sync period so potentially it's an issue at the edge case
• the attached state shows that both validator 206 and 484 belong to currentSyncCommittee and nextSyncCommittee, for sure they are at different index in current and next committee
• due to this line of code, we send the SyncCommitteeMessage to the wrong subnet

  lodestar/packages/lodestar/src/api/impl/beacon/pool/index.ts

  Line 120 in d8f702b

  const indexesInCommittee = state.currentSyncCommittee.validatorIndexMap.get(signature.validatorIndex);

also the ContributionAndProof invalid signature issue above is due to the wrong currentSyncCommittee usage

lodestar/packages/lodestar/src/chain/validation/signatureSets/syncCommitteeContribution.ts

Line 43 in d8f702b

const validatorIndex = state.currentSyncCommittee.validatorIndices[indexInCommittee];

[dapplion]

Oh so you mean we should still check if we are in the current or next sync committee instead of just using whatever head has

[twoeths]

Oh so you mean we should still check if we are in the current or next sync committee instead of just using whatever head has

yeah, we have that logic in SyncCommitteeMessage validation, just need to apply that same logic everywhere to make sure. Also the spec always does that, for example

if compute_sync_committee_period(get_current_epoch(state)) == compute_sync_committee_period(next_slot_epoch):
        sync_committee = state.current_sync_committee
    else:
        sync_committee = state.next_sync_committee