Resubscribing to core topics

[AgeManning]

Description

I've noticed that when a node starts and has a poor connection its peer count can fluctuate around a handful of peers and back to zero. In this case, I've noticed we emit logs about re-subscribing to the core topics. We should only subscribe once to the core topics.

Background Info

Core topics are ones that all nodes subscribe to. Such as a beacon_block and aggregate_attestations. We do not subscribe to these topics until a node has synced. This is because when a node is subscribed it must verify the messages on the topic and then forward them on. A node that doesn't have a sync'ed chain cannot verify the messages and therefore should not forward them, nor should it participate in the gossipsub topic.

Therefore lighthouse only subscribes to these topics once it has synced. It should never again need to re-subscribe or unsubscribe from these.

The subscription happens here: https://github.com/sigp/lighthouse/blob/stable/beacon_node/network/src/service.rs#L673
And it emits an info log: https://github.com/sigp/lighthouse/blob/stable/beacon_node/network/src/service.rs#L762

I've seen this info log occur a few times. Although its entirely harmless, we should not be subscribing again. I think the cause of this is because the sync status goes out of sync when we have no peers. When we have no peers our sync status changes to Stalled: https://github.com/sigp/lighthouse/blob/stable/beacon_node/lighthouse_network/src/types/sync_state.rs#L26

When we get more peers, we become syned again, and I think we re-call the subscribe to core topics. The message is being sent to the network task from sync. See here: https://github.com/sigp/lighthouse/blob/stable/beacon_node/network/src/sync/manager.rs#L461

From the last link, its seems a new client could be backfilling in the background and have their peers count fluctuate to 0 and back and this function could be called multiple times.

Resolution

We could try and prevent the function from being called mutliple times in the sync manager. The difficulty there would be that there are quite a few edge cases in how the sync state changes. We don't want to be in a state where a node never subscribes to the core topics (because of an edge case) as this would be catastrophic for the node.

I think a more defensive solution would be to handle it in the network service. When it gets called, it should just check to see if its already subscribed to the core topics and if so, just do nothing.

[divagant-martian]

Closed by #4271