-
Notifications
You must be signed in to change notification settings - Fork 38.9k
Stratum v2 Transport #30315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stratum v2 Transport #30315
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
Would it make sense to add a |
d78b16e to
d787c61
Compare
|
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
d787c61 to
1132807
Compare
1132807 to
6477011
Compare
|
It compiles and the tests run, but it's not pretty. The conversion between I also haven't put much thought into |
d4ecab9 to
697aa7d
Compare
|
I only had a very brief look, but my guess would be that it would be easier if |
697aa7d to
e108385
Compare
|
@sipa done and managed to clean up |
src/common/sv2_messages.h
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
639be6df22800d2e2a170c1af74d51951597834c: this namespace is outdated, since this isn't part of the node anymore.
e108385 to
2f673b5
Compare
2f673b5 to
cb40e0b
Compare
|
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
cb40e0b to
b0802dc
Compare
03557a0 to
6691069
Compare
|
🚧 At least one of the CI tasks failed. HintsMake sure to run all tests locally, according to the documentation. The failure may happen due to a number of reasons, for example:
Leave a comment here, if you need help tracking down a confusing failure. |
|
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. |
4c680d6 to
e28078f
Compare
Co-Authored-By: Christopher Coverdale <[email protected]>
This commit adds the simplest stratum v2 message. The remaining messages are introduced in later commits. Co-Authored-By: Christopher Coverdale <[email protected]>
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 <[email protected]> Co-Authored-By: Fi3
e28078f to
62b255c
Compare
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
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. |
Based on #29346. Followed by Sjors#50. Parent PR #29432.
Introduces
Sv2Transport::Transportwhich is very similar toV2Transport.This shoehorns
Sv2NetMsginto aCSerializedNetMsginSetMessageToSend, and into aCNetMessageinGetReceivedMessage.See discussion in #30209.