Replace libevent with our own HTTP and socket-handling implementation #32061

pinheadmz · 2025-03-13T19:32:14Z

This is a major component of removing libevent as a dependency of the project, by replacing the HTTP server used for RPC and REST with one implemented entirely within the Bitcoin Core codebase. The new HTTPServer class runs its own I/O thread, handling socket connections with code based on #30988, but tailored specifically for HTTP.

Some functional tests were added in #32408 to cover libevent behaviors that remain consistent with this branch.

Commit strategy:

Implement a few helper functions for strings, timestamps, etc needed by HTTP protocol
Isolate the existing libevent-based HTTP server in a namespace http_libevent
Implement HTTP in a new namespace http_bitcoin (classes like HTTPRequest, HTTPClient, etc...)
Switch bitcoind from the libevent server to the new server
Clean up (delete http_libevent)

I am currently seeing about a 10% speed up in the functional tests on my own arm/macos machine.

Integration testing:

I am testing the new HTTP server by forking projects that integrate with bitcoin via HTTP and running their integration tests with bitcoind built from this branch (on Github actions). I will continue adding integrations over time, and re-running these CI tests as this branch gets rebased:

DrahtBot · 2025-03-13T19:32:18Z

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/32061.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	laanwj, fjahr, w0xlt, furszy
Stale ACK	romanz, vasild

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34158 (torcontrol: Remove libevent usage by fjahr)
#33689 (http: replace WorkQueue and single threads handling for ThreadPool by furszy)
#32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)
#31929 (http: Make server shutdown more robust by hodlinator)
#31672 (rpc: add cpu_load to getpeerinfo by vasild)
#25665 (refactor: Add util::Result failure types and ability to merge result values 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 typos and grammar issues:

Unrecoverbale -> Unrecoverable [spelling error in comment making the English word misspelled]

^2026-01-09

DrahtBot · 2025-03-13T20:29:34Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/38735177073}

Hints

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.

laanwj · 2025-03-14T12:52:30Z

Concept ACK, nice work

vasild · 2025-03-14T13:57:43Z

Concept ACK

fjahr · 2025-03-14T20:29:05Z

Concept ACK

My understanding from the low-level networking discussion at CoreDev was that this wouldn't build on top of sockman. I guess the devil is in the details but can you address that in what sense the current approach follows what was discussed there? Thanks!

pinheadmz · 2025-03-15T00:03:45Z

My understanding from the low-level networking discussion at CoreDev was that this wouldn't build on top of sockman. I guess the devil is in the details but can you address that in what sense the current approach follows what was discussed there? Thanks!

Sure, by coredev I had already written most of this implementation (based on sockman) but the performance was bad, and that was part of the motivation behind the deep-dive talk. However, by the end of the week I had reviewed that code in person with smart attendees and not only improved the performance of my code but started to improve performance vs master branch as well! Those updates came in the days just after the deep-dive discussion.

SOME kind of sockman is needed to replace libevent. The one @vasild wrote does actually seem to work well for this purpose as well as for p2p, and it would be "nice" to only have to maintain one I/O loop structure in bitcoind. @theuni is investigating how a sockman for http could be optimized if it had no other purpose, and I think that is the kind of feedback that will help us decide which path to take.

vasild · 2025-03-19T12:22:15Z

SOME kind of sockman is needed to replace libevent ... it would be "nice" to only have to maintain one I/O loop structure in bitcoind.

💯

pinheadmz

Push to 63a9487:

Address review feedback from @fjahr on first 6 commits, changes included renaming functions and rewording commit messages for better clarity. I plan on opening a new PR with just these 6 commits shortly, some of that code can also be consumed by #34158

pinheadmz · 2026-01-07T18:32:21Z

src/netbase.h

 CService MaybeFlipIPv6toCJDNS(const CService& service);

+/** Get the bind address for a socket as CService. */
+CService GetBindAddress(const Sock& sock);


👍 re-worded the commit message

pinheadmz · 2026-01-07T18:34:25Z

src/netbase.cpp

+    sockaddr_storage storage;
+    socklen_t len = sizeof(storage);
+
+    auto sa = static_cast<sockaddr*>(static_cast<void*>(&storage));


Ah thanks, fixed this commit message

pinheadmz · 2026-01-07T18:36:04Z

src/netbase.cpp

+
+    auto sa = static_cast<sockaddr*>(static_cast<void*>(&storage));
+
+    if (!sock.GetSockName(sa, &len)) {


sure, taken. every little bit of legibility helps ;-)

pinheadmz · 2026-01-07T18:38:03Z

src/util/strencodings.h

 */
 template <typename T>
-std::optional<T> ToIntegral(std::string_view str)
+std::optional<T> ToIntegral(std::string_view str, size_t base = 10)


I think I will leave as-is, a bool would require explanation and this way we have explicit argument name and default value.

pinheadmz · 2026-01-07T18:46:11Z

src/util/strencodings.h

+        lowered.resize(s.size());
+        std::ranges::transform(s, lowered.begin(),
+            [](char c) {
+                // Avoid implicit-integer-sign-change UB by only


Ok taken. I was doubtful at first but learned about "case folding" in unicode so, the qualification is justified.

pinheadmz · 2026-01-07T18:50:06Z

src/util/strencodings.h

 };
 } // namespace detail

+struct CaseInsensitiveKeyEqual {


The important bit is the case-insensitivity of the HTTP protocol. I reference libevent functions in comments and commit messages throughout the PR because I referenced them when writing and also for reviewers to compare. I'll reword this commit message to better explain.

pinheadmz · 2026-01-07T19:47:28Z

src/util/time.h

 std::optional<int64_t> ParseISO8601DateTime(std::string_view str);

+/**
+ * RFC7231 formatting https://www.rfc-editor.org/rfc/rfc7231#section-7.1.1.1


Replaced the reference to HTTP spec with reference to the actual date/time spec implemented in the function.

pinheadmz · 2026-01-07T19:59:10Z

src/util/time.h

+ * RFC7231 formatting https://www.rfc-editor.org/rfc/rfc7231#section-7.1.1.1
+ * Used in HTTP/1.1 responses
+ */
+std::string FormatRFC7231DateTime(int64_t nTime);


Great catch, I had to dig through the forest of RFCs again, and rewrote the commit message and function name. I'm going with RFC1123 because that is the RFC referenced by libevent. I also wanted to keep the pattern of naming similar to neighbor functions like FormatISO8601DateTime()

pinheadmz · 2026-01-07T20:59:28Z

src/util/string.cpp

+        ++it;
+        ++count;
+        if (c == '\n') break;
+        if (count >= max_read) throw std::runtime_error("max_read exceeded by LineReader");


I think everything is correct as-is, but I refactored the tests in order to make sure I hit the edge correctly. I still could be missing an off-by-one though, so I appreciate your patience and re-review.

If max_read is 8, I want to read no more than 8 characters looking for \n. So 123567\n is fine, meaning that indeed we read up to AND including max_read.

However 12345678\n would throw an error because we would consume max_read characters and not find an EOL. In this case the EOL is "past max_read".

fjahr

nit-picking a bit more on LineReader edge cases :) Looking forward to the separate PR!

fjahr · 2026-01-08T13:15:49Z

src/test/util_string_tests.cpp

+        // but then throws because there is no EOL within max_read
+        LineReader reader2(input, /*max_read=*/23);
+        BOOST_CHECK_EQUAL(reader2.ReadLine(), "once upon a time there");
+        BOOST_CHECK_THROW(reader2.ReadLine(), std::runtime_error);


EOL is at position equal to max_read

This is a 23-char line so for me EOL is at max_read+1 again, same to what is said above in the 22-char line case:

EOL is +1 past max_read

Also, another edge case is a string without any \n or a last line without it, e.g. instead of EOL there is EOF. If I understand correctly, because of how the loop is structured, a string without \n and length 10 will throw the error if max_read=10 but a string with length 9 without a \n will not throw at all and just return the string. I would expect only a line of 11+ without \n to throw.

Maybe this ordering of the lines in the loop would do a bit better for this edge case? I didn't test it yet so it's possible there are other issues with it, feel free to ignore if that's the case.

while (it != end) { if (count >= max_read) throw std::runtime_error("max_read exceeded by LineReader"); ++count; auto c = static_cast<char>(*it); if (c == '\n') break; ++it; }

You're right and I think your original instinct on this was right as well. I modified the logic of the ReadLine() loop, added more comments, and covered the end-of-buffer case with another unit test. So it should be more clear: max_read is the length of the longest line we will return, and the line can be terminated either by \n (not counted) or by the end of the buffer.

max_read = 8

"12345678" OK
"12345678\n" OK
"123456789" bad
"123456789\n" bad

fjahr · 2026-01-08T13:28:20Z

src/util/string.cpp

+        ++it;
+        ++count;
+        if (c == '\n') break;
+        if (count >= max_read) throw std::runtime_error("max_read exceeded by LineReader");


Hm, yeah, I think the behavior is fine after looking at this again. I guess the first thing that made this a bit hard to parse is the ordering of the lines but I can see now that this is probably the most efficient way to do it. It may just not be the most readable and I needed some extra time to stare at it :)

I will add some nits at the appropriate lines.

fjahr · 2026-01-08T14:09:28Z

src/util/string.h

+    /**
+     * Returns a string from current iterator position up to next \n
+     * and advances iterator, does not return trailing \n or \r.
+     * Will not search for \n past max_read.


It sounds a bit like this might just return: "Will throw if a line length exceeds max_read."

Good point, updated

This is a helper struct to parse HTTP messages from data in buffers from sockets. HTTP messages begin with headers which are CRLF-terminated lines (\n or \r\n) followed by an arbitrary amount of body data. Whitespace is trimmed from the field lines but not the body. https://httpwg.org/specs/rfc9110.html#rfc.section.5

This commit is a no-op to isolate HTTP methods and objects that depend on libevent. Following commits will add replacement objects and methods in a new namespace for testing and review before switching over the server.

see: https://www.rfc-editor.org/rfc/rfc2616#section-4.2 https://www.rfc-editor.org/rfc/rfc7231#section-5 https://www.rfc-editor.org/rfc/rfc7231#section-7 https://httpwg.org/specs/rfc9111.html#header.field.definitions

HTTP Response message: https://datatracker.ietf.org/doc/html/rfc1945#section-6 Status line (first line of response): https://datatracker.ietf.org/doc/html/rfc1945#section-6.1 Status code definitions: https://datatracker.ietf.org/doc/html/rfc1945#section-9

HTTP Request message: https://datatracker.ietf.org/doc/html/rfc1945#section-5 Request Line aka Control Line aka first line: https://datatracker.ietf.org/doc/html/rfc1945#section-5.1 See message_read_status() in libevent http.c for how `MORE_DATA_EXPECTED` is handled there

…ocket Introduce a new low-level socket managing class `HTTPServer`. BindAndStartListening() was copied from CConnMan's BindListenPort() in net.cpp and modernized. Unit-test it with a new class `SocketTestingSetup` which mocks `CreateSock()` and will enable mock client I/O in future commits. Co-authored-by: Vasil Dimov <[email protected]>

AcceptConnection() is mostly copied from CConmann in net.cpp and then modernized. Co-authored-by: Vasil Dimov <[email protected]>

Co-authored-by: Vasil Dimov <[email protected]>

Socket handling methods are copied from CConnMan: `CConnman::GenerateWaitSockets()` `CConnman::SocketHandlerListening()` `CConnman::ThreadSocketHandler()` and `CConnman::SocketHandler()` are combined into ThreadSocketHandler()`. Co-authored-by: Vasil Dimov <[email protected]>

`SocketHandlerConnected()` adapted from CConnman Testing this requires adding a new feature to the SocketTestingSetup, inserting a "request" payload into the mock client that connects to us. Co-authored-by: Vasil Dimov <[email protected]>

Sockets-touching bits copied and adapted from `CConnman::SocketSendData()` Testing this requires adding a new feature to the SocketTestingSetup, returning the DynSock I/O pipes from the mock socket so the received data can be checked. Co-authored-by: Vasil Dimov <[email protected]>

See https://www.rfc-editor.org/rfc/rfc7230#section-6.3.2 > A server MAY process a sequence of pipelined requests in parallel if they all have safe methods (Section 4.2.1 of [RFC7231]), but it MUST send the corresponding responses in the same order that the requests were received. We choose NOT to process requests in parallel. They are executed in the order recevied as well as responded to in the order received. This prevents race conditions where old state may get sent in response to requests that are very quick to process but were requested later on in the queue.

This is a refactor to prepare for matching the API of HTTPRequest definitions in both namespaces http_bitcoin and http_libevent. In particular, to provide a consistent return type for GetRequestMethod() in both classes.

These methods are called by http_request_cb() and are present in the original http_libevent::HTTPRequest.

The original function is passed to libevent as a callback when HTTP requests are received and processed. It wrapped the libevent request object in a http_libevent::HTTPRequest and then handed that off to bitcoin for basic checks and finally dispatch to worker threads. In this commit we split the function after the http_libevent::HTTPRequest is created, and pass that object to a new function that maintains the logic of checking and dispatching. This will be the merge point for http_libevent and http_bitcoin, where HTTPRequest objects from either namespace have the same downstream lifecycle.

The original function was already naturally split into two chunks: First, we parse and validate the users' RPC configuration for IPs and ports. Next we bind libevent's http server to the appropriate endpoints. This commit splits these chunks into two separate functions, leaving the argument parsing in the common space of the module and moving the libevent-specific binding into the http_libevent namespace. A future commit will implement http_bitcoin::HTTPBindAddresses to bind the validate list of endpoints by the new HTTP server.

pinheadmz

Push to b6afd81:

Address feedback from @fjahr about LineReader(), fix edge-case handling.
At this point I'm going to split off the first 6 commits to a new PR and we can focus review there instead.

The new PR is #34242

pinheadmz · 2026-01-08T15:42:39Z

src/util/string.h

+    /**
+     * Returns a string from current iterator position up to next \n
+     * and advances iterator, does not return trailing \n or \r.
+     * Will not search for \n past max_read.


Good point, updated

pinheadmz · 2026-01-08T15:57:39Z

src/test/util_string_tests.cpp

+        // but then throws because there is no EOL within max_read
+        LineReader reader2(input, /*max_read=*/23);
+        BOOST_CHECK_EQUAL(reader2.ReadLine(), "once upon a time there");
+        BOOST_CHECK_THROW(reader2.ReadLine(), std::runtime_error);


You're right and I think your original instinct on this was right as well. I modified the logic of the ReadLine() loop, added more comments, and covered the end-of-buffer case with another unit test. So it should be more clear: max_read is the length of the longest line we will return, and the line can be terminated either by \n (not counted) or by the end of the buffer.

max_read = 8

"12345678" OK
"12345678\n" OK
"123456789" bad
"123456789\n" bad

pinheadmz mentioned this pull request Mar 13, 2025

Split CConnman #30988

Draft

pinheadmz marked this pull request as draft March 13, 2025 19:37

DrahtBot added the CI failed label Mar 13, 2025

laanwj added the RPC/REST/ZMQ label Mar 14, 2025

DrahtBot mentioned this pull request Mar 14, 2025

i2p: make a time gap between creating transient sessions and using them #32065

Open

This was referenced Mar 14, 2025

net: replace manual reference counting of CNode with shared_ptr #32015

Open

http: Make server shutdown more robust #31929

Open

This was referenced Mar 14, 2025

[IBD] specialize block serialization #31868

Draft

rpc: add cpu_load to getpeerinfo #31672

Open

refactor: Use std::span over Span #31519

Merged

net: option to disallow v1 connection on ipv4 and ipv6 peers #30951

Open

This was referenced Mar 14, 2025

p2p: Fill reconciliation sets (Erlay) attempt 2 #30116

Open

kernel, refactor: return error status on all fatal errors #29700

Draft

rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats' #29418

Open

pinheadmz force-pushed the http-rewrite-13march2025 branch 2 times, most recently from 640f38f to 49784b3 Compare March 18, 2025 19:44

pinheadmz commented Jan 7, 2026

View reviewed changes

fjahr reviewed Jan 8, 2026

View reviewed changes

Matthew Zipkin and others added 25 commits January 9, 2026 10:51

http: enclose libevent-dependent code in a namespace

1845484

This commit is a no-op to isolate HTTP methods and objects that depend on libevent. Following commits will add replacement objects and methods in a new namespace for testing and review before switching over the server.

HTTPServer: implement and test AcceptConnection()

b9cada3

AcceptConnection() is mostly copied from CConmann in net.cpp and then modernized. Co-authored-by: Vasil Dimov <[email protected]>

HTTPServer: generate sequential Ids for each newly accepted connection

573fa4e

Co-authored-by: Vasil Dimov <[email protected]>

http: Introduce HTTPClient class

e098c2a

HTTPserver: support "chunked" Transfer-Encoding

df00e97

HTTPServer: disconnect clients

9d364cb

Allow http workers to send data optimistically as an optimization

31c238d

define HTTP request methods at module level outside of class

77ae1a8

This is a refactor to prepare for matching the API of HTTPRequest definitions in both namespaces http_bitcoin and http_libevent. In particular, to provide a consistent return type for GetRequestMethod() in both classes.

Add helper methods to HTTPRequest to match original API

4eb5582

These methods are called by http_request_cb() and are present in the original http_libevent::HTTPRequest.

HTTPServer: implement control methods to match legacy API

e9e109b

HTTPServer: disconnect after idle timeout (-rpcservertimeout)

daa89a4

http: switch servers from libevent to bitcoin

0fabb84

fuzz: switch http_libevent::HTTPRequest to http_bitcoin::HTTPRequest

93fac91

http: delete libevent!

b6afd81

pinheadmz force-pushed the http-rewrite-13march2025 branch from 63a9487 to b6afd81 Compare January 9, 2026 16:23

pinheadmz commented Jan 9, 2026

View reviewed changes

pinheadmz mentioned this pull request Jan 9, 2026

Prepare string and net utils for future HTTP operations #34242

Open


		auto sa = static_cast<sockaddr>(static_cast<void>(&storage));

		if (!sock.GetSockName(sa, &len)) {

Replace libevent with our own HTTP and socket-handling implementation #32061

Are you sure you want to change the base?

Replace libevent with our own HTTP and socket-handling implementation #32061

Conversation

pinheadmz commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Integration testing:

Uh oh!

DrahtBot commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

DrahtBot commented Mar 13, 2025

Uh oh!

laanwj commented Mar 14, 2025

Uh oh!

vasild commented Mar 14, 2025

Uh oh!

fjahr commented Mar 14, 2025

Uh oh!

pinheadmz commented Mar 15, 2025

Uh oh!

vasild commented Mar 19, 2025

Uh oh!

pinheadmz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjahr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinheadmz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

pinheadmz commented Mar 13, 2025 •

edited

Loading

DrahtBot commented Mar 13, 2025 •

edited

Loading

pinheadmz left a comment •

edited

Loading