This is the top commit from #30572, where the new node service bit support commit is extracted on its own.
See the corresponding BIP draft for motivation: https://github.com/bitcoin/bips/pull/1663
Dissociating this change from halting the processing of unrequested transaction, allow the node service bit support to be used for further policies and mechanisms, beyond this mechanism only.
This does not add default signaling of NODE_TXRELAY_V2 as node local
services.
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.
328@@ -329,6 +329,9 @@ enum ServiceFlags : uint64_t {
329 // NODE_P2P_V2 means the node supports BIP324 transport
330 NODE_P2P_V2 = (1 << 11),
331
332+ // NODE_TXRELAY_V2 means the node supports BIPXXX transaction-relay v2 protocol
333+ NODE_TXRELAY_V2 = (1 << 12),
(in case you’re not waiting for concept acks)
0/src/protocol.cpp:94:13: error: enumeration value 'NODE_TXRELAY_V2' not handled in switch [-Werror,-Wswitch]
1 94 | switch ((ServiceFlags)service_flag) {
2 | ^~~~~~~~~~~~~~~~~~~~~~~~~~
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/29798727914
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.
🤔 There hasn’t been much activity lately and the CI seems to be failing.
If no one reviewed the current pull request by commit hash, a rebase can be considered. While the CI failure may be a false positive, the CI hasn’t been running for some time, so there may be a real issue hiding as well. A rebase triggers the latest CI and makes sure that no silent merge conflicts have snuck in.
Closing for now, due to inactivity for a few months, and failing CI, since it was opened.
Please leave a comment, if you want this reopened. Alternatively, you can also open a new pull, since there hasn’t been any discussion here anyway, except for discussions around the failing CI.
@maflcko Thanks if this PR can be re-opened.
The more ample rational behind this work has been published here:https://bitcoinops.org/en/newsletters/2024/12/06/ and it was under embargo for a bunch of months, as we were considering the impact on lightning implementations. I’ll go back on this PR + #30572 soon (tm). The problem is serious and for now it’s more conceptual review that is calling for than fixing a CI.
@glozow Thanks for the comment.
Have a look on the original PR on #21224, when all the existing tests at the time were fixed, before a real conversation on the conceptual trade-offs that one can evaluate at high-level was effectively done. Sorry, not sorry, it’s not rational to fix CI if it’s to waste time again, before there is conceptual convergence.
As a contributor yourself, before to be a maintainer, and I think someone able to conceptually review PRs yourself, if there are technical aspects of bitcoin P2P related to this PR that you don’t understand, feel free to ask questions, I’ll answer them.
