From f6ec3519a330e5c3899a4d9f7d26b7980a597b41 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 16 Oct 2025 06:55:55 -0400 Subject: [PATCH 1/2] init: Require explicit -asmap filename Currently, if `-asmap` is specified without a filename, bitcoind tries to load `ip_asn.map` data file. This change now requires `-asmap=ip_asn.map` or another filename to be specified explicitly. The change is intended to make behavior of the option explicit avoid confusion reported https://github.com/bitcoin/bitcoin/issues/33386 where documentation specifies a default file which is not actually loaded by default. It was originally implemented in https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3410302383 and various alternatives are discussed there. Co-authored-by: Fabian Jahr --- doc/files.md | 1 - doc/release-notes-33770.md | 4 +++ src/init.cpp | 9 +++-- test/functional/feature_asmap.py | 61 +++++++++++--------------------- 4 files changed, 31 insertions(+), 44 deletions(-) create mode 100644 doc/release-notes-33770.md diff --git a/doc/files.md b/doc/files.md index c8d8aab3a7a..12c6cefbc6d 100644 --- a/doc/files.md +++ b/doc/files.md @@ -66,7 +66,6 @@ Subdirectory | File(s) | Description `./` | `debug.log` | Contains debug information and general logging generated by `bitcoind` or `bitcoin-qt`; can be specified by `-debuglogfile` option `./` | `fee_estimates.dat` | Stores statistics used to estimate minimum transaction fees required for confirmation `./` | `guisettings.ini.bak` | Backup of former [GUI settings](#gui-settings) after `-resetguisettings` option is used -`./` | `ip_asn.map` | IP addresses to Autonomous System Numbers (ASNs) mapping used for bucketing of the peers; path can be specified with the `-asmap` option `./` | `mempool.dat` | Dump of the mempool's transactions `./` | `onion_v3_private_key` | Cached Tor onion service private key for `-listenonion` option `./` | `i2p_private_key` | Private key that corresponds to our I2P address. When `-i2psam=` is specified the contents of this file is used to identify ourselves for making outgoing connections to I2P peers and possibly accepting incoming ones. Automatically generated if it does not exist. diff --git a/doc/release-notes-33770.md b/doc/release-notes-33770.md new file mode 100644 index 00000000000..58e118635fd --- /dev/null +++ b/doc/release-notes-33770.md @@ -0,0 +1,4 @@ +`-asmap` requires explicit filename +----------------------------------- + +In previous releases, if `-asmap` was specified without a filename, this would try to load an `ip_asn.map` data file. Now loading an asmap file requires an explicit filename like `-asmap=ip_asn.map`. This change was made to make the option easier to understand, because it was confusing for there to be a default filename not actually loaded by default (https://github.com/bitcoin/bitcoin/issues/33386). Also this change makes the option more future-proof, because in upcoming releases, specifying `-asmap` will load embedded asmap data instead of an external file (https://github.com/bitcoin/bitcoin/pull/28792). diff --git a/src/init.cpp b/src/init.cpp index 97bf90e8969..d3139f028ad 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -158,7 +158,6 @@ static constexpr bool DEFAULT_STOPAFTERBLOCKIMPORT{false}; #endif static constexpr int MIN_CORE_FDS = MIN_LEVELDB_FDS + NUM_FDS_MESSAGE_CAPTURE; -static const char* DEFAULT_ASMAP_FILENAME="ip_asn.map"; /** * The PID file facilities. @@ -532,7 +531,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-addnode=", strprintf("Add a node to connect to and attempt to keep the connection open (see the addnode RPC help for more info). This option can be specified multiple times to add multiple nodes; connections are limited to %u at a time and are counted separately from the -maxconnections limit.", MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); - argsman.AddArg("-asmap=", 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), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-asmap=", "Specify asn mapping used for bucketing of the peers. Relative paths will be prefixed by the net-specific datadir location.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bantime=", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bind=[:][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet3: 127.0.0.1:%u=onion, testnet4: 127.0.0.1:%u=onion, signet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultChainParams->GetDefaultPort() + 1, testnetChainParams->GetDefaultPort() + 1, testnet4ChainParams->GetDefaultPort() + 1, signetChainParams->GetDefaultPort() + 1, regtestChainParams->GetDefaultPort() + 1), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-cjdnsreachable", "If set, then this host is configured for CJDNS (connecting to fc00::/8 addresses would lead us to the CJDNS network, see doc/cjdns.md) (default: 0)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -1548,7 +1547,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Read asmap file if configured std::vector asmap; if (args.IsArgSet("-asmap") && !args.IsArgNegated("-asmap")) { - fs::path asmap_path = args.GetPathArg("-asmap", DEFAULT_ASMAP_FILENAME); + fs::path asmap_path = args.GetPathArg("-asmap"); + if (asmap_path.empty()) { + InitError(_("-asmap requires a file path. Use -asmap=.")); + return false; + } if (!asmap_path.is_absolute()) { asmap_path = args.GetDataDirNet() / asmap_path; } diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py index 7f0103ece3b..eef599f1456 100755 --- a/test/functional/feature_asmap.py +++ b/test/functional/feature_asmap.py @@ -4,21 +4,9 @@ # 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 bitcoind in these cases: - -1. `bitcoind` with no -asmap arg, using /16 prefix for IP bucketing - -2. `bitcoind -asmap=`, using the unit test skeleton asmap - -3. `bitcoind -asmap=`, using the unit test skeleton asmap - -4. `bitcoind -asmap/-asmap=` with no file specified, using the default asmap - -5. `bitcoind -asmap` restart with an addrman containing new and tried entries - -6. `bitcoind -asmap` with no file specified and a missing default asmap file - -7. `bitcoind -asmap` with an empty (unparsable) default asmap file +Verify node behaviour and debug log when launching bitcoind with different +`-asmap` and `-noasmap` arg values, including absolute and relative paths, and +with missing and unparseable files. The tests are order-independent. @@ -29,7 +17,6 @@ import shutil from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal -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' @@ -79,22 +66,19 @@ class AsmapTest(BitcoinTestFramework): self.start_node(0, [f'-asmap={name}']) os.remove(filename) - def test_default_asmap(self): - shutil.copyfile(self.asmap_raw, self.default_asmap) + def test_unspecified_asmap(self): + msg = "Error: -asmap requires a file path. Use -asmap=." for arg in ['-asmap', '-asmap=']: - self.log.info(f'Test bitcoind {arg} (using default map file)') + self.log.info(f'Test bitcoind {arg} (and no filename specified)') 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) + self.node.assert_start_raises_init_error(extra_args=[arg], expected_msg=msg) def test_asmap_interaction_with_addrman_containing_entries(self): self.log.info("Test bitcoind -asmap restart with addrman containing new and tried entries") self.stop_node(0) - shutil.copyfile(self.asmap_raw, self.default_asmap) - self.start_node(0, ["-asmap", "-checkaddrman=1", "-test=addrman"]) + self.start_node(0, [f"-asmap={self.asmap_raw}", "-checkaddrman=1", "-test=addrman"]) self.fill_addrman(node_id=0) - self.restart_node(0, ["-asmap", "-checkaddrman=1", "-test=addrman"]) + self.restart_node(0, [f"-asmap={self.asmap_raw}", "-checkaddrman=1", "-test=addrman"]) with self.node.assert_debug_log( expected_msgs=[ "CheckAddrman: new 2, tried 2, total 4 started", @@ -102,29 +86,28 @@ class AsmapTest(BitcoinTestFramework): ] ): self.node.getnodeaddresses() # getnodeaddresses re-runs the addrman checks - os.remove(self.default_asmap) - def test_default_asmap_with_missing_file(self): - self.log.info('Test bitcoind -asmap with missing default map file') + def test_asmap_with_missing_file(self): + self.log.info('Test bitcoind -asmap with missing map file') self.stop_node(0) - msg = f"Error: Could not find asmap file \"{self.default_asmap}\"" - self.node.assert_start_raises_init_error(extra_args=['-asmap'], expected_msg=msg) + msg = f"Error: Could not find asmap file \"{self.datadir}{os.sep}missing\"" + self.node.assert_start_raises_init_error(extra_args=['-asmap=missing'], expected_msg=msg) def test_empty_asmap(self): self.log.info('Test bitcoind -asmap with empty map file') self.stop_node(0) - with open(self.default_asmap, "w", encoding="utf-8") as f: + empty_asmap = os.path.join(self.datadir, "ip_asn.map") + with open(empty_asmap, "w", encoding="utf-8") as f: f.write("") - msg = f"Error: Could not parse asmap file \"{self.default_asmap}\"" - self.node.assert_start_raises_init_error(extra_args=['-asmap'], expected_msg=msg) - os.remove(self.default_asmap) + msg = f"Error: Could not parse asmap file \"{empty_asmap}\"" + self.node.assert_start_raises_init_error(extra_args=[f'-asmap={empty_asmap}'], expected_msg=msg) + os.remove(empty_asmap) def test_asmap_health_check(self): self.log.info('Test bitcoind -asmap logs ASMap Health Check with basic stats') - shutil.copyfile(self.asmap_raw, self.default_asmap) msg = "ASMap Health Check: 4 clearnet peers are mapped to 3 ASNs with 0 peers being unmapped" with self.node.assert_debug_log(expected_msgs=[msg]): - self.start_node(0, extra_args=['-asmap']) + self.start_node(0, extra_args=[f'-asmap={self.asmap_raw}']) raw_addrman = self.node.getrawaddrman() asns = [] for _, entries in raw_addrman.items(): @@ -133,12 +116,10 @@ class AsmapTest(BitcoinTestFramework): if asn not in asns: asns.append(asn) assert_equal(len(asns), 3) - os.remove(self.default_asmap) def run_test(self): self.node = self.nodes[0] self.datadir = self.node.chain_path - self.default_asmap = os.path.join(self.datadir, DEFAULT_ASMAP_FILENAME) base_dir = self.config["environment"]["SRCDIR"] self.asmap_raw = os.path.join(base_dir, ASMAP) @@ -146,9 +127,9 @@ class AsmapTest(BitcoinTestFramework): self.test_noasmap_arg() self.test_asmap_with_absolute_path() self.test_asmap_with_relative_path() - self.test_default_asmap() + self.test_unspecified_asmap() self.test_asmap_interaction_with_addrman_containing_entries() - self.test_default_asmap_with_missing_file() + self.test_asmap_with_missing_file() self.test_empty_asmap() self.test_asmap_health_check() From 288b8c30be42f2879d1a1f3d5ec3cac13f87ace4 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 19 Nov 2025 09:08:24 -0500 Subject: [PATCH 2/2] doc: Drop (default: none) from -i2psam description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Suggested by Vojtěch Strnad (vostrnad) https://github.com/bitcoin/bitcoin/pull/33770#discussion_r2529117675 --- src/init.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index d3139f028ad..7e014eb5eac 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -553,7 +553,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) #else argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); #endif - argsman.AddArg("-i2psam=", "I2P SAM proxy to reach I2P peers and accept I2P connections (default: none)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-i2psam=", "I2P SAM proxy to reach I2P peers and accept I2P connections", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-i2pacceptincoming", strprintf("Whether to accept inbound I2P connections (default: %i). Ignored if -i2psam is not set. Listening for inbound I2P connections is done through the SAM proxy, not by binding to a local address and port.", DEFAULT_I2P_ACCEPT_INCOMING), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onlynet=", "Make automatic outbound connections only to network (" + Join(GetNetworkNames(), ", ") + "). Inbound and manual connections are not affected by this option. It can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-v2transport", strprintf("Support v2 transport (default: %u)", DEFAULT_V2_TRANSPORT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);