<pre><code>// if we selected an invalid or local address, restart
if (!addr.IsValid() || IsLocal(addr)) {
    break;
}
</code></pre>
<p>This looks like some kind of a belt-and-suspenders check which shouldn't happen under normal circumstances, because we shouldn't have added invalid or local addresses to AddrMan in the first place. I'm not sure why a break was chosen here - a continue here would probably be fine as well, it probably doesn't matter much.</p>
<pre><code>if (!IsReachable(addr))
    continue;
</code></pre>
<p>This, on the other hand, is something that could happen frequently, because <code>IsReachable</code> is dynamic, it depends on user configuration - we may have run on Tor in the past and deactivated with the next restart, so that all Tor addresses that were previously reachable are now unreachable.</p>
<p>Using <code>continue</code> instead of <code>break</code> in this case seems important to me: Encountering an unreachable address is no reason to skip the inner loop and have a 500ms sleep in the outer loop - we'd like to continue immediately without a sleep.</p>
<blockquote>
<p>why not restart, so addrman might get refreshed with more reachable
addresses?</p>
</blockquote>
<p>The process of AddrMan getting more addresses is independent of the ThreadOpenConnection logic - this happens in another area of the code (net_processing) and another Thread - whenever we receive addr messages from our peers, they are added to addrman.</p>

CC BY-SA 4.0
2023-04-07T15:37:17.637Z
2023-04-07T15:37:17.637Z
Lightlike
<p>In the inner loop of ThreadOpenConnections() in net.cpp sometimes there is a <code>break</code> to the outer loop and sometimes a <code>continue</code>. This is my understanding of the decision between break and continue in this part of the code:</p>
<ul>
<li>A break fully 'restarts' the search for an outward connection, the type of which will be determined again. Also new incoming potential peers will be included in the addrman tables.</li>
<li>A continue effectivily just asks addrman for a next candidate peer from the existing tables. The type of outward connection that we wish for remains the same.</li>
</ul>
<p>My question is about the choice of break versus continue in two cases. <a href="https://github.com/bitcoin/bitcoin/blob/c77c877a8e916878e09f64b2faa12eeca7528cc8/src/net.cpp#L1829" rel="nofollow noreferrer">Here</a> the code does a break:</p>
<pre><code>// if we selected an invalid or local address, restart
if (!addr.IsValid() || IsLocal(addr)) {
    break;
}
</code></pre>
<p>While a few lines below there is a continue:</p>
<pre><code>if (!IsReachable(addr))
    continue;
</code></pre>
<p>My question is: why 'restart' in the first code snippet and not continue? In other words: why not ask addrman a few times more, as it might supply a valid address soon? Or, for the second snippet, if we got an unreachable address: why not restart, so addrman might get refreshed with more reachable addresses?</p>
<p>I tried to find the answer by tracking back the history of this part of the code. The <code>break</code> in the first code snippet above appears to stem from the introduction of the address manager (<a href="https://github.com/bitcoin/bitcoin/pull/787" rel="nofollow noreferrer">pr/787</a>). It did not seem to answer my question.</p>

<bitcoin-core-development><network><p2p><source-code>
ThreadOpenConnections and break versus continue: why the one versus the other?
codo
# ThreadOpenConnections and break versus continue: why the one versus the other?

(Asked by: username_0 on 2023-02-02)

In the inner loop of ThreadOpenConnections() in net.cpp sometimes there is a `break` to the outer loop and sometimes a `continue`. This is my understanding of the decision between break and continue in this part of the code:

* A break fully 'restarts' the search for an outward connection, the type of which will be determined again. Also new incoming potential peers will be included in the addrman tables.
* A continue effectivily just asks addrman for a next candidate peer from the existing tables. The type of outward connection that we wish for remains the same.

My question is about the choice of break versus continue in two cases. [Here](https://github.com/bitcoin/bitcoin/blob/c77c877a8e916878e09f64b2faa12eeca7528cc8/src/net.cpp#L1829) the code does a break:

```
// if we selected an invalid or local address, restart
if (!addr.IsValid() || IsLocal(addr)) {
    break;
}

```

While a few lines below there is a continue:

```
if (!IsReachable(addr))
    continue;

```

My question is: why 'restart' in the first code snippet and not continue? In other words: why not ask addrman a few times more, as it might supply a valid address soon? Or, for the second snippet, if we got an unreachable address: why not restart, so addrman might get refreshed with more reachable addresses?

I tried to find the answer by tracking back the history of this part of the code. The `break` in the first code snippet above appears to stem from the introduction of the address manager ([pr/787](https://github.com/bitcoin/bitcoin/pull/787)). It did not seem to answer my question.

**Tags:** bitcoin-core-development, network, p2p, source-code


---

## 1 answers

### Answer by username_1 on 2023-04-07. Score: 2

```
// if we selected an invalid or local address, restart
if (!addr.IsValid() || IsLocal(addr)) {
    break;
}

```

This looks like some kind of a belt-and-suspenders check which shouldn't happen under normal circumstances, because we shouldn't have added invalid or local addresses to AddrMan in the first place. I'm not sure why a break was chosen here - a continue here would probably be fine as well, it probably doesn't matter much.

```
if (!IsReachable(addr))
    continue;

```

This, on the other hand, is something that could happen frequently, because `IsReachable` is dynamic, it depends on user configuration - we may have run on Tor in the past and deactivated with the next restart, so that all Tor addresses that were previously reachable are now unreachable.

Using `continue` instead of `break` in this case seems important to me: Encountering an unreachable address is no reason to skip the inner loop and have a 500ms sleep in the outer loop - we'd like to continue immediately without a sleep.

> why not restart, so addrman might get refreshed with more reachable
> addresses?

The process of AddrMan getting more addresses is independent of the ThreadOpenConnection logic - this happens in another area of the code (net\_processing) and another Thread - whenever we receive addr messages from our peers, they are added to addrman.


---

