cli: Replace libevent usage with simple http client

fjahr commented at 1:27 pm on January 19, 2026: contributor

Part of the effort to remove the libevent dependency altogether, see #31194

This takes the Read and FindFirst functions from the HTTPHeaders class from #32061 and puts them into bitcoin-cli as free functions in a separate namespace with a TODO comment to revisit potentially sharing this code somehow. I played with picking the necessary commits and moving the HTTPHeaders class to common or something similar but I am not loving it so I am going with this route for now since it also decouples the two PRs.

Otherwise the change itself replaces the libevent-based HTTP client with a simple synchronous implementation which uses the Sock class directly.

DrahtBot added the label Scripts and tools on Jan 19, 2026

DrahtBot commented at 1:28 pm on January 19, 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/34342.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	pinheadmz, bradleystachurski, w0xlt

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

Conflicts

No conflicts as of last run.

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

sock.Recv(recv_buf, sizeof(recv_buf), /*flags=*/0) in src/bitcoin-cli.cpp
sock.Recv(recv_buf, sizeof(recv_buf), /*flags=*/0) in src/bitcoin-cli.cpp
sock.Recv(recv_buf, sizeof(recv_buf), /*flags=*/0) in src/bitcoin-cli.cpp

2026-04-11 20:35:27

DrahtBot added the label CI failed on Jan 19, 2026

DrahtBot commented at 3:30 pm on January 19, 2026: contributor

🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/21139253857/job/60789761512 LLM reason (✨ experimental): Clang-Tidy failure: modernize-use-starts-ends-with flags substr usage in bitcoin-cli.cpp, treated as error and causing 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.

fjahr force-pushed on Jan 19, 2026

fjahr force-pushed on Jan 20, 2026

DrahtBot removed the label CI failed on Jan 20, 2026

DrahtBot added the label Needs rebase on Jan 23, 2026

fjahr force-pushed on Jan 23, 2026

DrahtBot removed the label Needs rebase on Jan 23, 2026

fjahr force-pushed on Jan 23, 2026

fjahr marked this as ready for review on Jan 23, 2026

pinheadmz commented at 0:13 am on January 24, 2026: member

concept ACK Nice moves, yo! 🔥

bradleystachurski commented at 8:23 pm on February 6, 2026: none

Concept ACK (and supportive of the broader libevent removal effort)

Reviewed efd74a822d9577c53259b3d27526957b0674f7d8

Found a minor behavioral regression: connection failures (e.g. unresolvable host, non-listening port) lose the help text shown on master.

Master:

0error: timeout on transient error: Could not connect to the server 127.0.0.1:19999
1Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
2Use "bitcoin-cli -help" for more info.

Branch:

0error: timeout on transient error: Could not connect to the server 127.0.0.1:19999

Catching instead of re-throwing preserves the help text:

 0diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
 1index 8414ad0317..3cd6e4b29c 100644
 2--- a/src/bitcoin-cli.cpp
 3+++ b/src/bitcoin-cli.cpp
 4@@ -1258,7 +1258,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co
 5     try {
 6         response = client.Post(endpoint, headers, strRequest);
 7     } catch (const CConnectionFailed&) {
 8-        throw;
 9+        response.status = 0;
10     }
11
12     if (response.status == 0) {

Verified this preserves the help text and passes interface_bitcoin_cli.py.

fjahr force-pushed on Feb 8, 2026

fjahr commented at 8:10 pm on February 8, 2026: contributor

@bradleystachurski

Found a minor behavioral regression: connection failures (e.g. unresolvable host, non-listening port) lose the help text shown on master.

You were right, good catch! Your suggested fix looks good to me as well, I have just applied that change.

in src/common/http.h:31 in 58d7fe0777

26+     */
27+    bool Read(util::LineReader& reader);
28+    std::string Stringify() const;
29+
30+private:
31+    std::unordered_map<std::string, std::string, util::AsciiCaseInsensitiveHash, util::AsciiCaseInsensitiveKeyEqual> m_map;

fanquake commented at 3:03 pm on February 25, 2026:

Note this is making use of code being removed in #32061 (https://github.com/bitcoin/bitcoin/pull/32061/changes/c24f5d3c3c879f7a43667866a92fa5c05b955dde).

pinheadmz commented at 3:24 pm on February 25, 2026:

I think there’s a few things we learned in #34158 as well that can be applied. This PR (bitcoin-cli) can maybe be draft until torcontol is ironed out.

fjahr commented at 10:07 pm on March 6, 2026:

It is already draft but thanks for the hints, I will address these once we have made some more progress on torcontol and the server.

fanquake marked this as a draft on Feb 25, 2026

w0xlt commented at 5:32 pm on March 10, 2026: contributor

Concept ACK

DrahtBot added the label Needs rebase on Mar 31, 2026

fjahr force-pushed on Apr 5, 2026

fjahr commented at 8:07 pm on April 5, 2026: contributor

Still very much draft mode but I rebased and resolved some of the conflicts with the latest changes in bitcoin-cli and the http server PR. I can drop the cherry-picked commits once #34905 is merged.

DrahtBot removed the label Needs rebase on Apr 5, 2026

DrahtBot added the label Needs rebase on Apr 9, 2026

fjahr force-pushed on Apr 9, 2026

fjahr commented at 1:38 pm on April 9, 2026: contributor

Rebased since most of the prerequisits have been merged with #34905. I also went through the code again and made several improvements, reducing the LoC count quite a bit. I was also never really sure whether I liked the idea of ripping the HTTPHeaders class out of the rest of the http code and moving it into common, when I really only need less than half of the functionality here. Instead have now taken the Read and FindFirst function that I needed and moved them into bitcoin-cli.cpp in a separate namespace with minor edits to be able to make them free functions. There is a big, fat TODO comment at the top to revisit sharing this code when #32061 is merged but for now I am at least sure that I don’t dislike the idea as much as the move to common, at least looking at it in isolation for now. The nice side effect is that this makes this PR now reviewable and possible to merge without having to wait for #32061 necessarily. Thus taking it out of draft status.

fjahr marked this as ready for review on Apr 9, 2026

DrahtBot removed the label Needs rebase on Apr 9, 2026

in src/bitcoin-cli.cpp:1259 in ca1ac2aeb7

1277+    std::chrono::seconds timeout_duration;
1278+    if (timeout > 0) {
1279+        timeout_duration = std::chrono::seconds(timeout);
1280+    } else {
1281+        // Use 5 year timeout for "indefinite"
1282+        timeout_duration = std::chrono::seconds(5 * 365 * 24 * 60 * 60);

maflcko commented at 1:59 pm on April 9, 2026:

nit: No need to do those manually. You can use std::chrono::years (since C++20), which is slightly larger: 365.2425 days (31556952s)

fjahr commented at 7:27 pm on April 9, 2026:

Nice, done, thanks!

DrahtBot added the label CI failed on Apr 9, 2026

DrahtBot commented at 2:20 pm on April 9, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/24192925766/job/70614859784 LLM reason (✨ experimental): CI failed due to a build compilation error while compiling bitcoin-cli.cpp (target bitcoin-cli, Error 1 from gmake).

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.

maflcko commented at 2:28 pm on April 9, 2026: member

If you want to fix the CI failure, you may adjust the CI script to exclude that warning as well:

0.github/ci-test-each-commit-exec.py:44:        "-DCMAKE_CXX_FLAGS=-Wno-error=unused-member-function",

Or you can adjust your code, up to you.

fjahr force-pushed on Apr 9, 2026

fjahr commented at 7:30 pm on April 9, 2026: contributor

If you want to fix the CI failure, you may adjust the CI script to exclude that warning as well:

Yeah, I saw the error and thought that check doesn’t make sense in this context but I didn’t have time to find the right place and address it yet. I added the exception for free functions.

DrahtBot removed the label CI failed on Apr 9, 2026

in src/bitcoin-cli.cpp:83 in 6dbb51040f

78+static constexpr size_t MAX_BODY_SIZE = 32 * 1024 * 1024;
79+
80+// TODO: These helpers may be replaced with the corresponding methods in HTTPHeaders
81+// from https://github.com/bitcoin/bitcoin/pull/32061 if that is moved to a shared
82+// location.
83+namespace http_bitcoin {

hodlinator commented at 8:26 pm on April 9, 2026:

nit: Seems like we should be going from local (bitcoin) to general (http):

0namespace bitcoin_http {

fjahr commented at 8:35 pm on April 11, 2026:

To be honest, I just copied the name of the temp namespace in #32061 since am not sure this one will stay around as well. But who knows, maybe it will stick around, so I renamed it.

in src/bitcoin-cli.cpp:841 in 6dbb51040f

834@@ -829,6 +835,376 @@ static std::optional<UniValue> CallIPC(BaseRequestHandler* rh, const std::string
835     return rh->ProcessReply(reply);
836 }
837 
838+/**
839+ * Simple synchronous HTTP client using Sock class.
840+ */
841+class HttpClient

hodlinator commented at 8:35 pm on April 9, 2026:

I find it confusing to have two classes with the same names between here and #32061 (okay, they might be in different contexts and the one in the other PR is cased as HTTPClient).

Most of the code style I’ve seen uses UPPERCASE for acronyms so I suggest aligning with #32061.
#32061’s client is the server’s perspective of the client, while this is the client’s own, “actor”. It seems to me like the CLI has a stronger claim and #32061 should rename to HTTPRemoteClient to resolve the conflict. cc @pinheadmz

fjahr commented at 8:35 pm on April 11, 2026:

Sure, renamed to HTTPClient. Happy to discuss another name change if you disagree with the suggestion above @pinheadmz

in src/common/url.cpp:41 in 6dbb51040f

36@@ -37,3 +37,25 @@ std::string UrlDecode(std::string_view url_encoded)
37 
38     return res;
39 }
40+
41+std::string UrlEncode(const std::string& str)

hodlinator commented at 9:01 pm on April 9, 2026:

Should avoid requiring a string to be constructed for the input param (also mirrors the decode function):

0std::string UrlEncode(std::string_view str)

hodlinator commented at 9:02 pm on April 9, 2026:

Possible test:

 0--- a/src/test/common_url_tests.cpp
 1+++ b/src/test/common_url_tests.cpp
 2@@ -5,6 +5,7 @@
 3 #include <common/url.h>
 4 
 5 #include <string>
 6+#include <string_view>
 7 
 8 #include <boost/test/unit_test.hpp>
 9 
10@@ -72,4 +73,41 @@ BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) {
11     BOOST_CHECK_EQUAL(UrlDecode("abc%00%00"), result2);
12 }
13 
14+BOOST_AUTO_TEST_CASE(url_encode) {
15+    BOOST_CHECK_EQUAL(UrlEncode(std::string_view{
16+        "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
17+        "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f"
18+        "\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f"
19+        "\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f"
20+        "\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f"
21+        "\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f"
22+        "\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f"
23+        "\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f"
24+        "\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f"
25+        "\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f"
26+        "\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf"
27+        "\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf"
28+        "\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf"
29+        "\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf"
30+        "\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef"
31+        "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff",
32+        256}),
33+        "%00%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F"
34+        "%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F"
35+        "%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F"
36+        "0123456789%3A%3B%3C%3D%3E%3F"
37+        "%40ABCDEFGHIJKLMNO"
38+        "PQRSTUVWXYZ%5B%5C%5D%5E_"
39+        "%60abcdefghijklmno"
40+        "pqrstuvwxyz%7B%7C%7D~%7F"
41+        "%80%81%82%83%84%85%86%87%88%89%8A%8B%8C%8D%8E%8F"
42+        "%90%91%92%93%94%95%96%97%98%99%9A%9B%9C%9D%9E%9F"
43+        "%A0%A1%A2%A3%A4%A5%A6%A7%A8%A9%AA%AB%AC%AD%AE%AF"
44+        "%B0%B1%B2%B3%B4%B5%B6%B7%B8%B9%BA%BB%BC%BD%BE%BF"
45+        "%C0%C1%C2%C3%C4%C5%C6%C7%C8%C9%CA%CB%CC%CD%CE%CF"
46+        "%D0%D1%D2%D3%D4%D5%D6%D7%D8%D9%DA%DB%DC%DD%DE%DF"
47+        "%E0%E1%E2%E3%E4%E5%E6%E7%E8%E9%EA%EB%EC%ED%EE%EF"
48+        "%F0%F1%F2%F3%F4%F5%F6%F7%F8%F9%FA%FB%FC%FD%FE%FF");
49+}
50+
51 BOOST_AUTO_TEST_SUITE_END()

fjahr commented at 8:35 pm on April 11, 2026:

Oh, nice, thanks. Weird, I feel like I had written a small unit test at some point but I guess it got lost and it certainly wasn’t that exhaustive anyway.

fjahr commented at 8:35 pm on April 11, 2026:

Sure, changed.

ci: Tolerate unused free functions in intermediate commits

When bigger changes are split across multiple commits, intermittend commits may introduce unused functions. Do not check for this error in intermittend commits.

567fb60f32

common: Add unused UrlEncode function

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>

2754c92176

cli: Add HTTP header parsing helpers 49e7c3c5b1

cli: Remove libevent usage fdf907d8cd

fjahr force-pushed on Apr 11, 2026

fjahr commented at 8:36 pm on April 11, 2026: contributor

Addressed feedback from @hodlinator , thanks a lot for the review!

cli: Replace libevent usage with simple http client #34342

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)