Add NODE_TXRELAY_V2. #30837

pull ariard wants to merge 1 commits into bitcoin:master from ariard:2024-node-txrelay-v2 changing 1 files +3 −0
  1. ariard commented at 6:46 pm on September 6, 2024: contributor

    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.

  2. Add NODE_TXRELAY_V2.
    This does not add default signaling of NODE_TXRELAY_V2 as node local
    services.
    50179789a5
  3. DrahtBot commented at 6:46 pm on September 6, 2024: contributor

    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.

  4. ariard marked this as a draft on Sep 6, 2024
  5. in src/protocol.h:333 in 50179789a5
    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),
    


    jonatack commented at 8:50 pm on September 6, 2024:

    (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      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
    

    ariard commented at 11:46 pm on October 13, 2024:
    will fix it, still looking more for reviews on the BIPs themselves for the disruption minimization approach of restraining the acceptance of unsolicited transactions among upgraded peers only.
  6. DrahtBot added the label CI failed on Sep 7, 2024
  7. DrahtBot commented at 8:22 am on September 7, 2024: contributor

    🚧 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.

  8. DrahtBot commented at 0:06 am on January 11, 2025: contributor

    🤔 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.

  9. maflcko commented at 7:52 am on January 20, 2025: member

    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.

  10. maflcko closed this on Jan 20, 2025

  11. ariard commented at 9:02 pm on January 20, 2025: contributor

    @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.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-01-21 06:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me