search

sigp / lighthouse

Ethereum consensus client in Rust
https://lighthouse.sigmaprime.io/
Apache License 2.0
2.87k stars 724 forks source link

Reconsider peer penalties for `BlockIsAlreadyKnown` #3772

Open michaelsproul opened 1 year ago

michaelsproul commented 1 year ago

Description

While looking at some Gnosis chain logs I noticed that we were penalising peers for BlockIsAlreadyKnown errors. This seems to occur if we fetch a block from RPC, then get it on gossip and fail the duplicate check, e.g.

Dec 02 08:33:31.078 INFO New RPC block received, hash: 0x976c…5166, slot: 6195334, module: network::beacon_processor::worker::sync_methods:95 Dec 02 08:33:31.098 DEBG Rejected gossip block, slot: 6195334, graffiti: Lighthouse:v3.3.0, error: BlockIsAlreadyKnown, service: beacon, module: beacon_chain::beacon_chain:2464 Dec 02 08:33:31.098 DEBG Could not verify block for gossip, ignoring the block, error: BlockIsAlreadyKnown, module: network::beacon_processor::worker::gossip_methods:796 Dec 02 08:33:31.098 DEBG Peer score adjusted, score: -0.99, peer_id: 16Uiu2HAm1dAXPtpFRVWd9fajwujcjJzMD8RtUH8HmpxFPv4LgtEc, msg: gossip_block_high, service: libp2p, module: lighthouse_network::peer_manager::peerdb:537

Version

Lighthouse v3.3.0

Present Behaviour

We apply a high-tolerance penalty here:

https://github.com/sigp/lighthouse/blob/bf533c8e42cc73c35730e285c21df8add0195369/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs#L793-L803

I guess we expect most blocks not to arrive via RPC, but it seems to happen quite often on Gnosis chain with the short block time and many late blocks. For blocks arriving on gossip the gossip duplicate filter should catch duplicates before processing is attempted.

Expected Behaviour

One option would be to remove the penalty and assume that the gossip duplicate filter will catch the vast majority of true duplicates sent by peers. I'm not well-versed enough in our gossip/networking code to understand whether this would come with any downsides. I'll let someone else weigh in on that.

Another option would be to adjust the gossip check so that we do not penalise gossip blocks if they race with RPC blocks.

djbakerman commented 1 year ago

I am seeing this on mainnet as well.

Dec 30 09:32:44.524 DEBG Could not verify block for gossip, ignoring the block, error: WouldRevertFinalizedSlot { block_slot: Slot(5463966), finalized_slot: Slot(5463968) }, module: network::beacon_processor::worker::gossip_methods:796 Dec 30 09:32:44.527 DEBG Could not verify block for gossip, ignoring the block, error: WouldRevertFinalizedSlot { block_slot: Slot(5463965), finalized_slot: Slot(5463968) }, module: network::beacon_processor::worker::gossip_methods:796 Dec 30 09:32:46.659 DEBG Could not verify block for gossip, ignoring the block, error: BlockIsAlreadyKnown, module: network::beacon_processor::worker::gossip_methods:796 Dec 30 09:32:46.666 DEBG Could not verify block for gossip, ignoring the block, error: BlockIsAlreadyKnown, module: network::beacon_processor::worker::gossip_methods:796 Dec 30 09:32:48.062 DEBG Could not verify block for gossip, ignoring the block, error: BlockIsAlreadyKnown, module: network::beacon_processor::worker::gossip_methods:796 repeats

My host has 3 validators, so one of the validators would attest slot 5463965 instead of 5463966

Discord: djbakerman#7065