#31260 scripted-diff: Type-safe settings retrieval

full analysis

https://github.com/bitcoin/bitcoin/pull/31260 · ryanofsky · +3713/-1113 in 100 files, 11 commits · labels: Refactoring, Needs rebase

Goal

  • Prevent recurring bugs and silent errors from misspelled option names, invalid types, or mismatched defaults
  • Ensure configuration and command-line settings are checked at compile time across the codebase

This PR introduces a `common::Setting` template class to provide compile-time type safety for command-line arguments and configuration settings. It migrates all existing `AddArg` and `GetArg*` calls across the codebase to `Setting::Register` and `Setting::Get` using an automated scripted diff.

Problem: Command-line arguments and configuration settings currently rely on string keys and scattered default values, allowing typos in setting names, mismatched defaults between help documentation and call sites, and inconsistent type interpretations to go undetected at compile time.

Category: Utilities (logging, arguments, libraries) (#7 of 66)

P2 · cleanup

  • P2 because it enforces compile-time safety across node configuration retrieval and registration
  • Prevents a long-standing source of subtle bugs from misspelled argument names and mismatched defaults
  • Unblocks planned runtime range and type checking across the configuration manager

P2 because replacing stringly-typed configuration retrieval with static type definitions removes a persistent, recurring class of bugs across the entire node. Mismatched defaults and negation footguns have caused multiple regressions and cleanups (#30529, #31212), and establishing this infrastructure directly unblocks type and range checking for arguments (#22978).

Membership: Implements core configuration and argument management infrastructure in src/common/setting.h and src/common/args.cpp.

Factors: security/stability 1, bug 1, performance 0, user value 0, leverage 2

Reviewability: Stale: Needs rebase

  • Review is blocked by merge conflicts with master until the author rebases

The PR has merge conflicts with current master and requires a rebase. Additionally, the author previously indicated an intent to restructure and split the changes into smaller PRs.

Author status: active; frequently rebasing and keeping branch building across refactors, though yet to split the PR or reply to recent review comments

Open concerns:

  • jeanpablojp noted that in `setting_internal.h`, null elements in settings.json arrays are dropped silently rather than throwing a type error, changing behavior from master
  • jeanpablojp noted the scripted-diff parser regex misses templated `GetArg<T>` calls, leaving five settings unmigrated
  • Author noted in April 2026 an intention to split the PR so the core `Setting` template class is introduced first with a few call sites, before applying the full tree-wide scripted diff

Resolved concerns:

  • Static and constexpr variable linkage cleanups in generated headers resolved with hodlinator
  • Extensibility and parameter handling of `HelpFn` updated based on feedback from maflcko
  • Circular dependency and documentation linter updates adjusted after reviewer review

Agreement: Mild

  • Strong concept support for compile-time guardrails on configuration settings
  • Much better to let the compiler enforce settings safety (l0rinc)
  • Praise for the safety improvements with testing and script refinements (hodlinator, jeanpablojp)
  • Discussion remains open on handling edge cases and possibly splitting the repository-wide diff

Strong concept consensus, but an unintended behavior difference in settings.json null handling remains unaddressed

Reviewers strongly favor replacing string-based argument access with typed declarations. However, a nonblocking comment from jeanpablojp regarding an unintended behavior difference in settings.json array parsing has not yet been addressed by the author.

  • hodlinator: 'Overdue Concept ACK... No (intended) changes in behavior when executing the code, just installing compile time guardrails.'
  • l0rinc: 'This looks really cool, strong concept ack!'
  • jeanpablojp: 'A null element gets dropped silently here, and numbers now get converted, where the old GetArgs() threw.'

Objections:

ReviewerKindHarmStatusBlockingAuthor repliedQuote
jeanpablojpcorrectnessA null element in a settings.json array gets dropped silently instead of throwing a type error as on masteropennono2026-08-20: 'A null element gets dropped silently here, and numbers now get converted, where the old GetArgs() threw... Raising it because the commit says behavior doesn't change.'
hodlinatormaintenanceNon-constexpr static variables duplicated across compilation units causing redundant allocationsresolvednoyes2024-11-20: "these other non-constexpr statics seem like they end up being \"file-local\" static vars in every compilation unit that includes them. Seems off?"
Settled: 2024-11-22: "they are switched to constexpr string literal types now"

Support:

  • hodlinator: Installs compile-time guardrails against settings typos and default mismatches without runtime performance degradation
  • l0rinc: Strong Concept ACK for having the compiler enforce settings safety rather than relying on runtime conventions
  • jeanpablojp: Concept ACK after verifying the scripted diff, building, and passing test suites

Participants: hodlinator (support), l0rinc (support), Pitan1993 (neutral), maflcko (support), yancyribbens (neutral), jeanpablojp (objection)

State derived from the lists: nonblocking objection open (jeanpablojp)

Review verdicts (DrahtBot): 0

Dependencies

Enables:

Files

1232 lines under test/bench/ci.

  • src/init_settings.h +913/-0
  • src/init.cpp +291/-350
  • src/test/argsman_tests.cpp +153/-152
  • src/test/getarg_tests.cpp +146/-145
  • src/test/argsman_tests_settings.h +222/-0
  • src/common/setting_internal.h +219/-0
  • src/common/setting.h +155/-0
  • src/wallet/init_settings.h +147/-0
  • src/bitcoin-cli_settings.h +138/-0
  • src/bitcoin-cli.cpp +52/-67
  • src/test/setting_tests.cpp +109/-0
  • src/bitcoin-tx_settings.h +107/-0
  • src/dummywallet_settings.h +97/-0
  • src/init/common_settings.h +87/-0
  • src/wallet/init.cpp +29/-35
  • src/init/common.cpp +32/-31
  • src/bitcoin-tx.cpp +25/-36
  • src/chainparamsbase_settings.h +56/-0
  • src/test/getarg_tests_settings.h +52/-0
  • src/bench/bench_bitcoin_settings.h +47/-0
  • src/dummywallet.cpp +23/-24
  • src/node/settings.h +45/-0
  • src/qt/bitcoin_settings.h +45/-0
  • src/wallet/wallet.cpp +20/-19
  • src/bitcoin-wallet_settings.h +36/-0
  • contrib/devtools/circular-dependencies.py +31/-2
  • src/bench/bench_bitcoin.cpp +15/-18
  • src/node/mempool_args.cpp +16/-15
  • src/common/args.cpp +19/-11
  • src/qt/bitcoin.cpp +15/-13
  • src/test/logging_tests_settings.h +24/-0
  • src/test/util/setup_common.cpp +11/-11
  • src/test/util/setup_common_settings.h +22/-0
  • src/bitcoin-wallet.cpp +11/-10
  • src/common/args_settings.h +21/-0
  • src/net.cpp +11/-10
  • src/chainparamsbase.cpp +10/-10
  • src/validation.h +0/-20
  • src/CMakeLists.txt +17/-1
  • src/httprpc.cpp +9/-8
  • src/node/settings.cpp +17/-0
  • src/chainparams.cpp +9/-7
  • src/bitcoin_settings.h +15/-0
  • src/httpserver.cpp +8/-7
  • src/node/chainstatemanager_args.cpp +8/-7
  • src/qt/test/optiontests_settings.h +15/-0
  • src/bitcoin-util_settings.h +13/-0
  • src/node/peerman_args.cpp +7/-6
  • src/qt/test/optiontests.cpp +7/-6
  • src/qt/intro.cpp +6/-4
  • src/bitcoind.cpp +5/-4
  • src/common/init.cpp +5/-4
  • src/wallet/test/init_tests.cpp +5/-4
  • src/test/util/common.h +6/-2
  • src/validation.cpp +0/-8
  • test/lint/check-doc.py +4/-4
  • src/bitcoin-util.cpp +4/-3
  • src/node/blockmanager_args.cpp +4/-3
  • src/node/caches.cpp +4/-3
  • src/test/logging_tests.cpp +4/-3
  • src/torcontrol.cpp +4/-3
  • src/wallet/load.cpp +4/-3
  • src/bitcoin.cpp +4/-2
  • src/init.h +6/-0
  • src/node/mining_args.cpp +4/-2
  • src/common/config.cpp +3/-2
  • src/node/coins_view_args.cpp +3/-2
  • src/node/kernel_notifications.cpp +3/-2
  • src/rpc/util.cpp +3/-2
  • src/wallet/dump.cpp +3/-2
  • src/wallet/spend.cpp +3/-2
  • src/wallet/wallettool.cpp +3/-2
  • src/wallet/walletutil.cpp +3/-2
  • src/qt/walletmodel.cpp +2/-2
  • src/addrdb.cpp +2/-1
  • src/kernel/chainstatemanager_opts.h +2/-1
  • src/node/chainstatemanager_args.h +0/-3
  • src/node/database_args.cpp +2/-1
  • src/node/interfaces.cpp +2/-1
  • src/node/mempool_persist_args.cpp +2/-1
  • src/node/miner.cpp +2/-1
  • src/qt/optionsmodel.cpp +2/-1
  • src/rpc/external_signer.cpp +2/-1
  • src/rpc/mempool.cpp +2/-1
  • src/rpc/request.cpp +2/-1
  • src/rpc/server.cpp +2/-1
  • src/test/addrman_tests.cpp +2/-1
  • src/test/fuzz/addrman.cpp +2/-1
  • src/test/fuzz/connman.cpp +2/-1
  • src/wallet/coincontrol.cpp +2/-1
  • src/wallet/db.cpp +2/-1
  • src/wallet/external_signer_scriptpubkeyman.cpp +2/-1
  • src/node/mining_args.h +0/-2
  • src/common/args.h +1/-0
  • src/node/blockstorage.cpp +1/-0
  • src/qt/CMakeLists.txt +1/-0
  • src/rpc/blockchain.cpp +1/-0
  • src/test/CMakeLists.txt +1/-0
  • src/test/util/CMakeLists.txt +1/-0
  • src/test/validation_chainstatemanager_tests.cpp +1/-0

Card

This PR refactors Bitcoin Core's settings management to use a compile-time type-safe Setting template class, converting all AddArg and GetArg call sites via a scripted diff. It solves the recurring problem of typos in option names, conflicting default values between documentation and call sites, and inconsistent type interpretations. Reviewers strongly endorse the concept, though the PR currently needs a rebase, has an unresolved minor behavioral observation regarding settings.json parsing, and the author previously noted an intention to split the template class from the large scripted diff.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T21:21, confidence high, input hash 1158d38b9967b451