torcontrol: Remove libevent usage

fjahr commented at 2:20 pm on December 27, 2025: contributor

This is part of the effort to remove the libevent dependency from our code base: #31194

The current approach tries to reuse existing code and follows roughly similar design decisions. It replaces the libevent-based async I/O with blocking I/O utilizing the existing Sock and CThreadInterrupt. The controller runs in a dedicated thread.

There are some optional code modernizations thrown in made along the way (namings, constexpr etc.). These are not strictly necessary but make the end result with the new code more consistent.

DrahtBot commented at 2:20 pm on December 27, 2025: 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/34158.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	pinheadmz
Concept ACK	sedited, pablomartin4btc, janb84, waketraindev

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:

#34486 (net: Reduce local network activity when networkactive=0 by willcl-ark)

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.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

m_sock->Recv(buf, sizeof(buf), MSG_DONTWAIT) in src/torcontrol.cpp
socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) in test/functional/feature_torcontrol.py

2026-01-30 13:30:16

sedited commented at 2:35 pm on December 27, 2025: contributor

Concept ACK

pinheadmz commented at 2:38 pm on December 27, 2025: member

concept ACK Quick review on github - where are you using LineReader?

fjahr force-pushed on Dec 27, 2025

DrahtBot added the label CI failed on Dec 27, 2025

DrahtBot commented at 3:32 pm on December 27, 2025: contributor

🚧 At least one of the CI tasks failed. Task macOS-cross to arm64: https://github.com/bitcoin/bitcoin/actions/runs/20540187798/job/59003472023 LLM reason (✨ experimental): Compilation error in fuzz tests: undefined type ConnectionCB and invalid TorControlConnection initialization (nullptr) in torcontrol.cpp.

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.

fjahr commented at 3:35 pm on December 27, 2025: contributor

where are you using LineReader?

Heh, I did quite a bit of moving around after I got things working and seems like I picked from the branch where i didn’t use it instead of the one where I did at the end. Pushed the right code for it now, using it in ProcessBuffer.

Will work on fixing the fuzz test which I completely missed 🙃 That’s what’s failing the CI.

pablomartin4btc commented at 4:24 pm on December 27, 2025: member

Concept ACK

fjahr force-pushed on Dec 28, 2025

DrahtBot removed the label CI failed on Dec 28, 2025

waketraindev commented at 3:33 pm on December 28, 2025: contributor

Concept ACK

There’s some uncovered new code in the coverage report, https://corecheck.dev/bitcoin/bitcoin/pulls/34158 And some Sonarcloud warnings to check out.

in src/torcontrol.cpp:137 in 872feec3eb

146-    if (b_conn)
147-        bufferevent_free(b_conn);
148-    b_conn = nullptr;
149+    if (m_sock) {
150+        m_sock.reset();
151+    }

janb84 commented at 4:08 pm on December 28, 2025:

0        m_sock.reset();

NIT: I think you can remove the redundant check. calling reset() on a std::unique_ptr is safe even if the pointer is already null

fjahr commented at 9:57 pm on December 28, 2025:

done

janb84 commented at 4:19 pm on December 28, 2025: contributor

Concept ACK 8d6f1b02f5868d918bb72059eac3afb881ccafe3

PR removes LibEvent usages from TorControl and cleans/modernizes the code a bit where touched.

code review, build and test. I do not have sufficient exp. to say something about the fuzz improvement commit. Small NIT found while doing the code review.

fjahr force-pushed on Dec 28, 2025

DrahtBot added the label CI failed on Dec 28, 2025

DrahtBot commented at 10:02 pm on December 28, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/20560088214/job/59049502835 LLM reason (✨ experimental): Python linting failed: an f-string had no placeholders (ruff warning F541) in test/functional/feature_torcontrol.py.

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.

fjahr commented at 10:16 pm on December 28, 2025: contributor

Addressed feedback.

There’s some uncovered new code in the coverage report, https://corecheck.dev/bitcoin/bitcoin/pulls/34158

The old code was largely uncovered as well but I have looked into it and added a simple functional test that provides some basic end-to-end coverage. I thought about unit tests too but I am not sure they provide much value aside from the functional and fuzz tests. But happy to be convinced otherwise if there are ideas for what they should cover.

And some Sonarcloud warnings to check out.

I don’t know what that is and the screenshot doesn’t allow me to see which lines are meant by the comment. Happy to look into them if you can transfer the comments matching to the correct lines here somehow.

waketraindev commented at 11:14 pm on December 28, 2025: contributor

I don’t know what that is and the screenshot doesn’t allow me to see which lines are meant by the comment. Happy to look into them if you can transfer the comments matching to the correct lines here somehow.

You can see the warnings on https://corecheck.dev/bitcoin/bitcoin/pulls/34158 scroll down the page, above benchmarks, below uncovered included code

DrahtBot removed the label CI failed on Dec 28, 2025

fjahr force-pushed on Dec 31, 2025

fjahr commented at 5:06 pm on December 31, 2025: contributor

You can see the warnings on https://corecheck.dev/bitcoin/bitcoin/pulls/34158 scroll down the page, above benchmarks, below uncovered included code

TIL, thanks. I addressed the ones that made sense to me.

DrahtBot added the label CI failed on Dec 31, 2025

DrahtBot commented at 6:18 pm on December 31, 2025: contributor

🚧 At least one of the CI tasks failed. Task macOS-cross to arm64: https://github.com/bitcoin/bitcoin/actions/runs/20623563561/job/59230100443 LLM reason (✨ experimental): Compilation failed due to use of std::jthread not available in the target standard library (missing C++20 jthread support).

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.

fjahr force-pushed on Dec 31, 2025

DrahtBot removed the label CI failed on Dec 31, 2025

in src/torcontrol.cpp:504 in 3a1f8c0947 outdated

500@@ -501,12 +501,12 @@ void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply&
501  *                  CookieString | ClientNonce | ServerNonce)
502  *
503  */
504-static std::vector<uint8_t> ComputeResponse(const std::string &key, const std::vector<uint8_t> &cookie,  const std::vector<uint8_t> &clientNonce, const std::vector<uint8_t> &serverNonce)
505+static std::vector<uint8_t> ComputeResponse(const std::string &key, const std::vector<uint8_t> &cookie,  const std::vector<uint8_t> &client_nonce, const std::vector<uint8_t> &serverNonce)

pinheadmz commented at 4:16 pm on January 14, 2026:

3a1f8c094798d09f41cd79c71c34afe2ea967394

why change client_nonce but not serverNonce? I’m guessing because there is a m_client_nonce at the call site, but the member property isn’t being explicitly handled here.

fjahr commented at 2:39 pm on January 17, 2026:

Yeah, I was initially focussing on the member variables in order to not let this get out of hand but the param here was touched too by accident. Doesn’t seem too bad to change the serverNonce too as well as the other local variables in this function, so taking care of all of them now.

in src/torcontrol.cpp:194 in c675b6f059 outdated

210+bool TorControlConnection::ProcessBuffer()
211+{
212+    if (m_recv_buffer.size() > MAX_LINE_LENGTH) {
213+        LogWarning("tor: Disconnecting because MAX_LINE_LENGTH exceeded");
214+        return false;
215+    }

pinheadmz commented at 7:45 pm on January 15, 2026:

c675b6f059dea662d9faa90df261de7bfbca13a1

We won’t ever get multiple lines in the receive buffer?

fjahr commented at 2:37 pm on January 17, 2026:

Right, that’s a good catch. If the processing is too slow this could fail unnecessarily and LineReader should catch this already fine, so I am removing this.

in src/torcontrol.cpp:200 in c675b6f059 outdated

216+
217+    // TODO: Maybe could give this as an option to LineReader instead
218+    auto last_newline = std::find(m_recv_buffer.rbegin(), m_recv_buffer.rend(), std::byte{'\n'});
219+    if (last_newline == m_recv_buffer.rend()) return true;
220+    size_t complete = last_newline.base() - m_recv_buffer.begin();
221+    util::LineReader reader({m_recv_buffer.data(), complete}, MAX_LINE_LENGTH);

pinheadmz commented at 7:51 pm on January 15, 2026:

c675b6f059dea662d9faa90df261de7bfbca13a1

I’m still learning how torcontrol protocol works all together, but without complete context it seems to me that all this could be simplified:

0util::LineReader reader(m_recv_buffer, MAX_LINE_LENGTH);

You shouldn’t have to find '\n', that’s LineReader’s job …?

torcontrol unit and functional tests still pass like this.

fjahr commented at 11:17 pm on January 16, 2026:

The idea here is that we may receive an incomplete line in the stream just because of packets arriving that way and if we would ask the LineReader for it we would receive it but couldn’t do anything with it. That’s why I am looking for the last \n and then only process the buffer up until that point. But this is something that could be done by LineReader, too. So maybe there could be a bool param and then LineReader would only return the lines that end with \n but I didn’t really spend time making up my mind if that is so much better. It looks cleaner here but this functionality would only have one user, so I don’t know… Either way I will wait until LineReader is merged and then I might try to add the functionality there if you think it’s better that way.

There should be a test for this though, currently the tests all send the data nice and complete.

pinheadmz commented at 2:50 pm on January 18, 2026:

Oh right, of course, incomplete lines. I think this applies to HTTP as well, and should probably be a change in #34242 like, if LineReader reaches the end of the buffer before finding a \n it should return null. Right now it treats end of buffer the same as end of line, but I think thats not actually what we want.

fjahr commented at 1:23 pm on January 19, 2026:

I only glossed over the usage of LineReader in the rest of the http PR so far but yeah, that makes sense to me. And that would make my code here simpler :)

pinheadmz commented at 6:03 pm on January 20, 2026:

I’m about to push an update to #34242 that solves this. The patch below fails the partial-send test in feature_torcontrol.py with your current LineReader but passes when I rebase on the updated version:

 0diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
 1index 94f925b8f4..bae7ac44a5 100644
 2--- a/src/torcontrol.cpp
 3+++ b/src/torcontrol.cpp
 4@@ -188,12 +188,8 @@ bool TorControlConnection::ReceiveAndProcess()
 5 
 6 bool TorControlConnection::ProcessBuffer()
 7 {
 8-    // TODO: Maybe could give this as an option to LineReader instead
 9-    auto last_newline = std::find(m_recv_buffer.rbegin(), m_recv_buffer.rend(), std::byte{'\n'});
10-    if (last_newline == m_recv_buffer.rend()) return true;
11-    size_t complete = last_newline.base() - m_recv_buffer.begin();
12-    util::LineReader reader({m_recv_buffer.data(), complete}, MAX_LINE_LENGTH);
13-
14+    util::LineReader reader(m_recv_buffer, MAX_LINE_LENGTH);
15+    auto start = reader.it;
16     while (auto line = reader.ReadLine()) {
17         // Skip short lines
18         if (line->size() < 4) continue;
19@@ -220,7 +216,7 @@ bool TorControlConnection::ProcessBuffer()
20         }
21     }
22 
23-    m_recv_buffer.erase(m_recv_buffer.begin(), m_recv_buffer.begin() + complete);
24+    m_recv_buffer.erase(m_recv_buffer.begin(), m_recv_buffer.begin() + std::distance(start, reader.it));
25     return true;
26 }
27

fjahr commented at 1:32 pm on January 21, 2026:

Cool, thank you! I will wait until the latest conversations have been resolved before rebasing, hoping everything in #34242 is clear and it can be merged soon :)

fjahr commented at 9:23 pm on January 23, 2026:

Taken now!

pinheadmz approved

pinheadmz commented at 8:24 pm on January 15, 2026: member

ACK 3de90a1159e606585aad32c876b6769d29f46ac1

Reviewed each commit, built and ran tests on arm64/macos.

Ran on mainnet with -onlynet=onion and observed tor connections in and out. Monitored bitcoind logs with -debug=tor and tor daemon logs for errors.

I also used wireshark to capture torcontrol packets on port 9051 to compare this branch against release 30.2 and observed identical behavior 👍

Checked the exponential retry backoff by killing the tor daemon process while bitcoind was running. Restarted tor after a few minutes and bitcoind continued as expected.

This is a very nice simplification of torcontrol and also demonstrates how we can replace libevent in bitcoin-cli as well ;-).

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA256
 2
 3ACK 3de90a1159e606585aad32c876b6769d29f46ac1
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmlpS64ACgkQ5+KYS2KJ
 7yTrIVhAAg03Hk1Hu/lEhqpcr1M1aTaxsyNUha2MOtj8L9JMkxYaylX2LlHDzDACq
 8g23L43GAECJtaMfSWH500Id9JqtWiNhn2tUhXLi9UEVDsbFsVf0usxDyF3kx2fec
 9CalOFldTrIV8gFOhmS650lyxt0HfyePQ8yTZBH9jeXLrWoyjOU77cjYGzPC3/wUW
10gsVqLFFY5FOVzB789abu0ufsv3J6LrwAqwwxhsJpJ7IKhfrR0orX5IOMD4r0JMD6
117lkU+4es68yyHUHnlZxnlhzZMMjaqxgA9/dglNivdP3glUqiEQ+dGVFtjZ0bepff
12gHjtm90wbFn59b+kPsTsiFVrYcBe6VBASljjx9QVxgPF8YW3rY64BIXjtOTNIduV
13enaiZgLDqkj8hq6mNSSjcbsnk2uDh+dHUTYNLnOaTjzilRXqiwf1SxFqnSnxDQ6f
14BESyS36JvznTXCctkaqMQ5KTFcjAOigLx+sPgaWrM0P0iioSvWaAXK1HICfWCCIZ
15RG1LS4b4fIIgdIhDjBgL+Mnfz7SoR3U0vvF1XUr6sN5wJVIpDhkzSOB5I9cIMS13
16NdbZ9idWLT4JW5sjXg3KZgVV6qADN3fVRvA0xww5B6halGD04qmO+R35nAifkm9h
17CYBuURa2xrSWsVTApWwQAM8O901vZicDq09jJqOflkgHzx6soRQ=
18=0WeV
19-----END PGP SIGNATURE-----

pinheadmz’s public key is on openpgp.org

 0Bitcoin Core client v30.99.0-3de90a1159e6 - server 70016/Satoshi:30.99.0/ - services wl2
 1
 2<->   type   net   serv  v  mping   ping send recv  txn  blk  hb addrp addrl  age id address                                                             version
 3 in        onion    wl2  2   1325   1325   46   46                   1          0  2 127.0.0.1:59473                                                     70016/Satoshi:30.0.0/
 4out   full onion  nwcl2  2    931    931    4    0         0      1000          1  0 wffaznxih3nah5tbjpiifbn5v3fzmr5tqzcinacr3cjtnags45owvaqd.onion:8333 70016/Satoshi:30.0.0/
 5out   full onion  nbwl2  2    934    934    4    0         0      1001          0  1 uathi7edrzgqsby7dikuqe7ehwpwjem2cy4mpsbahrsrtchjdymufxyd.onion:8333 70016/Satoshi:29.0.0/
 6out   full onion    nwl  1   2810   2810   11    0                1000          0  3 t423yolpdj5udh2yyemopmjtenmmwtfx3rxcdtneda5g2rw6isnjo7id.onion:8333 70016/Satoshi:25.0.0/
 7                               ms     ms  sec  sec  min  min                  min
 8
 9        onion   total   block
10in          1       1
11out         3       3       0
12total       4       4
13
14Local addresses
15zai7kzajktbkqom3edw45zyxnn6e5z5t45d2m7dhmhilgpkz2l3frmad.onion     port   8333    score      4

DrahtBot requested review from janb84 on Jan 15, 2026

DrahtBot requested review from pablomartin4btc on Jan 15, 2026

DrahtBot requested review from sedited on Jan 15, 2026

fjahr force-pushed on Jan 17, 2026

DrahtBot added the label CI failed on Jan 17, 2026

fjahr commented at 2:40 pm on January 17, 2026: contributor

Addressed feedback from @pinheadmz , thanks for the review!

EDIT: And also, I had actually lost sight of bitcoin-cli, I will look at that next to see what, if any, could be reusable from here to apply there.

DrahtBot removed the label CI failed on Jan 17, 2026

DrahtBot added the label Needs rebase on Jan 21, 2026

fjahr force-pushed on Jan 21, 2026

fjahr commented at 5:02 pm on January 21, 2026: contributor

Rebased after https://github.com/bitcoin/bitcoin/pull/34353

DrahtBot added the label CI failed on Jan 21, 2026

DrahtBot removed the label Needs rebase on Jan 21, 2026

fjahr commented at 10:30 pm on January 21, 2026: contributor

CI failure seems unrelated but I am waiting for #34242 anyway, hoping that makes it in soon.

sedited referenced this in commit 0871e104a2 on Jan 23, 2026

DrahtBot added the label Needs rebase on Jan 23, 2026

fjahr force-pushed on Jan 23, 2026

fjahr commented at 9:19 pm on January 23, 2026: contributor

Rebased since #34242 was merged.

fjahr force-pushed on Jan 23, 2026

DrahtBot removed the label Needs rebase on Jan 23, 2026

in src/torcontrol.h:146 in 1545c062a5

153-    CService service;
154+    TorControlConnection m_conn;
155+    std::string m_private_key;
156+    std::string m_service_id;
157+    bool m_reconnect;
158+    float m_reconnect_timeout;

pinheadmz commented at 2:59 pm on January 27, 2026:

4004ce5fd0bf249e2912ac7535218bef93907834

I thought it was funny at first to store a number of seconds for a timeout in a float, but I see it’s multiplied by 1.5 as an exponential backoff.

fjahr commented at 4:39 pm on January 30, 2026:

Hm, yeah, I didn’t really think much about it. I thought about this and seems to me like using std::chrono here is the nicer way of doing this, so I made that change.

in src/torcontrol.cpp:158 in 1545c062a5

168+    if (!m_sock->Wait(timeout, Sock::RECV, &event)) {
169+        return false;
170+    }
171+    if (event & Sock::ERR) {
172+        return false;
173+    }

pinheadmz commented at 3:06 pm on January 27, 2026:

dea33b173e227656a2368e21bc8b24a51a881700

You don’t wanna disconnect if there’s a socket error?

fjahr commented at 4:39 pm on January 30, 2026:

We should disconnect here if WaitForData returns false. However, when looking at it again, I think there might be some potential race if the socket is still connected somehow and calling Disconnect here shouldn’t hurt, so I added that.

in src/torcontrol.h:97 in 1545c062a5

92+    bool WaitForData(std::chrono::milliseconds timeout) const;
93+
94+    /**
95+     * Read available data from socket and process complete replies.
96+     * Dispatches to registered reply handlers.
97+     * @return true on success, false on connection error

pinheadmz commented at 3:13 pm on January 27, 2026:

dea33b173e227656a2368e21bc8b24a51a881700

Also returns false at “at end of stream” which I don’t think is totally correct since that will log "Error processing data from Tor control port" from ThreadControl().

fjahr commented at 4:39 pm on January 30, 2026:

(responding also to #34158 (review), marking the other one as resolved)

Yeah, the docs weren’t matching my real intent here. My thinking was that true simply means we are still connected and false that we are not anymore. Logging on the reason that we are not can be handled internally. I made this more consistent so at least it should be consistent now.

I you insist that returning a bool isn’t the right choice here I could use some enum as the return value (OK, ERR, END) to make the code more expressive. Would be fine for me, too.

in src/torcontrol.cpp:183 in 1545c062a5

196+    if (nread == 0) {
197+        LogDebug(BCLog::TOR, "End of stream");
198         return false;
199-    struct evbuffer *buf = bufferevent_get_output(b_conn);
200-    if (!buf)
201+    }

pinheadmz commented at 3:16 pm on January 27, 2026:

dea33b173e227656a2368e21bc8b24a51a881700

related to comment in torcontrol.h, I’m not sure return false is the right thing to do at end of stream.

in src/torcontrol.cpp:119 in 1545c062a5

125+        m_sock.reset();
126+        return false;
127+    }
128+
129+    // Set non-blocking mode on socket
130+    if (!m_sock->SetNonBlocking()) {

pinheadmz commented at 3:33 pm on January 27, 2026:

dea33b173e227656a2368e21bc8b24a51a881700

This would be already set if you used CreateSock() above.

But as long as we’re setting socket options here I wonder if we should also add SO_REUSEADDR (don’t wait to rebind to the socket if there is a restart)

fjahr commented at 4:39 pm on January 30, 2026:

(responding also to #34158 (review), marking the other as resolved)

I looked through that code and it seemed actually that ConnectDirectly() was an even better fit, so I am using that for now, let me know what you think.

in src/torcontrol.cpp:103 in 1545c062a5

 98@@ -166,41 +99,141 @@ bool TorControlConnection::Connect(const std::string& tor_control_center, const
 99         return false;
100     }
101 
102-    // Create a new socket, set up callbacks and enable notification bits
103-    b_conn = bufferevent_socket_new(base, -1, BEV_OPT_CLOSE_ON_FREE);
104-    if (!b_conn) {
105+    // Create a new socket
106+    SOCKET s = socket(control_service->GetSAFamily(), SOCK_STREAM, IPPROTO_TCP);

pinheadmz commented at 3:38 pm on January 27, 2026:

dea33b173e227656a2368e21bc8b24a51a881700

Why not use CreateSock() here?

pinheadmz approved

pinheadmz commented at 3:59 pm on January 27, 2026: member

re-ACK 1545c062a5973d7ec1aaf77d6224c5ad8a1686b2

Changes since last review are rebasing on master to include LineReader (#34242 ), as well as switching std::bind to std::bind_front (#34353) and addressing nits from review.

I dug a bit deeper into the socket code this time around as well and left some questions. Tested successfully again on mainnet and tried various edge cases like stopping and restarting the tor daemon while Bitcoin is running and switching the torcontrol port.

This can be added in a follow-up PR but once torcontrol is rid of libevent, we can also add the option to connect via UNIX sockets, like #27375

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA256
 2
 3ACK 1545c062a5973d7ec1aaf77d6224c5ad8a1686b2
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAml43+sACgkQ5+KYS2KJ
 7yTpOvQ/6A+msBLJZ307NW7aUWuAKMptOPPd0MlPWMtRIgZOEoepMlI4pwHW4mXMn
 8yP6c4ixGhrbt7iwdLVQA/+bU06x+CUC5GZMgImJu6IdsrSBxlUCz/aCOphEGixom
 9bsfF7EeULgX7ZOTgt4OMhgYX3riD5SmKbyAt18KjPqBcPAUkGTu1JprmB6O44yuX
10rp6jcrhfQ7RP6yY1n+9Pje6Oy9/x0oQCU476bosMpJLguyod68aI3oaDB70fO4j7
11rPI3HHhzQzkQ0vQ3KifgZwZje+OCKlm8JxJVMRDIwISWwZW0V3M3ouYK+xgffiTG
12BzcYyEXb13VDQlzWMGaqNE7hmTTa5rOqWUrZXv03QtluHcBXzaa50rHOj2eM491F
13mL2v6wV8lAepsWOowURvs1AsIcL6t3yjb3gmRh9wR4Ny3S+rzxo3TJEc0usS6vYy
14RSVtlz2RDWFf52PbchkfSIJ2c1dg4r5QlWi2lJaITebWh0IF2l+wMHFqBkf/jWmP
15X7q3ZV+RESiVVxHrmoTiDzEgIW09YxKftXeGNs7IHIbKQg7X4x3gZgsk5pGbvqsv
16qX7NxVLWPvzPThRvx/ckPHnZV2/Ee/RhjgIvKLtg30SphUfqjwr+dilF1QxyhUZc
17oLYKNkARab3U+6s8PIk4YPsr/4zsU/fn+COllKejImnlHuh2GgI=
18=kxiy
19-----END PGP SIGNATURE-----

pinheadmz’s public key is on openpgp.org

refactor: Use constexpr in torcontrol where possible 0fa9416e16

refactor: Modernize member variable names in torcontrol 7a3ddf0ecc

refactor: Get rid of unnecessary newlines in logs e2bc2d4932

torcontrol: Remove libevent usage

Replace libevent-based approach with using the Sock class and CThreadInterrupt.

a87e37f377

fuzz: Improve torcontrol fuzz test

Gets rid of the Dummy class and adds coverage of get_socks_cb.

389e95fc43

test: Add simple functional test for torcontrol b37e103374

test: Add test for partial message handling in torcontrol ddc4faafd2

fjahr force-pushed on Jan 30, 2026

DrahtBot removed the label CI failed on Jan 30, 2026

pinheadmz approved

pinheadmz commented at 7:34 pm on February 6, 2026: member

re-ACK ddc4faafd2150aed08a5956638a8781ede234e4e

Changes since last review were all addressing my review feedback, mostly just nits. Small change to chrono::duration for m_reconnect_timeout and a bigger change using ConnectDirectly() which is a very nice clean up.

Tested again on macos/arm64 on main net. The 1.5x backoff reconnection time worked as expected when tor was killed. Inbound and outbound tor connections worked as expected.

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA256
 2
 3ACK ddc4faafd2150aed08a5956638a8781ede234e4e
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmmGPz4ACgkQ5+KYS2KJ
 7yTqwdw//WxV6osT65TFiXT3UElMY8eLXK/j5koJNJw/ZgJLNEDjnq69vIhRcY9Yq
 85I8NqMtQRA4EU/7yLl55RWExjNwXgFIGlP/oVu5/VKmL5+10e8fjztAuCjA6WW8F
 9LIDP/X4Ol3K+uFypcYEyDWRVz129sdLN4B4QC85+18sU7BfZ0nGWfv5FLivhMq2z
10454Z49kf/rV17vBZRXeVU62En0h2sW0vGqiNG00hcviarlKRF5WXN0DQtkmS0oHK
11kfMwoFYe6xtcd1g66YZCRuOWadk00wI/v5+nyitUZdq0b5mhj0GcsQ+t5GbNocOo
12QEQktK2w32joyeJIkmvCHL0+Fmv3uiAFPyHlbQkoRQWCWmQIRJYti3wiNVKXEZ18
13AKWgY5cNiYlNp2DqE5wDI9PV603p7yvcQxvI2J1bzlW2gyInhRIYbhuclZxY8LHC
14iEZ88rrPtH1CN4Sh1xrZUbCpzt3mH9cOLMzKp7UJtgNWVtpKhe9gczUzl1uVKe6h
15FfsRZrZeBwr4G+8HNuQIcbzavG2b+vPIP6kQ3TjUTyMEf/1GpEU6MeumfuSNh0lO
16zZ0PSwC7li7V57gOytChrV7SNHESneM3j9BwAQFInF3GEk4K9AmQpZuik4z4nbnt
17IuUzz7hp5IX7COEcFfWUxZdEQwYKktiq3RuwrEXHsVAPaUm+HTE=
18=6wBj
19-----END PGP SIGNATURE-----

pinheadmz’s public key is on openpgp.org

in src/torcontrol.cpp:121 in a87e37f377

181 {
182-    if (b_conn) {
183-        Disconnect();
184-    }
185+    if (!m_sock) return false;
186+    std::string errmsg;

sedited commented at 1:57 pm on February 8, 2026:

Nit: How about logging this to debug or trace?

in src/torcontrol.cpp:706 in a87e37f377

721-     */
722-    if (!m_conn.Connect(m_tor_control_center, std::bind_front(&TorController::connected_cb, this),
723-         std::bind_front(&TorController::disconnected_cb, this) )) {
724-        LogWarning("tor: Re-initiating connection to Tor control port %s failed", m_tor_control_center);
725-    }
726+    LogDebug(BCLog::TOR, "Not connected to Tor control port %s, will retry", m_tor_control_center);

sedited commented at 5:57 pm on February 8, 2026:

Would a warn-level log line somewhere in the retry logic be useful?

in src/torcontrol.cpp:722 in a87e37f377

751+/**
752+ * TODO: TBD if introducing a global is the preferred approach here since we
753+ * usually try to avoid them. We could let init manage the lifecycle or make
754+ * this a part of NodeContext maybe instead.
755+ */
756+static std::unique_ptr<TorController> g_tor_controller;

sedited commented at 8:58 pm on February 8, 2026:

Seems like moving this to node context is a simple change and gets rid of some boilerplate that is only covered by functional tests:

 0diff --git a/src/init.cpp b/src/init.cpp
 1index 841fdec5b2..07de0c63d3 100644
 2--- a/src/init.cpp
 3+++ b/src/init.cpp
 4@@ -273 +273,3 @@ void Interrupt(NodeContext& node)
 5-    InterruptTorControl();
 6+    if (node.tor_controller) {
 7+        node.tor_controller->Interrupt();
 8+    }
 9@@ -316 +318,4 @@ void Shutdown(NodeContext& node)
10-    StopTorControl();
11+    if (node.tor_controller) {
12+        node.tor_controller->Join();
13+        node.tor_controller.reset();
14+    }
15@@ -2141 +2146 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
16-        StartTorControl(onion_service_target);
17+        node.tor_controller = std::make_unique<TorController>(gArgs.GetArg("-torcontrol", DEFAULT_TOR_CONTROL), onion_service_target);
18diff --git a/src/node/context.cpp b/src/node/context.cpp
19index d7d01b494e..164361601f 100644
20--- a/src/node/context.cpp
21+++ b/src/node/context.cpp
22@@ -19,0 +20 @@
23+#include <torcontrol.h>
24diff --git a/src/node/context.h b/src/node/context.h
25index 3a7488fd25..abe1bc8da2 100644
26--- a/src/node/context.h
27+++ b/src/node/context.h
28@@ -27,0 +28 @@ class PeerManager;
29+class TorController;
30@@ -92,0 +94 @@ struct NodeContext {
31+    std::unique_ptr<TorController> tor_controller;
32diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
33index be17a57488..1dbc037df0 100644
34--- a/src/torcontrol.cpp
35+++ b/src/torcontrol.cpp
36@@ -715,29 +714,0 @@ fs::path TorController::GetPrivateKeyFile()
37-/****** Thread ********/
38-
39-/**
40- * TODO: TBD if introducing a global is the preferred approach here since we
41- * usually try to avoid them. We could let init manage the lifecycle or make
42- * this a part of NodeContext maybe instead.
43- */
44-static std::unique_ptr<TorController> g_tor_controller;
45-
46-void StartTorControl(CService onion_service_target)
47-{
48-    assert(!g_tor_controller);
49-    g_tor_controller = std::make_unique<TorController>(gArgs.GetArg("-torcontrol", DEFAULT_TOR_CONTROL), onion_service_target);
50-}
51-
52-void InterruptTorControl()
53-{
54-    if (!g_tor_controller) return;
55-    LogInfo("tor: Thread interrupt");
56-    g_tor_controller->Interrupt();
57-}
58-
59-void StopTorControl()
60-{
61-    if (!g_tor_controller) return;
62-    g_tor_controller->Join();
63-    g_tor_controller.reset();
64-}
65-
66diff --git a/src/torcontrol.h b/src/torcontrol.h
67index 94561e3f40..ae4902fee0 100644
68--- a/src/torcontrol.h
69+++ b/src/torcontrol.h
70@@ -29,4 +28,0 @@ static const bool DEFAULT_LISTEN_ONION = true;
71-void StartTorControl(CService onion_service_target);
72-void InterruptTorControl();
73-void StopTorControl();
74-

in src/torcontrol.cpp:567 in a87e37f377

563@@ -501,7 +564,7 @@ void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply&
564  *                  CookieString | ClientNonce | ServerNonce)
565  *
566  */
567-static std::vector<uint8_t> ComputeResponse(const std::string &key, const std::vector<uint8_t> &cookie,  const std::vector<uint8_t> &client_nonce, const std::vector<uint8_t> &server_nonce)
568+static std::vector<uint8_t> ComputeResponse(const std::string_view &key, const std::vector<uint8_t> &cookie,  const std::vector<uint8_t> &client_nonce, const std::vector<uint8_t> &server_nonce)

sedited commented at 9:11 pm on February 8, 2026:

Nit: Can pass key by value and in the spirit of this line’s change the vectors could be spans?

in src/test/fuzz/torcontrol.cpp:52 in ddc4faafd2

53+            [&] {
54+                tor_controller.auth_cb(conn, tor_control_reply);
55             },
56             [&] {
57-                tor_controller.auth_cb(dummy_tor_control_connection, tor_control_reply);
58+                tor_controller.authchallenge_cb(conn, tor_control_reply);

sedited commented at 9:29 pm on February 8, 2026:

I’m getting a fuzzer failure that I’m guessing may be caused by a change in this function:

 0[#740](/bitcoin-bitcoin/740/)	NEW    cov: 661 ft: 1014 corp: 25/78b lim: 4 exec/s: 0 rss: 110Mb L: 4/4 MS: 5 ChangeBit-CMP-ChangeByte-ChangeBinInt-ShuffleBytes- DE: "\373\000\000\000"-
 1/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:1148:9: runtime error: reference binding to null pointer of type 'const value_type' (aka 'const std::basic_string<char>')
 2SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:1148:9 
 3AddressSanitizer:DEADLYSIGNAL
 4=================================================================
 5==21501==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x64b69d0d8101 bp 0x7ffe53d6ec10 sp 0x7ffe53d6eb40 T0)
 6==21501==The signal is caused by a READ memory access.
 7==21501==Hint: address points to the zero page.
 8    [#0](/bitcoin-bitcoin/0/) 0x64b69d0d8101 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::size() const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:1072:16
 9    [#1](/bitcoin-bitcoin/1/) 0x64b69d0d8101 in SplitTorReplyLine(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /home/drgrid/bitcoin/src/torcontrol.cpp:230:20
10    [#2](/bitcoin-bitcoin/2/) 0x64b69d0e3b7b in TorController::authchallenge_cb(TorControlConnection&, TorControlReply const&) /home/drgrid/bitcoin/src/torcontrol.cpp:582:48
11    [#3](/bitcoin-bitcoin/3/) 0x64b69b8d6321 in torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_5::operator()() const /home/drgrid/bitcoin/src/test/fuzz/torcontrol.cpp:52:32
12    [#4](/bitcoin-bitcoin/4/) 0x64b69b8d6321 in unsigned long CallOneOf<torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_3, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_4, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_5, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_6, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_7>(FuzzedDataProvider&, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_3, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_4, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_5, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_6, torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_7) /home/drgrid/bitcoin/src/test/fuzz/util.h:42:27
13    [#5](/bitcoin-bitcoin/5/) 0x64b69b8d6321 in torcontrol_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) /home/drgrid/bitcoin/src/test/fuzz/torcontrol.cpp:43:9
14    [#6](/bitcoin-bitcoin/6/) 0x64b69bb79cd5 in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
15    [#7](/bitcoin-bitcoin/7/) 0x64b69bb79cd5 in test_one_input(std::span<unsigned char const, 18446744073709551615ul>) /home/drgrid/bitcoin/src/test/fuzz/fuzz.cpp:88:5
16    [#8](/bitcoin-bitcoin/8/) 0x64b69bb79cd5 in LLVMFuzzerTestOneInput /home/drgrid/bitcoin/src/test/fuzz/fuzz.cpp:216:5
17    [#9](/bitcoin-bitcoin/9/) 0x64b69b115c14 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/drgrid/bitcoin/build_fuzz/bin/fuzz+0x1badc14) (BuildId: e39b175d556cbbeafdd76014c930896f3d6c8f6e)
18    [#10](/bitcoin-bitcoin/10/) 0x64b69b115309 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/drgrid/bitcoin/build_fuzz/bin/fuzz+0x1bad309) (BuildId: e39b175d556cbbeafdd76014c930896f3d6c8f6e)
19    [#11](/bitcoin-bitcoin/11/) 0x64b69b116af5 in fuzzer::Fuzzer::MutateAndTestOne() (/home/drgrid/bitcoin/build_fuzz/bin/fuzz+0x1baeaf5) (BuildId: e39b175d556cbbeafdd76014c930896f3d6c8f6e)
20    [#12](/bitcoin-bitcoin/12/) 0x64b69b117655 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/drgrid/bitcoin/build_fuzz/bin/fuzz+0x1baf655) (BuildId: e39b175d556cbbeafdd76014c930896f3d6c8f6e)
21    [#13](/bitcoin-bitcoin/13/) 0x64b69b10492f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/drgrid/bitcoin/build_fuzz/bin/fuzz+0x1b9c92f) (BuildId: e39b175d556cbbeafdd76014c930896f3d6c8f6e)
22    [#14](/bitcoin-bitcoin/14/) 0x64b69b12efb6 in main (/home/drgrid/bitcoin/build_fuzz/bin/fuzz+0x1bc6fb6) (BuildId: e39b175d556cbbeafdd76014c930896f3d6c8f6e)
23    [#15](/bitcoin-bitcoin/15/) 0x70df0082a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
24    [#16](/bitcoin-bitcoin/16/) 0x70df0082a28a in __libc_start_main csu/../csu/libc-start.c:360:3
25    [#17](/bitcoin-bitcoin/17/) 0x64b69b0f9914 in _start (/home/drgrid/bitcoin/build_fuzz/bin/fuzz+0x1b91914) (BuildId: e39b175d556cbbeafdd76014c930896f3d6c8f6e)
26
27AddressSanitizer can not provide additional info.
28SUMMARY: AddressSanitizer: SEGV /home/drgrid/bitcoin/src/torcontrol.cpp:230:20 in SplitTorReplyLine(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)
29==21501==ABORTING
30MS: 1 ChangeASCIIInt-; base unit: 66ab8fe3159190c0fece3131125619ef7349a19d
310x84,0x33,0xdb,0x67,
32\2043\333g
33artifact_prefix='./'; Test unit written to ./crash-01d7dad05a5562f6797a2b566c99fb79187bc471
34Base64: hDPbZw==

I am able to reproduce the crash locally with:

0cat crash-01d7dad05a5562f6797a2b566c99fb79187bc471 | base64                                                                         
1hDPbZw==

DrahtBot requested review from sedited on Feb 8, 2026

torcontrol: Remove libevent usage #34158

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)