Stratum v2 Transport #30315

Sjors · 2024-06-20T15:59:15Z

Based on #29346. Followed by Sjors#50. Parent PR #29432.

Introduces Sv2Transport::Transport which is very similar to V2Transport.

This shoehorns Sv2NetMsg into a CSerializedNetMsg in SetMessageToSend, and into a CNetMessage in GetReceivedMessage.

See discussion in #30209.

DrahtBot · 2024-06-20T15:59:19Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31045 (ci: Add missing -DWERROR=ON to test-each-commit by maflcko)
#30988 (Split CConnman by vasild)
#30595 (kernel: Introduce initial C header API by TheCharlatan)
#29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
#28792 (Embed default ASMap as binary dump header file by fjahr)

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.

Sjors · 2024-06-20T16:03:13Z

Would it make sense to add a libbitcoin_net library? Either at the same level as libbitcoin_common or on top of it. I could see how other node implementations could benefit from that, but it's also a way to limit libbitcoin_common in size. Afaik only libbitcoin_node needs networking (and zmq?), and a future libbitcoin_sv2, and maybe libbitcoin_rpc.

DrahtBot · 2024-06-20T16:05:34Z

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is 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.

Leave a comment here, if you need help tracking down a confusing failure.

_{Debug: https://github.com/bitcoin/bitcoin/runs/26477378291}

Sjors · 2024-06-21T11:18:24Z

It compiles and the tests run, but it's not pretty.

The conversion between Sv2NetMsg and CSerializedNetMsg / CNetMessage is done in 7d93767, which is a bit of a hack at the moment.

I also haven't put much thought into Sv2NetMsg ever since I took over #27854, so perhaps this design can be improved as well.

sipa · 2024-06-21T13:34:45Z

I only had a very brief look, but my guess would be that it would be easier if Sv2NetMsg did not contain an Sv2NetHeader, and just stored type and message. The Sv2 Transport would then construct the header at submitting or sending time instead. This means Sv2NetMsg would be more of a dumb container for what higher-level code cares about, while the protocol details would be more abstracted away in the Transport.

Sjors · 2024-06-21T15:44:59Z

@sipa done and managed to clean up sv2_messages.h a bit in the process.

Sjors · 2024-06-21T15:47:21Z

src/common/sv2_messages.h

+#include <string>
+#include <util/check.h>
+
+namespace node {


639be6d: this namespace is outdated, since this isn't part of the node anymore.

src/test/sv2_transport_tests.cpp

DrahtBot · 2024-06-21T18:14:59Z

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is 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.

Leave a comment here, if you need help tracking down a confusing failure.

_{Debug: https://github.com/bitcoin/bitcoin/runs/26526802432}

DrahtBot · 2024-09-20T09:25:33Z

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

Hints

Make sure to run all tests locally, according to the documentation.

The failure may 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 · 2024-09-20T15:56:50Z

Each of these CI failures seem spurious, some sort of timeout after many hours. However there's real failures in the followup PR. Going to rebase this to avoid confusion.

Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>

This commit adds the simplest stratum v2 message. The remaining messages are introduced in later commits. Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>

This avoids a circular dependency between bitcoin-sv2 and bitcoin-node.

This allows us to subclass Transport.

Implemented starting from a copy of V2Transport and the V2TransportTester, modifying it to fit Stratum v2 and Noise Protocol requirements. Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com> Co-Authored-By: Fi3

DrahtBot · 2024-10-08T15:35:01Z

🐙 This pull request conflicts with the target branch and needs rebase.

Sjors · 2024-10-16T08:57:01Z

I moved this to Sjors#67 so we can focus on building an interface for external applications to use, and getting multiprocess in a release.

Sjors force-pushed the 2024/06/sv2_transport branch from d78b16e to d787c61 Compare June 20, 2024 16:05

DrahtBot added the CI failed label Jun 20, 2024

Sjors mentioned this pull request Jun 20, 2024

Make Transport independent of CNetMessage and CSerializedNetMsg #30209

Closed

Sjors force-pushed the 2024/06/sv2_transport branch from d787c61 to 1132807 Compare June 20, 2024 16:15

Sjors mentioned this pull request Jun 20, 2024

Stratum v2 Template Provider (take 3) #29432

Closed

24 tasks

This was referenced Jun 21, 2024

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

Open

logging: use bitset for categories #26697

Closed

Sjors force-pushed the 2024/06/sv2_transport branch from 1132807 to 6477011 Compare June 21, 2024 11:14

Sjors force-pushed the 2024/06/sv2_transport branch 2 times, most recently from d4ecab9 to 697aa7d Compare June 21, 2024 13:24

DrahtBot removed the CI failed label Jun 21, 2024

Sjors force-pushed the 2024/06/sv2_transport branch from 697aa7d to e108385 Compare June 21, 2024 15:44

Sjors commented Jun 21, 2024

View reviewed changes

Sjors force-pushed the 2024/06/sv2_transport branch from e108385 to 2f673b5 Compare June 21, 2024 16:20

Sjors commented Jun 21, 2024

View reviewed changes

src/test/sv2_transport_tests.cpp Show resolved Hide resolved

Sjors force-pushed the 2024/06/sv2_transport branch from 2f673b5 to cb40e0b Compare June 21, 2024 18:14

DrahtBot added the CI failed label Jun 21, 2024

Sjors force-pushed the 2024/06/sv2_transport branch from cb40e0b to b0802dc Compare June 24, 2024 11:50

DrahtBot removed the CI failed label Jun 24, 2024

Sjors force-pushed the 2024/06/sv2_transport branch 2 times, most recently from 03557a0 to 6691069 Compare June 25, 2024 10:14

Sjors mentioned this pull request Jun 25, 2024

Stratum v2 connman #30332

Closed

2 tasks

DrahtBot mentioned this pull request Jun 25, 2024

Stratum v2 Noise Protocol #29346

Closed

Sjors force-pushed the 2024/06/sv2_transport branch from dfb9a6d to 4c680d6 Compare September 20, 2024 09:25

DrahtBot added the CI failed label Sep 20, 2024

Sjors force-pushed the 2024/06/sv2_transport branch from 4c680d6 to e28078f Compare September 20, 2024 16:21

DrahtBot removed the CI failed label Sep 20, 2024

This was referenced Sep 20, 2024

Embed default ASMap as binary dump header file #28792

Open

net: Replace libnatpmp with built-in PCP+NATPMP implementation #30043

Merged

DrahtBot added the CI failed label Sep 26, 2024

This was referenced Sep 27, 2024

Split CConnman #30988

Draft

build: Switch to Qt 6 #30997

Merged

DrahtBot added Needs rebase and removed CI failed labels Sep 30, 2024

Sjors and others added 8 commits October 4, 2024 12:09

Add sv2 log category for Stratum v2

b4a84ab

build: libbitcoin_sv2 scaffold

d08a2eb

ci: always build WITH_SV2=ON

5f36962

Add sv2 noise protocol

b588ff8

Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>

Add sv2 message CoinbaseOutputDataSize

1c8d46e

This commit adds the simplest stratum v2 message. The remaining messages are introduced in later commits. Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>

Move CNetMessage and Transport headers to common

8352f32

This avoids a circular dependency between bitcoin-sv2 and bitcoin-node.

Convert between Sv2NetMsg and CSerializedNetMsg

a8cbde1

This allows us to subclass Transport.

Introduce Sv2Transport

62b255c

Implemented starting from a copy of V2Transport and the V2TransportTester, modifying it to fit Stratum v2 and Noise Protocol requirements. Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com> Co-Authored-By: Fi3

Sjors force-pushed the 2024/06/sv2_transport branch from e28078f to 62b255c Compare October 4, 2024 12:06

DrahtBot removed the Needs rebase label Oct 4, 2024

DrahtBot mentioned this pull request Oct 7, 2024

ci: Add missing -DWERROR=ON to test-each-commit #31045

Merged

DrahtBot added the Needs rebase label Oct 8, 2024

Sjors mentioned this pull request Oct 16, 2024

Stratum v2 Transport Sjors/bitcoin#67

Closed

1 task

Sjors closed this Oct 16, 2024

Sjors mentioned this pull request Oct 16, 2024

Stratum v2 via IPC Mining Interface tracking issue #31098

Open

33 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Stratum v2 Transport #30315

Stratum v2 Transport #30315

Uh oh!

Sjors commented Jun 20, 2024 •

edited

Loading

Uh oh!

DrahtBot commented Jun 20, 2024 •

edited

Loading

Uh oh!

Sjors commented Jun 20, 2024 •

edited

Loading

Uh oh!

DrahtBot commented Jun 20, 2024

Uh oh!

Sjors commented Jun 21, 2024 •

edited

Loading

Uh oh!

sipa commented Jun 21, 2024

Uh oh!

Sjors commented Jun 21, 2024

Uh oh!

Sjors Jun 21, 2024

Uh oh!

Uh oh!

DrahtBot commented Jun 21, 2024

Uh oh!

DrahtBot commented Sep 20, 2024

Uh oh!

Sjors commented Sep 20, 2024

Uh oh!

DrahtBot commented Oct 8, 2024

Uh oh!

Sjors commented Oct 16, 2024

Uh oh!

Uh oh!

Stratum v2 Transport #30315

Stratum v2 Transport #30315

Uh oh!

Conversation

Sjors commented Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

Sjors commented Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jun 20, 2024

Uh oh!

Sjors commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipa commented Jun 21, 2024

Uh oh!

Sjors commented Jun 21, 2024

Uh oh!

Sjors Jun 21, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DrahtBot commented Jun 21, 2024

Uh oh!

DrahtBot commented Sep 20, 2024

Uh oh!

Sjors commented Sep 20, 2024

Uh oh!

DrahtBot commented Oct 8, 2024

Uh oh!

Sjors commented Oct 16, 2024

Uh oh!

Uh oh!

Sjors commented Jun 20, 2024 •

edited

Loading

DrahtBot commented Jun 20, 2024 •

edited

Loading

Sjors commented Jun 20, 2024 •

edited

Loading

Sjors commented Jun 21, 2024 •

edited

Loading