net: do not read/dump anchors if network is not active

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

We currently save 2 block-relay-only (anchor) connections when stopping a node and connect to them right after starting it. However, if you start a node with network disabled (setting -networkactive=0 or disabling using the RPC), it will read the anchors, obviously will not be able to connect to them and then, when stopping the node, considering it will have 0 block-relay-only connections, it will save 0 anchors, basically deleting the previous ones. With this PR, we will only read and write the anchors if the network is active. It will avoid losing the anchors when using the node without network activity and also avoid unecessary actions (like reading the anchors even knowing we have no network activity).

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

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

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.

net: do not read/dump anchors if network is not active 903a37c1e1

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.

rpc: add info about preserving anchors in setnetworkactive a52689837b

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.

net: do not read/dump anchors if network is not active #34213

Code Coverage & Benchmarks

Reviews