sigp / lighthouse

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

More robust subnet monitoring #5307

Open AgeManning opened 8 months ago

AgeManning commented 8 months ago

Description

The subnets and peer slots are becoming more valuable and we should be a little bit stricter about how peers are behaving.

In particular, lighthouse favours peers with long-lived subnets. We are getting these based on the meta-data or the enr. See here: https://github.com/sigp/lighthouse/blob/unstable/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs#L150

We don't want the cases where peers are faking these numbers by just setting their ENR bitfields or metadata fields. We want to ensure they are actually subscribed to the subnets.

We have events that tell us when a peer is subscribed to an event: https://github.com/sigp/lighthouse/blob/unstable/beacon_node/lighthouse_network/src/service/mod.rs#L1216

And when they unsubscribe: https://github.com/sigp/lighthouse/blob/unstable/beacon_node/lighthouse_network/src/service/mod.rs#L1269

It would be nice to ensure that the ENR/metadata bitfields match the subscriptions and penalize the peer if they do not.

Some simple logic could be:

Complications

There are some complications/race conditions that need to be solved. Firstly, meta-data can be sent at an arbitrary time after a peer connects and we won't know what their long-lived subnets are until we get this (for incoming peers). For peers we have dialed, we have their ENR and can match against this. When a peer connects, they send us all their gossipsub subscriptions, some of which will be short lived.

One idea, would be on every meta-data response we verify the peer is subscribed to all the subnets it says it is: https://github.com/sigp/lighthouse/blob/stable/beacon_node/lighthouse_network/src/peer_manager/mod.rs#L685

The other complication is that long-lived subnets rotate around every 24 hours. In principle, the subscription updates on gossipsub should happen instantly and the metadata responses should be slower, so I'd imagine we wouldn't hit too many race conditions there.

Perhaps a high tolerance peer penalty per meta-data response is a reasonable way to handle this.

AgeManning commented 8 months ago

@ackintosh - I assigned you, as i thought this might be something you may be interested in. Let me know if not tho :)