init: Improve -asmap option behavior and documentation

fjahr commented at 10:25 pm on October 14, 2025: contributor

Note: There is an alternative change that turns -asmap into a bool option and adds a -asmapfile option: #33631 Please indicate with your conceptual review which option you prefer, thanks!

This changes the behavior of the GetPathArg function to better work with the behavior of -asmap. When a path arg is given “1” (for example -asmap=1), this would currently be interpreted as a path but should be interpreted as using the arg without a parameter which activates the fallback. There is no explicit test added here because this is tested implicitly in the tests added in #28792.

Also improves the documentation of the -asmap default behavior.

See #33386 for further discussion. Closes #33386

common: Use fallback in GetPathArg if the path is 1

Otherwise -asmap=1, for example, would be interpreted as a path "1".

c1212f9d5e

DrahtBot commented at 10:25 pm on October 14, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33632.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33631 (init: Split file path handling out of -asmap option by fjahr)
#31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
#28792 (Embed default ASMap as binary dump header file by fjahr)
#17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)
#17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
#17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
#17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/init.cpp:535 in 0948cd1543

531@@ -532,7 +532,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
532                  ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
533 
534     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);
535-    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);
536+    argsman.AddArg("-asmap=<file>", strprintf("Specify asn mapping used for bucketing of the peers (default: disabled, but if -asmap or -asmap= is specified without a filename, %s will be used). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);

maflcko commented at 8:01 am on October 15, 2025:

Specify asn mapping used for bucketing of the peers -> Specify the ASN mapping used for bucketing of peers [adds missing article and capitalizes the ASN acronym to improve clarity]

fjahr commented at 10:09 am on October 15, 2025:

Done

maflcko commented at 8:02 am on October 15, 2025: member

(nit from the llm bot)

doc: Clarify -asmap default behavior

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

9d33b6d5c1

fjahr force-pushed on Oct 15, 2025

fjahr closed this on Oct 22, 2025

ryanofsky commented at 4:01 pm on October 22, 2025: contributor

I think this is ok to close, since my suggestion about improving the default behavior #33631 (comment) as a better alternative to that PR also seems like it could be a better alternative to this PR.

I do think the approach in the PR is reasonable, though. It could also be implemented by calling InterpretBool("-asmap", false) to detect the “1” value instead of hardcoding it into GetPathArg and affecting other users of GetPathArg which may not want to treat “1” specially.

init: Improve -asmap option behavior and documentation #33632

Code Coverage & Benchmarks

Reviews

Conflicts