averello/bitcoin-mirror
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-12-13 20:36:21 +01:00
Code Issues Packages Projects Releases Wiki Activity

init: Require explicit -asmap filename

Browse Source 
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 <fjahr@protonmail.com>
This commit is contained in:
Ryan Ofsky
2025-10-16 06:55:55 -04:00
parent 25c45bb0d0
commit f6ec3519a3
4 changed files with 31 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
doc/files.md
View File

@@ -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.

4
doc/release-notes-33770.md Normal file
View File

@@ -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).

9
src/init.cpp
View File

@@ -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=<ip>", 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=<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), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-asmap=<file>", "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=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-bind=<addr>[:<port>][=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<bool> 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=<file>."));
return false;
}
if (!asmap_path.is_absolute()) {
asmap_path = args.GetDataDirNet() / asmap_path;
}

61
test/functional/feature_asmap.py
View File

@@ -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=<absolute path>`, using the unit test skeleton asmap
3. `bitcoind -asmap=<relative path>`, 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=<file>."
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()