search

Sjors / bitcoin

Bitcoin Core integration/staging tree
https://bitcoin.org/en/download
MIT License
6 stars 10 forks source link

Stratum v2 connman #50

Open Sjors opened 2 months ago

Sjors commented 2 months ago

Based on https://github.com/bitcoin/bitcoin/pull/30315, https://github.com/bitcoin/bitcoin/pull/30205 and https://github.com/bitcoin/bitcoin/pull/30988. Followed by #49.

Parent PR https://github.com/bitcoin/bitcoin/pull/29432.

This PR introduces Sv2Connman, a subclass of SockMan (https://github.com/bitcoin/bitcoin/pull/30988). It uses the Sv2Transport introduced in https://github.com/bitcoin/bitcoin/pull/30315 to enable incoming connections from other Stratum v2 roles.

It's main target user is the Template Provider role introduced in #49.

There may be other Stratum v2 roles we want to support in the future.

Note to self, to keep this rebased until https://github.com/bitcoin/bitcoin/pull/30988 lands:

git rebase --rebase-merges HEAD~8

Earlier version(s): https://github.com/bitcoin/bitcoin/pull/30332

Sjors commented 2 months ago

@pinheadmz wrote in the original PR:

I am working on a libevent-replacing HTTP server using netbase primitives and yeah, I have some functions that look a lot like the sv2connamn in this branch.

I think it would be cool if possible to abstract the mechanisms in ThreadSocketHandler to work on "abstract clients" and I'd be happy to review and collaborate on that.

I plan to take a look at this.

Sjors commented 1 month ago

CMake rebase.

Sjors commented 2 weeks ago

Rebased to move everything into a bitcoin_sv2 library (use -DWITH_SV2=ON to compile).

Sjors commented 3 days ago

I turned Sv2Connman into a subclass of SockMan using https://github.com/bitcoin/bitcoin/pull/30988. This gets rid of most of the copy-paste stuff.

Two open questions:

  1. Does having m_disconnect_flag still make sense, given that I call CloseConnection everywhere?
  2. Should EventReadyToSend use the more flag?

MSAN is unhappy:

[15:40:15.757]   Uninitialized value was created by an allocation of 'storage' in the stack frame
[15:40:15.757]     #0 0x55dc46e92b31 in SockMan::AcceptConnection(Sock const&, CService&) ci/scratch/build-x86_64-pc-linux-gnu/src/./src/common/sockman.cpp:351:5
[15:40:15.757] 
[15:40:15.757] SUMMARY: MemorySanitizer: use-of-uninitialized-value ci/scratch/build-x86_64-pc-linux-gnu/src/./src/netaddress.cpp:812:5 in CService::SetSockAddr(sockaddr const*)

Maybe a4d82935024762b3c9500c2dafc8986a5d62836a makes it happy?

Sjors commented 2 days ago

MSAN is happy. The last push just adds the dependency on bitcoin_common, which on macOS isn't needed, but the Linux Guix build is more picky.