p2p: add more bad ports #32826

instead of updating the comment to match the code (which is the ultimate source of truth anyway), seems like a good opportunity to remove the duplication of the code below - which is already trivial, no need for a dead comment that doesn't add any new info.

👍 https://dev.mysql.com/doc/mysql-port-reference/en/mysql-port-reference-tables.html#GUID-65C1FF7E-5357-4E58-8D68-A0C3D24C0832__MYSQL-CLIENT-SERVER-PORTS

👍 https://www.datacentreplus.co.uk/support/knowledge-base/changing-the-rdp-port-on-a-windows-server

👍 https://www.postgresql.org/docs/current/app-postgres.html

👍 https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/6/html/deployment_guide/s1-connecting-to-vnc-server#sec-Configuring_the_Firewall_for_VNC

👍 https://www.mongodb.com/docs/manual/reference/default-mongodb-port

I was wondering if we could do a more functional approach here, since we already have a few count_if instances - not sure it's better or not, will let you decide, please resolve if you don't think it's a good alternative (assuming integer promotion for max + 1 here):

    using namespace std::ranges;
    auto ports{views::iota(1, std::numeric_limits<uint16_t>::max() + 1)};
    BOOST_CHECK_EQUAL(count_if(ports, IsBadPort), 85);

implemented

might as well inline it now that it's only used once

    BOOST_CHECK_EQUAL(std::ranges::count_if(ports, IsBadPort), 85);

done

👍, looks like this contains the end as well, no +1 needed. Checked with BOOST_CHECK_EQUAL(std::ranges::count_if(ports, [&](const int i) { return i == std::numeric_limits<uint16_t>::max(); }), 1); to make sure.

Note though that this is a substantial memory allocation, the previous suggestion was lazy. For the record here's the complete lazy solution I suggested (with fixed imports):

diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp
index e3bf482873..0404a45c79 100644
--- a/src/test/netbase_tests.cpp
+++ b/src/test/netbase_tests.cpp
@@ -2,6 +2,12 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <algorithm>
+#include <limits>
+#include <numeric>
+#include <ranges>
+#include <string>
+
 #include <net_permissions.h>
 #include <netaddress.h>
 #include <netbase.h>
@@ -13,12 +19,10 @@
 #include <util/strencodings.h>
 #include <util/translation.h>
 
-#include <string>
-#include <numeric>
-
 #include <boost/test/unit_test.hpp>
 
 using namespace std::literals;
+using namespace std::ranges;
 
 BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup)
 
@@ -604,10 +608,7 @@ BOOST_AUTO_TEST_CASE(isbadport)
     BOOST_CHECK(!IsBadPort(443));
     BOOST_CHECK(!IsBadPort(8333));
 
-    // Check all ports, there must be 85 bad ports in total.
-    using namespace std::ranges;
-    std::list<int> ports(std::numeric_limits<uint16_t>::max());
-    std::iota(ports.begin(), ports.end(), 1);
+    constexpr auto ports{views::iota(1, std::numeric_limits<uint16_t>::max() + 1)};
     BOOST_CHECK_EQUAL(count_if(ports, IsBadPort), 85);
 }

Got nerd-sniped by this commit using list for no reason and was about to suggest vector<uint16_t> as a better default. But a view is nicer conceptually and slightly nicer on the cache (memory usage difference seems negligible).

₿ poop "./build/bin/test_bitcoin_list -t netbase_tests/isbadport" "./build/bin/test_bitcoin_vector16 -t netbase_tests/isbadport"
Benchmark 1 (43 runs): ./build/bin/test_bitcoin_list -t netbase_tests/isbadport
  measurement          mean ± σ            min … max           outliers         delta
  wall_time           117ms ± 1.45ms     117ms …  125ms          2 ( 5%)        0%
  peak_rss           22.6MB ±  154KB    22.3MB … 22.9MB          0 ( 0%)        0%
  cpu_cycles          521M  ± 3.63M      510M  …  527M           1 ( 2%)        0%
  instructions       1.81G  ± 14.3M     1.77G  … 1.83G           0 ( 0%)        0%
  cache_references    957K  ± 28.7K      911K  … 1.01M           0 ( 0%)        0%
  cache_misses        184K  ± 8.55K      172K  …  214K           1 ( 2%)        0%
  branch_misses       164K  ±  702       163K  …  166K           1 ( 2%)        0%
Benchmark 2 (44 runs): ./build/bin/test_bitcoin_vector16 -t netbase_tests/isbadport
  measurement          mean ± σ            min … max           outliers         delta
  wall_time           115ms ±  181us     114ms …  115ms          0 ( 0%)        ⚡-  2.3% ±  0.4%
  peak_rss           22.5MB ±  171KB    22.3MB … 23.1MB          1 ( 2%)          -  0.2% ±  0.3%
  cpu_cycles          513M  ± 2.87M      505M  …  517M           2 ( 5%)        ⚡-  1.6% ±  0.3%
  instructions       1.80G  ± 15.7M     1.74G  … 1.82G           3 ( 7%)          -  0.6% ±  0.4%
  cache_references    684K  ± 35.5K      631K  …  771K           0 ( 0%)        ⚡- 28.5% ±  1.4%
  cache_misses        178K  ± 3.97K      172K  …  190K           1 ( 2%)        ⚡-  3.1% ±  1.5%
  branch_misses       165K  ±  523       163K  …  166K           1 ( 2%)          +  0.4% ±  0.2%

~/bitcoin
₿ poop "./build/bin/test_bitcoin_vector16 -t netbase_tests/isbadport" "./build/bin/test_bitcoin_view -t netbase_tests/isbadport"
Benchmark 1 (44 runs): ./build/bin/test_bitcoin_vector16 -t netbase_tests/isbadport
  measurement          mean ± σ            min … max           outliers         delta
  wall_time           115ms ± 2.52ms     114ms …  131ms          3 ( 7%)        0%
  peak_rss           22.5MB ±  209KB    22.2MB … 23.1MB          4 ( 9%)        0%
  cpu_cycles          512M  ± 3.19M      503M  …  518M           1 ( 2%)        0%
  instructions       1.80G  ± 13.7M     1.74G  … 1.81G           2 ( 5%)        0%
  cache_references    687K  ± 31.7K      633K  …  805K           1 ( 2%)        0%
  cache_misses        179K  ± 3.83K      174K  …  191K           2 ( 5%)        0%
  branch_misses       165K  ±  585       164K  …  166K           1 ( 2%)        0%
Benchmark 2 (44 runs): ./build/bin/test_bitcoin_view -t netbase_tests/isbadport
  measurement          mean ± σ            min … max           outliers         delta
  wall_time           115ms ± 1.19ms     114ms …  122ms          4 ( 9%)          -  0.2% ±  0.7%
  peak_rss           22.5MB ±  241KB    22.1MB … 23.0MB          0 ( 0%)          -  0.1% ±  0.4%
  cpu_cycles          512M  ± 4.47M      492M  …  518M           5 (11%)          +  0.0% ±  0.3%
  instructions       1.78G  ± 47.8M     1.49G  … 1.80G           6 (14%)          -  1.0% ±  0.8%
  cache_references    668K  ± 26.4K      632K  …  726K           0 ( 0%)        ⚡-  2.8% ±  1.8%
  cache_misses        177K  ± 4.16K      167K  …  187K           3 ( 7%)          -  1.0% ±  0.9%
  branch_misses       165K  ±  901       164K  …  168K           4 ( 9%)          +  0.1% ±  0.2%

I'll see myself out. 🚪🏃

Trying to connect to (or open( that port sounds like a great way to be flagged as malware / attacker.