Fix light client merkle proofs

[michaelsproul]

Issue Addressed

Fix a regression introduced in this PR:

• Fix electra light client types #6361

We were indexing into the MerkleTree with raw generalized indices, which was incorrect and triggering debug_assert failures, as described here:

• ApiTester::new_from_config doesn't work properly in Electra environment #7005

Proposed Changes

• Convert generalized_index to the correct leaf index prior to proof generation.
• Add sanity checks on indices used in BeaconState::generate_proof.
• Remove debug asserts from MerkleTree::generate_proof in favour of actual errors. This would have caught the bug earlier.
• Refactor the EF tests so that the merkle validity tests are actually run. They were misconfigured in a way that resulted in them running silently with 0 test cases, and the check_all_files_accessed.py script still had an ignore that covered the test files, so this omission wasn't detected.

Additional Info

This bug only affects LightClientFinality update and LightClientBootstrap, as they are the consumers of finalized root and sync committee merkle proofs respectively. The processing and broadcast of LightClientOptimisticUpdate is unaffected.

The buggy behaviour could be seen on our infra in the form of debug logs:

DEBG Light client invalid finality update error: InvalidLightClientFinalityUpdate, peer: 16Uiu2HAmAgnZHgwbHYVDiahFbT6pzeg5HT28fsYwwmGJD6fxbQQh

I am monitoring to ensure that these errors disappear after roll out.

[michaelsproul]

I've removed the logging for now, but it is running on our infra to help us spot more issues.

There are a lot of fields and subfields, so unless we do something like CompareFields (might be viable?), the output is going to be very long. Even as JSON it's quite unmanagable. E.g.

{
  "attested_header": {
    "beacon": {
      "slot": "3656194",
      "proposer_index": "1535871",
      "parent_root": "0x853c87a6bb14c9bbcfe98f366ea09948294cf209bc951d5310ec9e6dbfef86b3",
      "state_root": "0x906f1ff01ea2556bb6a05f4dfa7f3673f8a4a9d0f3ba0251600f61c756784fac",
      "body_root": "0x0f5cd8fbc7c4df7fd91e8c1b9c6cfa9ad711fc631a2a51d1197fe5e660049ad6"
    },
    "execution": {
      "parent_hash": "0x4113b40798acf7b57ee9391d3174e529fd85aa26610340eca9f63fabbca0bfce",
      "fee_recipient": "0x3b41d9736ed9bfc15a87d8fbb69c616190aed6c6",
      "state_root": "0x12de0a979f894c9cdbe94b85d14d01d24029d76cc196fd82fb385013fc4cd1fb",
      "receipts_root": "0xc02bb1e5c198cbccc21144765d0cef8f4bdec1c6a0d1b0a236f7c989da67d618",
      "logs_bloom": "0x84000000000000400080000000000000000000000400000000080040000000000100000000010000000001800000000400000000000800000040000000040000000000000000000000400000000000400000004000040000401000000000000100000000020000000000000000082800000000080000100000000000000000000000040004000004200000000000000000000000000000000000010000000000000000140000000000000100000004000000000000000002100004000000000040000000000000000100000000000000000000000000a00000000000000062000000000000000000209000000000000000000080008000000000000000100000",
      "prev_randao": "0x7306fb2898c6ccdaf4960d4ddfce66eaa81d9403e1155dd577e0f48b41519ff0",
      "block_number": "3369447",
      "gas_limit": "35964845",
      "gas_used": "652202",
      "timestamp": "1739776728",
      "extra_data": "0xd883010e0d846765746888676f312e32332e33856c696e7578",
      "base_fee_per_gas": "1254913",
      "block_hash": "0x190dfc8f7c4c32431477a771435bd3b0ed5c6df13fdab726d4ab900679ce7873",
      "transactions_root": "0xeba10054b5b2c487f259aa214e5e263d05a021eab955bc1b1529d5767dd1e99a",
      "withdrawals_root": "0xb3a26f1b2b68f43c04dea5e09172b1d9c0ae25dc13edad4d3d230149d8340bde",
      "blob_gas_used": "131072",
      "excess_blob_gas": "0"
    },
    "execution_branch": [
      "0x639ea3536f8ac6fc831cf4c3629f1f2a59e6baff009553dedef9bc0adcd40189",
      "0xd8857450a5aa5ef7f46be1ca6e14dfd20b698a047f30364e67c4c9dbd1dcd5f9",
      "0xdb56114e00fdd4c1f85c892bf35ac9a89289aaecb1ebd0a96cde606a748b5d71",
      "0x410b66c5afae89b1df7a65350ed742bf05942f75d6b663abfd295c626c1ec7f4"
    ]
  },
  "finalized_header": {
    "beacon": {
      "slot": "3656128",
      "proposer_index": "1867558",
      "parent_root": "0xee531cf5c2e68f032233aec782b73ac5bfd4bb289402868947a007aad8144d8d",
      "state_root": "0x83141bc902bcdd4557037df2b2b7c0bc79debb27d941a1d16f31218ea5a5dfcb",
      "body_root": "0xdeefa80e9b552e677cdb286f2813bb7e4e62118cec603b0dd3b932889adea866"
    },
    "execution": {
      "parent_hash": "0xd8cbcae880431bb6128ac8ef16a0a67c0e15c8b61e3fba3df902d3a5b38a0aa5",
      "fee_recipient": "0xb6c402298fcb88039bbfde70f5ace791f18cfac8",
      "state_root": "0xaf3cc69f7d9258acee618c36c903a1c8bd5cf6e22cb1c40a7d7c490fb763555d",
      "receipts_root": "0x024a1cd65cb6ef3d50a22ea42c4dbdef586a5851e4df91cb4a5443b8c988ad7d",
      "logs_bloom": "0x263320c08622632074924040000c0042344e0026b7800747e4ed407c07d0004a0173618728819201a0c01406c064000f5102801a0818069420440880803564814022206903255106c802c02a2ba0c54030008440888fa43200900000010648048841b5640241448780308a8f801a0a95b5429081013c511200da8cb2e8007c0481142d0d146c248a280404100084031600033060809040666e418280044c4034b289023424888647d52809402a30071061901430212210c80c28521102c130404110018288610180850580001103082180500c20870fb49880570cb2102c670d801d012c14400800248040342010e525242090b820e048540160006430103149",
      "prev_randao": "0x504aef912cc0fe4258063b34d3c5bba9992f86a1bfac9ad5ba04a61a5435fe10",
      "block_number": "3369387",
      "gas_limit": "35894605",
      "gas_used": "29816669",
      "timestamp": "1739775936",
      "extra_data": "0xd883010e0b846765746888676f312e32332e31856c696e7578",
      "base_fee_per_gas": "593270",
      "block_hash": "0x391a92b9caacccb4d2576346d945a43723b5c2a4450e55d86c1a9565acb76f1c",
      "transactions_root": "0xf64a6d39ebef81fe1bfdab817045ce711ab3c4b81b312d528dd3dc3efcc7878c",
      "withdrawals_root": "0x0bd45d0ed89944f58a97bb9594eae83be449f5f5aa73901e3e873903b6fdd73b",
      "blob_gas_used": "0",
      "excess_blob_gas": "0"
    },
    "execution_branch": [
      "0xd51aceed9aff38ded132761162c49a77523ae7c1dd74fa34f9acdcce9b172fc3",
      "0xb46f0c01805fe212e15907981b757e6c496b0cb06664224655613dcec82505bb",
      "0xdb56114e00fdd4c1f85c892bf35ac9a89289aaecb1ebd0a96cde606a748b5d71",
      "0x12fb543c0b5f8c609fa24f368dded12589f1ab387378ea8fbe40c46ae9969179"
    ]
  },
  "finality_branch": [
    "0x4ebe010000000000000000000000000000000000000000000000000000000000",
    "0x84aac78f6d6e4173f7aa524089ebe1322b50fa51332cfc4d61ab427e9e324d3c",
    "0xddff8ee532aa33a03e7839404940622daf61580f110efcb6459a536f8aee00e1",
    "0xc2c86a8bedbef9c6c9036c3a7b54587f25d9be732a44ffba66fdd49716679782",
    "0xd38854d340308969d869f85211f0918be250cdac61e43c272cf9f07d1172802e",
    "0x8192c89a42969df1a43afbc4032391b9e1d4771752313f742159db5910c86c1f"
  ],
  "sync_aggregate": {
    "sync_committee_bits": "0xf7ff7ffeffffdbfffdffeefafffff7dffffff6f7ffffbfeffbbfffffff9ffffdfffffeffdfbfffebffffffffffffffffeffffffefeefffffffffffffffffffe5",
    "sync_committee_signature": "0xa484f9534bf0014553dde697df805cb79f8b1269bd7bc6cab2f704d7fbb0702a22fde012c8eee224b8508a02c5bac0e7117feab34ad0f552e50d7849f15d30021b6b7ec2dfaa481bca7d6aa491eab1647aaf959d42748f19029ba3397e8fb046"
  },
  "signature_slot": "3656195"
}

[eserilev]

thanks for catching this, LGTM

[dapplion]

Should we make the GIndex a type to prevent these issues in the future?

[michaelsproul]

Yeah I think that sounds like a good idea. Maybe in a separate PR, as I'd like to keep this small and minimal for v7.0.0-beta.1 which we should release ASAP

[michaelsproul]

For now I think the EF tests and index checks are good insurance against future mistakes

[michaelsproul]

Update after running overnight:

• Very few occurrences of invalid finality updates floating around. There were 36 total, and the 3 I looked into were all Lodestar sending really stale updates. I've opened this issue on their repo: Stale LightClientFinalityUpdates sent during sync ChainSafe/lodestar#7476. This suggests that a staleness check would be an easy way to get better insights about the types of invalid LC updates. This was already being considered for different reasons here, which makes me feel like it is the right approach: Light client optimistic updates are requeued too eagerly #6994.
• The lack of invalid updates coming from Lighthouse peers (our un-updated nodes running v7.0.0-beta.0) is explained by the fact that we don't broadcast light client updates yet. So the impact of this bug for now is mostly that we don't forward updates that we should forward. It's possible that an attacker could forward junk updates that we would (wrongly) verify with v7.0.0-beta.0, so we should definitely fix this before v7.0.0 on mainnet, but whether we want to do v7.0.0-beta.1 will depend on the need for other fixes (e.g. builder API).
• No occurrences of invalid optimistic updates overnight, although there were a few yesterday prior to restart. I'll continue monitoring.