Skip recursive discovery query if no useful ENRs

[dapplion]

Issue Addressed

Lighthouse's basic simulator logs are completely un-usable because of the spam from the sequence of logs:

2024-07-30T08:50:55.2102984Z Jul 30 08:50:55.160 DEBG Starting a new peer discovery query     wanted: 6, outbound: 1, target: 6, connected: 1, service: libp2p, service: node_1
2024-07-30T08:50:55.2104783Z Jul 30 08:50:55.160 DEBG Starting a peer discovery request       target_peers: 6, service: libp2p, service: node_1
2024-07-30T08:50:55.2106299Z Jul 30 08:50:55.160 DEBG Discovery query completed               peers_found: 1, service: libp2p, service: node_1
2024-07-30T08:50:55.2108037Z Jul 30 08:50:55.160 DEBG Starting a new peer discovery query     wanted: 6, outbound: 1, target: 6, connected: 1, service: libp2p, service: node_1
2024-07-30T08:50:55.2109828Z Jul 30 08:50:55.160 DEBG Starting a peer discovery request       target_peers: 6, service: libp2p, service: node_1
2024-07-30T08:50:55.2111755Z Jul 30 08:50:55.160 DEBG Discovery query completed               peers_found: 1, service: libp2p, service: node_1

The root cause is that there are not enough peers in the network ever to satisfy the needs of the peer manager.

The peer manager enters an infinite loop of discovery queries that never yield new useful peers.

@AgeManning Tried to fix the issue with this PR but it doesn't work. The first condition of current_count < target is never met, so the spam still happens even with #6162

• Prevent continuous searches for low-peer networks #6162

Note that this infinite loop can theoretically happen in Mainnet too, if somehow the pool of ENRs shrinks to less than our target, or someone tries to run lighthouse with --target-peers 10000.

Proposed Changes

IMO the recursiveness is an optimization that we can safely skip under certain circumstances to prevent the infinite loop. This PR implements the heuristic that if:

• A query includes > 0 results
• None of those results is diable

Then we have likely exhausted the discv5 table and should stop the recursive query. Note that if there are false positives the heartbeat will trigger queries latter anyway.

I have tested this PR locally with cargo run --release --bin simulator basic-sim and there is no more spam of the logs above and the node still reaches the expected 5 target peers quickly.

[jxs]

LGTM! Lion

[AgeManning]

Yeah agreed. I think this is a good soln. Nice!

[AgeManning]

My changes to the DAS branch are superseded by this I think. So we should revert those when merging down.

[jimmygchen]

@mergify queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at f7f0bfc