scripted-diff: Type-safe settings retrieval #31260

pull ryanofsky wants to merge 9 commits into bitcoin:master from ryanofsky:pr/scripty changing 86 files +3562 βˆ’1084
  1. ryanofsky commented at 10:34 pm on November 8, 2024: contributor

    This PR changes the way settings are registered and retrieved to provide more compile-time safety. Currently settings are registered and retrieved like:

    0// Register setting
    1argsman.AddArg("-pid=<file>", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
    2
    3// Retrieve setting
    4args.GetPathArg("-pid", BITCOIN_PID_FILENAME)
    

    But this is not ideal because nothing checks that setting names are spelled correctly, or that default values (BITCOIN_PID_FILENAME) are used consistently in the help string and retrieval sites, or that settings are retrieved with consistent types (bool, int, string, path, or list). This PR addresses these issues by adding setting declarations which allow settings to be registered and retrieved like:

     0// Declare setting
     1using PidSetting = common::Setting<
     2    "-pid=<file>", fs::path, {.legacy = true},
     3    "Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)">
     4    ::DefaultFn<[] { return BITCOIN_PID_FILENAME; }>;
     5
     6// Register setting
     7PidSetting::Register(argsman);
     8
     9// Retrieve setting
    10PidSetting::Get(args)
    

    Suggestions for review

    All the real changes in this PR are in the last scripted-diff commit:

    If you take a few minutes to look at the changes applied in this scripted-diff commit, you’ll understand everything this PR is doing. A good place to start looking around in this commit is the generated src/init_settings.h file which declares the settings that get registered in src/init.cpp. Then you can look at the surrounding diffs and see they are just replacing AddArg and GetArg calls.

    The other notable commit is second commit, which implements the Setting class:

    The other commits do minor things like moving code or updating linters. The python script that implements the scripted diff is a temporary artifact that gets deleted. The python script is complicated because it does things like parsing c++ code to extract help strings, and figuring out the right types to declare settings with so code compiles. But the entire scope of the script is to (1) generate Setting type definitions, (2) add #includes, and (3) replace AddArg() calls with Register() calls and GetArg() calls with Get() calls. So there is not much the script can actually do wrong without triggering build and test failures.


    Extensions

    This PR only adds the ability to declare individual settings with built-in types. It doesn’t provide any new runtime behavior, but a branch in issue #22978 extends the Setting class implemented here to support runtime setting validation, additional types like std::variant, additional conversion options, custom types, custom validation, and groups of settings declared as options structs.

    This change is mostly orthogonal to #16545. #16545 only provides runtime type checking while this PR only provides compile-time checking with no new runtime behavior. But this change does allow a nicer way of declaring types in #16545, using c++ types like int instead of flags like ALLOW_INT, orstd::vector<std::string> instead of ALLOW_STRING | ALLOW_LIST.

  2. DrahtBot commented at 10:34 pm on November 8, 2024: 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/31260.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK l0rinc

    If your review is incorrectly listed, please react with πŸ‘Ž to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #31492 (Execute Discover() when bind=0.0.0.0 or :: is set by andremralves)
    • #31483 (kernel: Move kernel-related cache constants to kernel cache by TheCharlatan)
    • #31384 (mining: bugfix: Fix duplicate coinbase tx weight reservation by ismaelsadeeq)
    • #31250 (wallet: Disable creating and loading legacy wallets by achow101)
    • #31223 (net, init: derive default onion port if a user specified a -port by mzumsande)
    • #31215 (rpc: increase the defaults for -rpcthreads and -rpcworkqueue by vasild)
    • #31061 (refactor: Check translatable format strings at compile-time by maflcko)
    • #30965 (kernel: Move block tree db open to block manager by TheCharlatan)
    • #30951 (net: option to disallow v1 connection on ipv4 and ipv6 peers by stratospher)
    • #30727 (rpc: add address_type field in getaddressinfo by jonatack)
    • #30529 (Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior by ryanofsky)
    • #30343 (wallet, logging: Replace WalletLogPrintf() with LogInfo() by ryanofsky)
    • #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
    • #30221 (wallet: Ensure best block matches wallet scan state by achow101)
    • #30157 (Fee Estimation via Fee rate Forecasters by ismaelsadeeq)
    • #29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)
    • #29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
    • #29365 (Extend signetchallenge to set target block spacing by starius)
    • #29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)
    • #28802 (ArgsManager: support subcommand-specific options by ajtowns)
    • #28792 (Embed default ASMap as binary dump header file by fjahr)
    • #28710 (Remove the legacy wallet and BDB dependency by achow101)
    • #28463 (p2p: Increase inbound capacity for block-relay only connections by mzumsande)
    • #26988 (cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency by stratospher)
    • #26966 (index: initial sync speedup, parallelize process by furszy)
    • #26022 (Add util::ResultPtr class by ryanofsky)
    • #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
    • #25665 (refactor: Add util::Result failure values, multiple error and warning messages by ryanofsky)
    • #24539 (Add a “tx output spender” index by sstone)
    • #19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
    • #19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
    • #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)
    • #16545 (refactor: Implement missing error checking for ArgsManager flags by ryanofsky)
    • #10102 (Multiprocess bitcoin 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.

  3. ryanofsky marked this as a draft on Nov 8, 2024
  4. DrahtBot added the label Needs rebase on Nov 8, 2024
  5. ryanofsky commented at 11:36 pm on November 8, 2024: contributor
    Current status of this PR is that bitcoind and test_bitcoin binaries work and functional and unit tests pass, but there are compile errors in the other binaries that need to be fixed, and this also needs to be rebased. The PR is complete with all functionality described above implemented, but it probably needs more documentation. I also would like to add more commits replacing last remaining GetArg / GetIntArg / GetBoolArg / GetArgs / IsArgSet / IsArgNegated method uses with Setting::Get and dropping all those methods.
  6. ryanofsky force-pushed on Nov 13, 2024
  7. ryanofsky force-pushed on Nov 13, 2024
  8. ryanofsky commented at 0:36 am on November 13, 2024: contributor
    Updated 416860fc360b3d5aa1a0782ab8f8454b46e6d657 -> 47e30b40eca0eabf85d45e91fc247a74f3a346e8 (pr/scripty.1 -> pr/scripty.2, compare) getting the remaining binaries (not just test_bitcoin and bitcoind) to build, simplifying the way optional and default values are used, making many other cleanups and fixes. Rebased 47e30b40eca0eabf85d45e91fc247a74f3a346e8 -> d8a4a0a8f760cf82cc7f71dfbd3140d11db95790 (pr/scripty.2 -> pr/scripty.3, compare) due to conflicts Updated d8a4a0a8f760cf82cc7f71dfbd3140d11db95790 -> ad32a27e966eac118587cf0e03c1eedaf0c051ed (pr/scripty.3 -> pr/scripty.4, compare) with a fix for the feature_logging.py test that got broken by the previous optional/default changes. Also includes cmake changes to fix some CI build errors. Updated ad32a27e966eac118587cf0e03c1eedaf0c051ed -> 534f9713c9a4a12b8c276585ed6ae96a25d4bc09 (pr/scripty.4 -> pr/scripty.5, compare) replacing IsArgSet calls with Setting::Value().isNull() instead of using std::optional and Setting::Get() to avoid changing any behavior since Get() fully parses the setting and can throw exceptions, and to reduce uses of std::optional which complicated code. Also add basic unit tests and make various script improvements. Rebased 534f9713c9a4a12b8c276585ed6ae96a25d4bc09 -> 7e572376bdf2e13e896a1aa8f87c3701e2446684 (pr/scripty.5 -> pr/scripty.6, compare) with a number of changes intended to fix CI errors, and with a fix for silent merge conflict with #31174 Rebased 7e572376bdf2e13e896a1aa8f87c3701e2446684 -> 1f243467cfcaa72a8f665141bdb4e5c5af668dfe (pr/scripty.6 -> pr/scripty.7, compare) due to conflict #31287 and many workarounds for various compiler issues in CI and fixes for more linters
  9. DrahtBot added the label CI failed on Nov 13, 2024
  10. DrahtBot commented at 1:21 am on November 13, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/32896439725

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  11. DrahtBot removed the label Needs rebase on Nov 13, 2024
  12. ryanofsky force-pushed on Nov 13, 2024
  13. ryanofsky force-pushed on Nov 14, 2024
  14. DrahtBot added the label Needs rebase on Nov 14, 2024
  15. ryanofsky force-pushed on Nov 14, 2024
  16. DrahtBot removed the label Needs rebase on Nov 14, 2024
  17. DrahtBot added the label Needs rebase on Nov 15, 2024
  18. ryanofsky force-pushed on Nov 15, 2024
  19. ryanofsky renamed this:
    WIP: scripted-diff: Type-safe settings retrieval
    scripted-diff: Type-safe settings retrieval
    on Nov 15, 2024
  20. ryanofsky marked this as ready for review on Nov 15, 2024
  21. ryanofsky commented at 9:32 pm on November 15, 2024: contributor

    Pushed a lot of fixes and updates over the past week and marking this PR as no longer wip/draft. This PR is just a refactoring and doesn’t change runtime behavior, but it should be a good start to having better defined settings with clear types and default values, avoiding confusion and bugs caused by the current settings API seen in #30529 and #31212 and other PRs, and making API more extensible to support custom types and validation in the future.


    Updated 1f243467cfcaa72a8f665141bdb4e5c5af668dfe -> c42d27d8c844b721b5aff384b3d014b1e0cc4783 (pr/scripty.7 -> pr/scripty.8, compare) to fix remaining CI issues.

  22. DrahtBot removed the label Needs rebase on Nov 15, 2024
  23. ryanofsky force-pushed on Nov 16, 2024
  24. DrahtBot commented at 7:22 am on November 20, 2024: contributor
    This needs the tidy CI task issues fixed up
  25. DrahtBot added the label Needs rebase on Nov 20, 2024
  26. in src/common/setting.h:39 in f9910935c8 outdated
    34+    constexpr StringLiteral(std::nullptr_t) {
    35+    }
    36+    T value{};
    37+};
    38+
    39+StringLiteral(std::nullptr_t) -> StringLiteral<1, std::nullptr_t>;
    


    hodlinator commented at 3:16 pm on November 20, 2024:

    In f9910935c880c782c2b1d35acd38e1e18e6b4ad8: What is StringLiteral(std::nullptr_t) ->? I take it some kind of template specialization but haven’t come across regular braces and arrows being used in this way before.

    Edit: It could be a constructor but I would expect StringLiteral::StringLiteral as it appears outside the type definition. It kind of looks like a function prototype C++11-style return type, but there’s no body. Happy to learn more modern rules for things, I’m a bit out of date.


    ryanofsky commented at 9:02 pm on November 22, 2024:

    re: #31260 (review)

    What is StringLiteral(std::nullptr_t) ->?

    This is a template deduction guide used to be able to pass nullptr_t as a string literal (used for a few settings that are only retrieved and never registered). I could explain more but chatgpt is much more great at this type of question: https://chatgpt.com/share/6740f058-e900-800a-afdf-7b56760db068

  27. in src/common/args.cpp:282 in c42d27d8c8 outdated
    279+    return SettingToPath(GetSetting(arg)).value_or(default_value);
    280+}
    281+
    282+std::optional<fs::path> SettingToPath(const common::SettingsValue& value)
    283+{
    284+    if (value.isFalse()) return fs::path{};
    


    hodlinator commented at 4:06 pm on November 20, 2024:
    Is isFalse() equivalent to the former IsArgNegated() use?

    ryanofsky commented at 9:06 pm on November 22, 2024:

    re: #31260 (review)

    Is isFalse() equivalent to the former IsArgNegated() use?

    Yes exactly,, if you look at the definition of IsArgNegated() it is just returning isFalse():

    https://github.com/bitcoin/bitcoin/blob/2638fdb4f934be96b7c798dbac38ea5ab8a6374a/src/common/args.cpp#L454

    This works because of the translation of negated values to false in InterpretValue:

    https://github.com/bitcoin/bitcoin/blob/2638fdb4f934be96b7c798dbac38ea5ab8a6374a/src/common/args.cpp#L121

  28. in src/bench/bench_bitcoin_settings.h:17 in c42d27d8c8 outdated
    12+#include <vector>
    13+
    14+static const char* DEFAULT_BENCH_FILTER = ".*";
    15+static constexpr int64_t DEFAULT_MIN_TIME_MS{10};
    16+/** Priority level default value, run "all" priority levels */
    17+static const std::string DEFAULT_PRIORITY{"all"};
    


    hodlinator commented at 4:39 pm on November 20, 2024:
    In b51a156f388926175afdbc50c2d563db404e3b81: I’ve learned to accept static constexpr (+inline…) in headers, but these other non-constexpr statics seem like they end up being “file-local” static vars in every compilation unit that includes them. Seems off?

    ryanofsky commented at 9:36 pm on November 22, 2024:

    re: #31260 (review)

    In b51a156: I’ve learned to accept static constexpr (+inline…) in headers, but these other non-constexpr statics seem like they end up being “file-local” static vars in every compilation unit that includes them. Seems off?

    Good catch. I wanted to avoid changing these to keep the commit move-only as much as possible but it doesn’t make sense to have std::strings that could do allocations duplicated across translation units, so they are switched to constexpr string literal types now. I think the other static const values should not be a problem since they should just be stripped out if not referenced and not create any symbols, so I did leave those, but could change if preferred.

  29. in contrib/devtools/circular-dependencies.py:80 in c42d27d8c8 outdated
    72@@ -55,8 +73,19 @@ def module_name(path):
    73             if match:
    74                 include = match.group(1)
    75                 included_module = module_name(include)
    76-                if included_module is not None and included_module in deps and included_module != module:
    77+                if included_module and included_module in deps and included_module != module:
    78                     deps[module].add(included_module)
    79+                included_by[include].add(arg)
    80+
    81+# Trigger an error if any module listed as being nontransitive if ever included
    


    hodlinator commented at 4:50 pm on November 20, 2024:
    0# Trigger an error if any module listed as being nontransitive is ever included
    

    ryanofsky commented at 9:41 pm on November 22, 2024:

    re: #31260 (review)

    Thanks, fixed comment

  30. in src/init.cpp:914 in c42d27d8c8 outdated
    912         }
    913     }
    914 
    915     // If -forcednsseed is set to true, ensure -dnsseed has not been set to false
    916-    if (args.GetBoolArg("-forcednsseed", DEFAULT_FORCEDNSSEED) && !args.GetBoolArg("-dnsseed", DEFAULT_DNSSEED)){
    917+    if (ForcednsseedSetting::Get(args) && !DnsseedSetting::Get(args, DEFAULT_DNSSEED)){
    


    hodlinator commented at 5:03 pm on November 20, 2024:

    How come we keep the default value in one of the cases here but not the other, both are using .GetBoolArg in the original?

    Seems to compile fine when switching DnsseedSetting from using ::HelpArgs<DEFAULT_DNSSEED> to ::Default<DEFAULT_DNSSEED> like the ForcednsseedSetting.


    ryanofsky commented at 9:43 pm on November 22, 2024:

    re: #31260 (review)

    How come we keep the default value in one of the cases here but not the other, both are using .GetBoolArg in the original?

    The scripted diff will only assign default values to Setting types if the same default value is used every place a setting is retrieved.

    In this case the -dnsseed setting is retrieved 5 different places, and 4 of the places look like args.GetBoolArg("-dnsseed", DEFAULT_DNSSEED) where DEFAULT_DNSSEED is true and the return value is bool but one place looks like args.GetBoolArg("-dnsseed") where the return value is std::optional<bool> and the default value is std::nullopt.

    Looking at the surrounding code, it seems like this inconsistency is intentional. The code treats an explicitly set -dnsseed differently than the default value and triggers an error if it is specified along with with -onlynet. Conceptually it seems like the -dnseed setting really has three values: “yes” “no” and “always” and the default value is “yes” but an explicitly set -dnsseed value means “always” and triggers an error if it cannot be satisfied.

    A cleaner way to implement support for -dnsseed would probably be to define an Options struct and interpret -dnseed and related settings one place instead of 5 different places. But for the purposes of this PR, no behavior is changing and new code basically looks the same as the old code and shouldn’t be much more or much less confusing.


    l0rinc commented at 1:53 pm on December 10, 2024:
    I’m also having trouble with these defaults

    ryanofsky commented at 0:02 am on December 12, 2024:

    re: #31260 (review)

    I’m also having trouble with these defaults

    Hopefully it should be obvious that behavior is not changing:

    • ForceDnsSeedSetting::Get(args) returns the same thing as args.GetBoolArg("-forcednsseed", DEFAULT_FORCEDNSSEED) because of the type and default value specified in the ForceDnsSeedSetting definition
    • DnsSeedSetting::Get(args, DEFAULT_DNSSEED) returns the same thing as args.GetBoolArg("-dnsseed", DEFAULT_DNSSEED) because of the type and default value specified in the DnsSeedSetting definition

    But let me know if there is something else you would like to know!

  31. in src/test/setting_tests.cpp:65 in c42d27d8c8 outdated
    59+BOOST_AUTO_TEST_CASE(GetOptional)
    60+{
    61+    BOOST_CHECK_EQUAL(SettingTest<std::optional<int>>{}.Parse().Get(), std::nullopt);
    62+    BOOST_CHECK_EQUAL(SettingTest<std::optional<int>>{}.AddArg("-s=3").Parse().Get(), 3);
    63+    BOOST_CHECK_EQUAL(SettingTest<std::optional<int>>{}.AddArg("-s=3").Parse().Value().write(), "\"3\"");
    64+    BOOST_CHECK_EQUAL(SettingTest<std::optional<int>>{}.AddArg("-nos=1").Parse().Get(), 0);
    


    hodlinator commented at 5:09 pm on November 20, 2024:

    Maybe add value-less negation?

    0BOOST_CHECK_EQUAL(SettingTest<std::optional<int>>{}.AddArg("-nos").Parse().Get(), 0);
    

    ryanofsky commented at 10:22 pm on November 22, 2024:

    re: #31260 (review)

    Maybe add value-less negation?

    Thanks! Added. The test is currently very minimal, and I hope to do more with it, especially after this PR when I would like to add support for runtime settings validation.

  32. in contrib/devtools/reg-settings.py:446 in f33c0b1969 outdated
    441+            # FIXME handle these by generating synthetic AddArg calls without corresponding Register()
    442+            if get.add is None:
    443+                #import pprint
    444+                #print("*"*80)
    445+                #print(f"Bad get call with no corresponding type")
    446+                #pprint.pprint(get)
    


    hodlinator commented at 7:59 pm on November 20, 2024:
    In f33c0b1969ee4c1c36475c876f46d9085feee134: WIP?

    ryanofsky commented at 10:33 pm on November 22, 2024:

    re: #31260 (review)

    WIP?

    Unclear right now. Adding this wasn’t as necessary as I initially thought it might be to remove most of the GetArg* calls in the codebase. But I am looking into more ways to expand the PR and maybe drop the GetArg methods methods entirely. Depending on how much work this it could be a follow-up and in any case doesn’t need to block anything.

  33. in src/common/setting.h:66 in c42d27d8c8 outdated
    61+    static value_t Get(auto& manager, const value_t& default_value)
    62+    {
    63+        return internal::SettingGet<value_t, options>(manager, summary.value, [&] { return default_value; }, get_fn);
    64+    }
    65+
    66+    static SettingsValue Value(auto& manager)
    


    hodlinator commented at 9:07 pm on November 20, 2024:

    In f9910935c880c782c2b1d35acd38e1e18e6b4ad8: Better to document type? This style of using auto as function-arg-type when not necessary feels like the opposite of C++ concepts, potentially leading to downstream compile errors. But interested to hear reasoning behind it.

     0    static void Register(ArgsManager& manager, auto&&... register_options)
     1    {
     2        internal::SettingRegister<T, options, help>(manager, summary, category, help_fn, default_fn, get_fn, register_options...);
     3    }
     4
     5    static T Get(const ArgsManager& manager)
     6    {
     7        return internal::SettingGet<T, options>(manager, summary.value, default_fn, get_fn);
     8    }
     9
    10    static value_t Get(const ArgsManager& manager, const value_t& default_value)
    11    {
    12        return internal::SettingGet<value_t, options>(manager, summary.value, [&] { return default_value; }, get_fn);
    13    }
    14
    15    static SettingsValue Value(const ArgsManager& manager)
    

    ryanofsky commented at 10:35 pm on November 22, 2024:

    reL #31260 (review)

    In f991093: Better to document type? This style of using auto as function-arg-type when not necessary feels like the opposite of C++ concepts, potentially leading to downstream compile errors. But interested to hear reasoning behind it.

    Good catch, this was just done early in a prototype and I never changed it. Updated to use explicit types now.

  34. in src/common/setting_internal.h:147 in c42d27d8c8 outdated
    142+    using type = decltype(value);
    143+    constexpr auto operator()() { return value; }
    144+};
    145+
    146+template<typename T, SettingOptions options, auto help>
    147+void SettingRegister(auto& manager, auto summary, auto category, auto help_fn, auto default_fn, auto get_fn, auto&&... register_options)
    


    hodlinator commented at 9:08 pm on November 20, 2024:

    Document types and remove unused T?

    0template<SettingOptions options, auto help>
    1void SettingRegister(ArgsManager& manager, auto summary, OptionsCategory category, auto help_fn, auto default_fn, auto get_fn, auto&&... register_options)
    

    ryanofsky commented at 10:36 pm on November 22, 2024:

    reL #31260 (review)

    Document types and remove unused T?

    Thanks! Added types. I do want to keep T around because my changes in #22978 branch adding runtime setting validation need to know the setting type (also so T and options are available consistently as template parameters).

  35. in src/common/setting_internal.h:171 in c42d27d8c8 outdated
    166+        return default_fn();
    167+    }
    168+}
    169+
    170+template<typename T, SettingOptions options>
    171+T SettingGet(auto& manager, std::string_view summary, auto default_fn, auto get_fn)
    


    hodlinator commented at 9:21 pm on November 20, 2024:
    0T SettingGet(const ArgsManager& manager, std::string_view summary, auto default_fn, auto get_fn)
    

    ryanofsky commented at 10:37 pm on November 22, 2024:

    re: #31260 (review)

    Thanks! Applied change.

  36. hodlinator commented at 9:35 pm on November 20, 2024: contributor

    High-level review of c42d27d8c844b721b5aff384b3d014b1e0cc4783

    Review instructions are a nice introduction. Feels a bit risky having links to personal branch instead of specific commit, but it’s good to keep it flexible until ACKs come in.


    Negation?

    I’m curious how you foresee negation checks being done. Having worked on proper handling of negated args recently, to me it feels like user code should have to jump through a hoop on the way to the non-negated value, if an arg supports negation, to encourage conscious negation logic. common::Disabled is currently only used in tests, this goes against this comment from #16545: https://github.com/bitcoin/bitcoin/blob/b5ef85497436c3e9e60c760465d8991592efef07/src/common/args.h#L112-L114


    Impact on compile time from template magic?

    Compiler: GCC 13.3.0 Ran ccache --clear and deleted cmake build directory before each run.

    Before

    0Run [#1](/bitcoin-bitcoin/1/)
    1real	5m13.786s
    2user	63m16.112s
    3sys	4m18.988s
    4
    5Run [#2](/bitcoin-bitcoin/2/)
    6real	5m18.751s
    7user	65m23.916s
    8sys	4m29.586s
    

    After

    0Run [#1](/bitcoin-bitcoin/1/)
    1real	5m16.808s
    2user	65m15.770s
    3sys	4m27.238s
    4
    5Run [#2](/bitcoin-bitcoin/2/)
    6real	5m21.099s
    7user	66m22.265s
    8sys	4m32.162s
    

    Conclusion

    Slightly faster before, but overlapping timings. Should be okay.


    Commit message for f310e17e73bd1cea74aa45fd94192f24df122f86:

    Typo: “headers from being >be< included”


    Value(args).isNull()

    DatadirSetting::Value(args).isNull() vs DatadirSettingPath::Get(args) - feels weird to call the former Value() … it’s like std::optional::value() but in reverse. The fact that settings are backed by UniValues help explain it, but still looks inelegant. I see this style comes from a latter revision to trigger parsing and possible exceptions. Maybe a DatadirSetting::IsArgSet(args) wrapper could be added?


    CamelCasing

    Couldn’t we add a dictionary for proper CamelCasing of settings names so that

    • ZmqpubrawtxSetting -> ZmqPubRawTxSetting
    • StopafterblockimportSetting -> StopAfterBlockImportSetting etc?
  37. ryanofsky force-pushed on Nov 22, 2024
  38. DrahtBot removed the label Needs rebase on Nov 22, 2024
  39. ryanofsky commented at 11:37 pm on November 22, 2024: contributor

    Rebased c42d27d8c844b721b5aff384b3d014b1e0cc4783 -> 27083ecbc221095585cfff3bb208850e564723fd (pr/scripty.8 -> pr/scripty.9, compare) to fix conflict with #31317 including suggested changes and some changes to the scripted diff to cover more arguments. Updated 27083ecbc221095585cfff3bb208850e564723fd -> b9628a08df0b64c24ee15fd15cdc3bb3b2c6d129 (pr/scripty.9 -> pr/scripty.10, compare) expanding to cover hidden arguments and fixing temporary bug in early commit to fix for-each-commit job Updated b9628a08df0b64c24ee15fd15cdc3bb3b2c6d129 -> 4e33ed4eb054e230436b68c681d978d0e7bea0a1 (pr/scripty.10 -> pr/scripty.11, compare) with fix for lint error https://cirrus-ci.com/task/6667770497073152 Rebased 4e33ed4eb054e230436b68c681d978d0e7bea0a1 -> a4a5226b5b7f240a87a5f078cf0b6a475679b8ab (pr/scripty.11 -> pr/scripty.12, compare) due to conflict with #31212

    re: #31260#pullrequestreview-2448864512

    Thanks for the review and testing! The compiler benchmarking is especially interesting and commit message typo should be fixed now. The camelcasing dictionary is also a great idea and I will definitely implement that.

    Negation?

    I’m curious how you foresee negation checks being done. Having worked on proper handling of negated args recently, to me it feels like user code should have to jump through a hoop on the way to the non-negated value, if an arg supports negation, to encourage conscious negation logic. common::Disabled is currently only used in tests, this goes against this comment from #16545:

    In most cases, it makes sense for negated bool arguments to be false, negated int arguments to be 0, and negated string/path/list arguments to be empty. With the Setting class and default types these conversions are made automatically, except that for backward compatibility when .legacy = true is set, negated strings are returned as “0”. In cases where the default behavior doesn’t make sense, there is support for std::variant in my #22978 branch, where custom negation behavior can be implemented by choosing a setting type like std::variant<Disabled, T>.

    From #22978 (comment): “std::variant<Disabled, T> might be useful for special cases or backwards compatibility to detect negated values and treat them differently from 0 and “”. For example if you had a -bwlimit setting and wanted to treat -nobwlimit as “use unlimited bandwidth” and -bwlimit=0 as “use no bandwidth” using a variant with a Disabled member allows that.”

    Value(args).isNull()

    DatadirSetting::Value(args).isNull() vs DatadirSettingPath::Get(args) - feels weird to call the former Value() … it’s like std::optional::value() but in reverse. The fact that settings are backed by UniValues help explain it, but still looks inelegant. I see this style comes from a latter revision to trigger parsing and possible exceptions.

    I tend to agree but just to explain the thinking, the method is called Value() because it returns the JSON SettingsValue type, and the name makes some sense because it is returning the raw JSON value without interpreting it as a bool/int/string/path or other type. Using the Value() method should be pretty strongly discouraged, so it’s not neccesarily a bad thing if usage looks inelegant. This PR is only using Value() in the scripted-diff commit to emulate IsArgSet and IsArgNegated behavior and avoid the parsing exceptions you mentioned which could change behavior of the current code and break compatibility.

    But all the code using Value() would be better off not using it. This could be done by switching to better default values or by switching to std::optional<> or std::variant<> types. There should be basically be no reason to call Value() in new code and your PR #31212 and my #30529 will eliminate a bunch of uses of Value() here assuming they are merged before this PR.

    Maybe a DatadirSetting::IsArgSet(args) wrapper could be added?

    I would want to avoid this just because I think the IsArgSet function is a footgun. I think it’s preferable to use Default<> and DefaultFn<> to set static defaults whenever possible, and to use to use the std::optional<> when dynamic defaults are needed, and to never call IsArgSet in any case.

  40. DrahtBot removed the label CI failed on Nov 23, 2024
  41. DrahtBot added the label Refactoring on Nov 23, 2024
  42. ryanofsky force-pushed on Nov 25, 2024
  43. DrahtBot added the label CI failed on Nov 25, 2024
  44. DrahtBot commented at 9:43 pm on November 25, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/33503226227

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  45. ryanofsky force-pushed on Nov 27, 2024
  46. DrahtBot removed the label CI failed on Nov 27, 2024
  47. common: expose SettingToPath function bbc782eff9
  48. common: Add Setting class to support typed Settings 8472506ee3
  49. test: Add test for common::Setting class 6cebe45a71
  50. init, refactor: Prepare AddArg calls for scripted-diff
    Avoid string concatenation and add missing namespace names so AddArg calls can
    be moved to header files in an upcoming scripted-diff.
    d2cd327188
  51. move-only: move AddArg default values to headers
    Move constant declarations referenced in AddArg calls to headers so AddArg
    calls be moved to header files in an upcoming scripted-diff.
    9a6572d38d
  52. refactor: Prepare AddHiddenArgs call for scripted-diff
    Drop hidden_args vector so all AddHiddenArgs calls can be replaced in upcoming
    scripted diff.
    8ae020bafe
  53. lint: Fixes for _settings.h headers
    Linter changes to deal with _settings.h headers added in the next commit.
    
    In circular-dependencies.py, prevent _settings.h headers from being included
    transitively. Add a new check that makes it an error to include _settings.h
    from another header file, rather than a .cpp file. Use this fact to avoid
    errors about .cpp files including _settings.h files being circularly dependent
    based on each other.
    
    In lint-format-strings.py, skip _settings.h because they contain lines like
    strformat(fmt, ...) where fmt is a compile time constant that is checked at
    compile time by the compiler and can't be checked by this linter.
    
    In check-doc.py update test to look for Setting::Get and Setting::Register
    instead of GetArg and AddArg.
    c55ed9f483
  54. contrib: Add script to replace AddArgs / GetArgs calls with Setting Register / Get calls 1149c867c6
  55. scripted-diff: Replace AddArgs / GetArgs calls with Setting Register / Get calls
    This commit is a pure refactoring and does not change behavior in any way.
    
    -BEGIN VERIFY SCRIPT-
    python contrib/devtools/reg-settings.py
    git add -N src/bench/bench_bitcoin_settings.h src/bitcoin-tx_settings.h src/bitcoin-util_settings.h src/bitcoin-wallet_settings.h src/chainparamsbase_settings.h src/common/args_settings.h src/init/common_settings.h src/init_settings.h src/qt/bitcoin_settings.h src/test/argsman_tests_settings.h src/test/logging_tests_settings.h src/wallet/init_settings.h src/dummywallet_settings.h src/qt/test/optiontests_settings.h src/test/getarg_tests_settings.h
    git rm contrib/devtools/reg-settings.py
    -END VERIFY SCRIPT-
    e12edf7661
  56. ryanofsky force-pushed on Dec 4, 2024
  57. in src/chainparams.cpp:30 in a4a5226b5b outdated
    26@@ -25,11 +27,11 @@ using util::SplitString;
    27 
    28 void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& options)
    29 {
    30-    if (args.IsArgSet("-signetseednode")) {
    31-        options.seeds.emplace(args.GetArgs("-signetseednode"));
    32+    if (!SignetseednodeSetting::Value(args).isNull()) {
    


    l0rinc commented at 11:44 am on December 10, 2024:

    existence checking seems to be relied on often, maybe we could add an IsSet method to Settings (maybe even an bool isSet() const { return (typ != VNULL); } to UniValie):

    0    static bool IsSet(const ArgsManager& manager)
    1    {
    2        return !Value(manager).isNull();
    3    }
    

    simplifying these to:

    0    if (SignetseednodeSetting::IsSet(args)) {
    

    ryanofsky commented at 8:49 pm on December 11, 2024:

    re: #31260 (review)

    existence checking seems to be relied on often, maybe we could add an IsSet method to Settings (maybe even an bool isSet() const { return (typ != VNULL); } to UniValie):

    Existence checking is a pattern that causes many bugs and be discouraged The case you looking at here is removed by #30529, which will hopefully be merged before this PR. All other calls like this should also be removed.

    The simplest, safest, and most reliable way of handling unset settings is to use Default<> or DefaultFn<> options to provide explicit defaults that can be automatically returned by Setting::Get calls and shown in help information.

    In cases where the default behavior can’t be expressed as an explicit default value, the next best thing is to declare the setting with std::optional type so checking for unset values at least is mandatory and can be done reliably, avoiding bugs seen in #30529 and other places.


    l0rinc commented at 2:03 pm on December 12, 2024:
    Nice, adding sensible default values is indeed a lot better than checking if it was set or not. Can you make this PR a draft until the dependent PRs are merged to signal that this isn’t its final form?

    ryanofsky commented at 8:31 pm on December 13, 2024:

    re: #31260 (review)

    Can you make this PR a draft until the dependent PRs are merged to signal that this isn’t its final form?

    This PR is ready to review, doesn’t have any dependent PRs, and is not a draft, though it is possible I may split some of the initial commits here off into another PR, and in that case I will mark this as a draft.

    There isn’t much intersection between this PR and #30529, except that if #30529 gets merged before this PR (which I assume it will), then this line in the scripted diff output will disappear.

  58. in src/common/setting.h:70 in a4a5226b5b outdated
    44+namespace common {
    45+//! Setting template class used to declare compile-time Setting types which are
    46+//! used to register and retrieve settings.
    47+template<StringLiteral summary, typename T, SettingOptions options = SettingOptions{}, StringLiteral help = nullptr, auto help_fn = nullptr, auto default_fn = nullptr, auto get_fn = nullptr, OptionsCategory category = OptionsCategory::OPTIONS>
    48+struct Setting {
    49+    using value_t = internal::RemoveOptional<T>::type;
    


    l0rinc commented at 11:48 am on December 10, 2024:

    My IDE suggests that we use the typename keyword before such dependent types:

    0    using value_t = typename internal::RemoveOptional<T>::type;
    

    ryanofsky commented at 10:34 pm on December 11, 2024:

    re: #31260 (review)

    My IDE suggests that we use the typename keyword before such dependent types:

    I think this code is working with at least 3 different compilers (gcc, clang, msvc). Do you think the compilers are failing to flag this? Or that this is change would be helpful for style reasons?


    l0rinc commented at 2:05 pm on December 12, 2024:
    I hinted at the IDE complaining since I could exactly find out why this was necessary - if you can’t see the value in it either, please resolve this comment.

    ryanofsky commented at 8:33 pm on December 13, 2024:

    re: #31260 (review)

    I hinted at the IDE complaining since I could exactly find out why this was necessary - if you can’t see the value in it either, please resolve this comment.

    No problem, thanks for clarifying and thanks for the hint!

  59. in src/chainparamsbase_settings.h:10 in a4a5226b5b outdated
     5+#include <common/setting.h>
     6+
     7+#include <string>
     8+#include <vector>
     9+
    10+using SignetseednodeSetting = common::Setting<
    


    l0rinc commented at 11:57 am on December 10, 2024:

    It seems to me we’re using suffix naming to group values - could we use a namespace instead?

    0namespace Setting {
    1using Signetseednode = common::Setting<
    2    "-signetseednode", std::vector<std::string>, common::SettingOptions{.legacy = true, .disallow_negation = true},
    3    "Specify a seed node for the signet network, in the hostname[:port] format, e.g. sig.net:1234 (may be used multiple times to specify multiple seed nodes; defaults to the global default signet test network seed node(s))">
    4    ::Category<OptionsCategory::CHAINPARAMS>;
    

    referring to it as:

    0        options.seeds.emplace(Setting::Signetseednode::Get(args));
    

    ryanofsky commented at 10:27 pm on December 11, 2024:

    re: #31260 (review)

    It seems to me we’re using suffix naming to group values - could we use a namespace instead?

    I think it’s best when c++ namespaces reflect directory structure of the project, so when you see a util:: reference you know it is referring to something in src/util/, wallet:: you know it is referring to something in src/wallet/, etc.

    I don’t think I see value in having a Setting namespace, and think it could cause confusion if there could be clashes between a top level Setting namespace and a wallet::Setting namespace and the common::Setting class. In general I think overloading like this should be avoided if it is just going add mental overhead without providing other value.

    Also BlocksDirSetting::Get reads better to me than Setting::BlocksDir::Get, “blocks dir setting get” over “setting blocksdir get”, but of course that’s very subjective.


    l0rinc commented at 2:38 pm on December 12, 2024:

    The part that bothers me is that this seems “Stringly-Typed” this way, global scoped and the user doesn’t even have proper code completion since there isn’t a common prefix or namespace or enum to narrow the options for code completion as far as I can tell. I’m not recommending a top level settings (but even that would be better than these globals), but rather targeted nested namespaces (e.g., wallet::setting, init::setting) or something that would group these in a way that the compiler understands instead of just having a naming convention.

    Also BlocksDirSetting::Get reads better to me than Setting::BlocksDir::Get

    What about inverting it and making the Get method accept different types based on the context, something like: Init::Setting::Get<TestActivationHeight>(args)? I haven’t thought this through (I can’t meaningfully review a change this big, we need to find a way to narrow it down), I’m just wondering how we could make the compiler work for us instead of coming up with special names.


    ryanofsky commented at 8:43 pm on December 13, 2024:

    re: #31260 (review)

    The part that bothers me is that this seems “Stringly-Typed” this way, global scoped and the user doesn’t even have proper code completion since there isn’t a common prefix or namespace or enum to narrow the options for code completion as far as I can tell.

    That’s a good point. I think better code completion is a real reason to want to group these identifiers. I will try to implement that in my next push, probably using namespace settings nested in any existing namespaces. Sorry for resisting before, I think I just really didn’t like the ambiguity of having a class and namespace with the same name.

    On the inversion idea writing Get<TestActivationHeight> instead of TestActivationHeight::Get, it wouldn’t be my preference just because of the template syntax, but it seems fine if other reviewers prefer it or it solves other problems.

  60. in src/common/setting_internal.h:160 in a4a5226b5b outdated
    155+        help_str = help.value;
    156+    }
    157+    manager.AddArg(summary.value, help_str, SettingFlags<options>(), category);
    158+}
    159+
    160+template<typename T, SettingOptions options>
    


    l0rinc commented at 12:00 pm on December 10, 2024:
    options seems unused here

    ryanofsky commented at 10:45 pm on December 11, 2024:

    re: #31260 (review)

    options seems unused here

    It should be used in followup PR (options will need to be passed to the traits class to implement support for std::variant settings). Also I’m tying to be consistent about accepting T and options together most places so almost all the functions accept the same template parameters.


    l0rinc commented at 2:39 pm on December 12, 2024:
    Could we introduce it in the PR that will actually need it instead?

    ryanofsky commented at 8:56 pm on December 13, 2024:

    re: #31260 (review)

    Could we introduce it in the PR that will actually need it instead?

    I don’t want to introduce an inconsistency and make options inaccessible to code which needs to use it, encouraging hackier less consistent ways of passing options around. I’ll just move it to the setting traits class so it will be used.

  61. in src/common/setting_internal.h:171 in a4a5226b5b outdated
    166+        return default_fn();
    167+    }
    168+}
    169+
    170+template<typename T, SettingOptions options>
    171+T SettingGet(const ArgsManager& manager, std::string_view summary, auto default_fn, auto get_fn)
    


    l0rinc commented at 12:11 pm on December 10, 2024:
    auto default_fn is bit surprising here, superficially seems like an over-generalized getter. Will review it later in more detail (is it some constexpr constraint? Will check later…).

    ryanofsky commented at 10:46 pm on December 11, 2024:

    re: #31260 (review)

    auto default_fn is bit surprising here, superficially seems like an over-generalized getter. Will review it later in more detail (is it some constexpr constraint? Will check later…).

    If specified it should be a lambda. I don’t think there is another type besides auto this could realistically use.

  62. in src/common/setting_internal.h:147 in a4a5226b5b outdated
    142+    using type = decltype(value);
    143+    constexpr auto operator()() { return value; }
    144+};
    145+
    146+template<typename T, SettingOptions options, auto help>
    147+void SettingRegister(ArgsManager& manager, auto summary, OptionsCategory category, auto help_fn, auto default_fn, auto get_fn, auto&&... register_options)
    


    l0rinc commented at 12:12 pm on December 10, 2024:
    get_fn seems unused

    ryanofsky commented at 10:50 pm on December 11, 2024:

    re: #31260 (review)

    get_fn seems unused

    Good catch. Will follow up and probably drop from this PR. This comes from the branch in #22978 and is used to support custom types for settings like structs and enums.

  63. in src/common/setting_internal.h:31 in a4a5226b5b outdated
    26+template<typename T, auto get_fn = nullptr>
    27+struct SettingTraitsBase
    28+{
    29+    using setting_t = T;
    30+    static constexpr bool is_list{false};
    31+    static bool Get(const common::SettingsValue& setting, setting_t& out)
    


    l0rinc commented at 12:15 pm on December 10, 2024:
    nit: we’re already inide common

    ryanofsky commented at 10:34 pm on December 11, 2024:

    re: #31260 (review)

    nit: we’re already inide common

    Nice catch, simplified here and elsewhere.

  64. in src/common/setting_internal.h:44 in a4a5226b5b outdated
    39+        }
    40+        return false;
    41+    }
    42+};
    43+
    44+template<typename T, SettingOptions options>
    


    l0rinc commented at 12:24 pm on December 10, 2024:

    In other cases we’ve used a single template method, instead of each type or case having its own explicit implementation - would it make sense to unify this as well?

     0template<typename T, SettingOptions options>
     1struct SettingTraits
     2{
     3    static bool Get(const SettingsValue& setting, T&)
     4    {
     5        if constexpr (std::is_same_v<T, Unset>) return setting.isNull();
     6        if constexpr (std::is_same_v<T, Enabled>) return setting.isTrue();
     7        if constexpr (std::is_same_v<T, Disabled>) return setting.isFalse();
     8        // ... other specializations (using options param as well)
     9        return false;
    10    }
    11};
    

    ryanofsky commented at 10:55 pm on December 11, 2024:

    re: #31260 (review)

    In other cases we’ve used a single template method, instead of each type or case having its own explicit implementation - would it make sense to unify this as well?

    I think this is a good idea to keep in mind as long as the traits class only contains a single method, but in practice I have followup changes extending it to have more methods, so the same if constexpr conditions would need to be repeated in each method. Depending on what happens with followups, though, should revisit this.


    l0rinc commented at 2:41 pm on December 12, 2024:
    Doesn’t this signal that the design is lacking a properly typed system? The compiler should be able to warn us of those missing conditions/cases (though I’m overwhelmed here and can’t recommend anything more concrete yet).

    ryanofsky commented at 9:02 pm on December 13, 2024:

    re: #31260 (review)

    Doesn’t this signal that the design is lacking a properly typed system? The compiler should be able to warn us of those missing conditions/cases (though I’m overwhelmed here and can’t recommend anything more concrete yet).

    I don’t really know what you mean by this, but there is a lot going on in this PR so I think I think it should become less overwhelming when the bigger picture comes into focus. There are no missing conditions/cases, though. Everything is type safe. You can write the same code with template specialization or with if statements, and I’m just saying here template specialization will lead to less repetition in followup changes which add more traits methods.

  65. in src/bench/bench_bitcoin.cpp:135 in a4a5226b5b outdated
    138-        args.output_csv = argsman.GetPathArg("-output-csv");
    139-        args.output_json = argsman.GetPathArg("-output-json");
    140-        args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER);
    141-        args.sanity_check = argsman.GetBoolArg("-sanity-check", false);
    142-        args.priority = parsePriorityLevel(argsman.GetArg("-priority-level", DEFAULT_PRIORITY));
    143+        args.asymptote = parseAsymptote(AsymptoteSetting::Get(argsman));
    


    l0rinc commented at 12:32 pm on December 10, 2024:
    Given that parsing can still invalidate a stored value, I wonder if it’s possible to include parsing/validation/normalization into each setting as well - to have a final value instead of later noticing that the value was provided but it’s invalid… It would return something like an std::variant<T, error> in that case instead. It would also fix weird defaults like DEFAULT_MIN_TIME_MS which should already be in their target type (i.e. std::chrono::milliseconds instead of int64_t)

    ryanofsky commented at 8:48 pm on December 11, 2024:

    re: #31260 (review)

    Given that parsing can still invalidate a stored value, I wonder if it’s possible to include parsing/validation/normalization into each setting as well

    Yes! The idea is very much to add parsing and validation behavior, std::variant support, and support for std::chrono types as followups.

    All of this can be seen in https://github.com/ryanofsky/bitcoin/blob/pr/argtype/src/test/argsman_tests.cpp from #22978, though that code was written before this PR and there are a lot of improvements I want to make to the interface and implementation.


    l0rinc commented at 2:43 pm on December 12, 2024:
    I understand that often search and replace can change a lot of code and is very convenient for the author, but all those unrelated changes are very hard to review. Would it be possible to go end-to-end for a very narrow part of the settings and slowly strangle out the old version (i.e. depth-first-search instead of breadth first search)?

    ryanofsky commented at 9:17 pm on December 13, 2024:

    re: #31260 (review)

    I understand that often search and replace can change a lot of code and is very convenient for the author, but all those unrelated changes are very hard to review.

    I don’t think I understand what the unrelated changes are. The only changes in this commit are replacing GetArgs calls with equivalent Get calls, AddArgs calls with equivalent Register calls.

    Would it be possible to go end-to-end for a very narrow part of the settings and slowly strangle out the old version (i.e. depth-first-search instead of breadth first search)?

    If I’m understanding the request correctly, you want to see what parsing/validation/normalization behavior looks like in combination with this PR for a narrow set of settings. I think you can look at https://github.com/ryanofsky/bitcoin/blob/pr/argtype/src/test/argsman_tests.cpp from #22978 for an idea of that. And I am happy to update that branch because it precedes this PR and is somewhat stale at this point.

    I think it is important to be clear about the relationship between this PR and #22978 though. This PR is not changing any functionality or providing any new behavior. It is only doing a mechanical search and replace of GetArgs with Get calls and AddArgs with Register calls, so that settings can be accessed from a type-safe API and will have associated types and default values. #22978 implements various new pieces of functionality on top of the type-safe API.

  66. in src/bench/bench_bitcoin_settings.h:14 in a4a5226b5b outdated
     9+
    10+#include <cstdint>
    11+#include <string>
    12+#include <vector>
    13+
    14+static const char* DEFAULT_BENCH_FILTER = ".*";
    


    l0rinc commented at 12:33 pm on December 10, 2024:
    I know it’s a move, but could we modernize these (in a separate commit, of course)?

    ryanofsky commented at 8:48 pm on December 11, 2024:

    re: #31260 (review)

    I know it’s a move, but could we modernize these (in a separate commit, of course)?

    I’m not actually sure what the modern way of writing this is, but if there is a better way let me know here. I am planning to open a new PR that could be merged before this one with small cleanups like this, to reduce the size of this one.


    l0rinc commented at 2:45 pm on December 12, 2024:
    I just meant constexpr and brace init here (but at least consistency with the other lines here)
  67. in src/bench/bench_bitcoin.cpp:29 in a4a5226b5b outdated
    38-    argsman.AddArg("-output-csv=<output.csv>", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
    39-    argsman.AddArg("-output-json=<output.json>", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
    40-    argsman.AddArg("-sanity-check", "Run benchmarks for only one iteration with no output", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
    41-    argsman.AddArg("-priority-level=<l1,l2,l3>", strprintf("Run benchmarks of one or multiple priority level(s) (%s), default: '%s'",
    42-                                                           benchmark::ListPriorities(), DEFAULT_PRIORITY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
    43+    AsymptoteSetting::Register(argsman);
    


    l0rinc commented at 12:34 pm on December 10, 2024:
    This looks really cool, strong concept ack!
  68. in src/bench/bench_bitcoin_settings.h:20 in a4a5226b5b outdated
    15+static constexpr int64_t DEFAULT_MIN_TIME_MS{10};
    16+/** Priority level default value, run "all" priority levels */
    17+static constexpr auto DEFAULT_PRIORITY{"all"};
    18+
    19+using AsymptoteSetting = common::Setting<
    20+    "-asymptote=<n1,n2,n3,...>", std::string, common::SettingOptions{.legacy = true},
    


    l0rinc commented at 12:44 pm on December 10, 2024:

    a lot of bare template parameters here, it’s not immediately obvious what std::string refers to - what if we commented them:

    0using AsymptoteSetting = common::Setting<
    1    /*summary=*/"-asymptote=<n1,n2,n3,...>",
    2    /*type=*/std::string,
    3    /*options=*/{.legacy = true},
    4    /*help=*/"Test asymptotic growth of the runtime of an algorithm, if supported by the benchmark">;
    

    ryanofsky commented at 8:49 pm on December 11, 2024:

    re: #31260 (review)

    a lot of bare template parameters here, it’s not immediately obvious what std::string refers to - what if we commented them:

    There should be only 3 unnamed parameters passed to the Setting class:

    • Summary string
    • Help string
    • C++ type

    I would expect the basic meanings of these parameters to be pretty obvious in context, especially with current _settings.h files which define dozens of settings next to each other. And the parts that aren’t obvious should be explained by the common::Setting class documentation, which should be easy to jump to from the usage.

    Other options passed to the Setting class are passed as named parameters using named SettingsOptions, Category, Default, HelpArgs, etc fields. These fields should also be easy to jump to and well-documented.

    I don’t think it would be good to add lots of /*summary=*/ /*type=*/ /*help=*/ comments, especially in files defining dozens of settings, because they could become inconsistent over time and be a maintenance burden, add noise, and make it harder to see differences between settings. I think it should be ok to make the 3 most common parameters unnamed and make everything else named.

  69. in test/lint/check-doc.py:18 in a4a5226b5b outdated
    14@@ -15,11 +15,11 @@
    15 
    16 FOLDER_GREP = 'src'
    17 FOLDER_TEST = 'src/test/'
    18-REGEX_ARG = r'\b(?:GetArg|GetArgs|GetBoolArg|GetIntArg|GetPathArg|IsArgSet|get_net)\("(-[^"]+)"'
    19-REGEX_DOC = r'AddArg\("(-[^"=]+?)(?:=|")'
    20+REGEX_ARG = r'([A-Za-z0-9]+Setting)::Get'
    


    l0rinc commented at 1:00 pm on December 10, 2024:
    If this linter is still relevant, we can use \w instead of [A-Za-z0-9] throughout (and we could sort args_unknown = sorted(args_docd.difference(args_used)) for consistent output)

    ryanofsky commented at 11:07 pm on December 11, 2024:

    re: #31260 (review)

    If this linter is still relevant, we can use \w instead of [A-Za-z0-9] throughout (and we could sort args_unknown = sorted(args_docd.difference(args_used)) for consistent output)

    Will do this in in a followup. I’m not actually sure of everything this linter is doing and I don’t know if it still adds value after this PR, so I opted to keep it for now.


    l0rinc commented at 2:46 pm on December 12, 2024:
    If you keep it, please consider my suggestions above
  70. in src/wallet/walletutil.cpp:20 in a4a5226b5b outdated
    17 
    18-    if (gArgs.IsArgSet("-walletdir")) {
    19-        path = gArgs.GetPathArg("-walletdir");
    20+    if (!WalletdirSetting::Value(gArgs).isNull()) {
    21+        path = WalletdirSetting::Get(gArgs);
    22         if (!fs::is_directory(path)) {
    


    l0rinc commented at 1:01 pm on December 10, 2024:
    as hinted above, I think WalletdirSetting should either return a valid result or empty/error - so would be cool if the is_directory check could be done inside

    ryanofsky commented at 11:11 pm on December 11, 2024:

    re: #31260 (review)

    as hinted above, I think WalletdirSetting should either return a valid result or empty/error - so would be cool if the is_directory check could be done inside

    To be sure, no behavior is be changing here, but the handling of -walletdir definition doesn’t make a lot of sense and should be cleaned up. I haven’t looked into it much, but could take suggestions here if you have them for another PR, either before or after this one.


    l0rinc commented at 2:47 pm on December 12, 2024:
    Absolutely, low and high risk changes should be intertwined
  71. in src/wallet/wallettool.cpp:143 in a4a5226b5b outdated
    144 
    145     if (command == "create") {
    146         DatabaseOptions options;
    147         ReadDatabaseArgs(args, options);
    148         options.require_create = true;
    149         // If -legacy is set, use it. Otherwise default to false.
    


    l0rinc commented at 1:04 pm on December 10, 2024:
    comment doesn’t make a lot of sense anymore
  72. in src/wallet/wallettool.cpp:136 in a4a5226b5b outdated
    137+    if (command == "create" && WalletSetting::Value(args).isNull()) {
    138         tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n");
    139         return false;
    140     }
    141-    const std::string name = args.GetArg("-wallet", "");
    142+    const std::string name = WalletSetting::Get(args);
    


    l0rinc commented at 1:10 pm on December 10, 2024:
    In other cases we’ve provided the default values for the settings - should we add ::Default<{""}> to the definition (or whatever compiles)?

    ryanofsky commented at 11:18 pm on December 11, 2024:

    re: #31260 (review)

    In other cases we’ve provided the default values for the settings - should we add ::Default<{""}> to the definition (or whatever compiles)?

    I don’t think that would be an improvement in general, but the wallet tool setting can be changed to follow whatever style seems appropriate.

    In general, settings are default initialized, so if you don’t specify a default and the setting type is a string, the default value is “”, if it’s an int default is 0, if it’s std::optional default is std::nullopt, vector default is {}, etc.

    I think using simple default values should generally be encouraged, and when more notable default values need to be set, it is better if they stand out and are not lost in surrounding ::Default<0> ::Default<false> :DefaultFn<[] { return std::string{}; }> boilerplate.

  73. in src/wallet/wallettool.cpp:144 in a4a5226b5b outdated
    146         DatabaseOptions options;
    147         ReadDatabaseArgs(args, options);
    148         options.require_create = true;
    149         // If -legacy is set, use it. Otherwise default to false.
    150-        bool make_legacy = args.GetBoolArg("-legacy", false);
    151+        bool make_legacy = LegacySetting::Get(args);
    


    l0rinc commented at 1:11 pm on December 10, 2024:

    Same, are we not specifying false defaults? We seem to do it for e.g. DEFAULT_NAMED

    0using LegacySetting = common::Setting<
    1    "-legacy", bool, common::SettingOptions{.legacy = true},
    2    "Create legacy wallet. Only for 'create'">
    3    ::Default<false>;
    

    ryanofsky commented at 11:25 pm on December 11, 2024:

    re: #31260 (review)

    Same, are we not specifying false defaults? We seem to do it for e.g. DEFAULT_NAMED

    Yes, empty defaults are intentionally not specified so nonempty defaults stand out.

    Regarding DEFAULT_NAMED, followups to this PR should remove DEFAULT_XXX constants like this which no longer provide benefits after this PR, and only make it harder to see what default values are.

  74. in src/wallet/wallet.cpp:2994 in a4a5226b5b outdated
    2990@@ -2990,8 +2991,8 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
    2991     // TODO: Can't use std::make_shared because we need a custom deleter but
    2992     // should be possible to use std::allocate_shared.
    2993     std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), FlushAndDeleteWallet);
    2994-    walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
    2995-    walletInstance->m_notify_tx_changed_script = args.GetArg("-walletnotify", "");
    2996+    walletInstance->m_keypool_size = std::max(KeypoolSetting::Get(args), int64_t{1});
    


    l0rinc commented at 1:13 pm on December 10, 2024:
    shouldn’t this also be a validation error instead - preferably in KeypoolSetting

    ryanofsky commented at 11:27 pm on December 11, 2024:

    re: #31260 (review)

    shouldn’t this also be a validation error instead - preferably in KeypoolSetting

    That would probably be a good idea, but it’s not definitely clear to me, and this would be a better topic for a PR intending to change behavior.

  75. in src/wallet/init.cpp:114 in a4a5226b5b outdated
    112 
    113         return true;
    114     }
    115 
    116-    if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) {
    117+    if (BlocksonlySetting::Get(gArgs, DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) {
    


    l0rinc commented at 1:14 pm on December 10, 2024:
    can we migrate gArgs.SoftSetBoolArg("-walletbroadcast", false) as well?

    ryanofsky commented at 11:30 pm on December 11, 2024:

    re: #31260 (review)

    can we migrate gArgs.SoftSetBoolArg("-walletbroadcast", false) as well?

    Maybe, but I think all uses of SoftSet/ForceSet are all hacks that abuse global state and that would be better to handle other ways. I think it would be best to deal with these in a PR eliminating SoftSet/ForceSet functions, and in any case discourage more calls to these functions.

  76. in src/torcontrol.cpp:463 in a4a5226b5b outdated
    459@@ -459,7 +460,7 @@ void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply&
    460 
    461         // Now that we know Tor is running setup the proxy for onion addresses
    462         // if -onion isn't set to something else.
    463-        if (gArgs.GetArg("-onion", "") == "") {
    464+        if (OnionSetting::Get(gArgs) == "") {
    


    l0rinc commented at 1:17 pm on December 10, 2024:
    Q: how can we set something to an empty string? -onion=""? -noonion? -noonion=""?

    ryanofsky commented at 11:33 pm on December 11, 2024:

    re: #31260 (review)

    Q: how can we set something to an empty string? -onion=""? -noonion? -noonion=""?

    The onion setting is just empty by default so there should be no need to specify these. This PR isn’t concerned with changing any runtime behavior, though #16545 is and documents all these cases.


    l0rinc commented at 2:49 pm on December 12, 2024:
    So is == "" the same as checking if the value was set? If so, we could use that here as well

    ryanofsky commented at 9:45 pm on December 13, 2024:

    re: #31260 (review)

    So is == "" the same as checking if the value was set? If so, we could use that here as well

    Maybe we are talking past each other with different terminology, but the point of allowing the settings framework to specify default values is to avoid needing to care whether a setting was set or unset. The setting can simply be retrieved and if the user did not set a value, the framework can provide an appropriate default.

    So I guess I would answer the question as no. The == "" check is just checking if the setting is empty, not checking if it is set. The setting might have been unset or it might have been set by the user to empty, and the code retrieving the setting should not care which of these was the case.

  77. in src/test/util/setup_common.h:279 in a4a5226b5b outdated
    278@@ -279,6 +279,10 @@ inline std::ostream& operator<<(std::ostream& os, const std::optional<T>& v)
    279     return v ? os << *v
    280              : os << "std::nullopt";
    281 }
    282+inline std::ostream& operator<<(std::ostream& os, const std::nullopt_t)
    283+{
    284+    return os << "std::nullopt";
    285+}
    


    l0rinc commented at 1:25 pm on December 10, 2024:

    We should be able to unify them now:

    0inline std::ostream& operator<<(std::ostream& os, const std::nullopt_t)
    1{
    2    return os << "std::nullopt";
    3}
    4template <typename T>
    5std::ostream& operator<<(std::ostream& os, const std::optional<T>& v)
    6{
    7    return v ? os << *v
    8             : os << std::nullopt;
    9}
    

    ryanofsky commented at 11:37 pm on December 11, 2024:

    re: #31260 (review)

    We should be able to unify them now:

    Makes sense, applied suggestion.

  78. in src/qt/walletmodel.cpp:18 in a4a5226b5b outdated
    12@@ -13,6 +13,7 @@
    13 #include <qt/recentrequeststablemodel.h>
    14 #include <qt/sendcoinsdialog.h>
    15 #include <qt/transactiontablemodel.h>
    16+#include <wallet/init_settings.h>
    17 
    18 #include <common/args.h> // for GetBoolArg
    


    l0rinc commented at 1:30 pm on December 10, 2024:
    comment needs update now

    ryanofsky commented at 11:39 pm on December 11, 2024:

    re: #31260 (review)

    comment needs update now

    Made a note. I think I will do this in a separate PR because I don’t want to add unnecessary changes to this one.

  79. in src/qt/optionsmodel.cpp:721 in a4a5226b5b outdated
    717@@ -717,7 +718,7 @@ bool OptionsModel::isRestartRequired() const
    718 
    719 bool OptionsModel::hasSigner()
    720 {
    721-    return gArgs.GetArg("-signer", "") != "";
    722+    return wallet::SignerSetting::Get(gArgs) != "";
    


    l0rinc commented at 1:31 pm on December 10, 2024:
    is "" the default for SignerSetting now?

    ryanofsky commented at 11:40 pm on December 11, 2024:

    re: #31260 (review)

    is "" the default for SignerSetting now?

    To be sure, no default values or other runtime behaviors are changing in this PR, but the answer is yes. Default for -signer and most string settings is ""

  80. in src/qt/intro.cpp:147 in a4a5226b5b outdated
    141@@ -140,9 +142,9 @@ Intro::Intro(QWidget *parent, int64_t blockchain_size_gb, int64_t chain_state_si
    142 
    143     const int min_prune_target_GB = std::ceil(MIN_DISK_SPACE_FOR_BLOCK_FILES / 1e9);
    144     ui->pruneGB->setRange(min_prune_target_GB, std::numeric_limits<int>::max());
    145-    if (gArgs.IsArgSet("-prune")) {
    146+    if (!PruneSetting::Value(gArgs).isNull()) {
    147         m_prune_checkbox_is_default = false;
    148-        ui->prune->setChecked(gArgs.GetIntArg("-prune", 0) >= 1);
    149+        ui->prune->setChecked(PruneSetting::Get(gArgs, 0) >= 1);
    


    l0rinc commented at 1:33 pm on December 10, 2024:
    PruneSetting seems to have a explicit default value of 0 - maybe we can bake it into the definition

    ryanofsky commented at 11:43 pm on December 11, 2024:

    re: #31260 (review)

    PruneSetting seems to have a explicit default value of 0 - maybe we can bake it into the definition

    The script-diff would have dropped the 0 if it was actually the default everywhere, but there is code in blockmanager_args.cpp that retrieves the setting with a different default value, so the 0 is actually necessary here.

    It would definitely be better if PruneSetting did have a consistent default value but that is something that needs to be cleaned up manually and can’t be scripted.

  81. in src/node/chainstatemanager_args.cpp:29 in a4a5226b5b outdated
    24@@ -24,36 +25,36 @@
    25 namespace node {
    26 util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts)
    27 {
    28-    if (auto value{args.GetIntArg("-checkblockindex")}) {
    29+    if (auto value{CheckblockindexSettingInt::Get(args)}) {
    30         // Interpret bare -checkblockindex argument as 1 instead of 0.
    


    l0rinc commented at 1:36 pm on December 10, 2024:
    this seems weird, shouldn’t CheckblockindexSetting itself normalize this?

    ryanofsky commented at 11:44 pm on December 11, 2024:

    re: #31260 (review)

    this seems weird, shouldn’t CheckblockindexSetting itself normalize this?

    Yes, this is implemented in #16545 but current settings parsing behavior has lots of weird cases like this.

  82. in src/net.cpp:3813 in a4a5226b5b outdated
    3809@@ -3809,7 +3810,7 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
    3810     AssertLockNotHeld(m_total_bytes_sent_mutex);
    3811     size_t nMessageSize = msg.data.size();
    3812     LogDebug(BCLog::NET, "sending %s (%d bytes) peer=%d\n", msg.m_type, nMessageSize, pnode->GetId());
    3813-    if (gArgs.GetBoolArg("-capturemessages", false)) {
    3814+    if (CapturemessagesSetting::Get(gArgs, false)) {
    


    l0rinc commented at 1:38 pm on December 10, 2024:

    I’m not sure I understand when the default values are kept, why not just:

    0    if (CapturemessagesSetting::Get(gArgs)) {
    

    ryanofsky commented at 11:49 pm on December 11, 2024:

    re: #31260 (review)

    I’m not sure I understand when the default values are kept, why not just:

    The scripted-diff consolidates default values when it knows they are all the same, and in this case it doesn’t because code here is treating the default value as false, while code elsewhere is treating default value as PeerManager::Options{}.capture_messages, which is also false, but scripted-diff has no way of knowing that.

    Will make a note of this and clean this up in another PR since it should be pretty easy to fix.


    l0rinc commented at 2:52 pm on December 12, 2024:
    I don’t see the value of scripted diffs when something is this complicated - the code should probably be pre-processed and refactored so that the scripted diffs don’t have any exceptional cases.

    ryanofsky commented at 9:52 pm on December 13, 2024:

    re: #31260 (review)

    I don’t see the value of scripted diffs when something is this complicated - the code should probably be pre-processed and refactored so that the scripted diffs don’t have any exceptional cases.

    The change you are suggesting would not be possible to make without a script. If you create a PR where you have manually gone through GetArg calls in the codebase and looked at what default values they were passed, and then moved those default values into a settings definition in a separate header when those default values were all the same, but left them in place when they were not all the same, the repeated this for 200 settings in the codebase, the result would be inevitably buggy because there is no way a human being could make those changes reliably. The changes would also be basically impossible to review, and the PR would also be difficult to rebase and update correctly as GetArg calls were added and removed from the codebase.

    A script is the way to make these changes reliably in a way that is possible to review and update.

  83. in src/init_settings.h:762 in a4a5226b5b outdated
    761+    ::Category<OptionsCategory::NODE_RELAY>;
    762+
    763+using DatacarriersizeSetting = common::Setting<
    764+    "-datacarriersize", int64_t, common::SettingOptions{.legacy = true},
    765+    "Relay and mine transactions whose data-carrying raw scriptPubKey "
    766+                             "is of this size or less (default: %u)">
    


    l0rinc commented at 1:41 pm on December 10, 2024:

    in other cases we wrote out the lines:

    0    "Relay and mine transactions whose data-carrying raw scriptPubKey is of this size or less (default: %u)">
    

    ryanofsky commented at 11:51 pm on December 11, 2024:

    re: #31260 (review)

    in other cases we wrote out the lines:

    I’m not sure how to implement this suggestion in the scripted diff, and I’m not sure it necessarily would be worth the effort. I guess I could change this in earlier commit 537caa82a66392b0bc010d3a4de9b303e1ce59ab, though. Will make a note.

  84. in src/init.cpp:1461 in a4a5226b5b outdated
    1457@@ -1497,19 +1458,19 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
    1458     // Requesting DNS seeds entails connecting to IPv4/IPv6, which -onlynet options may prohibit:
    1459     // If -dnsseed=1 is explicitly specified, abort. If it's left unspecified by the user, we skip
    1460     // the DNS seeds by adjusting -dnsseed in InitParameterInteraction.
    1461-    if (args.GetBoolArg("-dnsseed") == true && !g_reachable_nets.Contains(NET_IPV4) && !g_reachable_nets.Contains(NET_IPV6)) {
    1462+    if (DnsseedSetting::Get(args) == true && !g_reachable_nets.Contains(NET_IPV4) && !g_reachable_nets.Contains(NET_IPV6)) {
    


    l0rinc commented at 1:50 pm on December 10, 2024:
    Does it still make sense to do == true?

    ryanofsky commented at 11:53 pm on December 11, 2024:

    re: #31260 (review)

    Does it still make sense to do == true?

    This isn’t the clearest way to write the code but the scripted diff can’t change this without changing behavior because this is calling a retrieval function that returns std::optional (both before and after the change).

  85. l0rinc commented at 2:02 pm on December 10, 2024: contributor
    Concept ACK, it’s a lot better to have the compiler do the job for us. I just scrolled through to understand the gist of it - but it’s a humongous change, I’m toast for today. Do you think it would be possible to split this into tiny PRs that change each settings-block one-by-one instead? That way we can iterate on the first to see if we could validate and parse and normalize the values there as well.
  86. ryanofsky force-pushed on Dec 12, 2024
  87. ryanofsky commented at 0:35 am on December 12, 2024: contributor

    Thanks for the reviews!

    Updated a4a5226b5b7f240a87a5f078cf0b6a475679b8ab -> 7a24a9dea0387637f604852d85aaac761203bcb6 (pr/scripty.12 -> pr/scripty.13, compare) implementing various suggestions like the camelcase map and extending to cover more test settings. At this point am getting pretty close to being able to drop the GetArg/GetArgs/IsArgSet/etc methods entirely.


    re: #31260#pullrequestreview-2492047895

    Do you think it would be possible to split this into tiny PRs that change each settings-block one-by-one instead? That way we can iterate on the first to see if we could validate and parse and normalize the values there as well.

    There are around 200 settings, so I feel like a single atomic scripted-diff commit t is probably the best approach to take getting this change over with, but depending on what your concern is and what you are trying to verify I can definitely change the script to produce other output.

    If the idea is to spot-check individual settings, and make sure the replacements are valid, the way I’ve been doing that is just to view the scripted-diff commit with git log -p -n1 and then search the diff for individual settings like LimitAncestorSizeSetting and confirm the diffs in the places where it is registered & retrieved & defined make sense and don’t change previous behavior.

  88. in src/common/setting.h:35 in 7a24a9dea0 outdated
    30+struct StringLiteral {
    31+    constexpr StringLiteral(const char (&str)[N]) {
    32+        std::copy_n(str, N, value);
    33+    }
    34+    constexpr StringLiteral(std::nullptr_t) {
    35+    }
    


    hodlinator commented at 10:42 am on December 12, 2024:

    nit: developer-notes.md state:

    Braces on new lines for classes, functions, methods.

    clang-format tolerates {} though.

    0    constexpr StringLiteral(const char (&str)[N])
    1    {
    2        std::copy_n(str, N, value);
    3    }
    4    constexpr StringLiteral(std::nullptr_t) {}
    

    Similar story for class SettingTest.

    Running

    0git diff -U0 HEAD~9.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v
    

    otherwise shows mainly

    0-template<
    1+template <
    

    as something I would recommend.

  89. in src/common/setting.h:110 in 7a24a9dea0 outdated
    105+    //! Default accessor which sets a default value for the Setting. The
    106+    //! specified value will be returned by Setting::Get calls if the setting
    107+    //! was not specified in the command line, config file, or settings.json
    108+    //! file. The specified value will also be substituted in the help string if
    109+    //! HelpArgs or HelpFn are not specified.
    110+    using Default = Setting<summary, T, options, help, help_fn, internal::Constant<_default>{}, get_fn, category>;
    


    hodlinator commented at 1:46 pm on December 12, 2024:

    template-parts of the declarations start to dangle above the comment for the rest of the expressions here.

    0    //! Default accessor which sets a default value for the Setting. The
    1    //! specified value will be returned by Setting::Get calls if the setting
    2    //! was not specified in the command line, config file, or settings.json
    3    //! file. The specified value will also be substituted in the help string if
    4    //! HelpArgs or HelpFn are not specified.
    5    template <auto _default>
    6    using Default = Setting<summary, T, options, help, help_fn, internal::Constant<_default>{}, get_fn, category>;
    
  90. in src/common/setting_internal.h:28 in 7a24a9dea0 outdated
    23+    using type = U;
    24+};
    25+
    26+template<typename T, auto get_fn = nullptr>
    27+struct SettingTraitsBase
    28+{
    


    hodlinator commented at 1:48 pm on December 12, 2024:

    nit: Brace on same line according to developer-notes.md, here and further down.

    0struct SettingTraitsBase {
    
  91. in src/common/args.cpp:300 in 7a24a9dea0 outdated
    295@@ -288,8 +296,8 @@ fs::path ArgsManager::GetBlocksDirPath() const
    296     // this function
    297     if (!path.empty()) return path;
    298 
    299-    if (IsArgSet("-blocksdir")) {
    300-        path = fs::absolute(GetPathArg("-blocksdir"));
    301+    if (!BlocksDirSetting::Value(*this).isNull()) {
    302+        path = fs::absolute(BlocksDirSettingPath::Get(*this));
    


    hodlinator commented at 2:17 pm on December 12, 2024:

    Not sure what to make of BlocksDirSetting/BlocksDirSettingPath pairs..

    • BlocksdirSettingPath is not registered to avoid collisions.
    • I guess you are avoidant when it comes to adding types.. but this seems worse.

    Would suggest:

    0    if (!BlocksDirSetting::Value(*this).isNull()) {
    1        path = fs::absolute(BlocksDirSetting::Get(*this));
    

    and having BlocksDirSetting contain an fs::path directly.


    ryanofsky commented at 9:28 pm on December 13, 2024:

    re: #31260 (review)

    Not sure what to make of BlocksDirSetting/BlocksDirSettingPath pairs..

    I guess you are avoidant when it comes to adding types.. but this seems worse.

    These changes just come from scripted-diff trying not to change behavior in any way, but I can add a new commit before the scripted diff to avoid creation of ConfSettingPath, DataDirSettingPath, and BlocksDirSettingPath types.

    These types are created because current code is inconsistent about using GetArg and GetPathArg when retrieving these settings, so the script needs to be conservative and declaring separate types for the inconsistent retrievals.

    Part of the point of this PR is to uncover inconsistencies like this, so thanks for noticing it!

  92. hodlinator commented at 2:46 pm on December 12, 2024: contributor

    Reviewed 7a24a9dea0387637f604852d85aaac761203bcb6

    Thanks for adding NAME_MAP to get CamelCase!

    Surface-level nit time I’m afraid, aside from my curiosity regarding your reasoning around *SettingPath.

  93. l0rinc commented at 2:59 pm on December 12, 2024: contributor

    There are around 200 settings, so I feel like a single atomic scripted-diff commit is probably the best approach to take

    If the idea is to spot-check individual settings, and make sure the replacements are valid

    I’d like to offer an alternative, since I can only ACK if I have reviewed every line manually - but that’s not something I can do here. So instead of approaching this row-by-row (apply each change to every occurance and continue with different changeset in different PRs), I would be interested in doing the changes end-to-end for a small subset of the settings instead (i.e. settings with validation and normalization and error handling and type safety for the helpers only, keeping the others completely intact) - i.e. column-by-column (using a tabular iteration as a metaphor).

    I know it’s more work that way, but I’m afraid I can’t meaningfully review the changes otherwise - but would like to help, I think this is a good general direction.

  94. ryanofsky force-pushed on Dec 13, 2024
  95. ryanofsky commented at 11:04 pm on December 13, 2024: contributor

    Thanks for the additional reviews!

    Updated 7a24a9dea0387637f604852d85aaac761203bcb6 -> e12edf7661384f593b5868b3f3374613773019d9 (pr/scripty.13 -> pr/scripty.14, compare) with a cleanups to the scripted diff to make it possible to run incrementally and deal with some of l0rinc’s concerns.

    I didn’t get to make most of the other requested updates yet, but have a list of changes I want to include in the next push:


    re: #31260 (comment)

    I’d like to offer an alternative, since I can only ACK if I have reviewed every line manually - but that’s not something I can do here. So instead of approaching this row-by-row (apply each change to every occurance and continue with different changeset in different PRs), I would be interested in doing the changes end-to-end for a small subset of the settings instead (i.e. settings with validation and normalization and error handling and type safety for the helpers only, keeping the others completely intact) - i.e. column-by-column (using a tabular iteration as a metaphor).

    You might consider giving a partial ACK to the parts of the PR you are ok with, and pointing out other parts you don’t feel able to sign off on.

    But I think the underlying issue here might be that you haven’t totally grokked yet which changes are included and are not included in this PR, which is understandable because the scripted diff is very big, so it is hard to see that changes you might be expecting are not being made, and that the scope of the PR is in fact, very limited.

    To help clarify things, I decided to follow up with your idea from last review and show what this change would look like in tiny commits instead of in a big commit. I would encourage you to maybe just look at few of these commits at random, and either see that that changes are obvious or ask me any questions about what is not obvious.

    If you still couldn’t ack the PR after this because you couldn’t review every line manually that would be understandable. But you should at least be able to see that the script is really just replacing GetArg calls with equivalent Get calls and AddArg calls with equivalent Register calls and not doing anything else beyond that.

    For reference the branch with tiny commits can be found at https://github.com/ryanofsky/bitcoin/commits/pr/scripty-tiny and there aren’t any differences between it and this PR, other than being split up into more commits. The branch was generated by running the script with a new --git-commit option to automatically create a git commit after each setting replacement.

    • 629c302c18d4281f41a4995b125c9da00842fe72 init_settings.h: Add CheckAddrManSetting
    • de6edd0be26028fd075db5f2dc6b89933047d1b1 bench/bench_bitcoin_settings.h: Add AsymptoteSetting
    • f77817559f49fe24b4b925dbfef3e1159917207b bench/bench_bitcoin_settings.h: Add FilterSetting
    • 3f6a9a5a30f965c40b35046c58ce57909ae189f7 bench/bench_bitcoin_settings.h: Add ListSetting
    • d99f3abd8ccfb46c80c9612a2c4bd3046fea790f bench/bench_bitcoin_settings.h: Add MinTimeSetting
    • beec29c6ae836053d0bf27bc077984accd3c3c68 bench/bench_bitcoin_settings.h: Add OutputCsvSetting
    • 882691605648043f6bd23ffcd8d35b3dc227999b bench/bench_bitcoin_settings.h: Add OutputJsonSetting
    • 9af4351a845b92b18ccf83dd0834fb16102b6dcd bench/bench_bitcoin_settings.h: Add SanityCheckSetting
    • 1c6b7c1573fad99d358b5fb4ade29ee2d04346a9 bench/bench_bitcoin_settings.h: Add PriorityLevelSetting
    • 905da021f53dbcb689b787f56d2589ae5986c1e3 bitcoin-cli_settings.h: Add VersionSetting
    • 0b0ebac2527346ecf2f9ea6941bc5431f52dea1f bitcoin-tx_settings.h: Add VersionSetting
    • e65ce5c42529269ebb7419ddca3cb47059114e10 bitcoin-util_settings.h: Add VersionSetting
    • a385cc0e1dd0f598594444c7d80107ab76d0a06a bitcoin-wallet_settings.h: Add VersionSetting
    • a2a5bef599fb47be1e8fbc9d6c96c39c5b3e57df init_settings.h: Add VersionSetting
    • 83aa8e1774b26fe01970f896fdff62b3c9473243 bitcoin-cli_settings.h: Add ConfSetting
    • 8c975b648a35052c8cdebb61ddfe26259f96de62 init_settings.h: Add ConfSetting
    • 8c838fc3f10ebdb109628100eed2ee67fc24fcd9 bitcoin-cli_settings.h: Add DataDirSetting
    • fb3adf1d712e2d561f49b9b6fb8d3bfb720a7f22 bitcoin-wallet_settings.h: Add DataDirSetting
    • ee24f487c433f44ea28d37d47918385a56fa7b98 init_settings.h: Add DataDirSetting
    • 9b362e5927dcd986764f5195da7ea0f6171e4add bitcoin-cli_settings.h: Add GenerateSetting
    • e09cdbf684c143797d401386f57866d71fe11192 bitcoin-cli_settings.h: Add AddrInfoSetting
    • 8a218f6487999cb3c651beb1fb7b34e5c53b0bac bitcoin-cli_settings.h: Add GetInfoSetting
    • aa3735d5351369f53f7a04200e7fbfc1618a1b91 bitcoin-cli_settings.h: Add NetInfoSetting
    • 823c35145f47d653613cb6cefb3ed74a33057755 bitcoin-cli_settings.h: Add ColorSetting
    • 40a01303e0969c73028a9cef7fa38d3e39540ef3 bitcoin-cli_settings.h: Add NamedSetting
    • e17f559ed909a3af27381b5b79be91c2dc616f4c bitcoin-cli_settings.h: Add RpcClientTimeoutSetting
    • a04605877bf237b77f1a476f615f9edd188abccb bitcoin-cli_settings.h: Add RpcConnectSetting
    • 8d025240f63bda53d6f1698d70bb76c8c6bf7a3e bitcoin-cli_settings.h: Add RpcCookieFileSetting
    • 4d9c4c3d879b73e581cb6e170eec2925ee7ecc0e init_settings.h: Add RpcCookieFileSetting
    • b944834b4f8d0739f363156bef49336a2b907e06 bitcoin-cli_settings.h: Add RpcPasswordSetting
    • 9013423e6c9696096df676c1ab24603cedbcb62d init_settings.h: Add RpcPasswordSetting
    • 887be1f9ab45ebb64a98b214ac837f7bb9f3ae0c bitcoin-cli_settings.h: Add RpcPortSetting
    • 9a50a1c78b1d60b345cbc3f8567bb5cc0cfc22b2 init_settings.h: Add RpcPortSetting
    • 61acb9c19f8987e56a1a48a61e1286c5baeb5339 bitcoin-cli_settings.h: Add RpcUserSetting
    • e5a521f0ba3c757f43804a0c652f483a652eba6a init_settings.h: Add RpcUserSetting
    • 806970e5cbcdf90b8fdde2b7eda5c80ae0147656 bitcoin-cli_settings.h: Add RpcWaitSetting
    • 0759754a0efee0ea777aa816340edbec66c81e7f bitcoin-cli_settings.h: Add RpcWaitTimeoutSetting
    • 5f994b0e62827540e873bffa18899feb0fb4d3d8 bitcoin-cli_settings.h: Add RpcWalletSetting
    • 7501ba291648a928010196536e98c2fccd13b170 bitcoin-cli_settings.h: Add StdinSetting
    • 9d6269672e0eac2fd3146994bbfabaaea3cf598f bitcoin-cli_settings.h: Add StdinRpcPassSetting
    • ece4be24cd8b975c44f350139ca6b6cd10d046fd bitcoin-cli_settings.h: Add StdinWalletPassphraseSetting
    • 7fddf02115f99677e70f7178515763fc64dc205a bitcoin-tx_settings.h: Add CreateSetting
    • 24774031d2aa0935e6bd3baa3b220b861a77ffe0 bitcoin-tx_settings.h: Add JsonSetting
    • 51c5e9bba705135349caeb99f1efe578795ade39 bitcoin-tx_settings.h: Add TxIdSetting
    • 45938ac6f3a9106a98ff6bc48f9cf0963136b046 bitcoin-tx_settings.h: Add DelInSetting
    • 83baad12ee849a0fc8828768eaff54343e16720e bitcoin-tx_settings.h: Add DelOutSetting
    • f9afb2ee4a2dc63a4ae83e700c9e03285d586c0a bitcoin-tx_settings.h: Add InSetting
    • dd0094d7820e23e078e6dff54dd2853deddfa6d1 bitcoin-tx_settings.h: Add LockTimeSetting
    • 10a8a2a2d0e343e5f094ff61bd416c22becf7be5 bitcoin-tx_settings.h: Add NVersionSetting
    • 3a1cde32ece50d791df69c5ef448f2fde40d3940 bitcoin-tx_settings.h: Add OutAddrSetting
    • 0ff44bc8d904f9ae37a81cb999bb23ac5bc2b021 bitcoin-tx_settings.h: Add OutDataSetting
    • 2b13f62fdc9e8375dd48b13fd4da74d70d830688 bitcoin-tx_settings.h: Add OutMultiSigSetting
    • 2e601c4c1584fec7559827d1bcdacc662eca2336 bitcoin-tx_settings.h: Add OutPubKeySetting
    • 7d536cf7d5b708fc958291ac8ad373c244cf2735 bitcoin-tx_settings.h: Add OutScriptSetting
    • 0c930c4bb1c78beb7b6e10b3675bee2711c2857d bitcoin-tx_settings.h: Add ReplaceableSetting
    • aa908568de2338a04a4ba109d7d553b1a0c692e7 bitcoin-tx_settings.h: Add SignSetting
    • d64d5a35317fa49f137a64f7af5c095fd8cbb2ca bitcoin-tx_settings.h: Add LoadSetting
    • 12f800e39743eb7c8b5221d612ebcde51b3288d0 bitcoin-tx_settings.h: Add SetSetting
    • c02c8be1472e981958f88e2006722375774eaeaa bitcoin-wallet_settings.h: Add WalletSetting
    • 0c005ddf2fd2fb3c0cf43ad24b1c3e3d2b5e1a60 dummywallet_settings.h: Add WalletSettingHidden
    • 62cdd5505eca429190f6a9eb22d7148c859e53c1 wallet/init_settings.h: Add WalletSetting
    • 4525e0552d5799578b90523e0f6bd38e57b27c96 bitcoin-wallet_settings.h: Add DumpFileSetting
    • a556efef3fcd4e6b0a38d45f05a3b043df209429 bitcoin-wallet_settings.h: Add DebugSetting
    • abbb8b30fcebea1a9b048d5d636fcb53c4e826bf init/common_settings.h: Add DebugSetting
    • da8a53a4bb3482ebe174715e7a9aa8cddd283cf0 bitcoin-wallet_settings.h: Add DescriptorsSetting
    • 91b8fdcf602e50abce7a9aac3ef278636ce81059 bitcoin-wallet_settings.h: Add LegacySetting
    • 33024701ae24cf36ead528e72a005927b5062556 bitcoin-wallet_settings.h: Add FormatSetting
    • 02a675be3d231e248cbdddf89ad25033376600f2 bitcoin-wallet_settings.h: Add PrintToConsoleSetting
    • 3e9a3ed1468de8c3e87f051d9c4fd32684e93a05 init/common_settings.h: Add PrintToConsoleSetting
    • a8011d247f85f22a907dd24394832d1426127f21 bitcoin-wallet_settings.h: Add WithInternalBdbSetting
    • 0cd3abc3d1efdae32bfb46818144aa06fc15a364 init_settings.h: Add DaemonSetting
    • 5d49aa7a154403831e9f9be7087edbc34988775e init_settings.h: Add DaemonSetting
    • 37db1a3d3eab543883b5bb3294c8358dca2a8b4b init_settings.h: Add DaemonWaitSetting
    • 1922241b7479293be87bec83bae3d11a40bff452 init_settings.h: Add DaemonWaitSetting
    • 8b8342663d2f8414c20c1345818e21cee1c6682c chainparamsbase_settings.h: Add SignetSeedNodeSetting
    • 308213012464d5a860e9ce5ee1bfaa973e08b9c1 chainparamsbase_settings.h: Add SignetChallengeSetting
    • c32750713ec2f7d56a66f44fbe387c5f5cf1ad39 init_settings.h: Add FastPruneSetting
    • ffc8916fe3d57a9e371edfb76fe593f94a016300 chainparamsbase_settings.h: Add TestActivationHeightSetting
    • 5cedd342ef18e0533114a0b3122f6d12654823bd chainparamsbase_settings.h: Add VbParamsSetting
    • ddb714affc34c6fc15708525666d1c39aff937fd chainparamsbase_settings.h: Add ChainSetting
    • 58362384726e112c17ac2871799e97c2c2ba074b chainparamsbase_settings.h: Add RegTestSetting
    • 052bff16d83846f7b6a7ea699e0c1c6a33ebf6d4 test/argsman_tests_settings.h: Add RegTestSetting
    • d0b4246e9721d611439abbdd41c125920855085c chainparamsbase_settings.h: Add TestNetSetting
    • c5ac6a3effbce3ddd55cb5608662294257584454 test/argsman_tests_settings.h: Add TestNetSetting
    • 946a5e550da9a2c88e85f09cf8a1b2925a48fba8 chainparamsbase_settings.h: Add TestNet4Setting
    • 185435faae18a6773a06cb939b198863dba521f6 chainparamsbase_settings.h: Add SignetSetting
    • 37f90b1c7853b768c974a30fe14617d6585cda34 common/args_settings.h: Add HelpSetting
    • 445e3389901d6f2afb6301d04eddda33779f9fba common/args_settings.h: Add HSettingHidden
    • 0d0bfed05ba7512ad660cce4dc50b8e39fe0f497 test/argsman_tests_settings.h: Add HSetting
    • 03e5da2267f6f0d44dba8bdc676b5b44f811eb17 common/args_settings.h: Add QSettingHidden
    • 7370e40e9b90c4f0838fd0ed50cb7432ed5363bd init_settings.h: Add BlocksDirSetting
    • 55790e70ecc08ea3fdd67351325c33a57d7dec6a init_settings.h: Add SettingsSetting
    • b92f21eaacc7c6d24bd89f773c02b85b3f339b26 init_settings.h: Add HelpDebugSetting
    • bc6d1edf29386532d47ef79356bc13d21f59838b init_settings.h: Add TestSetting
    • 1579e188c8dc747dad9d1208df79682417202496 init_settings.h: Add AllowIgnoredConfSetting
    • b68649f668921ba312845b36c2d2c2e37dc206bc dummywallet_settings.h: Add AddressTypeSettingHidden
    • 1c7f776320d2a330010c3d63196052666514cf1f wallet/init_settings.h: Add AddressTypeSetting
    • 54ced9a1450063a3dc6a28db6fe062a70ad67c1d dummywallet_settings.h: Add AvoidPartialSpendsSettingHidden
    • 7002a86aae96cc0d5b7af9180d809d1654d446bf wallet/init_settings.h: Add AvoidPartialSpendsSetting
    • a5ac859fe003e7d648e8447711fbc984d92deb3f dummywallet_settings.h: Add ChangeTypeSettingHidden
    • 3373cb01df9a6afc9193a4f25219bfbdb4ed504a wallet/init_settings.h: Add ChangeTypeSetting
    • c718f08485a02e2c324593881d446d1f637305f4 dummywallet_settings.h: Add ConsolidateFeeRateSettingHidden
    • 344bcd6b3d41c68cfabc21095da6578050c8de37 wallet/init_settings.h: Add ConsolidateFeeRateSetting
    • 108bbc99688f3e78d315c3228efb82c5e03639a8 dummywallet_settings.h: Add DisableWalletSettingHidden
    • 05b93c0f7eb08f28b99f41131d18a86542a4f1a3 wallet/init_settings.h: Add DisableWalletSetting
    • c12eba219182b02042a22ca84e365e2be3e9ad37 dummywallet_settings.h: Add DiscardFeeSettingHidden
    • deda151e0b7fdd0054f80e70f239a558ade35488 wallet/init_settings.h: Add DiscardFeeSetting
    • f48dd55391f8f68d8759c0ffeb8f6b76643898ce dummywallet_settings.h: Add FallbackFeeSettingHidden
    • 9e2597924fcf4a31f43949f374d7677a080e6845 wallet/init_settings.h: Add FallbackFeeSetting
    • c8e5428029fd9bc469ed39b5372d3eddc8512a0b dummywallet_settings.h: Add KeyPoolSettingHidden
    • 490c2f37dbec61e97dd8fb3027cb6f1e0edf69e3 wallet/init_settings.h: Add KeyPoolSetting
    • f9566d3742ca49dfb97cc087b280634df7831801 dummywallet_settings.h: Add MaxApsFeeSettingHidden
    • 376fdee411779eae1002caa0af9bff5058c5abdf wallet/init_settings.h: Add MaxApsFeeSetting
    • cbaf3922cbe60a3723c7eb5a44f1412c427291ce dummywallet_settings.h: Add MaxTxFeeSettingHidden
    • 69ee8a665d5fa3a922f30d436f6e4a23d8889acd wallet/init_settings.h: Add MaxTxFeeSetting
    • b934bcf0eac23125db80527397fa3f847c210f0b dummywallet_settings.h: Add MinTxFeeSettingHidden
    • e4362bdc78f89cb7551c2385f5829c54d70ac614 wallet/init_settings.h: Add MinTxFeeSetting
    • c146a592e95bb020228822c68406e5f5f8856462 dummywallet_settings.h: Add PayTxFeeSettingHidden
    • 66ef6eb9083946b031c2acdb2efc56cdf5bf3857 wallet/init_settings.h: Add PayTxFeeSetting
    • 76cf2750323a37050efd024d31df686654f20316 dummywallet_settings.h: Add SignerSettingHidden
    • 1f1e3c79285ed9942e4e534b79210fb792c05efa wallet/init_settings.h: Add SignerSetting
    • 615d346860e6ba356f6c2df0befdc313ad73972d dummywallet_settings.h: Add SpendZeroConfChangeSettingHidden
    • 441426730c57ed023a8d99669a4ba6944bb3c7fd wallet/init_settings.h: Add SpendZeroConfChangeSetting
    • 8faf6280d2f980280045c880a5bd5745de7662ec dummywallet_settings.h: Add TxConfirmTargetSettingHidden
    • 0e43216d8e433988c70cb610ee2024a356c6145c wallet/init_settings.h: Add TxConfirmTargetSetting
    • ef3552976575b8cdaab7f9bc0f3daa009e7e1220 dummywallet_settings.h: Add WalletBroadcastSettingHidden
    • 3c645b32df118052126e3020b81f789f27be4b70 wallet/init_settings.h: Add WalletBroadcastSetting
    • 5081192ab3fb1474c76e900a7fa6b0a0370b3105 dummywallet_settings.h: Add WalletDirSettingHidden
    • cf23fcb5ab2ada9427b8dc816f5cb130e01488b7 wallet/init_settings.h: Add WalletDirSetting
    • f0991aa925b894f673a14c36b2f23fa9db81064e dummywallet_settings.h: Add WalletNotifySettingHidden
    • ce7f97b648eb33e653713c0995f4286ac44ea098 wallet/init_settings.h: Add WalletNotifySetting
    • 5737f6f76ce37523f30a414cfae438575c0abaaa dummywallet_settings.h: Add WalletRbfSettingHidden
    • da7aa72e8d4f55ce7344d22a26dfa22ebf326c38 wallet/init_settings.h: Add WalletRbfSetting
    • 08750010a611e1c37196dc181502a9ecd7a26d8f dummywallet_settings.h: Add DbLogSizeSettingHidden
    • 99138f876cde5361e4f4ba67a0c0fee964d6a168 wallet/init_settings.h: Add DbLogSizeSetting
    • afce0b2c173ebe4614ea0caec1f6cedc994b9759 wallet/init_settings.h: Add DbLogSizeSetting
    • f76b5659591ad31703b870cc5f9b55acdd77ae6e dummywallet_settings.h: Add FlushWalletSettingHidden
    • 607e1524fd51d66988140c2da66e44b89cf831fc wallet/init_settings.h: Add FlushWalletSetting
    • d3cf734028614f80a83ea4e0005650c775da026d dummywallet_settings.h: Add PrivDbSettingHidden
    • 0443bd1d9771c04763aea4597a3e3777cfb7545f wallet/init_settings.h: Add PrivDbSetting
    • 967d2ab2287130d6a9183cfd6f1e9149c044bef2 dummywallet_settings.h: Add WalletRejectLongChainsSettingHidden
    • 138ade7c65ddc259fc9c1b7e5f3a7d9850568748 wallet/init_settings.h: Add WalletRejectLongChainsSetting
    • 9eb1d33fe99f5d90340a9b09746b46ea4303a6a9 dummywallet_settings.h: Add WalletCrossChainSettingHidden
    • f0850b79b7a8444b15b583c899a84ecab5c58c6e wallet/init_settings.h: Add WalletCrossChainSetting
    • aa5759a0326b4fd334410c8139b646d1dd79e5d3 dummywallet_settings.h: Add UnsafeSqliteSyncSettingHidden
    • a523c051ce391955e1dadeddc408730c69aeb282 wallet/init_settings.h: Add UnsafeSqliteSyncSetting
    • 760042994b366c3c20ca3a02a45b3dded715af2e wallet/init_settings.h: Add UnsafeSqliteSyncSetting
    • 04718dd1160020cb6d2be8f9501b0f1499e59c56 dummywallet_settings.h: Add SwapBdbEndianSettingHidden
    • bb4a6db55375fc235994957be7cf7f0952fbe862 wallet/init_settings.h: Add SwapBdbEndianSetting
    • c7a4801965a517a3a6eb4a7d4ba1185ea5391140 init_settings.h: Add RpcCookiePermsSetting
    • 32e5ecab28fd7b7ca903d4d1403d217b75b25bce init_settings.h: Add RpcAuthSetting
    • 9f3b099d226ee45ec9dc1b8d60c52d8b95806232 init_settings.h: Add RpcWhitelistDefaultSetting
    • 1b8c00a9bf84432883e6d7f7d3fff3b41b37232b init_settings.h: Add RpcWhitelistSetting
    • dc35e93d702cd61353e950de6af1804ba88b6d74 init_settings.h: Add RpcAllowIpSetting
    • 3edc421ec0c6ccad17c2fedd97b1fd439f8a5fe8 init_settings.h: Add RpcBindSetting
    • ebc782b68f99d7b82aeca85d98ba5f74384d7176 init_settings.h: Add RpcServerTimeoutSetting
    • 5d7ad0fca3dee767e8cff4207042dab152a509c9 init_settings.h: Add RpcWorkQueueSetting
    • caae5926c3b1d71bb0429e7ea9e12d0ba8181e88 init_settings.h: Add RpcThreadsSetting
    • 82051ef57edc3b397f611cfce0a9492cf2fd0b3e init_settings.h: Add AlertNotifySetting
    • 075c8c840cbab00f05b6fb02a4fe171717ce1b57 init_settings.h: Add AssumeValidSetting
    • 315fb27bc90daf2675ac10266637baf96f506f71 init_settings.h: Add BlocksXorSetting
    • 14bdf003fcbbcf500e4f8fed99970ed3d6d94066 init_settings.h: Add BlockNotifySetting
    • 479ba69378a6997c82738760ef5360140dda2f5e init_settings.h: Add BlockReconstructionExtraTxnSetting
    • 347792a9b0f7f03dfbded6de58f6c5c5c49e4e4a init_settings.h: Add BlocksOnlySetting
    • a5bcedc57fbdc55eece6d5c9cd227f4463ae0fdb init_settings.h: Add CoinStatsIndexSetting
    • db4921818b56f87bf02e6e8f81600641af8110f1 init_settings.h: Add DbBatchSizeSetting
    • 3f6fa286a8cf5d63ed03bb8af5629e84a8e721c4 init_settings.h: Add DbCacheSetting
    • 0264cced794bbc7a4ee0e30df0faea07bfb8b47a init_settings.h: Add IncludeConfSetting
    • 6ee33acffd4cdb28e905a696e0c94999e55e3700 init_settings.h: Add LoadBlockSetting
    • 6425f0b5ff76f8cea05de43e0c98525cdd873a98 init_settings.h: Add MaxMemPoolSetting
    • 85dab4356573c9ec8b30a42a8d1cc0a561cebd56 init_settings.h: Add MaxOrphanTxSetting
    • e619ecd41a7e2357ed7d222a0e6a877459e52f42 init_settings.h: Add MempoolExpirySetting
    • df3e9a063a08d2f2874c19c58158eee00c27db0e init_settings.h: Add MinimumChainWorkSetting
    • 2764eb8b28c7f10a8ac806183e1f86c7ca6f217f init_settings.h: Add ParSetting
    • 17376faf931325c229c6b8762f38a6eec960ca63 init_settings.h: Add PersistMempoolSetting
    • fb41ffc33e1a0a6ea61738927b4d4d1ac843e859 init_settings.h: Add PersistMempoolV1Setting
    • b1f729c0e4bc7b440a03f80ddd9fb7c6a6e04042 init_settings.h: Add PidSetting
    • 609f12c2bf62ce64bf21e8b354472a56ca34fcc8 init_settings.h: Add PruneSetting
    • d2fa73374d1f98dcd3f102fc2310d135a31014ac init_settings.h: Add ReIndexSetting
    • e263abc4776f9c6a83be1e9c0388b9d85297810c init_settings.h: Add ReIndexChainstateSetting
    • 9ec093ac80e193be93b2109ce4565ae31afaddf1 init_settings.h: Add StartupNotifySetting
    • e3ce95432f4bab29314739b38251dc89d67abc64 init_settings.h: Add ShutdownNotifySetting
    • cbd8bc6ba6fc86e51fbb3d61b94caf99451f57ae init_settings.h: Add TxIndexSetting
    • ae32edd1904452b71d9c136c9253034b6639c0f8 init_settings.h: Add BlockFilterIndexSetting
    • 6d0d5d5fef80770dcd95bccb6f31e46e086ca582 init_settings.h: Add AddNodeSetting
    • 7f72dca6ae0dd4760770690c7cabc6f5c97973c5 init_settings.h: Add AsMapSetting
    • 991ec891e03ee275d033c95dc4d4d6dd3a7e2fda init_settings.h: Add BanTimeSetting
    • 87974e3b99d4be7238784b453abe0c19872c45c4 init_settings.h: Add BindSetting
    • f0e10515cb5e2eb91bbd84b3cd7e02592cc11dfd init_settings.h: Add CJdnsReachableSetting
    • 7ea60e5f2165c565501ecd56966c109a4fbc401f init_settings.h: Add ConnectSetting
    • ddadc92aaf641e015a7c2953947a7d84f0f0f1dd init_settings.h: Add DiscoverSetting
    • da9b692ce0fc1a8f605cde928f9230c38f21900e init_settings.h: Add DnsSetting
    • 356de6d1d5275cb946fd93a0a29c99af264120b9 init_settings.h: Add DnsSeedSetting
    • e2cde81ffd7c34f7af872533232b9779d82a03d8 init_settings.h: Add ExternalIpSetting
    • f7fcc6c40cbc517ac6f4e375da2c8ee45dbc4ea5 init_settings.h: Add FixedSeedsSetting
    • 75f09e9b65fa046dd84b40c0fc7f9d94a485daec init_settings.h: Add ForceDnsSeedSetting
    • 54fae03b659343f14b9695a876466af7c3eacd13 init_settings.h: Add ListenSetting
    • ecc06b0a6cc1deefe7f039822d9fd836a5b06ae0 qt/test/optiontests_settings.h: Add ListenSetting
    • 20455952b6352b8ba40f29cf9c9d4fc45fc6a106 init_settings.h: Add ListenOnionSetting
    • dde91440bf3cd0c94c408f943c2496ad911d8cf0 qt/test/optiontests_settings.h: Add ListenOnionSetting
    • cceb11ee8e595a0ba62a34a25199a209a15ae6d3 init_settings.h: Add MaxConnectionsSetting
    • 87e799c87ad975281e9224e77dbad3a39ca2bbbf init_settings.h: Add MaxReceiveBufferSetting
    • e7e6e6079759d95a36da29f2e970ee42c1d9d307 init_settings.h: Add MaxSendBufferSetting
    • 80064afed91d448078b553fcd665a68a8a78ab00 init_settings.h: Add MaxUploadTargetSetting
    • 85cd5f61b2b56d9e916909823e9b0905e76d54ad init_settings.h: Add OnionSetting
    • f14a1f37b4956d78d39db4c5b29f7ad227bdd3d8 init_settings.h: Add OnionSetting2
    • cacf9025457ef2a6f0c77a5f8ffa5ac5b1583cb5 init_settings.h: Add I2pSamSetting
    • 7e7cfd57fd66e96db46dfa5c439cdb68f358a543 init_settings.h: Add I2pAcceptIncomingSetting
    • 6854657aecb42774c37b985e183cbb002c0e8831 init_settings.h: Add OnlyNetSetting
    • 466180bb889b61c171176570ffbd2ff94e770ddd init_settings.h: Add V2TransportSetting
    • f2eee78c218348cc6010d12f8de025bd69fb16c2 init_settings.h: Add PeerBloomFiltersSetting
    • 20a40d7ef3eb96dc8c6412e443dd39211a753e51 init_settings.h: Add PeerBlockFiltersSetting
    • 1a066f1e34ca77cc7d574318406318f3ee9fb9a2 init_settings.h: Add TxReconciliationSetting
    • 1f1fedb4b56ed627e3cf16a24ccd0f13474b72e8 init_settings.h: Add PortSetting
    • 3009fccda000391d0bfb3470be4185e3428aa48d init_settings.h: Add ProxySetting
    • 2ef2c4d6785d21dc3d0a28ebf7880e81c0e7eb75 init_settings.h: Add ProxySetting2
    • 8565e2a94029bf9fba41ba82d6a97982b445abb2 init_settings.h: Add ProxyRandomizeSetting
    • 7b24325590eb2b564e05b1ede7a1ecf0c6c7637b init_settings.h: Add SeedNodeSetting
    • c9dc6adb4885f5572d706b63cda728c08542eed0 init_settings.h: Add NetworkActiveSetting
    • f6af1a1b6c858f9398bae0d0dd84e4a0c7563cf2 init_settings.h: Add TimeoutSetting
    • 65d5d86c0e0cfe3f28adc283bba86240d0590c57 init_settings.h: Add PeerTimeoutSetting
    • da1674addc078c43840887ce08362fd1e062fecf init_settings.h: Add TorControlSetting
    • 01bc31938c64ff5b1e829d8c748123535482cfbf init_settings.h: Add TorPasswordSetting
    • 1fad250fc57b75f425de2d8e38fd1374dfaaa4a8 init_settings.h: Add UpnpSetting
    • 337276ffa7a92df7ce469003395f8b34931b5836 init_settings.h: Add NatPmpSetting
    • 1a55acf1b7d06e3dd3974022581ddeae931dd8fb init_settings.h: Add WhiteBindSetting
    • 0ad26ac1193f0448b948879ce93e0c7c1a924169 init_settings.h: Add WhiteListSetting
    • a429fc7db7347fdd9612d90e92f593a03e485fa5 init_settings.h: Add ZmqPubHashBlockSetting
    • 8005821b84fb89932fbd1bd64c8b3d9867981357 init_settings.h: Add ZmqPubHashBlockSetting
    • 638051f79db799eb80dbbbdf24c6fe552892b050 init_settings.h: Add ZmqPubHashTxSetting
    • 6d75dba3c4ee64905aeee9c88e254c8b495d6d79 init_settings.h: Add ZmqPubHashTxSetting
    • 160dbaa74fcf44ee05d94462973c8951a76a3680 init_settings.h: Add ZmqPubRawBlockSetting
    • 8b248f34a18fdd418e84ac012f719cd0b5e53ed3 init_settings.h: Add ZmqPubRawBlockSetting
    • 73eb0539da59895427c29d0f2d044ddac5574139 init_settings.h: Add ZmqPubRawTxSetting
    • 97b2eaf057e17c04a5a14902356afad80f76bab6 init_settings.h: Add ZmqPubRawTxSetting
    • 9b318310279f54f5156a9f378c3b2b0ff614912b init_settings.h: Add ZmqPubSequenceSetting
    • 6327fc1420c224c52289947013671123c839168d init_settings.h: Add ZmqPubSequenceSetting
    • 2a2b8d612c1a625101c6fd3e8d286e42ad3e8aab init_settings.h: Add ZmqPubHashBlockHwmSetting
    • 7f8941580176df724dc618ef2d7e9ae2bfe43b06 init_settings.h: Add ZmqPubHashBlockHwmSetting
    • 1519547c97e3dfb70346c696f9d0696e1b2f2aba init_settings.h: Add ZmqPubHashTxHwmSetting
    • f41423cc01ea02d96f9299b9889aa790c02fb2d8 init_settings.h: Add ZmqPubHashTxHwmSetting
    • 1ab11c6ea8655f4397fd1451c69db12046bea942 init_settings.h: Add ZmqPubRawBlockHwmSetting
    • fb42ff4ea80ec4e8be4325be984cb9169a1a6377 init_settings.h: Add ZmqPubRawBlockHwmSetting
    • bcbe35bbdd5fc1621aaa656f461034b3c6621948 init_settings.h: Add ZmqPubRawTxHwmSetting
    • 22b652d6196deaff39eade7fa330e5a86243a53a init_settings.h: Add ZmqPubRawTxHwmSetting
    • 265da9af822b937b91053680533a5d272fd92846 init_settings.h: Add ZmqPubSequenceHwmSetting
    • 3424d69f65254d4f5994969b25cb115ae69705a4 init_settings.h: Add ZmqPubSequenceHwmSetting
    • ec6797de12390ca03f39bf50830c3fd8acc09ed6 init_settings.h: Add CheckBlocksSetting
    • c03829ae2b1c710ace052127de58debb33cb8efa init_settings.h: Add CheckLevelSetting
    • affd1dbef5ee585cadf9211a545d1c7e38582d0d init_settings.h: Add CheckBlockIndexSetting
    • 848b7024f74ef23c5be554a2362a1fa76d90d2d7 init_settings.h: Add CheckMempoolSetting
    • 201bd2712eb6ded936414d009f5b7665de7912bd init_settings.h: Add CheckPointsSetting
    • a40b6331421427efe2f079ede6c4cd90ae6ecb96 init_settings.h: Add DeprecatedRpcSetting
    • aa1b89261cf7974bef3165ed756138456f7f186f init_settings.h: Add StopAfterBlockImportSetting
    • 2b3b1f9ecc993873f032f45783be6c974b5937b5 init_settings.h: Add StopAtHeightSetting
    • 773fdfc870b4d86b11b03a1d393e45c4e6b2e7ce init_settings.h: Add LimitAncestorCountSetting
    • 3a183d53e5fcafb49ecf35167a8752822fbcc548 init_settings.h: Add LimitAncestorSizeSetting
    • 8b406660d03573b1bc6bb7ba7450ae47fa901009 init_settings.h: Add LimitDescendantCountSetting
    • cb881aaf7fba23433519c7ca0a3477568af4e03d init_settings.h: Add LimitDescendantSizeSetting
    • 6e756937b70480f68b7db94bfea73c60b12f4587 init_settings.h: Add CaptureMessagesSetting
    • bee79e1cff694d5a6f12e5795b7dbc0dea91ce41 init_settings.h: Add MockTimeSetting
    • 807a5bd3e5d391cdf5968fbf5aa7ae87f2e24b68 init_settings.h: Add MaxSigCacheSizeSetting
    • adfaae91480e2fe955c3bdbf90cdaab9ce45b320 init_settings.h: Add MaxTipAgeSetting
    • 8e98f004e5e5a8b7c1bb8e06f309d24367321e89 init_settings.h: Add PrintPrioritySetting
    • 10431d0836b22a2b74c7e75007e65edb23e5e4b4 init_settings.h: Add UaCommentSetting
    • a56d7446006b120af0f8e253e7e3e5334d2822c1 init_settings.h: Add AcceptNonstdTxnSetting
    • a0e7932b201a0a1ad2ce320800a695e05276ecd6 init_settings.h: Add IncrementalRelayFeeSetting
    • 980180219aafa82401b58b6f1081352695b32ce1 init_settings.h: Add DustRelayFeeSetting
    • 7cd2d20e0014ae7eac2a51bfb6e5bc1077cb172d init_settings.h: Add AcceptStaleFeeEstimatesSetting
    • c95daffbba472d12427786e5bbd030d649d0660a init_settings.h: Add BytesPerSigOpSetting
    • 86d4d5117b0aeaa20d8e4c7ef5073f2ba6eae929 init_settings.h: Add DataCarrierSetting
    • b04fe015e9bae11b6e5ca7fbeee9b580e8d50c7e init_settings.h: Add DataCarrierSizeSetting
    • 735eb320c14964b65415fb16afff51e6960584e4 init_settings.h: Add PermitBareMultiSigSetting
    • 076881bbb817d47c55145f8ad6aad879ca8b048f init_settings.h: Add MinRelayTxFeeSetting
    • 36de8b45709ccbb36663bbb3a5fa1f4822d9dd2a init_settings.h: Add WhiteListForceRelaySetting
    • 0d7db4e95291358cb33a881088feb2dec9d52058 init_settings.h: Add WhiteListRelaySetting
    • 8e287d6c7a8acc859d93fbc051af7a18d64ebfb0 init_settings.h: Add BlockMaxWeightSetting
    • e066fac23b979569ec278dfe9ec4376fc733cd86 init_settings.h: Add BlockMinTxFeeSetting
    • 80d3188b6cd7c7bf74f829c184c01ad0e74d2dab init_settings.h: Add BlockVersionSetting
    • 10c206f312e0b515eceb164f277df18378963dcb init_settings.h: Add RestSetting
    • 12e26bd486145d4a94745924c5c89462e76c11fb init_settings.h: Add RpcDocCheckSetting
    • c43e5ac59444d1b8b6c7e6147250a155633bbacb init_settings.h: Add ServerSetting
    • aea9101f7af9900cd941c44d8bcbb212f6ead58d init_settings.h: Add IpcBindSetting
    • f80898a27a2c92a6e5cbc93c1f1845c8154dc799 init_settings.h: Add DbCrashRatioSettingHidden
    • 52542f98f2c223299bd1989d523f1141f845a5a3 init_settings.h: Add ForceCompactDbSettingHidden
    • cada2e835b8f297ce6e1ecffc4d96adfadd7678b init_settings.h: Add ChooseDataDirSettingHidden
    • c82eb648f2c1b85d4c8cf601c87ec250f1e7961b qt/bitcoin_settings.h: Add ChooseDataDirSetting
    • 60ef0aef9576bdceaf660608fc90abccebff0946 init_settings.h: Add LangSettingHidden
    • 4f2ce372dd825cd024efd2a27ee654df3d076260 qt/bitcoin_settings.h: Add LangSetting
    • f8ad80eadedb9cc4e8fe130f510c61222668410c init_settings.h: Add MinSettingHidden
    • 90340b22f599f8d04feb5134f5ca859bb8aca2aa qt/bitcoin_settings.h: Add MinSetting
    • d54d6b357d717e60f1d1ed885d241d6471f56f5d init_settings.h: Add ResetGuiSettingsSettingHidden
    • b0917f426b828764309e7f2ae37e9bff538643f2 qt/bitcoin_settings.h: Add ResetGuiSettingsSetting
    • 9221d42140ed1f4be9f768c65aaba1d81ad07ec0 init_settings.h: Add SplashSettingHidden
    • d49aee0381c7b0c31ece9489c9a4e7d8f89c3064 qt/bitcoin_settings.h: Add SplashSetting
    • fccda5314bde902936488c8939663c721b580f0d init_settings.h: Add UiPlatformSettingHidden
    • 0b5e165cee59f63b8de431108076eeab5b085b4f qt/bitcoin_settings.h: Add UiPlatformSetting
    • 023a48ca857e7ab60f59e55389e535d4f5085b26 init/common_settings.h: Add DebugLogFileSetting
    • df0357f631f82572140f2cb92e7461043c389710 init/common_settings.h: Add DebugExcludeSetting
    • c5e2b10e1a7142eaf8c75a3a908347c8d07d5804 init/common_settings.h: Add LogIpsSetting
    • bbd771ad67c767f53fa5cae56480d325cc31d2af init/common_settings.h: Add LogLevelSetting
    • 7ef24bde66cf21ce9f65d48a3cebadfff89d6e78 test/logging_tests_settings.h: Add LogLevelSetting2
    • eb2e4920a5fada57ea7155604feaf770a94ec87f test/logging_tests_settings.h: Add LogLevelSetting3
    • cf456d2eac9f76fc34c0aaa6421d0a10e83660ac test/logging_tests_settings.h: Add LogLevelSetting4
    • 195c9df9065727f8c74c9de625afdaf0492caab7 init/common_settings.h: Add LogTimestampsSetting
    • 285f6b9d16d4cc881ed1d4cf438588a0c813c8f3 init/common_settings.h: Add LogThreadNamesSetting
    • 00df3ca690289b2ce931f8cddf47db6e84a26b69 init/common_settings.h: Add LogSourceLocationsSetting
    • 1fcfdf07c14605d68b6caeb040bc63f81a3d8e12 init/common_settings.h: Add LogTimeMicrosSetting
    • 5aa9b6a81ecdebb058b4252b17cf41d4d3d1f8ea init/common_settings.h: Add LogLevelAlwaysSetting
    • fa4fdddff5d3e7e27a6a7832f2f727111aad1495 init/common_settings.h: Add ShrinkDebugFileSetting
    • 3afbe803afecb315867d0c51be4504bc2465ab49 test/argsman_tests_settings.h: Add ValueSetting
    • d5c4257f896bb2c4dc8b22d8db25be5d6933a7e6 test/argsman_tests_settings.h: Add ASetting
    • 99e47d470c7724062832e8a9f6a2c6c1eea35b03 test/argsman_tests_settings.h: Add BSetting
    • f53edaca22653363289b3cb0ed1ee5f33cba9a05 test/argsman_tests_settings.h: Add CccSetting
    • 77ac1340e0f5390a8bd5d4b2d5cdb65c6ef3cf1f test/argsman_tests_settings.h: Add FSetting
    • 14069b581bc63df9af21380dd6765132a189ae52 test/argsman_tests_settings.h: Add FSetting2
    • 76077a22c60b4ec86c54da8b12930d9a4e1d5e2c test/argsman_tests_settings.h: Add DSetting
    • 8be21a3e3b12caf4cd7c03d76c511f9199979338 test/argsman_tests_settings.h: Add NobSetting
    • 4c945dfd16dafd0d11afa73463db6d9332cc5c26 test/argsman_tests_settings.h: Add CSetting
    • f94dd3e186dd377da8fc28809a084221558293df test/argsman_tests_settings.h: Add ESetting
    • 5d4d4594ee3f8bf55a7579b2db377a0537ad3af2 test/argsman_tests_settings.h: Add FooSetting
    • 6b759d61c290b332fd81c4a4dab65ef39d6f7a01 test/getarg_tests_settings.h: Add FooSetting
    • 1497a6cefabaa5982eeceb7811a15e24aa933be4 test/getarg_tests_settings.h: Add FooSetting2
    • 55ae97e84dbd0f78f0ccb0b71b0fd5346d8393ea test/argsman_tests_settings.h: Add BarSetting
    • 7afc408fd96a3d2f7540a0e2b8ce30a35b3f5fe6 test/getarg_tests_settings.h: Add BarSetting
    • 06d3bb955bb3dd8882de960f992b696bc0d0df05 test/argsman_tests_settings.h: Add FffSetting
    • 0f00fd2f8c201491d88d8177c9d3f36d9bc4e3e8 test/argsman_tests_settings.h: Add GggSetting
    • c5177532722115fcd2ce9e2457f1d9a3d39fb6cc test/argsman_tests_settings.h: Add ISetting
    • 2d42d3761444d4593ad686113ac0851012198e03 test/argsman_tests_settings.h: Add ZzzSetting
    • 9a61b59d61ef25a2f6a3b8181051331ac260b3d9 test/argsman_tests_settings.h: Add IiiSetting
    • 398784c6270c81bcb62b4f181caf095494322315 test/argsman_tests_settings.h: Add NofffSetting
    • 47daaf52e9a66b2f3090bab57ee0450de69560ab test/argsman_tests_settings.h: Add NogggSetting
    • c027b9658e31511d48a042ab472ebefc662d1b2c test/argsman_tests_settings.h: Add NohSetting
    • 1c9ca198b97cda54dc6612af1f16727fb9b261a9 test/argsman_tests_settings.h: Add NoiSetting
    • 474cc8e15ca67e94a11a196389daa7ef071b07bd test/argsman_tests_settings.h: Add StrTest1Setting
    • 4379937cc3278231ad133750c6469d31acf03df6 test/argsman_tests_settings.h: Add StrTest2Setting
    • 9bd1f1650252abc85f7c0f18ce2f828b8a0b261f test/argsman_tests_settings.h: Add IntTest1Setting
    • f1605dcf380403103ddcaab3c3586995f3af7526 test/argsman_tests_settings.h: Add IntTest2Setting
    • 6cd811986c8fa45e6f017e46a5ab88b6a83567f9 test/argsman_tests_settings.h: Add IntTest3Setting
    • a35142c3a0eb357bb69f406ff884d9fdefd9166e test/argsman_tests_settings.h: Add BoolTest1Setting
    • ebaf5760e023cbe43d0eb74bdda0e8d90df02ba7 test/argsman_tests_settings.h: Add BoolTest2Setting
    • bff218dbe13e40af8ba3d16a2929f9058534614a test/argsman_tests_settings.h: Add BoolTest3Setting
    • 041c36954883fe37e4233cf1b6c732b2780eed56 test/argsman_tests_settings.h: Add BoolTest4Setting
    • 52884b3d4f2879f056d520cb751709b641328960 test/argsman_tests_settings.h: Add PriTest1Setting
    • f11eef2216b82ce5a3ba7618849b85d1e7954135 test/argsman_tests_settings.h: Add PriTest2Setting
    • 0827f502993ede574a9a76013986625c5eea9bb9 test/argsman_tests_settings.h: Add PriTest3Setting
    • 0d91519f66bf4539efacc33194f78194f12723b7 test/argsman_tests_settings.h: Add PriTest4Setting
    • 2418f99972124708fa9d6e3a1783d395f757a39b test/getarg_tests_settings.h: Add FoSetting
    • ea6a25c224f418b23c60aa9f0000854d6adcae1c test/getarg_tests_settings.h: Add FooOSetting
    • 865a9323ab3761045bad9abece5d5eebcdec0507 test/getarg_tests_settings.h: Add DirSetting
    • f9fffcbda7100c633235261f6507e2258bc41ed1 test/util/setup_common_settings.h: Add TestDataDirSetting
  96. DrahtBot commented at 1:02 am on December 14, 2024: contributor

    πŸ™ This pull request conflicts with the target branch and needs rebase.

  97. DrahtBot added the label Needs rebase on Dec 14, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-22 09:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me