// if we chosen an invalid or native tackle, restart
if (!addr.IsValid() || IsLocal(addr)) {
break;
}
This appears like some type of a belt-and-suspenders examine which should not occur below regular circumstances, as a result of we should not have added invalid or native addresses to AddrMan within the first place. I am unsure why a break was chosen right here – a proceed right here would in all probability be positive as properly, it in all probability does not matter a lot.
if (!IsReachable(addr))
proceed;
This, alternatively, is one thing that might occur incessantly, as a result of IsReachable is dynamic, it will depend on consumer configuration – we might have run on Tor previously and deactivated with the following restart, so that each one Tor addresses that have been beforehand reachable are actually unreachable.
Utilizing proceed as a substitute of break on this case appears vital to me: Encountering an unreachable tackle is not any purpose to skip the internal loop and have a 500ms sleep within the outer loop – we might prefer to proceed instantly and not using a sleep.
why not restart, so addrman would possibly get refreshed with extra reachable
addresses?
The method of AddrMan getting extra addresses is impartial of the ThreadOpenConnection logic – this occurs in one other space of the code (net_processing) and one other Thread – every time we obtain addr messages from our friends, they’re added to addrman.