net: Restore default whitelistrelay to true

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.

fanquake added the label P2P on Aug 16, 2019

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.

Sjors approved

Sjors commented at 9:15 am on August 16, 2019: member

utACK, agree this also needs a more structural solution.

NicolasDorier commented at 12:15 pm on August 16, 2019: contributor

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

NicolasDorier force-pushed on Aug 16, 2019

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

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

MarcoFalke added this to the milestone 0.19.0 on Aug 16, 2019

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.

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.

NicolasDorier force-pushed on Aug 16, 2019

emilengler commented at 1:58 pm on August 16, 2019: contributor

Concept ACK

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)

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.

NicolasDorier force-pushed on Aug 16, 2019

NicolasDorier commented at 2:24 pm on August 16, 2019: contributor

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

NicolasDorier force-pushed on Aug 16, 2019

NicolasDorier commented at 3:22 pm on August 16, 2019: contributor

~Fixing the chmods on #16635~

[Fix] The default whitelistrelay should be true ce7eac3cb0

Reformat p2p_permissions.py 3b05f0f70f

NicolasDorier force-pushed on Aug 16, 2019

NicolasDorier commented at 3:44 pm on August 16, 2019: contributor

I rebased and also fixed rpc_setban

promag commented at 8:10 pm on August 18, 2019: member

ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb.

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

Sjors commented at 8:46 am on August 19, 2019: member

re-ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb

laanwj renamed this:
~~[Fix] The default whitelistrelay should be true~~
net: The default whitelistrelay should be true
on Aug 19, 2019

laanwj commented at 9:19 am on August 19, 2019: member

ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb

laanwj renamed this:
~~net: The default whitelistrelay should be true~~
net: Restore default whitelistrelay to true
on Aug 19, 2019

laanwj referenced this in commit ed9a2a37c1 on Aug 19, 2019

laanwj merged this on Aug 19, 2019

laanwj closed this on Aug 19, 2019

sidhujag referenced this in commit 3a217a7a05 on Aug 19, 2019

luke-jr referenced this in commit cf3d6caf65 on Sep 3, 2019

luke-jr referenced this in commit 7961ed773d on Sep 3, 2019

deadalnix referenced this in commit 861ef49bb1 on May 2, 2020

PastaPastaPasta referenced this in commit d2b9464231 on Jul 18, 2021

PastaPastaPasta referenced this in commit 07b1471b03 on Jul 18, 2021

PastaPastaPasta referenced this in commit 7d0f3a51bf on Jul 19, 2021

PastaPastaPasta referenced this in commit 1c0bfbecbb on Jul 19, 2021

PastaPastaPasta referenced this in commit 909a13082f on Jul 20, 2021

DrahtBot locked this on Dec 16, 2021

net: Restore default whitelistrelay to true #16631