torcontrol: Improve connection logging #36759

ViniciusCestarii commented at 4:36 PM on November 4, 2026: contributor

Before eae193e750235e4023b3e6313284eede2cd7a022, Connect() only started the connection and returned true. If tor wasn't running the refusal was reported later and only logged under -debug=tor. Since eae193e750 (in v32), every failed reconnect attempt logs:

2026-11-04T14:14:08Z connect() to 127.0.0.1:9051 failed after wait: Connection refused (111)
2026-11-04T14:14:08Z [warning] tor: Error connecting to address 127.0.0.1:9051
2026-11-04T14:14:08Z [warning] tor: Initiating connection to Tor control port 127.0.0.1:9051 failed

On a default configured node without tor, this pollutes debug.log with 18 lines every hour.

This makes the following improvements:

Log connection failures as warning if tor config is explicit set (-torcontrol, -torpassword or -listenonion), otherwise at debug level.
Log at info level when an active onion service is lost.
Log info at the first tor connection failure if no tor config is explicit set.

The first two commits are from @fjahr.

jpk68 commented at 6:40 PM on November 4, 2026: contributor

ACK ac6d31e7a9320773b98934478d0e2ea3c17dfb65

fanquake commented at 6:48 PM on November 4, 2026: member

cc @fjahr @pinheadmz

in src/torcontrol.cpp:98 in ac6d31e7a9

  94 | @@ -95,9 +95,9 @@ bool TorControlConnection::Connect(const std::string& tor_control_center)
  95 |          return false;
  96 |      }
  97 |  
  98 | -    m_sock = ConnectDirectly(control_service.value(), /*manual_connection=*/true);
  99 | +    m_sock = ConnectDirectly(control_service.value(), /*manual_connection=*/false);

pinheadmz commented at 7:03 PM on November 4, 2026:

Why did you change this line? I think it actually shouldn't matter, its just residue of how all of our socket handling code has the perspective of bitcoin p2p even when that's not what we're doing with the socket.

ViniciusCestarii commented at 12:12 AM on November 5, 2026:

Because it is used only on:

    if (manual_connection) {
        LogInfo("%s", error_message);
    } else {
        LogDebug(BCLog::NET, "%s", error_message);
    }

Setting it to false makes it log as debug, otherwise it would still log "connect() to 127.0.0.1:9051 failed after wait: Connection refused (111)"

pinheadmz commented at 7:07 PM on November 4, 2026: member

I've noticed these annoying warnings as well on my own node without tor and looked into it. There is an exponential backoff with limit of RECONNECT_TIMEOUT_MAX{600.0}; // 10 min cap so after ~20 minutes or so the log messages drop to once every ten minutes. We keep attempting this connection so that if the user starts tor later in the node's lifetime, we just pick it up and work with it.

Personally I think it's annoying when you first start the node but then I dont mind seeing the attempts 6x/hour. Reminds me I need to start tor!

So I need help understanding the motivation for this change.

ViniciusCestarii commented at 12:23 AM on November 5, 2026: contributor

So I need help understanding the motivation for this change.

My main motivation is that it can be confusing for users that are not running tor since it is a warning level message they might think it is a config error or open new issue (#36691) and also pollutes logs. If the user wants to see tor debug logs they can set -debug=tor like it was before v32

pinheadmz approved

pinheadmz commented at 8:36 PM on November 5, 2026: member

ACK ac6d31e7a9320773b98934478d0e2ea3c17dfb65

Built and tested on macos/arm64. Code changes are justified and makes the logs cleaner / more like pre-32 behavior. Thanks @ViniciusCestarii for answering my questions!

Also wanted to throw out the option of using the new LogWarnThenDebug() if you think that makes sense. Happy to re review.

<details><summary>Show Signature</summary>

pinheadmz's public key is on openpgp.org

</details>

DrahtBot commented at 8:36 PM on November 5, 2026: contributor

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

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36759
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36759.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36759#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Stale ACK	jpk68, pinheadmz, winterrdog

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36743 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36743.svg"> ([RFC] build: build with -Wmissing-declarations/prototypes by fanquake)
#35713 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35713.svg"> (Remove boost as a unit test runner by rustaceanrob)
#31260 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/31260.svg"> (scripted-diff: Type-safe settings retrieval by ryanofsky)

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.

fanquake commented at 9:05 PM on November 5, 2026: member

If we decide to do this, we should probably also backport into 32.x.

sedited requested review from fjahr on Nov 5, 2026

winterrdog commented at 11:15 PM on November 5, 2026: contributor

tested ACK ac6d31e7a9320773b98934478d0e2ea3c17dfb65

successfully built and tested with clang++-20 on Debian, targeting x86_64.

logs look less noisy (quite peaceful now) when i ran bitcoind in regtest

re: #36759#pullrequestreview-5791717944

LogWarnThenDebug strikes a balance between various user needs, i think it is OK to take it up. the log message that shows failure to connect to tor control is logged exactly once. logs will look sth like this:

...
2026-11-05T22:59:01Z torcontrol thread start
2026-11-05T22:59:01Z initload thread exit
2026-11-05T22:59:01Z [warning] Error connecting to address 127.0.0.1:9051
2026-11-05T22:59:01Z [warning] Initiating connection to Tor control port 127.0.0.1:9051 failed
2026-11-05T22:59:01Z Bound to 127.0.0.1:18445
2026-11-05T22:59:01Z Bound to [::]:18444
2026-11-05T22:59:01Z Bound to 0.0.0.0:18444
2026-11-05T22:59:01Z Loaded 0 addresses from "anchors.dat"
...

</details>

fjahr commented at 2:04 AM on November 6, 2026: contributor

I agree that the current behavior isn't ideal but it seems that an bigger problem than noisy logs is that you wouldn't see any logs unless you had -debug=tor if you had -torcontrol set explicitly, i.e. you clearly want to use Tor but there is an issue and it doesn't work. That's why I think the current v32 behavior was an improvement to the previous behavior. Instead of reverting, I would suggest a more complete fix that works well for both scenarios: When the user configured Tor they should see logs and when they didn't they should not see them (only with -debug=tor).

I have drafted the change so people can decide if they like this more: https://github.com/fjahr/bitcoin/commits/2026-11-torlog/ But note that this also includes a bit of additional improvements like making the logging less noisy when it does happen and deals with losing the connection along the way.

ViniciusCestarii commented at 12:20 PM on November 6, 2026: contributor

Thanks for reviewing! I agree that logging for tor users is useful. I like @fjahr proposal but also tor is meant to work out of box (without the need of any explicit config), so users that never set tor specific config would not know that tor isn't being used unless they run with -debug=tor.

The best both worlds would be what @fjahr proposed + if no explicit tor config, log the first failure once at info level (e.g. "Not using Tor (no control port at 127.0.0.1:9051)") similar to @pinheadmz's LogWarnThenDebug() but at info. @fjahr if you're OK with it, I can pull your commits into this PR (keeping you as author) and add the info-once part on top. Or if you'd rather open your own PR, I'll close this one in favor of it.

fjahr commented at 10:12 AM on November 7, 2026: contributor

if you're OK with it, I can pull your commits into this PR

Sure, go ahead!

ViniciusCestarii force-pushed on Nov 7, 2026

ViniciusCestarii renamed this:
~~torcontrol: log control port connection failures at debug level~~
torcontrol: Improve connection logging
on Nov 7, 2026

ViniciusCestarii force-pushed on Nov 7, 2026

DrahtBot added the label CI failed on Nov 7, 2026

ViniciusCestarii commented at 4:23 PM on November 7, 2026: contributor

Force-pushed d5f6a9b6c96b30eaa9740eda5b9a98474a3aef59 adding @fjahr proposed changes + log the first failure once at info level. Updated PR title and description.

DrahtBot removed the label CI failed on Nov 7, 2026

in src/torcontrol.h:144 in 996846efe6

 140 | @@ -140,6 +141,7 @@ class TorController
 141 |      CThreadInterrupt m_interrupt;
 142 |      std::thread m_thread;
 143 |      const std::string m_tor_control_center;
 144 | +    const bool m_manual_connection{false};

pinheadmz commented at 3:52 PM on November 9, 2026:

996846efe6d0e7ee67d7fc7857321f46d26df1bc

I feel like the value should keep the name m_tor_control_configured that is declared by init.cpp. "Manual" connection doesn't make sense to me in the tor control context. For that matter, I also feel like the the value in netbase could probably also be renamed log_as_info (or verbose_failure) or something because it has no effect until LogConnectFailure() is called which could be called by P2P, torcontrol, Proxy, even bitcoin-cli...

ViniciusCestarii commented at 12:08 PM on November 10, 2026:

Agree it should be renamed, but it's out of scope here. I could do it as follow up

pinheadmz commented at 6:49 PM on November 9, 2026: member

I think this has gotten perhaps too complicated. Even with Claude making me a chart I can't wrap my head around all the details:

There's messages logging at Warn, Info, and Debug levels.
There's torcontrol options being set manually and by default.
There's the user's log level setting at default (info) or debug.
There's first connection error and not-first error.
There's a re-implementation of LogWarnThenDebug() but for Info level and then debug (I think?)
Netbase and torcontrol both write logs, at different levels

I think we should discuss simpler logic that cleans up the log and still follows the guidelines about log level.

Always set "manual" to false" netbase only logs debug. Handle more interesting logging directly from torcontrol
Use LogWarnThenDebug instead of adding new state and effectively LogInfoThenDebug

I "generated" a commit along with @fjahr "lost connection" commit here: https://github.com/bitcoin/bitcoin/compare/master...pinheadmz:bitcoin:torcontrol-log-simple

Tested by turning tor on and off during / before bitcoind. This feels right to me:

default user

start with tor: Got tor service ID... break tor: Lost connection... (only once) restart tor: Got tor service ID...

start without tor: <nothing> restart tor: Got service ID... break tor: Lost connection... (only once)

-listenonion / -torcontrol / -torpassword user

Same as default, plus a single [warning] tor: Error connecting to address... the first time any connection attempt fails (at startup if tor isn't running, or on the first reconnect attempt after tor breaks). Never repeated for the lifetime of the process.

-debug user

logs every single retry attempt

torcontrol: Log when an active onion service is lost e998f12b55

torcontrol: Only warn once about failed connections if Tor control is configured

The Tor control thread retries the connection indefinitely. Previously
every failed attempt logged two warnings plus an info line from netbase,
even for default nodes where the user never asked for Tor.

Only warn (once, via LogWarnThenDebug) if the user explicitly configured
Tor control with -listenonion, -torcontrol or -torpassword. Otherwise
log failures at debug level only. The netbase connect() failure is always
logged at debug level, and the redundant "Initiating connection" warning
is removed.

ffd928d32d

ViniciusCestarii force-pushed on Nov 10, 2026

ViniciusCestarii commented at 12:23 PM on November 10, 2026: contributor

Thanks for reviewing! Force-pushed ffd928d32de2137ff7178847d078ade064671563 with @pinheadmz suggested commits. I agree with renaming tor_control_configured and that net logs were "leaking".

This drops the info log on d5f6a9b6c96b30eaa9740eda5b9a98474a3aef59, which purpose was to remind a default user that wanted tor to activate tor. Fine leaving it out to keep this simple.