Attestation verification uses head state fork

[paulhauner]

Description

In the epoch after the Capella epoch DSRV (as a Lido operator) had an invalid block produced by Lighthouse.

Missed Block Details

• Slot: 6209557
• Epoch: 194048 (This is the Capella fork epoch 👀)
• Block hash: 0xa009b1b9d1de0edcb14db82d098ba6e2d902e6b13424062a6ac3933e59a0c15e
• Proposer index: 211576

Missed Block Logs

The we have a variety of logs. Those from Lighthouse and Nimbus showed a block which failed batch signature verification, meaning one of the signatures in the block is invalid. Prysm had more useful logs which showed that three attestations had invalid signatures:

beacon-chain[284213]: time="2023-04-13 06:31:50" level=debug msg="Received block" blockSlot=6209557 graffiti="Lighthouse/v4.0.1-a53830f
beacon-chain[284213]: " prefix=sync proposerIndex=211576 sinceSlotStartTime=3.41225891s
beacon-chain[284213]: time="2023-04-13 06:31:50" level=debug msg="Inserting in invalid block cache" prefix=sync root=0xa009b1b9d1de0edcb14db82d098ba6e2d902e6b13424062a6ac3933e59a0c15e
beacon-chain[284213]: time="2023-04-13 06:31:50" level=error msg="Could not handle p2p pubsub" error="could not process block: could not batch verify signature: some signatures are invalid. details:
beacon-chain[284213]: signature 'attestation signature' is invalid. signature: 0x8b9f6a72fe819f4d9e9c02a7c7b9d0fcc72236c5af54d2d31f1b427507a170017d154d16804346b8b8a4e1f8a3ac4a2d03fa109a71bbaf27ea5799db59d855159f7acb721dcd8da3b6daa43be6455a641eaa3058b7c856c829a66067d84ebd84, public key: 0xab6f91513b1d8118a5786a17f051d9664e0abf8e68e770fe67f1eef0c2da8f342349bbb856c12e3e3685da1285449832, message: 0x42d167388f1f3aefb7cd367817d2bf4c77f43a059b4314d3423d7632221c3dce
beacon-chain[284213]: signature 'attestation signature' is invalid. signature: 0x850287d27342e9e5ddba46d2a173061c9b5ae85203cd36f7677dbbed6e3c80d4151085165218be1772414d54cfe59f1b0213ee2ab721bf0e9f4481b57ad1907ede79bdc9947a87d7d4d37d3c15860892f7e1049c84d4a6f16eaa1a6a9f9f069c, public key: 0x83b8b961d70960af358ce221a1389e797f45f109e479c3ca4ea01cc2e176e87ab326ddfc305bf758c8feab1a689e3103, message: 0xdbbe100ff936d0d119eed1832a9aabbf90308a0cafe4e753d0ad6a60a3e4e8ef
beacon-chain[284213]: signature 'attestation signature' is invalid. signature: 0xa4715668f83b2973954aef35b4ff6aa505b65c679992522339b130524d93fce50d65f9eda1858bef055cee3b24f3be2719a55d7beba1e85139e9e2b6cd60c5f669e04b68912ecf7c59469c6ada65f5140dd83fa698e5be5c7704564132bdb0f8, public key: 0xae916e59115029ff5225b8cec6b331411c252b7ffe8d82bb8dd0d62b175ed1c2cea41d65aa7576fb66c356da64033129, message: 0xa82b2131661bcb773b13d2f80c4b5cb8532f82354b71f02629138dd1718f64a7" prefix=sync topic="/eth2/bba4da96/beacon_block/ssz_snappy"

Unfortunately we don't have the actual block (hence #4194) so we're limited in the information that we know.

Whilst investigating I found an issue that I think caused this. Even if it didn't we need to fix it anyway.

Batch Attestation Signature Verification Issue

There are two places in batch attestation verification (only batch verification, not individual verification) where we use the Fork of the block at the head of the chain for signature verification:

lighthouse/beacon_node/beacon_chain/src/attestation_verification/batch.rs

Line 68 in 693886b

let fork = chain.canonical_head.cached_head().head_fork();

lighthouse/beacon_node/beacon_chain/src/attestation_verification/batch.rs

Line 172 in 693886b

let fork = chain.canonical_head.cached_head().head_fork();

Consider the following scenario:

1. The Capella fork happens at epoch 194048.
2. We are at the fourth second of the first slot of epoch 194048.
3. We haven't yet received the first block of epoch 194048, so our head is the last block in the epoch of 194047. This mean our "head fork" has current_version: bellatrix and previous_version: altair
4. We start receiving valid attestations that were created in the first slot of epoch 194048. They will be signed using a fork with current_version: capella and previous_version: bellatrix (because they will have advanced their beacon state into the current epoch and therefore run upgrade_to_capella before creating the attestation).

In this scenario, we will reject those valid attestations (this is bad). Additionally, anyone that forgot to update their node will be signing attestations using current_version: bellatrix and previous_version: altair. We will accept those attestations as invalid and put them in the op pool.

Then, when we're packing a block we'll use those invalid attestations (because anything in the op pool should be valid) and therefore produce an invalid block (just like the one DSRV made).

Potential Solution

The naive solution would be to always clone the beacon state and run the state transition function to bring it up to the same slot as the attestation. However that clone and state processing is way too expensive so this would be impractical.

Since we only need to state.fork structure upgraded, a much simpler and cheaper method would be to check to see if there is an "upgrade" function (e.g., upgrade_to_capella) in the slots between the slot of the head state and the slot of the attestation. If so, just update the Fork and use that. I think this would be quite easy to implement and very cheap at runtime.

We should also add some tests so that we prevent any regressions.

It's also worth noting that this is only a problem at fork boundaries. This issue will not demonstrate itself again until our next upgrade (Deneb). However, I think we should fix it quickly so that (a) we don't forget about it and (b) we don't do invalid stuff on the Deneb testnets.

[jimmygchen]

Hi @paulhauner I'd like to look into this one!

[jimmygchen]

Just to add to the above - slot 6209557 is slot 21 of the Capella epoch, and not one of the earlier slots in the epoch, but the scenario described by @paulhauner could still happen because the proposer of slot 21 may already had the invalid attestation added to the op pool early in the epoch, however it wasn't included in any of the earlier 20 slots (7 out of 20 were missed proposals, and others could potentially be other CL clients) and remained in its op pool, hence when the validator gets a chance to propose a block, it included the invalid attestation, resulting in an invalid block. (diagnosis from @michaelsproul)

I'm thinking to add the following test cases:

1. an attestation with data.slot >= first capella slot signed with the pre-Capella fork should be REJECTED (currently ACCEPTED if our head is not up to date)
2. an attestation with data.slot >= first capella slot signed with the Capella fork should be ACCEPTED (currently REJECTED if our head is not up to date)

[jimmygchen]

I've managed to write tests for the above cases (thanks to @michaelsproul's help!), and interestingly only the 1st scenario fails.

With the 1st scenario (other node isn't aware of the fork and signed using bellatrix), our head fork (bellatrix) is used for batch verification so the attestation signed using bellatrix passes, and the attestation is ACCEPTED. This needs to be fixed.

In the 2nd scenario (attestation signed with capella fork), the head fork (bellatrix) is used for batch verification so the attestation signed using capella fails, and after this we do individual verification, where the target epoch fork (capella) is used, and this passes the verification, hence the attestation is accepted, which is the right result, but costs the extra verification.

If we update the fork used with the proposed solution, it should fix the issue with the 1st scenario, and also eliminate the need for individual verification in the 2nd scenario.

[michaelsproul]

Great progress @jimmygchen, interesting that only the first case fails! I think that's still consistent with the invalid block we saw: the invalid attestation from the 1st case would end up in the op pool and get packed in a block.

[michaelsproul]

Stumbled upon this old write-up from 2 years ago where Teku had essentially the same bug: https://hackmd.io/@ajsutton/rJmzlJyfY

[paulhauner]

Resolved via #4263 🎉