net: Extract download permission from noban

Github-Pull: bitcoin#19191
Rebased-From: 111109a

doc/reduce-traffic.md

@@ -23,8 +23,8 @@ longer serving historic blocks (blocks older than one week).
 Keep in mind that new nodes require other nodes that are willing to serve
 historic blocks.
 
-Whitelisted peers will never be disconnected, although their traffic counts for
-calculating the target.
+Peers with the `download` permission will never be disconnected, although their
+traffic counts for calculating the target.
 
 ## 2. Disable "listening" (`-listen=0`)
 

src/init.cpp

@@ -437,7 +437,7 @@ void SetupServerArgs()
     gArgs.AddArg("-maxreceivebuffer=<n>", strprintf("Maximum per-connection receive buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     gArgs.AddArg("-maxsendbuffer=<n>", strprintf("Maximum per-connection send buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     gArgs.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
-    gArgs.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h), 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    gArgs.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'download' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     gArgs.AddArg("-misbehavinglimit=<n>", strprintf("Maximum number of misbehaving peers to deprioritise (default: %s)", DEFAULT_MISBEHAVING_LIMIT), ArgsManager::ALLOW_INT, OptionsCategory::CONNECTION);
     gArgs.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor hidden services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     gArgs.AddArg("-onlynet=<net>", "Make outgoing connections only through network <net> (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@@ -464,10 +464,11 @@ void SetupServerArgs()
     gArgs.AddArg("-whitebind=<[permissions@]addr>", "Bind to given address and whitelist peers connecting to it. "
         "Use [host]:port notation for IPv6. Allowed permissions are bloomfilter (allow requesting BIP37 filtered blocks and transactions), "
         "blockfilters (serve compact block filters to peers per BIP 157), "
-        "noban (do not ban for misbehavior), "
+        "noban (do not ban for misbehavior; implies download), "
         "forcerelay (relay transactions that are already in the mempool; implies relay), "
         "relay (relay even in -blocksonly mode), "
-        "and mempool (allow requesting BIP35 mempool contents). "
+        "mempool (allow requesting BIP35 mempool contents), "
+        "and download (allow getheaders during IBD, no disconnect after maxuploadtarget limit). "
         "Specify multiple permissions separated by commas (default: noban,mempool,relay). Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
 
     gArgs.AddArg("-whitelist=<[permissions@]IP address or network>", strprintf("Whitelist peers using the given IP address (e.g. 1.2.3.4) or "

src/net_permissions.cpp

@@ -38,6 +38,7 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output,
             else if (permission == "noban") NetPermissions::AddFlag(flags, PF_NOBAN);
             else if (permission == "forcerelay") NetPermissions::AddFlag(flags, PF_FORCERELAY);
             else if (permission == "mempool") NetPermissions::AddFlag(flags, PF_MEMPOOL);
+            else if (permission == "download") NetPermissions::AddFlag(flags, PF_DOWNLOAD);
             else if (permission == "all") NetPermissions::AddFlag(flags, PF_ALL);
             else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY);
             else if (permission == "addr") NetPermissions::AddFlag(flags, PF_ADDR);
@@ -77,6 +78,7 @@ std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags)
     if (NetPermissions::HasFlag(flags, PF_RELAY)) strings.push_back("relay");
     if (NetPermissions::HasFlag(flags, PF_MEMPOOL)) strings.push_back("mempool");
     if (NetPermissions::HasFlag(flags, PF_ADDR)) strings.push_back("addr");
+    if (NetPermissions::HasFlag(flags, PF_DOWNLOAD)) strings.push_back("download");
     return strings;
 }
 

src/net_permissions.h

@@ -19,8 +19,10 @@ enum NetPermissionFlags
     // Always relay transactions from this peer, even if already in mempool
     // Keep parameter interaction: forcerelay implies relay
     PF_FORCERELAY = (1U << 2) | PF_RELAY,
+    // Allow getheaders during IBD and block-download after maxuploadtarget limit
+    PF_DOWNLOAD = (1U << 18),
     // Can't be banned for misbehavior
-    PF_NOBAN = (1U << 4),
+    PF_NOBAN = (1U << 4) | PF_DOWNLOAD,
     // Can query the mempool
     PF_MEMPOOL = (1U << 5),
     // Can request addrs without hitting a privacy-preserving cache
@@ -33,7 +35,7 @@ enum NetPermissionFlags
 
     // True if the user did not specifically set fine grained permissions
     PF_ISIMPLICIT = (1U << 31),
-    PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL | PF_ADDR | PF_BLOCKFILTERS,
+    PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL | PF_ADDR | PF_BLOCKFILTERS | PF_DOWNLOAD,
 };
 class NetPermissions
 {

src/net_processing.cpp

@@ -1495,8 +1495,8 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c
     }
     const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
     // disconnect node in case we have reached the outbound limit for serving historical blocks
-    // never disconnect whitelisted nodes
-    if (send && connman->OutboundTargetReached(true) && ( ((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_FILTERED_WITNESS_BLOCK) && !pfrom->HasPermission(PF_NOBAN))
+    // nodes with the download permission may exceed target
+    if (send && connman->OutboundTargetReached(true) && ( ((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_FILTERED_WITNESS_BLOCK) && !pfrom->HasPermission(PF_DOWNLOAD))
     {
         LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom->GetId());
 
@@ -2738,7 +2738,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
         }
 
         LOCK(cs_main);
-        if (::ChainstateActive().IsInitialBlockDownload() && !pfrom->HasPermission(PF_NOBAN)) {
+        if (::ChainstateActive().IsInitialBlockDownload() && !pfrom->HasPermission(PF_DOWNLOAD)) {
             LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because node is in initial block download\n", pfrom->GetId());
             return true;
         }

src/test/netbase_tests.cpp

@@ -400,14 +400,15 @@ BOOST_AUTO_TEST_CASE(netpermissions_test)
     BOOST_CHECK_EQUAL(connection_direction, ConnectionDirection::Both);
 
     const auto strings = NetPermissions::ToStrings(PF_ALL);
-    BOOST_CHECK_EQUAL(strings.size(), 7U);
+    BOOST_CHECK_EQUAL(strings.size(), 8U);
     BOOST_CHECK(std::find(strings.begin(), strings.end(), "blockfilters") != strings.end());
     BOOST_CHECK(std::find(strings.begin(), strings.end(), "bloomfilter") != strings.end());
     BOOST_CHECK(std::find(strings.begin(), strings.end(), "forcerelay") != strings.end());
     BOOST_CHECK(std::find(strings.begin(), strings.end(), "relay") != strings.end());
     BOOST_CHECK(std::find(strings.begin(), strings.end(), "noban") != strings.end());
     BOOST_CHECK(std::find(strings.begin(), strings.end(), "mempool") != strings.end());
     BOOST_CHECK(std::find(strings.begin(), strings.end(), "addr") != strings.end());
+    BOOST_CHECK(std::find(strings.begin(), strings.end(), "download") != strings.end());
 }
 
 BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters)

test/functional/feature_maxuploadtarget.py

@@ -137,9 +137,9 @@ def run_test(self):
 
         self.nodes[0].disconnect_p2ps()
 
-        self.log.info("Restarting node 0 with noban permission and 1MB maxuploadtarget")
+        self.log.info("Restarting node 0 with download permission and 1MB maxuploadtarget")
         self.stop_node(0)
-        self.start_node(0, ["-whitelist=noban@127.0.0.1", "-maxuploadtarget=1"])
+        self.start_node(0, ["-whitelist=download@127.0.0.1", "-maxuploadtarget=1"])
 
         # Reconnect to self.nodes[0]
         self.nodes[0].add_p2p_connection(TestP2PConn())
@@ -152,9 +152,12 @@ def run_test(self):
 
         getdata_request.inv = [CInv(2, big_old_block)]
         self.nodes[0].p2p.send_and_ping(getdata_request)
-        assert_equal(len(self.nodes[0].getpeerinfo()), 1) #node is still connected because of the whitelist
 
-        self.log.info("Peer still connected after trying to download old block (whitelisted)")
+        self.log.info("Peer still connected after trying to download old block (download permission)")
+        peer_info = self.nodes[0].getpeerinfo()
+        assert_equal(len(peer_info), 1)  # node is still connected
+        assert_equal(peer_info[0]['permissions'], ['download'])
+
 
 if __name__ == '__main__':
     MaxUploadTest().main()

test/functional/p2p_permissions.py

@@ -39,21 +39,21 @@ def run_test(self):
         self.checkpermission(
             # default permissions (no specific permissions)
             ["-whitelist=127.0.0.1"],
-            ["relay", "noban", "mempool", "addr"],
+            ["relay", "noban", "mempool", "addr", 'download'],
             True)
 
         self.checkpermission(
             # relay permission removed (no specific permissions)
             ["-whitelist=127.0.0.1", "-whitelistrelay=0"],
-            ["noban", "mempool", "addr"],
+            ["noban", "mempool", "addr", 'download'],
             True)
 
         self.checkpermission(
             # forcerelay and relay permission added
             # Legacy parameter interaction which set whitelistrelay to true
             # if whitelistforcerelay is true
             ["-whitelist=127.0.0.1", "-whitelistforcerelay"],
-            ["forcerelay", "relay", "noban", "mempool", "addr"],
+            ["forcerelay", "relay", "noban", "mempool", "addr", 'download'],
             True)
 
         # Let's make sure permissions are merged correctly
@@ -64,32 +64,32 @@ def run_test(self):
         self.checkpermission(
             ["-whitelist=noban@127.0.0.1"],
             # Check parameter interaction forcerelay should activate relay
-            ["noban", "bloomfilter", "forcerelay", "relay"],
+            ["noban", "bloomfilter", "forcerelay", "relay", 'download'],
             False)
         self.replaceinconfig(1, "whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1")
 
         self.checkpermission(
             # legacy whitelistrelay should be ignored
             ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"],
-            ["noban", "mempool"],
+            ["noban", "mempool", 'download'],
             False)
 
         self.checkpermission(
             # legacy whitelistforcerelay should be ignored
             ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"],
-            ["noban", "mempool"],
+            ["noban", "mempool", 'download'],
             False)
 
         self.checkpermission(
             # missing mempool permission to be considered legacy whitelisted
             ["-whitelist=noban@127.0.0.1"],
-            ["noban"],
+            ["noban", 'download'],
             False)
 
         self.checkpermission(
             # all permission added
             ["-whitelist=all@127.0.0.1"],
-            ["blockfilters", "addr"] +
+            ["blockfilters", "addr", 'download'] +
             ["forcerelay", "noban", "mempool", "bloomfilter", "relay"],
             False)