net: preserve anchors when network is disabled

brunoerg commented at 9:26 pm on January 6, 2026: contributor

Problem

When a node is shut down while network activity is disabled (e.g. via -networkactive=0 or setnetworkactive false), anchors.dat is overwritten with an empty list. This happens because StopNodes() collects anchors via GetCurrentBlockRelayOnlyConns(), which returns nothing when there are no active connections. The node then loses its anchors and cannot reconnect to them on the next startup.

Fix

In SetNetworkActive(false), save the current block-relay-only connections into m_anchors before the connections are closed, so the anchor list is preserved even after the network is deactivated.
In StopNodes(), if the network is inactive and m_anchors is non-empty, use those stored anchors as the source for anchors.dat instead of the (empty) live connection list. Also skip DumpAnchors entirely when the network is inactive and there is nothing to write.

Also, log that “X anchors will be tried for connections” only if network is active.

DrahtBot added the label P2P on Jan 6, 2026

DrahtBot commented at 9:26 pm on January 6, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34213.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	waketraindev, danielabrozzoni
Stale ACK	bensig, Bortlesboat

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32394 (net: make m_nodes_mutex non-recursive by vasild)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

brunoerg force-pushed on Jan 6, 2026

DrahtBot added the label CI failed on Jan 6, 2026

DrahtBot commented at 9:31 pm on January 6, 2026: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/20762589891/job/59621113321 LLM reason (✨ experimental): Linting failed: ruff detected errors (trailing whitespace) in Python code, causing the CI to fail.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

brunoerg force-pushed on Jan 6, 2026

DrahtBot removed the label CI failed on Jan 6, 2026

in src/net.cpp:3348 in 903a37c1e1 outdated

3344@@ -3345,7 +3345,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
3345         std::shuffle(seed_nodes.begin(), seed_nodes.end(), FastRandomContext{});
3346     }
3347 
3348-    if (m_use_addrman_outgoing) {
3349+    if (m_use_addrman_outgoing && fNetworkActive) {

waketraindev commented at 0:21 am on January 7, 2026:

I start some nodes with network inactive.

micro nit (in both cases):

0    if (fNetworkActive && m_use_addrman_outgoing) {

brunoerg commented at 6:25 pm on January 7, 2026:

I’ll leave as-is for now.

luke-jr commented at 8:27 pm on January 22, 2026:

If the user subsequently uses setnetworkactive to enable network, should we restore the anchors then?

brunoerg commented at 12:36 pm on January 27, 2026:

Yes, I think so. I will implement it in a follow-up.

waketraindev commented at 0:22 am on January 7, 2026: contributor

Concept ACK

waketraindev commented at 1:50 am on January 7, 2026: contributor

Have you considered also adding a DumpAnchors() if anchors_to_dump.size() > 1 when setting network active from true to false?

That would persist the anchors for nodes that do a setnetworkactive false and after shut down and skip dumping them again at shutdown.

brunoerg commented at 6:26 pm on January 7, 2026: contributor

Have you considered also adding a DumpAnchors() if anchors_to_dump.size() > 1 when setting network active from true to false?

Yes, but would leave it for a follow-up, it will require more changes than the simple ones to cover the -networkactive behavior.

bensig commented at 6:40 pm on January 7, 2026: contributor

ACK 903a37c1e142b4ee6b83fee99735ab69716084ee

Tested, feature_anchors.py passes

DrahtBot requested review from waketraindev on Jan 7, 2026

w0xlt commented at 8:59 pm on January 7, 2026: contributor

This seems like a reasonable change, but it may be better to document that setting fNetworkActive to false via the setnetworkactive RPC can prevent anchors from being preserved.

brunoerg commented at 7:48 pm on January 8, 2026: contributor

This seems like a reasonable change, but it may be better to document that setting fNetworkActive to false via the setnetworkactive RPC can prevent anchors from being preserved.

Sounds good, I will add this information in the setnetworkactive RPC.

brunoerg commented at 1:07 pm on January 9, 2026: contributor

Force-pushed addressing #34213#pullrequestreview-3636779749

danielabrozzoni commented at 4:39 pm on January 13, 2026: member

Concept ACK. I think this is a good improvement, makes sense to have, and I can’t find any additional surface of attack introduced here. To recap, this is how we use anchors at the moment:

Anchors are used to protect from eclipse attacks. When our node cleanly shutdown, it saves our outbound block relay only connections to the anchors.dat file. The attack this is preventing is: if an attacker can manipulate our addrman to make sure that, on restart, we will only connect to the attacker nodes, and we happen to restart our node, the attacker can eclipse us. Anchors protect us because on restart we would connect to some of our previous block relay only peer, and one honest peer is sufficient for us not to become eclipsed.
We do not persist anchors if we uncleanly shutdown (power outage, software crash, etc.), because if one of our anchor is malicious and figures out a way to crash our node, and on restart we will reconnect to it, the attacker can keep crashing our node. See: #17428 (comment)

I started thinking along the lines of the last bullet point, to see whether the code introduced in this PR could be dangerous in any way, but I couldn’t think of anything. While I couldn’t find a way to exploit the bug this PR is fixing (if an attacker can figure out how to manipulate our bitcoind setting to set networkactive=0, we likely have bigger problems!), I still think it makes sense to fix this, as it’s low risk, and would avoid unnecessarily losing anchors.

However, while reviewing it I started thinking about the anchor logic, and I wonder if we should harden it a bit to make sure that we don’t delete anchors if networkactive is not set, but the user is not connected to the internet. I would appreciate feedback from someone who has thought about this area more deeply than me :)

At the moment, the PR works as intended if we set -networkactive=0, but if we simply disconnect the wifi between restarts, Bitcoin Core is still deleting anchors, similarly to master.

For example, I tried on my own node:

Start the node, keep it running until there’s block only connections, shut it down and check that the anchors.dat file is present
Disconnect internet
Restart the node

From the logs, we can see that the node reads the anchors, can’t connect to them, and at the time of flushing to disk, it writes 0 addresses.

02026-01-13T11:48:49Z Loaded 2 addresses from "anchors.dat"
12026-01-13T11:48:49Z 2 block-relay-only anchors will be tried for connections.
22026-01-13T11:53:26Z DumpAnchors: Flush 0 outbound block-relay-only peer addresses to anchors.dat started
32026-01-13T11:53:26Z DumpAnchors: Flush 0 outbound block-relay-only peer addresses to anchors.dat completed (0.00s)

The same happens if we start the node, disconnect the wifi, wait for long enough that all the peers are disconnected.

This happens because fNetworkActive only refers to whether -networkactive is set or not.

I think it would be beneficial to fix this edge case, because if an attacker can figure out how to disconnect us from the internet, then we would lose the anchors and be vulnerable to an eclipse attack. Of course, finding a way to disconnect a node from the internet is not an easy task! I suppose an attacker could try with a BGP attack, but I’m not entirely sure. I haven’t looked into how we would implement this, and I’m not sure it’s entirely doable.

brunoerg commented at 12:27 pm on January 14, 2026: contributor

I started thinking along the lines of the last bullet point, to see whether the code introduced in this PR could be dangerous in any way, but I couldn’t think of anything. While I couldn’t find a way to exploit the bug this PR is fixing (if an attacker can figure out how to manipulate our bitcoind setting to set networkactive=0, we likely have bigger problems!), I still think it makes sense to fix this, as it’s low risk, and would avoid unnecessarily losing anchors.

To clarify, I don’t think this is exploitable, it is mostly about the own user restarting the node (which happened to me) disabling the network and ending up losing the anchors. Also, since the network is disabled it makes sense to not perform any network activity like this.

waketraindev commented at 6:32 pm on January 14, 2026: contributor

still wish anchors would have been persisted if available when network activity is turned off instead just on exit.

edit: shouldn’t anchors be loaded at startup even if networkactive is false and maybe just skip saving them if network is inactive?

With this case both loading and saving will be skipped if node is started with network active false

0    if (m_use_addrman_outgoing && fNetworkActive) {
1        // Load addresses from anchors.dat
2        m_anchors = ReadAnchors(gArgs.GetDataDirNet() / ANCHORS_DATABASE_FILENAME);
3        if (m_anchors.size() > MAX_BLOCK_RELAY_ONLY_ANCHORS) {
4            m_anchors.resize(MAX_BLOCK_RELAY_ONLY_ANCHORS);
5        }
6        LogInfo("%i block-relay-only anchors will be tried for connections.\n", m_anchors.size());
7    }

How I see it:

Anchors should be loaded on startup regardless if network activity is disabled (it might be turned on after)
Anchors should not be popped back, attempted connection, if network activity is disabled
Anchors should be saved on exit from GetCurrentBlockRelayOnlyConns() if populated or m_anchors if empty

in src/rpc/net.cpp:894 in a52689837b

889@@ -890,7 +890,8 @@ static RPCHelpMan setnetworkactive()
890 {
891     return RPCHelpMan{
892         "setnetworkactive",
893-        "Disable/enable all p2p network activity.\n",
894+        "Disable/enable all p2p network activity.\n"
895+        "Disabling it can prevent anchors from being preserved.\n",

luke-jr commented at 8:26 pm on January 22, 2026:

This seems like a bug… Maybe we should re-save the original anchors if there’s no new ones at a clean shutdown? Perhaps update the “original anchors” list just before disabling network activity?

brunoerg commented at 12:36 pm on January 27, 2026:

Maybe we should re-save the original anchors if there’s no new ones at a clean shutdown?

I don’t think so. What if these anchors are not available anymore? I mean, we tried to connect to them but they are not available anymore.

ajtowns commented at 9:52 am on March 20, 2026:

I think maintaining m_anchors as something like:

 0    auto cutoff = now() - 1m;
 1    vector<CAddress> anchors;
 2    set<CAddress> old{m_anchors};
 3    for (const CNode* pnode : m_nodes) {
 4        if (!pnode->IsBlockOnlyConn()) continue;
 5        if (!pnode->fSuccessfullyConnected) continue;
 6        if (pnode->m_connected > cutoff) continue;
 7        anchors.push_back(pnode->addr);
 8        old.erase(pnode->addr);
 9    }
10    for (auto& o : old) {
11        if (m_nodes.size() >= 2) break;
12        anchors.push_back(o);
13    }
14    m_anchors.swap(anchors);

could work? ie update it with the address of current blocks-only connections that have been working, but if there aren’t any (or enough) such connections, keep the old addresses.

brunoerg commented at 6:14 pm on March 20, 2026:

Would it prevent us to not save addresses that we tried to connect to and didn’t work?

Perhaps when disabling the network, we could save the anchors (we’re connected to) to m_anchors and use it in StopNodes if GetCurrentBlockRelayOnlyConns doesn’t return any peer.

sedited requested review from danielabrozzoni on Mar 16, 2026

Bortlesboat commented at 3:01 pm on March 18, 2026: none

ACK a52689837bfa210e4426092e34a5d2c6ea24f351

Ran feature_anchors.py locally — passes on both transports.

DrahtBot requested review from waketraindev on Mar 18, 2026

Bortlesboat commented at 3:04 pm on March 18, 2026: none

Re the open question about whether anchors should be loaded at startup even with -networkactive=0: I think skipping the load is correct. Anchors protect against eclipse attacks on restart — if the node starts with the network off, there is no connection activity to eclipse. If the user later calls setnetworkactive true, the node has its addrman for peer diversity. The anchors.dat file stays on disk, so the next clean restart with network active loads them normally.

l0rinc commented at 3:07 pm on March 18, 2026: contributor

@Bortlesboat, please point your bot to a different repo, this isn’t useful…

fanquake commented at 3:08 pm on March 18, 2026: member

cc @willcl-ark given #34486.

sedited removed review request from waketraindev on Mar 19, 2026

sedited requested review from ajtowns on Mar 19, 2026

DrahtBot requested review from waketraindev on Mar 20, 2026

net: store anchors in SetNetworkActive

When disabling the network activity, store anchors on m_anchors.

9a7c4970cd

net: preserve anchors on shutdown when network is inactive

During StopNodes(), anchors are collected via GetCurrentBlockRelayOnlyConns().
When the network has been deactivated, that call returns an empty list since
there are no active connections, causing anchors.dat to be overwritten with no
entries and losing the anchors that were saved at deactivation time (by
SetNetworkActive).

Fix this by falling back to m_anchors as the source when the network is
inactive and m_anchors is non-empty. Also skip DumpAnchors entirely if the
network is inactive and there is nothing to write, avoiding unnecessary
file I/O.

71d3aa1957

test: disabling network activity should not affect anchors a6d411664e

net: guard m_anchors with mutex

m_anchors can be read and written from different threads.
Introduce m_anchors_mutex, guard the vector with GUARDED_BY,
and update EXCLUSIVE_LOCKS_REQUIRED annotations on all callers.

dce37280c3

brunoerg force-pushed on Mar 27, 2026

brunoerg renamed this:
~~net: do not read/dump anchors if network is not active~~
net: preserve anchors when network is disabled
on Mar 27, 2026

net: log that anchors will be tried for connections only if network is active 3aa8484020

DrahtBot added the label CI failed on Mar 27, 2026

brunoerg commented at 2:52 pm on March 27, 2026: contributor

I just changed the approach of this PR and changed PR title and description. It addresses concerns like #34213 (review). Now this PR addresses cases involving disabling network via RPC and starting the node with the network disabled via flag.

cc @danielabrozzoni @luke-jr @ajtowns

DrahtBot removed the label CI failed on Mar 27, 2026

net: preserve anchors when network is disabled #34213

Problem

Fix

Code Coverage & Benchmarks

Reviews

Conflicts