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
Concept ACK	sedited, pablomartin4btc, janb84, waketraindev
Stale ACK	pinheadmz

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:

#31260 (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.

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):

socket(control_service->GetSAFamily(), SOCK_STREAM, IPPROTO_TCP) in src/torcontrol.cpp
m_sock->Recv(buf, sizeof(buf), MSG_DONTWAIT) in src/torcontrol.cpp

2026-01-23 21:23:48

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:195 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

refactor: Use constexpr in torcontrol where possible acae95708a

refactor: Modernize member variable names in torcontrol 4004ce5fd0

refactor: Get rid of unnecessary newlines in logs 2cfdf33af5

fjahr force-pushed on Jan 23, 2026

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

Rebased since #34242 was merged.

torcontrol: Remove libevent usage

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

dea33b173e

fuzz: Improve torcontrol fuzz test

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

0e5faaaa0e

test: Add simple functional test for torcontrol 515c1e4d35

test: Add test for partial message handling in torcontrol 1545c062a5

fjahr force-pushed on Jan 23, 2026

DrahtBot removed the label Needs rebase on Jan 23, 2026

torcontrol: Remove libevent usage #34158

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)