Incorrectly marking bad blocks halts chain

[siladu]

Problem: If Besu puts an INVALID block in the bad block cache and that block is subsequently valid, then we can only recover with a manual restart.

Some reasons that Besu might 'incorrectly' mark blocks as INVALID:

• unhandled internal error during block import (e.g. RocksDB error)
  • These have previously been mitigated with https://github.com/hyperledger/besu/blob/c85841d308f9d4bc37b6d3a0f61737a3f4b18c35/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockManager.java#L93C19-L99 but it relies on knowing the possible errors ahead of time
• CL has a bug and temporarily sends a bad block
  • Best case the problem might be transient or the fix might be to restart CL, but then you also have to restart besu as well to clear the bad block cache.

There are other scenarios where besu marks blocks as INVALID such as when backwards syncing that probably should continue to do that so we don't end up with besu on a fork.

Potential Solutions:

1. Maintain both an "operational" and an "archive" bad block cache. Always add to the "archive", maybe don't add to the "operational" (this might be useful even alongside the other options).
2. Have the cache expire after some time or after repeated hits (converse of (3)).
3. Use a circuit breaker approach where we only add it to the bad block cache if we get the same bad block after X retries (converse of (2)).
4. Extreme version of (1) where we only maintain an "archive", no "operational" cache and so always re-process bad blocks - this is the most lenient approach.

(2) is what geth do: https://github.com/ethereum/go-ethereum/blob/73697529994e14996b7740730481e926d5ec3e40/eth/catalyst/api.go#L105-L109

Geth's response about how they handle it https://discord.com/channels/595666850260713488/892088344438255616/1078206539548078122:

Seems I was somewhat wrong about that. In the 'core' blockchain, we do have badblocks-tagging, and we do store it to disk, but never ever consult it ourselves during processing. So I was right about that. However, the beacon API module has it's own cache for bad blocks, to avoid re-validating already known 'bad' blocks, and I didn't know about that. The cache is pretty small, though, and apparently the idea was to actually sometimes revalidate them anyway and not be too strict about it.

The reasoning behind not caching bad blocks would be just what happened here: a (temporary) situation occurs, which marks a good block as bad. Later, after the situation has been resolved, the reputation of certain blocks lingers, and causes long-term problems / longer splits than necessary.

Related to:

• Investigate why we couldn't automatically recover from bad block #5094 (comment)
• Remove BadBlockManager from ProtocolSpec #5153

Original discussion here: https://discord.com/channels/905194001349627914/905205502940696607/1077867357487243315

[non-fungible-nelson]

expire with persistence on disk

[non-fungible-nelson]

@siladu - any update on this issue?