The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process. A summary of reviews will appear here.
Reviewers, 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.
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.
🚧 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.
It compiles and the tests run, but it’s not pretty.
The conversion between Sv2NetMsg and CSerializedNetMsg / CNetMessage is done in https://github.com/bitcoin/bitcoin/pull/30315/commits/7d937674de425d8e2525bac9e719ac93a775ea45, 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.
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.
9+#include <span.h>
10+#include <streams.h>
11+#include <string>
12+#include <util/check.h>
13+
14+namespace node {
namespace is outdated, since this isn’t part of the node anymore.
242+
243+ // Payload
244+ Span<const std::byte> payload_plain = MakeByteSpan(msg.data());
245+ // TODO: truncate very long messages, about 100 bytes at the start and end
246+ // is probably enough for most debugging.
247+ LogPrintLevel(BCLog::SV2, BCLog::Level::Trace, "Payload: %s\n", HexStr(payload_plain));
MakeByteSpan(msg), not MakeByteSpan(msg.data()).
🚧 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.
libbitcoin_common. See https://github.com/Sjors/bitcoin/pull/47 for a more ambitious attempt to introduce a whole new libbitcoin_net, but that became real big and distracting real fast :-)
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 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