Introduce SockMan ("lite"): low-level socket handling for HTTP #32747

pinheadmz · 2025-06-13T18:36:49Z

Introduces a new low-level socket manager SockMan as an abstract class with virtual functions for implementing higher-level networking protocols like HTTP. This is the next step in #31194

This is a minimal, alternative version of #30988 ("Split CConnman") without any changes to working code (P2P is not affected). It adds a stripped-down version of the SockMan introduced in that pull request that implements only what is needed for the HTTP server implemented in #32061 (i.e. no I2P stuff and for now, no outbound connection stuff). Exclusions from the original SockMan pull request can be checked with:

git diff vasild/sockman \
src/common/sockman.h \
src/common/sockman.cpp

The commit order has been flipped quite a bit because the original PR incrementally pulls logic out of net wheras this PR builds a new system from the bottom-up. Otherwise I tried to keep all the SockMan code in order so reviewers of the original PR would be familiar with it.

It also adds unit tests by introducing a SocketTestingSetup which mocks network sockets by returning a queue of DynSock from CreateSock(). HTTP server tests in #32061 will be rebased on this framework as well.

DrahtBot · 2025-06-13T18:36:52Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	w0xlt, hodlinator
Stale ACK	vasild

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32015 (net: replace manual reference counting of CNode with shared_ptr by vasild)
#30988 (Split CConnman by vasild)

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:

an unique -> a unique [“unique” begins with a consonant sound; “a unique” is grammatically correct]
datasent -> data sent [two words; “data sent” is the intended phrase]

^{drahtbot_id_5_m}

DrahtBot · 2025-06-13T19:03:15Z

🚧 At least one of the CI tasks failed.
_{Task MSan, depends: https://github.com/bitcoin/bitcoin/runs/44070031695}
_{LLM reason (✨ experimental): MemorySanitizer detected use of uninitialized memory during sockman_tests, causing CI failure.}

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.

Sjors · 2025-06-16T08:21:26Z

I was able to switch Sjors#50 to use this instead of #30988 without have to change anything. So it makes sense to me to focus on this PR first, as it's smaller. Thoughts @vasild?

w0xlt

Concept ACK.
Are the functions in this PR to replace Connam functions like CConnman::BindListenPort, CConnman::InitBinds, etc...?

pinheadmz · 2025-06-18T10:04:16Z

replace Connman functions

In this PR they are just protocol-agnostic copies of those ConnMan functions. Just the socket stuff without the Bitcoin stuff. We could still proceed with the reactor in #30988 after this is merged and actually replace the socket stuff in ConnMan with SockMan but this PR is minimized with the focus being the socket needs of the HTTP server in #32061

vasild

Approach ACK 5f79411

vasild · 2025-06-20T10:15:06Z

src/test/sockman_tests.cpp

+    TestSockMan sockman;
+
+    // This address won't actually get used because we stubbed CreateSock()
+    const std::optional<CService> addr_bind{Lookup("0.0.0.0", 0, false)};


9299d5d SockMan: introduce class and implement binding to listening socket

nit, ensure Lookup() succeeded before continuing because below addr_bind.value() is used unconditionally:

BOOST_REQUIRE(addr.has_value());

vasild · 2025-06-20T12:14:30Z

src/common/sockman.cpp

+/** Get the bind address for a socket as CService. */
+static CService GetBindAddress(const Sock& sock)
+{
+    CService addr_bind;
+    struct sockaddr_storage sockaddr_bind;
+    socklen_t sockaddr_bind_len = sizeof(sockaddr_bind);
+    if (!sock.GetSockName((struct sockaddr*)&sockaddr_bind, &sockaddr_bind_len)) {
+        addr_bind.SetSockAddr((const struct sockaddr*)&sockaddr_bind, sockaddr_bind_len);
+    } else {
+        LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "getsockname failed\n");
+    }
+    return addr_bind;
+}


GetBindAddress() is the same as in net.cpp. It is nice to have this PR remove 0 lines, but I think it is better to make an exception and move the function from net.cpp to netbase.{h,cpp} and use that from both net.cpp and common/sockman.cpp.

Oh yes thanks, done by inserting a move-only commit

vasild · 2025-06-23T09:48:37Z

src/common/sockman.cpp

+        SocketHandlerConnected(io_readiness);
+
+        // Accept new connections from listening sockets.
+        SocketHandlerListening(io_readiness.events_per_sock);


674a5ff SockMan: handle connected sockets: read data from socket

In the commit message: s/conencts/connects/

vasild · 2025-06-23T09:57:18Z

src/test/sockman_tests.cpp

+        // Received data is written here by the SockMan I/O thread
+        // and tested by the main thread.
+        Mutex m_received_mutex;
+        std::vector<uint8_t> m_received;


674a5ff SockMan: handle connected sockets: read data from socket

m_received would better be per-client:

std::unordered_map<Id, std::vector<uint8_t>> m_received;

and then adjust EventGotData() to plug the data in the client's slot:

m_received[id].assign(data.begin(), data.end());

and GetReceivedData() to get the data for the client:

GetReceivedData(Id id) { return m_received[id]; }

Good catch, taken. I was hoping to get away with only ever using one client in this test but this makes more sense for coverage anyway ;-)

vasild · 2025-06-23T10:02:53Z

src/common/sockman.cpp

+    return m_connected.erase(id) > 0;
+}
+
+ssize_t SockMan::SendBytes(Id id,


3f796b6 SockMan: handle connected sockets: write data to socket

In the commit message: s/recevied/received/

pinheadmz

Rebase to address review by @vasild THANKS!

pinheadmz · 2025-06-23T18:18:35Z

src/test/sockman_tests.cpp

+    TestSockMan sockman;
+
+    // This address won't actually get used because we stubbed CreateSock()
+    const std::optional<CService> addr_bind{Lookup("0.0.0.0", 0, false)};


pinheadmz · 2025-06-23T18:32:08Z

src/common/sockman.cpp

+/** Get the bind address for a socket as CService. */
+static CService GetBindAddress(const Sock& sock)
+{
+    CService addr_bind;
+    struct sockaddr_storage sockaddr_bind;
+    socklen_t sockaddr_bind_len = sizeof(sockaddr_bind);
+    if (!sock.GetSockName((struct sockaddr*)&sockaddr_bind, &sockaddr_bind_len)) {
+        addr_bind.SetSockAddr((const struct sockaddr*)&sockaddr_bind, sockaddr_bind_len);
+    } else {
+        LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "getsockname failed\n");
+    }
+    return addr_bind;
+}


Oh yes thanks, done by inserting a move-only commit

pinheadmz · 2025-06-23T18:47:07Z

src/test/sockman_tests.cpp

+        // Received data is written here by the SockMan I/O thread
+        // and tested by the main thread.
+        Mutex m_received_mutex;
+        std::vector<uint8_t> m_received;


Good catch, taken. I was hoping to get away with only ever using one client in this test but this makes more sense for coverage anyway ;-)

pinheadmz · 2025-06-23T18:47:22Z

src/common/sockman.cpp

+        SocketHandlerConnected(io_readiness);
+
+        // Accept new connections from listening sockets.
+        SocketHandlerListening(io_readiness.events_per_sock);


pinheadmz · 2025-06-23T18:51:26Z

src/common/sockman.cpp

+    return m_connected.erase(id) > 0;
+}
+
+ssize_t SockMan::SendBytes(Id id,


vasild

ACK 3e7abce

jonatack

Began reviewing, then realized that I was reviewing the same code twice with the two "modernize" commits following "original" commits.

Propose to either (1) make clear in the commit message where the code is being pulled from, and that it will be re-styled afterward, or (2) squash the modernization commits into the previous ones. Thereby easing life a bit for your reviewers :)

jonatack · 2025-07-23T22:24:36Z

src/test/util/setup_common.h

 #include <pubkey.h>
 #include <stdexcept>
 #include <test/util/random.h>
+#include <test/util/net.h>


535daaf nit, sort

fixed, thanks

jonatack · 2025-07-23T22:25:30Z

src/test/sockman_tests.cpp

@@ -0,0 +1,152 @@
+// Copyright (c) 2021-2022 The Bitcoin Core developers


535daaf (edit, or is this due to the code already existing, if yes, please mention this in the commit message)

Suggested change

// Copyright (c) 2021-2022 The Bitcoin Core developers

// Copyright (c) 2025-present The Bitcoin Core developers

Thanks, removed the years entirely which seems to be the style for new files.

vasild

ACK e007e1b57d5d42c2a8d932d5b91eec8a3ca76e14

l0rinc

I'm not immersed in this to give a meaningful ack yet, but please see a few of my comments.

My main observation is that we're introducing tests and new code at the same time and I find it hard to follow what the relation between the two is - how do I tell if they're doing what they're supposed to.
Given that this is meant to replace libevent, I wonder if we could add either a wrapper to libevent and cover it with tests as a first step (could be done in a separate standalone PR as well) and slowly replace the methods we rely on from libevent with our own implementation while the tests keep passing and as a last step delete libevent abstraction completely. That would reassure me personally that the new code isn't just dead weight we're adding to the code, but that we're slowly strangling out a dependency with small focused changes covered with tests where I can debug both the old or new behavior and make absolutely sure they behave in the same way.

I would also prefer new code to be fully covered with tests, especially since none of the error cases and ipv6 part seem to be.

l0rinc · 2025-09-30T02:40:18Z

src/common/sockman.cpp

        return false;
    }

+    int one{1};


nit: I know this was the original name as well, but variables (by their name) should reflect the purpose of their usage, not their value. ipv6_only{1} or tcp_nodelay{1} might make the context more obvious (if that's indeed what they're goal is.

Good call, this value is setting a boolean option to true so I made the variable name more explicit, in the "modernize" commit.

l0rinc · 2025-09-30T02:43:40Z

src/common/sockman.cpp

+        err_msg = Untranslated(strprintf("Bind address family for %s not supported", to.ToStringAddrPort()));
+        return false;


if error message is tied to a false return value, can we return std::optional<bilingual_str> instead of having both a return value and an output parameter?

std::optional<bilingual_str> BindAndStartListening(const CService& to) { sockaddr_storage storage; socklen_t len{sizeof(storage)}; if (!to.GetSockAddr(reinterpret_cast<sockaddr*>(&storage), &len)) { return Untranslated(strprintf("Bind address family for %s not supported", to.ToStringAddrPort())); } ...

Good call, will use util::Result here and remove the error string as an "out" parameter.

l0rinc · 2025-09-30T02:54:48Z

src/netbase.cpp

+    CService addr_bind;
+    struct sockaddr_storage sockaddr_bind;
+    socklen_t sockaddr_bind_len = sizeof(sockaddr_bind);
+    if (!sock.GetSockName((struct sockaddr*)&sockaddr_bind, &sockaddr_bind_len)) {


whenreinterpret_cast<sockaddr*>(&storage) as in SockMan::BindAndStartListening and when c-style cast?

good idea, i'll insert a new "modernize" commit after the move-only GetBindAddress commit

l0rinc · 2025-09-30T03:06:25Z

src/common/sockman.cpp

+    // and enable it by default or not. Try to enable it, if possible.
+    if (to.IsIPv6()) {
+#ifdef IPV6_V6ONLY
+        if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_V6ONLY, &one, sizeof(one)) == SOCKET_ERROR) {


are we testing this code in any way?
And can we enable compile and be type-checking and make it if constexpr instead of using macros?

I don't think we could reliably test these paths in ci because such socket operations depend so much on the platform.

We could create additional child classes of DynSock for the unit tests, so we have mock socket variants that fail each specific option, but I'm not sure we'd get enough value from those tests to be worth the work. Could be a follow up.

l0rinc · 2025-09-30T03:10:30Z

src/common/sockman.cpp

+    sockaddr_storage storage;
+    socklen_t len{sizeof(storage)};
+    if (!to.GetSockAddr(reinterpret_cast<sockaddr*>(&storage), &len)) {
+        err_msg = Untranslated(strprintf("Bind address family for %s not supported", to.ToStringAddrPort()));


seems to me we're not testing any of these unhappy paths - could we do that or is it too much work?

I'll add coverage for as many as I can. But I think all I can do from this level is try to bind to an address with an invalid family (i.e. "NET_ONION").

l0rinc · 2025-09-30T03:16:52Z

src/common/sockman.cpp

+
+    // According to the internet TCP_NODELAY is not carried into accepted sockets
+    // on all platforms.  Set it again here just to be sure.
+    const int on{1};


what's the difference between this an the one (without const) above? Can we unify their naming so that they represent the meaning of what they're storing instead of the value?

good call ill set a static constexpr for all the times we do this in the module, overriding https://github.com/bitcoin/bitcoin/pull/32747/files#r2420892943

l0rinc · 2025-09-30T03:18:12Z

src/common/sockman.cpp

+    // on all platforms.  Set it again here just to be sure.
+    const int on{1};
+    if (sock->SetSockOpt(IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) == SOCKET_ERROR) {
+        LogDebug(BCLog::NET, "connection from %s: unable to set TCP_NODELAY, continuing anyway\n",


nit: most logs don't need a trailing newline anymore

that's nice, thanks

l0rinc · 2025-09-30T03:19:56Z

src/common/sockman.cpp

+std::unique_ptr<Sock> SockMan::AcceptConnection(const Sock& listen_sock, CService& addr)
+{
+    // Make sure we only operate on our own listening sockets
+    Assume(std::ranges::find_if(m_listen, [&](const auto& sock) { return sock.get() == &listen_sock; }) != m_listen.end());


We don't actually need the value here, just that it exists, right?

Suggested change

Assume(std::ranges::find_if(m_listen, [&](const auto& sock) { return sock.get() == &listen_sock; }) != m_listen.end());

Assume(std::ranges::any_of(m_listen, [&](const auto& sock) { return sock.get() == &listen_sock; }));

l0rinc · 2025-09-30T03:40:32Z

src/common/sockman.cpp

+    // Create socket for listening for incoming connections
+    sockaddr_storage storage;
+    socklen_t len{sizeof(storage)};
+    if (!to.GetSockAddr(reinterpret_cast<sockaddr*>(&storage), &len)) {


I understand that we need different sizes for ipv4 and ipv6, hence these ugly polimorphism-mimicking structures, but I'm wondering if there's a way to hide this ugliness in the C++ code with either decicated structures or something lke std::variant<sockaddr_in, sockaddr_in6>

This is a reasonable note but I think it's out of scope for this PR, since GetSockAddr() is used so many other places in the code. Wrapping sockaddr_storage for C++ style could also be applied to Accept(), Connect(), Bind() and probably a few others. I'll open a new PR for that cleanup, similar to #33378

l0rinc · 2025-09-30T03:44:10Z

src/common/sockman.cpp

+    }
+
+    int flags{MSG_NOSIGNAL | MSG_DONTWAIT};
+#ifdef MSG_MORE


can we have some explanation here, it's not self-explanatory. The source location had:

// We rely on the 'more' value returned by GetBytesToSend to correctly predict whether more // bytes are still to be sent, to correctly set the MSG_MORE flag. As a sanity check, // verify that the previously returned 'more' was correct.

sure, added.

pinheadmz

Rebase to 03ce73a820b485fbdbb854d9d3947ff1e6d87880 address conflicts with master, as well as silent conflict from #33378. Address feedback from @l0rinc THANKS -- mostly improving "modernization" of the code borrowed from Connman.

There is a bigger question about the approach in this PR, which adds code that is dead for now but required for #32061. I am going to explore other ways to introduce this code more incrementally, or maybe we can start off by writing SockMan as a libevent wrapper, and then replace libevent with our own code once the libevent behavior is asserted by tests.

pinheadmz · 2025-10-10T15:37:47Z

src/common/sockman.cpp

        return false;
    }

+    int one{1};


Good call, this value is setting a boolean option to true so I made the variable name more explicit, in the "modernize" commit.

pinheadmz · 2025-10-10T17:12:37Z

src/common/sockman.cpp

+    // Create socket for listening for incoming connections
+    sockaddr_storage storage;
+    socklen_t len{sizeof(storage)};
+    if (!to.GetSockAddr(reinterpret_cast<sockaddr*>(&storage), &len)) {


This is a reasonable note but I think it's out of scope for this PR, since GetSockAddr() is used so many other places in the code. Wrapping sockaddr_storage for C++ style could also be applied to Accept(), Connect(), Bind() and probably a few others. I'll open a new PR for that cleanup, similar to #33378

pinheadmz · 2025-10-10T19:43:49Z

src/common/sockman.cpp

+        err_msg = Untranslated(strprintf("Bind address family for %s not supported", to.ToStringAddrPort()));
+        return false;


Good call, will use util::Result here and remove the error string as an "out" parameter.

pinheadmz · 2025-10-10T19:51:53Z

src/common/sockman.cpp

+    sockaddr_storage storage;
+    socklen_t len{sizeof(storage)};
+    if (!to.GetSockAddr(reinterpret_cast<sockaddr*>(&storage), &len)) {
+        err_msg = Untranslated(strprintf("Bind address family for %s not supported", to.ToStringAddrPort()));


I'll add coverage for as many as I can. But I think all I can do from this level is try to bind to an address with an invalid family (i.e. "NET_ONION").

pinheadmz · 2025-10-14T14:24:23Z

src/common/sockman.cpp

+    // and enable it by default or not. Try to enable it, if possible.
+    if (to.IsIPv6()) {
+#ifdef IPV6_V6ONLY
+        if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_V6ONLY, &one, sizeof(one)) == SOCKET_ERROR) {


I don't think we could reliably test these paths in ci because such socket operations depend so much on the platform.

We could create additional child classes of DynSock for the unit tests, so we have mock socket variants that fail each specific option, but I'm not sure we'd get enough value from those tests to be worth the work. Could be a follow up.

pinheadmz · 2025-10-14T14:25:46Z

src/common/sockman.cpp

+    // on all platforms.  Set it again here just to be sure.
+    const int on{1};
+    if (sock->SetSockOpt(IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) == SOCKET_ERROR) {
+        LogDebug(BCLog::NET, "connection from %s: unable to set TCP_NODELAY, continuing anyway\n",


that's nice, thanks

pinheadmz · 2025-10-14T15:35:47Z

src/common/sockman.cpp

+std::unique_ptr<Sock> SockMan::AcceptConnection(const Sock& listen_sock, CService& addr)
+{
+    // Make sure we only operate on our own listening sockets
+    Assume(std::ranges::find_if(m_listen, [&](const auto& sock) { return sock.get() == &listen_sock; }) != m_listen.end());


pinheadmz · 2025-10-14T15:58:33Z

src/common/sockman.cpp

+
+    // According to the internet TCP_NODELAY is not carried into accepted sockets
+    // on all platforms.  Set it again here just to be sure.
+    const int on{1};


good call ill set a static constexpr for all the times we do this in the module, overriding https://github.com/bitcoin/bitcoin/pull/32747/files#r2420892943

pinheadmz · 2025-10-14T16:10:40Z

src/common/sockman.cpp

+    }
+
+    int flags{MSG_NOSIGNAL | MSG_DONTWAIT};
+#ifdef MSG_MORE


sure, added.

pinheadmz · 2025-10-14T16:42:30Z

src/netbase.cpp

+    CService addr_bind;
+    struct sockaddr_storage sockaddr_bind;
+    socklen_t sockaddr_bind_len = sizeof(sockaddr_bind);
+    if (!sock.GetSockName((struct sockaddr*)&sockaddr_bind, &sockaddr_bind_len)) {


good idea, i'll insert a new "modernize" commit after the move-only GetBindAddress commit

vasild

ACK 03ce73a820b485fbdbb854d9d3947ff1e6d87880

theuni · 2025-10-17T21:27:13Z

My $.02 and repeating what I've said elsewhere... I still just don't think that this is the correct level of abstraction. IMO socket-based protocols are different enough to warrant their own logic. Either that, or the abstraction would need to provide a bazillion options and flags, ala libevent/libev/libuv/libkj/etc.

For ex: presumably in order to remain generic, SendBytes may "succeed" at sending only one byte of many. Meaning that implementations will have to maintain their own send buffers, and presumably a thread that does a write/sleep/write/sleep dance. Imo that alone kinda defeats the purpose of this abstraction.

IOReadiness and Sock already implement a good chunk of the low-level handling. Sure, a custom event-loop per-protocol isn't exactly trivial, but it's a better result imo than a master-of-none abstraction.

I'd rather see this and a custom HTTP server collapsed together into a tight implementation. If another another socket-based protocol is implemented in the future, that would be a good time to abstract away the commonalities. But the fact that CConnman isn't a good a good candidate for abstraction is a hint to me that the next one won't be either.

pinheadmz · 2025-10-21T11:00:06Z

@theuni I hear you, maybe I should re write the PR description: The purpose is not to establish a generic socket manager but to just advance the bigger goal of replacing libevent with #32061. In that context, this PR is just the first 11 commits of the HTTP server replacement, broken off for more focused unit testing and review (which has all been great so far).

So with that in mind, what do you recommend? Is it higher level organization like removing the abstract class stuff and moving all the code into HTTPServer directly? Or are there actual lower-level implementation details here you think are ineffective for HTTP?

I'm waiting for approval on this socket chunk before rebasing #32061 but I could also ditch this initial PR and just focus all the work on the bigger picture.

Introduce a new low-level socket managing class `SockMan`. BindListenPort() is copied from CConnMan in net.cpp and will be modernized in the next commit. Unit-test it with a new class `SocketTestingSetup` which mocks `CreateSock()` and will enable mock client I/O in future commits. `SockMan` and `SocketTestingSetup` are designed to be generic and reusbale for higher-level network protocol implementation and testing. Co-authored-by: Vasil Dimov <[email protected]>

It was copied verbatim from `CConnman::BindListenPort()` in the previous commit. Modernize its variables and style and log the error messages from the caller. Also categorize the informative messages to the "net" category because they are quite specific to the networking layer. Co-authored-by: Vasil Dimov <[email protected]>

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

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

Replace the C-style casting with C++ reinterpret_cast

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

`CConnman::SocketHandlerConnected()` copied to `SockMan::SocketHandlerConnected()`. 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 from `CConnman::SocketSendData()` copied to `SockMan::SendBytes()`. 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]>

Copy from some parts of `CConnman::SocketHandlerConnected()` and `CConnman::ThreadSocketHandler()` to: `EventIOLoopCompletedForOne(id)` and `EventIOLoopCompletedForAll()`. Co-authored-by: Vasil Dimov <[email protected]>

pinheadmz · 2025-11-18T16:19:51Z

rebased on master to fix conflict but also converted to draft.

The plan is now to integrate this I/O loop directly with the new http server. So I will rebase #32061 on this branch to include the feedback the socket stuff has received, and then combine this sockman with the http server and remove the abstraction.

pinheadmz · 2025-11-26T11:01:39Z

Closing - these commits are all merged into #32061 including all the great review feedback <3

pinheadmz mentioned this pull request Jun 13, 2025

Remove libevent as a dependency (HTTP / cli / torcontrol) #31194

Open

6 tasks

pinheadmz force-pushed the sockman-lite branch from 33c5d77 to d8274e9 Compare June 13, 2025 19:03

DrahtBot added the CI failed label Jun 13, 2025

pinheadmz force-pushed the sockman-lite branch 2 times, most recently from 55a329a to 2d79cd7 Compare June 13, 2025 20:12

This was referenced Jun 14, 2025

Split CConnman #30988

Draft

test: add end-to-end tests for CConnman and PeerManager #26812

Open

pinheadmz force-pushed the sockman-lite branch from 2d79cd7 to 5f79411 Compare June 14, 2025 14:55

DrahtBot removed the CI failed label Jun 14, 2025

Sjors mentioned this pull request Jun 16, 2025

Stratum v2 connman Sjors/bitcoin#50

Closed

3 tasks

w0xlt reviewed Jun 17, 2025

View reviewed changes

pinheadmz force-pushed the sockman-lite branch from 5f79411 to 1307088 Compare June 21, 2025 22:24

pinheadmz mentioned this pull request Jun 22, 2025

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

Open

vasild reviewed Jun 23, 2025

View reviewed changes

pinheadmz force-pushed the sockman-lite branch from 1307088 to 3e7abce Compare June 23, 2025 23:20

pinheadmz commented Jun 24, 2025

View reviewed changes

This was referenced Jun 24, 2025

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

Open

Broadcast own transactions only via short-lived Tor or I2P connections #29415

Merged

vasild approved these changes Jun 25, 2025

View reviewed changes

DrahtBot requested a review from w0xlt June 25, 2025 15:57

pinheadmz requested review from Sjors and removed request for w0xlt July 2, 2025 18:08

pinheadmz requested a review from hodlinator July 15, 2025 18:05

jonatack reviewed Jul 23, 2025

View reviewed changes

DrahtBot requested a review from w0xlt July 23, 2025 22:42

pinheadmz force-pushed the sockman-lite branch from 3e7abce to 598bee6 Compare August 7, 2025 15:34

DrahtBot mentioned this pull request Sep 22, 2025

net: support overriding the proxy selection in ConnectNode() #33454

Merged

vasild approved these changes Sep 29, 2025

View reviewed changes

DrahtBot requested a review from hodlinator September 29, 2025 13:37

l0rinc reviewed Sep 30, 2025

View reviewed changes

DrahtBot added the Needs rebase label Oct 6, 2025

pinheadmz force-pushed the sockman-lite branch from e007e1b to 03ce73a Compare October 14, 2025 17:33

pinheadmz commented Oct 14, 2025

View reviewed changes

DrahtBot removed the Needs rebase label Oct 14, 2025

vasild approved these changes Oct 17, 2025

View reviewed changes

DrahtBot added the Needs rebase label Oct 24, 2025

pinheadmz and others added 11 commits November 18, 2025 11:15

SockMan: implement and test AcceptConnection()

02c2ac1

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

style: modernize the style of SockMan::AcceptConnection()

9dd3576

SockMan: generate sequential Ids for each newly accepted connection

5a24444

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

[move-only] Make GetBindAddress() callable from outside net.cpp

3261981

Modernize GetBindAddress()

2d4f121

Replace the C-style casting with C++ reinterpret_cast

SockMan: dispatch cyclical events from I/O loop

40c9700

Copy from some parts of `CConnman::SocketHandlerConnected()` and `CConnman::ThreadSocketHandler()` to: `EventIOLoopCompletedForOne(id)` and `EventIOLoopCompletedForAll()`. Co-authored-by: Vasil Dimov <[email protected]>

pinheadmz force-pushed the sockman-lite branch from 03ce73a to 40c9700 Compare November 18, 2025 16:15

pinheadmz marked this pull request as draft November 18, 2025 16:17

DrahtBot removed the Needs rebase label Nov 18, 2025

l0rinc mentioned this pull request Nov 25, 2025

RFC: bench: Add multi thread benchmarking #33740

Closed

pinheadmz closed this Nov 26, 2025

		@@ -0,0 +1,152 @@
		// Copyright (c) 2021-2022 The Bitcoin Core developers

	// Copyright (c) 2021-2022 The Bitcoin Core developers
	// Copyright (c) 2025-present The Bitcoin Core developers

		err_msg = Untranslated(strprintf("Bind address family for %s not supported", to.ToStringAddrPort()));
		return false;

	Assume(std::ranges::find_if(m_listen, [&](const auto& sock) { return sock.get() == &listen_sock; }) != m_listen.end());
	Assume(std::ranges::any_of(m_listen, [&](const auto& sock) { return sock.get() == &listen_sock; }));

Introduce SockMan ("lite"): low-level socket handling for HTTP #32747

Introduce SockMan ("lite"): low-level socket handling for HTTP #32747

Uh oh!

Conversation

pinheadmz commented Jun 13, 2025

Uh oh!

DrahtBot commented Jun 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 Jun 13, 2025

Uh oh!

Sjors commented Jun 16, 2025

Uh oh!

w0xlt left a comment

Choose a reason for hiding this comment

Uh oh!

pinheadmz commented Jun 18, 2025

Uh oh!

vasild 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!

Choose a reason for hiding this comment

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!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

jonatack 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!

jonatack Jul 23, 2025 • 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!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

l0rinc left a comment

Choose a reason for hiding this comment

DrahtBot commented Jun 13, 2025 •

edited

Loading

jonatack left a comment •

edited

Loading

jonatack Jul 23, 2025 •

edited

Loading