Merge #17812: asmap feature refinements and functional tests

src/addrman.cpp

@@ -6,9 +6,9 @@
 #include <addrman.h>
 
 #include <hash.h>
-#include <serialize.h>
-#include <streams.h>
 #include <logging.h>
+#include <streams.h>
+#include <serialize.h>
 
 int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asmap) const
 {
@@ -16,7 +16,7 @@ int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asma
     uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetHash().GetCheapHash();
     int tried_bucket = hash2 % ADDRMAN_TRIED_BUCKET_COUNT;
     uint32_t mapped_as = GetMappedAS(asmap);
-    LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to tried bucket %i.\n", ToStringIP(), mapped_as, tried_bucket);
+    LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to tried bucket %i\n", ToStringIP(), mapped_as, tried_bucket);
     return tried_bucket;
 }
 
@@ -27,7 +27,7 @@ int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std:
     uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetHash().GetCheapHash();
     int new_bucket = hash2 % ADDRMAN_NEW_BUCKET_COUNT;
     uint32_t mapped_as = GetMappedAS(asmap);
-    LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to new bucket %i.\n", ToStringIP(), mapped_as, new_bucket);
+    LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to new bucket %i\n", ToStringIP(), mapped_as, new_bucket);
     return new_bucket;
 }
 
@@ -658,12 +658,12 @@ std::vector<bool> CAddrMan::DecodeAsmap(fs::path path)
     FILE *filestr = fsbridge::fopen(path, "rb");
     CAutoFile file(filestr, SER_DISK, CLIENT_VERSION);
     if (file.IsNull()) {
-        LogPrintf("Failed to open asmap file from disk.\n");
+        LogPrintf("Failed to open asmap file from disk\n");
         return bits;
     }
     fseek(filestr, 0, SEEK_END);
     int length = ftell(filestr);
-    LogPrintf("Opened asmap file %s (%d bytes) from disk.\n", path, length);
+    LogPrintf("Opened asmap file %s (%d bytes) from disk\n", path, length);
     fseek(filestr, 0, SEEK_SET);
     char cur_byte;
     for (int i = 0; i < length; ++i) {

src/addrman.h

@@ -6,23 +6,22 @@
 #ifndef BITCOIN_ADDRMAN_H
 #define BITCOIN_ADDRMAN_H
 
+#include <clientversion.h>
 #include <netaddress.h>
 #include <protocol.h>
 #include <random.h>
 #include <sync.h>
 #include <timedata.h>
 #include <util.h>
-#include <clientversion.h>
 
+#include <fs.h>
+#include <hash.h>
+#include <iostream>
 #include <map>
 #include <set>
 #include <stdint.h>
-#include <vector>
-#include <iostream>
 #include <streams.h>
-#include <fs.h>
-#include <hash.h>
-
+#include <vector>
 
 /**
  * Extended statistics about a CAddress

src/init.cpp

@@ -511,6 +511,7 @@ void SetupServerArgs()
     gArgs.AddArg("-timestampindex", strprintf("Maintain a timestamp index for block hashes, used to query blocks hashes by a range of timestamps (default: %u)", DEFAULT_TIMESTAMPINDEX), false, OptionsCategory::INDEXING);
     gArgs.AddArg("-txindex", strprintf("Maintain a full transaction index, used by the getrawtransaction rpc call (default: %u)", DEFAULT_TXINDEX), false, OptionsCategory::INDEXING);
 
+    gArgs.AddArg("-asmap=<file>", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), false, OptionsCategory::CONNECTION);
     gArgs.AddArg("-addnode=<ip>", "Add a node to connect to and attempt to keep the connection open (see the `addnode` RPC command help for more info). This option can be specified multiple times to add multiple nodes.", false, OptionsCategory::CONNECTION);
     gArgs.AddArg("-allowprivatenet", strprintf("Allow RFC1918 addresses to be relayed and connected to (default: %u)", DEFAULT_ALLOWPRIVATENET), false, OptionsCategory::CONNECTION);
     gArgs.AddArg("-banscore=<n>", strprintf("Threshold for disconnecting misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), false, OptionsCategory::CONNECTION);
@@ -543,7 +544,6 @@ void SetupServerArgs()
     gArgs.AddArg("-timeout=<n>", strprintf("Specify connection timeout in milliseconds (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), false, OptionsCategory::CONNECTION);
     gArgs.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), false, OptionsCategory::CONNECTION);
     gArgs.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", false, OptionsCategory::CONNECTION);
-    gArgs.AddArg("-asmap=<file>", "Specify asn mapping used for bucketing of the peers. Path should be relative to the -datadir path.", false, OptionsCategory::CONNECTION);
 #ifdef USE_UPNP
 #if USE_UPNP
     gArgs.AddArg("-upnp", "Use UPnP to map the listening port (default: 1 when listening and no -proxy)", false, OptionsCategory::CONNECTION);
@@ -1901,6 +1901,31 @@ bool AppInitMain()
             return InitError(ResolveErrMsg("externalip", strAddr));
     }
 
+    // Read asmap file if configured
+    if (gArgs.IsArgSet("-asmap")) {
+        fs::path asmap_path = fs::path(gArgs.GetArg("-asmap", ""));
+        if (asmap_path.empty()) {
+            asmap_path = DEFAULT_ASMAP_FILENAME;
+        }
+        if (!asmap_path.is_absolute()) {
+            asmap_path = GetDataDir() / asmap_path;
+        }
+        if (!fs::exists(asmap_path)) {
+            InitError(strprintf(_("Could not find asmap file %s"), asmap_path));
+            return false;
+        }
+        std::vector<bool> asmap = CAddrMan::DecodeAsmap(asmap_path);
+        if (asmap.size() == 0) {
+            InitError(strprintf(_("Could not parse asmap file %s"), asmap_path));
+            return false;
+        }
+        const uint256 asmap_version = SerializeHash(asmap);
+        g_connman->SetAsmap(asmap);
+        LogPrintf("Using asmap version %s for IP bucketing\n", asmap_version.ToString());
+    } else {
+        LogPrintf("Using /16 prefix for IP bucketing\n");
+    }
+
 #if ENABLE_ZMQ
     g_zmq_notification_interface = CZMQNotificationInterface::Create();
 
@@ -2458,25 +2483,6 @@ bool AppInitMain()
         return false;
     }
 
-    // Read asmap file if configured
-    if (gArgs.IsArgSet("-asmap")) {
-        std::string asmap_file = gArgs.GetArg("-asmap", "");
-        if (asmap_file.empty()) {
-            asmap_file = DEFAULT_ASMAP_FILENAME;
-        }
-        const fs::path asmap_path = GetDataDir() / asmap_file;
-        std::vector<bool> asmap = CAddrMan::DecodeAsmap(asmap_path);
-        if (asmap.size() == 0) {
-            InitError(strprintf(_("Could not find or parse specified asmap: '%s'"), asmap_path));
-            return false;
-        }
-        g_connman->SetAsmap(asmap);
-        const uint256 asmap_version = SerializeHash(asmap);
-        LogPrintf("Using asmap version %s for IP bucketing.\n", asmap_version.ToString());
-    } else {
-        LogPrintf("Using /16 prefix for IP bucketing.\n");
-    }
-
     // ********************************************************* Step 13: finished
 
     SetRPCWarmupFinished();

src/netaddress.cpp

@@ -202,6 +202,11 @@ bool CNetAddr::IsRFC7343() const
 
 bool CNetAddr::IsTor() const { return m_net == NET_ONION; }
 
+bool CNetAddr::IsHeNet() const
+{
+    return (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x04 && GetByte(12) == 0x70);
+}
+
 bool CNetAddr::IsLocal() const
 {
     // IPv4 loopback
@@ -439,9 +444,8 @@ std::vector<unsigned char> CNetAddr::GetGroup(const std::vector<bool> &asmap) co
     {
         nStartByte = 6;
         nBits = 4;
-    }
+    } else if (IsHeNet()) {
         // for he.net, use /36 groups
-    else if (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x04 && GetByte(12) == 0x70)
         nBits = 36;
     // for the rest of the IPv6 network, use /32 groups
     else

src/netaddress.h

@@ -109,8 +109,9 @@ class CNetAddr
         bool IsRFC4843() const; // IPv6 ORCHID (deprecated) (2001:10::/28)
         bool IsRFC7343() const; // IPv6 ORCHIDv2 (2001:20::/28)
         bool IsRFC4862() const; // IPv6 autoconfig (FE80::/64)
-        bool IsRFC6052() const; // IPv6 well-known prefix (64:FF9B::/96)
-        bool IsRFC6145() const; // IPv6 IPv4-translated address (::FFFF:0:0:0/96)
+        bool IsRFC6052() const; // IPv6 well-known prefix for IPv4-embedded address (64:FF9B::/96)
+        bool IsRFC6145() const; // IPv6 IPv4-translated address (::FFFF:0:0:0/96) (actually defined in RFC2765)
+        bool IsHeNet() const;   // IPv6 Hurricane Electric - https://he.net (2001:0470::/36)
         bool IsTor() const;
         bool IsLocal() const;
         bool IsRoutable() const;

test/functional/feature_asmap.py

@@ -0,0 +1,106 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test asmap config argument for ASN-based IP bucketing.
+
+Verify node behaviour and debug log when launching dashd in these cases:
+
+1. `dashd` with no -asmap arg, using /16 prefix for IP bucketing
+
+2. `dashd -asmap=<absolute path>`, using the unit test skeleton asmap
+
+3. `dashd -asmap=<relative path>`, using the unit test skeleton asmap
+
+4. `dashd -asmap/-asmap=` with no file specified, using the default asmap
+
+5. `dashd -asmap` with no file specified and a missing default asmap file
+
+6. `dashd -asmap` with an empty (unparsable) default asmap file
+
+The tests are order-independent.
+
+"""
+import os
+import shutil
+
+from test_framework.test_framework import BitcoinTestFramework
+
+DEFAULT_ASMAP_FILENAME = 'ip_asn.map' # defined in src/init.cpp
+ASMAP = '../../src/test/data/asmap.raw' # path to unit test skeleton asmap
+VERSION = 'fec61fa21a9f46f3b17bdcd660d7f4cd90b966aad3aec593c99b35f0aca15853'
+
+def expected_messages(filename):
+    return ['Opened asmap file "{}" (59 bytes) from disk'.format(filename),
+            'Using asmap version {} for IP bucketing'.format(VERSION)]
+
+class AsmapTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = False
+        self.num_nodes = 1
+
+    def test_without_asmap_arg(self):
+        self.log.info('Test dashd with no -asmap arg passed')
+        self.stop_node(0)
+        with self.node.assert_debug_log(['Using /16 prefix for IP bucketing']):
+            self.start_node(0)
+
+    def test_asmap_with_absolute_path(self):
+        self.log.info('Test dashd -asmap=<absolute path>')
+        self.stop_node(0)
+        filename = os.path.join(self.datadir, 'my-map-file.map')
+        shutil.copyfile(self.asmap_raw, filename)
+        with self.node.assert_debug_log(expected_messages(filename)):
+            self.start_node(0, ['-asmap={}'.format(filename)])
+        os.remove(filename)
+
+    def test_asmap_with_relative_path(self):
+        self.log.info('Test dashd -asmap=<relative path>')
+        self.stop_node(0)
+        name = 'ASN_map'
+        filename = os.path.join(self.datadir, name)
+        shutil.copyfile(self.asmap_raw, filename)
+        with self.node.assert_debug_log(expected_messages(filename)):
+            self.start_node(0, ['-asmap={}'.format(name)])
+        os.remove(filename)
+
+    def test_default_asmap(self):
+        shutil.copyfile(self.asmap_raw, self.default_asmap)
+        for arg in ['-asmap', '-asmap=']:
+            self.log.info('Test dashd {} (using default map file)'.format(arg))
+            self.stop_node(0)
+            with self.node.assert_debug_log(expected_messages(self.default_asmap)):
+                self.start_node(0, [arg])
+        os.remove(self.default_asmap)
+
+    def test_default_asmap_with_missing_file(self):
+        self.log.info('Test dashd -asmap with missing default map file')
+        self.stop_node(0)
+        msg = "Error: Could not find asmap file \"{}\"".format(self.default_asmap)
+        self.node.assert_start_raises_init_error(extra_args=['-asmap'], expected_msg=msg)
+
+    def test_empty_asmap(self):
+        self.log.info('Test dashd -asmap with empty map file')
+        self.stop_node(0)
+        with open(self.default_asmap, "w", encoding="utf-8") as f:
+            f.write("")
+        msg = "Error: Could not parse asmap file \"{}\"".format(self.default_asmap)
+        self.node.assert_start_raises_init_error(extra_args=['-asmap'], expected_msg=msg)
+        os.remove(self.default_asmap)
+
+    def run_test(self):
+        self.node = self.nodes[0]
+        self.datadir = os.path.join(self.node.datadir, self.chain)
+        self.default_asmap = os.path.join(self.datadir, DEFAULT_ASMAP_FILENAME)
+        self.asmap_raw = os.path.join(os.path.dirname(os.path.realpath(__file__)), ASMAP)
+
+        self.test_without_asmap_arg()
+        self.test_asmap_with_absolute_path()
+        self.test_asmap_with_relative_path()
+        self.test_default_asmap()
+        self.test_default_asmap_with_missing_file()
+        self.test_empty_asmap()
+
+
+if __name__ == '__main__':
+    AsmapTest().main()

test/functional/test_runner.py

@@ -168,6 +168,7 @@ BASE_SCRIPTS= [
     'feature_dip0020_activation.py',
     'feature_uacomment.py',
     'p2p_unrequested_blocks.py',
+    'feature_asmap.py',
     'feature_logging.py',
     'p2p_node_network_limited.py',
     'feature_blocksdir.py',