net: Add -allowinbound config option #25718

pull fjahr wants to merge 2 commits into bitcoin:master from fjahr:2022-07-allowinbound changing 4 files +18 −3
  1. fjahr commented at 10:52 pm on July 26, 2022: contributor

    (This is just a very rough draft, I am only looking for conceptual feedback right now, so this is the bare minimum code to understand what it should do. I will add tests and fix docs when the conceptual review hurdle is cleared.)

    This is an alternative approach to what was already attempted with #25690 and #24835. Please have a look there for more background information.

    The -allowinbound option lets users explicitly mark networks as available for inbound connections, even if that network may be explicitly deactivated for outbound connections or if it is blocked for other reasons that are out of the user’s control. The effect is that the -externalip addresses of that network may be announced and the -getnetworkinfo RPC will show these addresses. These several users seem to have run into these issues, see for example #25669.

  2. net: Add -allowinbound config option c262ad096d
  3. rpc: Improve docs for getnetworkinfo RPC 2468101b4d
  4. fjahr commented at 10:58 pm on July 26, 2022: contributor
    @mzumsande has suggested having separate -onlynet options for inbound and outbound. I think that’s a valid option as well and if reviewers prefer that approach I can draft that one as well.
  5. DrahtBot added the label P2P on Jul 27, 2022
  6. vasild commented at 12:03 pm on July 28, 2022: contributor

    I think this can and should be resolved in a simpler way by removing IsReachable() from AddLocal() and IsPeerAddrLocalGood().

    The new option -allowinbound would be difficult to explain to users. By its name, I would guess that -allowinbound=0 should do the same as -listen=0.

  7. Rspigler commented at 6:13 pm on July 28, 2022: contributor
    I like the onlynet inbound/outbound. There is consistently confusion about the network settings and how they interact (and bugs found), and I think this would really clear things up. Also, this would translate easily to GUI settings.
  8. vasild commented at 4:30 am on July 29, 2022: contributor
    What would -onlynet inbound do? Is that not the same as omitting the network from -bind=?
  9. DrahtBot commented at 7:36 am on July 29, 2022: contributor

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #25775 (docs: remove non-signaling mentions of BIP125 by glozow)
    • #24170 (p2p, rpc: Manual block-relay-only connections with addnode by mzumsande)

    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.

  10. Rspigler commented at 6:19 pm on July 30, 2022: contributor
    Maybe I don’t understand the purpose of the PR, but I thought it would make changes such as: -cjdnsreachable -> -allowinbound=cjdns -onlynet=cjdns -> '-allowoutbound=cjdns
  11. DrahtBot commented at 10:38 am on August 22, 2022: contributor

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

    Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

  12. DrahtBot added the label Needs rebase on Aug 22, 2022
  13. DrahtBot commented at 1:40 am on December 12, 2022: contributor

    There hasn’t been much activity lately and the patch still needs rebase. What is the status here?

    • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
    • Is it no longer relevant? ➡️ Please close.
    • Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.
  14. DrahtBot commented at 1:17 am on March 12, 2023: contributor

    There hasn’t been much activity lately and the patch still needs rebase. What is the status here?

    • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
    • Is it no longer relevant? ➡️ Please close.
    • Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.
  15. fjahr commented at 3:22 pm on March 12, 2023: contributor
    Closing this for now as I currently don’t have time to continue with this. I hope I can revisit it in the future.
  16. fjahr closed this on Mar 12, 2023

  17. bitcoin locked this on Mar 11, 2024

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: 2026-02-17 09:13 UTC

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