net: Restore default whitelistrelay to true #16631

pull NicolasDorier wants to merge 2 commits into bitcoin:master from NicolasDorier:fix/default-whiterelay changing 5 files +43 −36
  1. NicolasDorier commented at 8:47 AM on August 16, 2019: contributor

    I thought whitelistrelay default was false when it is true.

    The root of the issue come from the fact that all references to DEFAULT_ are not in the scope of this file, so hard coding of default values are used everywhere in net.cpp. I think that in a separate PR we should fix that more fundamentally everywhere.

  2. fanquake added the label P2P on Aug 16, 2019
  3. in src/net.cpp:916 in 2433fc1880 outdated 
     910 | @@ -911,7 +911,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
 911 |      if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_ISIMPLICIT)) {
 912 |          NetPermissions::ClearFlag(permissionFlags, PF_ISIMPLICIT);
 913 |          if (gArgs.GetBoolArg("-whitelistforcerelay", false)) NetPermissions::AddFlag(permissionFlags, PF_FORCERELAY);
 914 | -        if (gArgs.GetBoolArg("-whitelistrelay", false)) NetPermissions::AddFlag(permissionFlags, PF_RELAY);
 915 | +        if (gArgs.GetBoolArg("-whitelistrelay", true)) NetPermissions::AddFlag(permissionFlags, PF_RELAY);
    Sjors commented at 9:14 AM on August 16, 2019:

    Add comment DEFAULT_WHITELISTRELAY = true, so if anyone changes that default they won't miss this spot.

    NicolasDorier commented at 1:49 PM on August 16, 2019:

    Actually I can easily move the constant (https://github.com/bitcoin/bitcoin/pull/16631#discussion_r314708245) testing it.

  4. Sjors approved
  5. Sjors commented at 9:15 AM on August 16, 2019: member

    utACK, agree this also needs a more structural solution.

  6. NicolasDorier commented at 12:15 PM on August 16, 2019: contributor

    Is there something strange with travis? all PRs seems not passing.

  7. NicolasDorier force-pushed on Aug 16, 2019
  8. in src/net.cpp:915 in 59542e8a66 outdated 
     909 | @@ -910,8 +910,10 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
 910 |      bool legacyWhitelisted = false;
 911 |      if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_ISIMPLICIT)) {
 912 |          NetPermissions::ClearFlag(permissionFlags, PF_ISIMPLICIT);
 913 | +        // DEFAULT_WHITELISTFORCERELAY = false
 914 |          if (gArgs.GetBoolArg("-whitelistforcerelay", false)) NetPermissions::AddFlag(permissionFlags, PF_FORCERELAY);
 915 | -        if (gArgs.GetBoolArg("-whitelistrelay", false)) NetPermissions::AddFlag(permissionFlags, PF_RELAY);
 916 | +        // DEFAULT_WHITELISTRELAY = true
    MarcoFalke commented at 12:56 PM on August 16, 2019:

    What is this?

    DEFAULT_WHITELISTRELAY has nothing to do with validity of blocks or transactions. Please just move it from validation to net or something

  9. in test/functional/p2p_permissions.py:35 in 59542e8a66 outdated 
      32 |          True)
  33 |  
  34 | +        self.checkpermission(
  35 | +        # relay permission removed (no specific permissions)
  36 | +        ["-whitelist=127.0.0.1", "-whitelistrelay=0"],
  37 | +        ["noban", "mempool"],
    MarcoFalke commented at 12:58 PM on August 16, 2019:

    Generally we intend with 4 spaces after an opening ( that is immediately followed by a new line

    NicolasDorier commented at 1:46 PM on August 16, 2019:

    ~you mean that you want me to move the comment above?~ got it

  10. MarcoFalke added this to the milestone 0.19.0 on Aug 16, 2019
  11. emilengler commented at 1:51 PM on August 16, 2019: contributor

    NACK sorry, I think removing args shouldn't be done instantly. They should go through a depraction process and be removed after one major version has been released. Otherwise it could break compatibility.

  12. NicolasDorier commented at 1:51 PM on August 16, 2019: contributor

    @emilengler this PR is precisely about restoring compatibility that I broke during previous PR.

  13. NicolasDorier force-pushed on Aug 16, 2019
  14. emilengler commented at 1:58 PM on August 16, 2019: contributor

    Concept ACK

  15. NicolasDorier commented at 1:58 PM on August 16, 2019: contributor

    As @MarcoFalke suggested, I moved DEFAULT_WHITELISTRELAY/DEFAULT_WHITELISTFORCERELAY to net.h. It build fine, and is a more natural place for those constant to be.

    I also reformatted the p2p_permissions.py file (indent after (\n opening)

  16. Sjors commented at 2:10 PM on August 16, 2019: member

    ACK 2e35132c084c812a536d2738ce1993e79112908d, modulo missing +x permission for p2p_permissions.py (squash https://github.com/Sjors/bitcoin/commit/e79aa1e7990ddbecde5699a6db0902f70389af2c if that's tricky on Windows).

    Is there something strange with travis? all PRs seems not passing.

    Yes, one more rebase should make that go away, see #16633.

  17. NicolasDorier force-pushed on Aug 16, 2019
  18. NicolasDorier commented at 2:24 PM on August 16, 2019: contributor

    @Sjors oops, fixed the chmod, sorry for that.

  19. NicolasDorier force-pushed on Aug 16, 2019
  20. NicolasDorier commented at 3:22 PM on August 16, 2019: contributor

    ~Fixing the chmods on #16635~

  21. [Fix] The default whitelistrelay should be true ce7eac3cb0
  22. Reformat p2p_permissions.py 3b05f0f70f
  23. NicolasDorier force-pushed on Aug 16, 2019
  24. NicolasDorier commented at 3:44 PM on August 16, 2019: contributor

    I rebased and also fixed rpc_setban

  25. promag commented at 8:10 PM on August 18, 2019: member

    ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb.

    nit, could split the moved code to a move-only commit.

  26. Sjors commented at 8:46 AM on August 19, 2019: member

    re-ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb

  27. laanwj renamed this:
    [Fix] The default whitelistrelay should be true
    net: The default whitelistrelay should be true
    on Aug 19, 2019
  28. laanwj commented at 9:19 AM on August 19, 2019: member

    ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb

  29. laanwj renamed this:
    net: The default whitelistrelay should be true
    net: Restore default whitelistrelay to true
    on Aug 19, 2019
  30. laanwj referenced this in commit ed9a2a37c1 on Aug 19, 2019
  31. laanwj merged this on Aug 19, 2019
  32. laanwj closed this on Aug 19, 2019
  33. sidhujag referenced this in commit 3a217a7a05 on Aug 19, 2019
  34. luke-jr referenced this in commit cf3d6caf65 on Sep 3, 2019
  35. luke-jr referenced this in commit 7961ed773d on Sep 3, 2019
  36. deadalnix referenced this in commit 861ef49bb1 on May 2, 2020
  37. PastaPastaPasta referenced this in commit d2b9464231 on Jul 18, 2021
  38. PastaPastaPasta referenced this in commit 07b1471b03 on Jul 18, 2021
  39. PastaPastaPasta referenced this in commit 7d0f3a51bf on Jul 19, 2021
  40. PastaPastaPasta referenced this in commit 1c0bfbecbb on Jul 19, 2021
  41. PastaPastaPasta referenced this in commit 909a13082f on Jul 20, 2021
  42. DrahtBot locked this on Dec 16, 2021
Contributors
NicolasDorierSjorsMarcoFalkeemilenglerpromaglaanwj
Labels
P2P

Milestone
0.19.0

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-05-02 18:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me