refactor: Implement missing error checking for ArgsManager flags #16545

ryanofsky · 2019-08-04T14:13:12Z

Trigger startup errors if bitcoin is configured with bad setting values according to flags. Also raise internal errors if settings are registered and retrieved with inconsistent flags.

This change has no effect on behavior because the new ArgsManager flags added here are not used outside of tests yet.

It'll probably be easier to start applying type checking flags to new options than existing options. But for examples of how type checking flags can make sense applied to existing options, see the new example_options unit test added in this PR.

Followups:

Support for flag combinations is possible and is added in commit c919f51 (branch)
ALLOW_LIST flags are added and enforced in #17580
Bad IsArgSet usages with ALLOW_LIST are removed in #30529 and prevented in #17783
Confusing and ignored multiple assignments in config files are disallowed in #17493
Confusing reverse-precedence settings merging code is removed in #17581
Type flags added in this PR implement runtime part of a change to add more compile-time checking in #22978

ryanofsky · 2019-08-04T14:14:53Z

FYI @hebasto, this adds error checking for the flags from #16097

src/util/system.h

DrahtBot · 2019-08-04T14:59:06Z

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/16545.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	laanwj, hodlinator
Approach ACK	l0rinc
Approach NACK	ajtowns
Stale ACK	hebasto

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

hebasto · 2019-08-04T15:21:27Z

It seems that not providing a new InterpretNegated() function has some benefits:

no need for key_name local variables
diff gets much smaller

promag

Looks good, read the code and tried locally. Will review tests, but for now just a couple of comments.

src/util/system.cpp

src/util/system.h

ryanofsky

Thanks for reviews!

re: #16545 (comment)

It seems that not providing a new InterpretNegated() function has some benefits:

no need for key_name local variables

diff gets much smaller

The function is only 9 lines long, so I don't see it having a very big impact on the size of the diff, but there are a few reasons why I think it's important for this to be separate and not part of FlagsOfKnownArg:

FlagsOfKnownArg is now called at argument retrieval time, not jut argument parsing time, so it's more efficient and easier to reason about if it is only doing a pure map lookup without string manipulation and parsing.
Having this be a separate function makes InterpretOption shorter and more understandable. Instead of parsing option names, parsing option values, and dealing with flags, it now only parses option values and no longer deals with flags or names at all.
Getting rid of the InterpretNegated function may reduce the size of this diff, but it would increase the diff in #15934, because it would leave more code for that PR to deduplicate and update. It's nice to take care of some work for #15934 when the changes make sense here as well.

src/util/system.h

src/util/system.cpp

src/util/system.h

promag · 2019-08-07T10:58:55Z

Side note, I find FlagsOfKnownArg confusing because it also returns for unknown arguments and it doesn't tell if it's known or not. I'd reword to GetArgFlags.

ryanofsky

Promag's suggestion to work on a commit using the flags
#16545 (comment) was really useful. Turns out after trying this out with wallet flags, the empty string check for TYPE_STRING is a lot less useful than I thought it would be, so I'm going to simplify the flags a little and get rid of it.

It does seem good to just start off with a minimal set of checks,, and then in the future when we think of checks that actually are useful in practice (maybe checks for ip addresses, timestamps, hex strings, value ranges) we can add them at that point.

I'm going to:

Make a little followup PR applying new types to some wallet args
Tweak flags a little in this PR to be less crazy about empty strings
Add example of using flags in the PR description as suggested and link to wallet flag PR
Improve comments, make /* Standard value types. */ comment more descriptive and add notes to IsArgSet and IsArgNegated methods explaining how there's no need to call these if the argument has a type declared.

Actual changes to this PR will be very small, so it still should be reviewable now.

Side note, I find FlagsOfKnownArg confusing because it also returns for unknown arguments and it doesn't tell if it's known or not. I'd reword to GetArgFlags.

Both seem good to me, but get GetArgFlags does seem a little more standard, so I'll rename if @hebasto also says it's better or just as good.

src/util/system.h

hebasto · 2019-08-10T16:09:28Z

Concept ACK 01ca54a

@promag

Side note, I find FlagsOfKnownArg confusing because it also returns for unknown arguments and it doesn't tell if it's known or not. I'd reword to GetArgFlags.

For unknown arguments it returns ArgsManager::NONE.

@ryanofsky

Both seem good to me, but get GetArgFlags does seem a little more standard, so I'll rename if @hebasto also says it's better or just as good.

Naming is the hardest part of coding ;)
Agree with @promag's suggestion about renaming.

src/util/system.h

Discussed: bitcoin#16545 (comment)

Rename FlagsOfKnownArg to GetArgFlags Discussed: bitcoin#16545 (comment)

ryanofsky

Updated c7d018d -> b5e8b2a (pr/argcheck.5 -> pr/argcheck.6, compare) with changes described in #16545 (review).

Agree with @promag's suggestion about renaming.

Now renamed in 0779ce5.

src/util/system.h

laanwj · 2019-10-23T10:58:53Z

Concept ACK, consistent argument error checking is good.

Rename suggested by João Barbosa <[email protected]> bitcoin#16545 (comment) This also gets rid of ArgsManager::NONE constant, which was an implementation detail not meant to be used by ArgsManager callers. Finally this reverts a change from 7f40528 bitcoin#15934 adding "-" characters to argument names. Better for GetArgFlags to require "-" prefixes for consistency with other argument methods, and to be more efficient later when GetArg functions need to call GetArgFlags. This commit does not change behavior.

ryanofsky

Updated 5a94560 -> b5ef854 (pr/argcheck.42 -> pr/argcheck.43, compare) implementing review suggestions

src/common/args.cpp

ryanofsky · 2024-10-02T16:02:31Z

src/common/args.cpp

-        return std::nullopt;
+    if (value) {
+        int64_t parsed_int;
+        if ((flags & ArgsManager::ALLOW_STRING) || !IsTypedArg(flags) || value->empty()) return *value;


re: #16545 (comment)

It's interesting to see what that looks like but I see few disadvantages to that approach:

Superficially, the resulting code is longer and the diff is bigger. Current PR is trying to minimally change this function.

It seems to be duplicating some code like "Cannot set -%s with no value" error, increasing risk that differences between the two copies of code will be introduced accidentally, and introducing unintended differences in behavior for typed and untyped arguments.

It seems to make it harder to get a high level sense of how argument parsing is intended to work, for example parsing of negated arguments is split up across three functions instead of being handled one place.

It seems to do more string manipulation, breaking up error messages into smaller strings that might be harder to search for and translate (if we want to support that later).

It might not work as well with my branch in RFC: ArgsManager type and range checking #22978. In this PR, distinction between typed and legacy arguments is all-or-nothing, while in the other branch I want allow supporting and disabling legacy behavior with granular options like SettingOptions::allow_double_negation to make code clearer and offer more flexibility for migrating away from legacy behaviors.

ryanofsky · 2024-10-02T16:21:14Z

src/test/fuzz/system.cpp

                auto help = fuzzed_data_provider.ConsumeRandomLengthString(16);
                auto flags = fuzzed_data_provider.ConsumeIntegral<unsigned int>() & ~ArgsManager::COMMAND;
+                // Avoid hitting "ALLOW_INT flag is incompatible with ALLOW_STRING", etc exceptions
+                if (flags & ArgsManager::ALLOW_ANY) flags &= ~(ArgsManager::ALLOW_BOOL | ArgsManager::ALLOW_INT | ArgsManager::ALLOW_STRING);


re: #16545 (comment)

let's use the flag helper methods here as well

Can see response to #16545 (comment), but logic in this fuzzing code is already obscure and I feel like hiding the flags in the if conditions while continuing to use the flags in the body of the if statements would only make it more obscure.

src/test/fuzz/system.cpp

src/test/util/setup_common.h

src/test/argsman_tests.cpp

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

95a0104 test: Add tests for directories in place of config files (Hodlinator) e85abe9 args: Catch directories in place of config files (Hodlinator) e4b6b18 test: Add tests for -noconf (Hodlinator) 483f0da args: Properly support -noconf (Hodlinator) 312ec64 test refactor: feature_config_args.py - Stop nodes at the end of tests, not at the beginning (Hodlinator) 7402658 test: -norpccookiefile (Hodlinator) 39cbd4f args: Support -norpccookiefile for bitcoind and bitcoin-cli (Hodlinator) e82ad88 logs: Use correct path and more appropriate macros in cookie-related code (Hodlinator) 6e28c76 test: Harden testing of cookie file existence (Hodlinator) 75bacab test: combine_logs.py - Output debug.log paths on error (Hodlinator) bffd92f args: Support -nopid (Hodlinator) 12f8d84 args: Disallow -nodatadir (Hodlinator) 6ff9662 scripted-diff: Avoid printing version information for -noversion (Hodlinator) e8a2054 doc args: Document narrow scope of -color (Hodlinator) Pull request description: - Document `-color` as only applying to `-getinfo`, to be less confusing for bitcoin-cli users. - No longer print version information when getting passed `-noversion`. - Disallow `-nodatadir` as we cannot run without one. It was previously interpreted as a mix of unset and as a relative path of "0". - Support `-norpccookiefile` - Support `-nopid` - Properly support `-noconf` (instead of working by accident). Also detect when directories are specified instead of files. Prompted by investigation in #16545 (review). ACKs for top commit: l0rinc: utACK 95a0104 achow101: ACK 95a0104 ryanofsky: Code review ACK 95a0104. Looks good! Thanks for all your work on this breaking the changes down and making them simple. Tree-SHA512: 5174251e6b9196a9c6d135eddcb94130295c551bcfccc78e633d9e118ff91523b1be0d72828fb49603ceae312e6e1f8ee2651c6a2b9e0f195603a73a9a622785

ryanofsky

Appreciate the reviews.

Updated b5ef854 -> d28a5c8 (pr/argcheck.43 -> pr/argcheck.44, compare) with some suggested changes.
Updated d28a5c8 -> 2057315 (pr/argcheck.44 -> pr/argcheck.45, compare) to fix typo in fuzz check

Updated 2057315 -> 4090dcf (pr/argcheck.45 -> pr/argcheck.46, compare) to fix typo in fuzz check

While I think this PR is in a good state, I'd encourage review of #31260 ahead of this, because #31260 allows setting types to be specified with C++ types instead of flags, which should semantics implemented here more obvious when this can be rebased on top.

Rebased 4090dcf -> 7cb06ef (pr/argcheck.46 -> pr/argcheck.47, compare)

Rebased 7cb06ef -> 68909e1 (pr/argcheck.47 -> pr/argcheck.48, compare)

ryanofsky · 2024-12-09T19:59:53Z

src/common/args.cpp

-        return std::nullopt;
+    if (value) {
+        int64_t parsed_int;
+        if ((flags & ArgsManager::ALLOW_STRING) || !IsTypedArg(flags) || value->empty()) return *value;


re: #16545 (comment)

Currently I need to know too much about the possible states on a single level, would prefer excluding some states while I'm reviewing.

I think a good strategy is to review the function twice. Review it one time assuming the argument is typed, and another time assuming the argument is untyped, and make sure behavior makes sense in both cases. This approach should also make it easier to see that there aren't unnecessary inconsistencies between the two cases, since there aren't two functions that need to be compared.

Also, I expect this function to get simpler if #31260 is merged first and this is rebased on top. The int/string/bool parsing logic currently here should move into the SettingTraits classes introduced in that PR.

ryanofsky · 2024-12-09T20:00:52Z

src/common/args.cpp

    }
+
+    // Disallow flag combinations that would result in nonsensical behavior or a bad UX.
+    if ((flags & ALLOW_ANY) && (flags & (ALLOW_BOOL | ALLOW_INT | ALLOW_STRING))) {


re: #16545 (comment)

I wouldn't really consider this encapsulating. It is adding a level of indirection and changing syntax used to access flags without reducing the need for callers to know everything about them. #31260 adds a better way of specifying types without flags so that could be a way forward here.

ryanofsky · 2024-12-09T20:19:45Z

src/test/fuzz/system.cpp

                auto help = fuzzed_data_provider.ConsumeRandomLengthString(16);
                auto flags = fuzzed_data_provider.ConsumeIntegral<unsigned int>() & ~ArgsManager::COMMAND;
+                // Avoid hitting "ALLOW_INT flag is incompatible with ALLOW_STRING", etc exceptions
+                if (flags & ArgsManager::ALLOW_ANY) flags &= ~(ArgsManager::ALLOW_BOOL | ArgsManager::ALLOW_INT | ArgsManager::ALLOW_STRING);


re: #16545 (comment)

I mean, you've already extracted a IsTypedArg(flags) method for ArgsManager::ALLOW_BOOL | ArgsManager::ALLOW_INT | ArgsManager::ALLOW_STRING, right?

IsTypedArg is used in higher level code to determine if type checking and coercion should be done. I think that code is easier to understand if the distinction between typed and untyped arguments is explicit and it is not dealing with individual flags.

By contrast, this code is low level fuzz code directly dealing with interactions between the flags, and I think it is clearer if it can reference them directly.

src/test/fuzz/system.cpp

…case behavior a85e8c0 doc: Add some general documentation about negated options (Ryan Ofsky) 490c8fa doc: Add release notes summarizing negated option behavior changes. (Ryan Ofsky) 458ef0a refactor: Avoid using IsArgSet() on -connect list option (Ryan Ofsky) 752ab9c test: Add test to make sure -noconnect disables -dnsseed and -listen by default (Ryan Ofsky) 3c2920e refactor: Avoid using IsArgSet() on -signetseednode and -signetchallenge list options (Ryan Ofsky) d056689 refactor: Avoid using IsArgSet() on -debug, -loglevel, and -vbparams list options (Ryan Ofsky) 3d1e8ca Normalize inconsistent -noexternalip behavior (Ryan Ofsky) ecd590d Normalize inconsistent -noonlynet behavior (Ryan Ofsky) 5544a19 Fix nonsensical bitcoin-cli -norpcwallet behavior (Ryan Ofsky) 6e8e7f4 Fix nonsensical -noasmap behavior (Ryan Ofsky) b6ab350 Fix nonsensical -notest behavior (Ryan Ofsky) 6768389 Fix nonsensical -norpcwhitelist behavior (Ryan Ofsky) e03409c Fix nonsensical -norpcbind and -norpcallowip behavior (Ryan Ofsky) 40c4899 Fix nonsensical -nobind and -nowhitebind behavior (Ryan Ofsky) 5453e66 Fix nonsensical -noseednode behavior (Ryan Ofsky) Pull request description: The PR changes behavior of negated `-noseednode`, `-nobind`, `-nowhitebind`, `-norpcbind`, `-norpcallowip`, `-norpcwhitelist`, `-notest`, `-noasmap`, `-norpcwallet`, `-noonlynet`, and `-noexternalip` options, so negating these options just clears previously specified values doesn't have other side effects. Negating options on the command line can be a useful way of resetting options that may have been set earlier in the command line or config file. But before this change, negating these options wouldn't fully reset them, and would have confusing and undocumented side effects (see commit descriptions for details). Now, negating these options just resets them and behaves the same as not specifying them. Motivation for this PR is to fix confusing behaviors and also to remove incorrect usages of the `IsArgSet()` function. Using `IsArgSet()` tends to lead to negated option bugs in general, but it especially causes bugs when used with list settings returned by `GetArgs()`, because when these settings are negated, `IsArgSet()` will return true but `GetArgs()` will return an empty list. This PR eliminates all uses of `IsArgSet()` and `GetArgs()` together, and followup PR #17783 makes it an error to use `IsArgSet()` on list settings, since calling `IsArgSet()` is never actually necessary. Most of the changes here were originally made in #17783 and then moved here to be easier to review and avoid a dependency on #16545. ACKs for top commit: achow101: ACK a85e8c0 danielabrozzoni: re-ACK a85e8c0 hodlinator: re-ACK a85e8c0 Tree-SHA512: dd4b19faac923aeaa647b1c241d929609ce8242b43e3b7bc32523cc48ec92a83ac0dc5aee79f1eba8794535e0314b96cb151fd04ac973671a1ebb9b52dd16697

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

DrahtBot · 2025-12-12T19:59:59Z

🚧 At least one of the CI tasks failed.
_{Task macOS native, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/20166545486/job/57891252209}
_{LLM reason (✨ experimental): Fuzzing failed due to an uncaught std::logic_error (Can't call ForceSetArg) while processing the system fuzz corpus, causing non-zero exit.}

Hints

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.

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

This commit just adds documentation for the type flags. The flags are actually implemented in the following two commits.

… startup This commit implements support for new ALLOW_BOOL, ALLOW_INT, ALLOW_STRING, and ALLOW_LIST flags by validating settings with these flags earlier on startup and providing detailed error messages to users. The new flags implement stricter error checking than ALLOW_ANY. For example, a double negated option like -nosetting=0 is treated like an error instead of true, and an unrecognized bool value like -setting=true is treated like an error instead of false. And if a non-list setting is assigned multiple times in the same section of a configuration file, the later assignments trigger errors instead of being silently ignored. The new flags also provide type information that allows ArgsManager GetSettings() and GetSettingsList() methods to return typed integer and boolean values instead of unparsed strings. The changes in this commit have no effect on current application behavior because the new flags are only used in unit tests. The existing ALLOW_ANY checks in the argsman_tests/CheckValueTest confirm that no behavior is changing for current settings, which use ALLOW_ANY.

…LLOW flags Update GetArg, GetArgs, GetBoolArg, and GetIntArg helper methods to work conveniently with ALLOW_BOOL, ALLOW_INT, and ALLOW_STRING flags. The GetArg methods are convenience wrappers around the GetSetting method. The GetSetting method returns the originally parsed settings values in their declared bool/int/string types, while the GetArg wrappers provide extra type-coercion and default-value fallback features as additional conveniences for callers. This commit makes two changes to GetArg, GetArgs, GetBoolArg, and GetIntArg helper methods when BOOL/INT/STRING flags are used: 1. GetArg methods will now raise errors if they are called with inconsistent flags. For example, GetArgs will raise a logic_error if it is called on a non-LIST setting, GetIntArg will raise a logic_error if it is called on a non-INT setting. 2. GetArg methods will now avoid various type coersion footguns when they are called on new BOOL/INT/STRING settings. Existing ALLOW_ANY settings are unaffected. For example, negated settings will return "" empty strings instead of "0" strings (in the past the "0" strings caused strangeness like "-nowallet" options creating wallet files named "0"). The new behaviors are fully specified and checked by the `CheckValueTest` unit test. The ergonomics of the GetArg helper methods are subjective and the behaviors they implement can be nitpicked and debated endlessly. But behavior of these helper methods does not dictate application behavior, and they can be bypassed by calling GetSetting and GetSettingList methods instead. If it's necessary, behavior of these helper methods can also be changed again in the future. The changes have no effect on current application behavior because the new flags are only used in unit tests. The `setting_args` unit test and ALLOW_ANY checks in the `CheckValueTest` unit test are unchanged and confirm that `GetArg` methods behave the same as before for ALLOW_ANY flags (returning the same values and throwing the same exceptions).

Co-authored-by: Hodlinator <[email protected]>

The type flags aren't currently used to validate or convert settings in the settings.json file, but they should be in the future. Add test to check current behavior that can be extended when flags are applied. Co-authored-by: Hodlinator <[email protected]>

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

ryanofsky force-pushed the pr/argcheck branch from 42ce595 to 4ca5242 Compare August 4, 2019 14:23

hebasto reviewed Aug 4, 2019

View reviewed changes

src/util/system.h Outdated Show resolved Hide resolved

hebasto reviewed Aug 4, 2019

View reviewed changes

src/util/system.h Outdated Show resolved Hide resolved

DrahtBot added Tests Utils/log/libs labels Aug 4, 2019

hebasto mentioned this pull request Aug 5, 2019

Disallow double negation syntax (-nofoo=0) for options #16508

Closed

ryanofsky force-pushed the pr/argcheck branch from 4ca5242 to 0097330 Compare August 5, 2019 19:52

ryanofsky marked this pull request as ready for review August 5, 2019 19:58

promag reviewed Aug 7, 2019

View reviewed changes

src/util/system.cpp Outdated Show resolved Hide resolved

src/util/system.h Outdated Show resolved Hide resolved

ryanofsky commented Aug 7, 2019

View reviewed changes

src/util/system.h Outdated Show resolved Hide resolved

src/util/system.h Outdated Show resolved Hide resolved

src/util/system.cpp Outdated Show resolved Hide resolved

src/util/system.h Outdated Show resolved Hide resolved

ryanofsky force-pushed the pr/argcheck branch from 0097330 to 01ca54a Compare August 7, 2019 10:42

ryanofsky commented Aug 8, 2019

View reviewed changes

src/util/system.h Outdated Show resolved Hide resolved

hebasto reviewed Aug 10, 2019

View reviewed changes

src/util/system.h Outdated Show resolved Hide resolved

ryanofsky force-pushed the pr/argcheck branch from 01ca54a to c7d018d Compare August 13, 2019 00:47

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Sep 9, 2019

Rename FlagsOfKnownArg to GetArgFlags

0779ce5

Discussed: bitcoin#16545 (comment)

ryanofsky force-pushed the pr/argcheck branch from c7d018d to b5e8b2a Compare September 9, 2019 20:34

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Sep 9, 2019

argcheck # Rename FlagsOfKnownArg to GetArgFlags

36f2ade

Rename FlagsOfKnownArg to GetArgFlags Discussed: bitcoin#16545 (comment)

ryanofsky commented Sep 9, 2019

View reviewed changes

src/util/system.h Outdated Show resolved Hide resolved

ryanofsky mentioned this pull request Oct 23, 2019

Merge settings one place instead of five places #15934

Merged

DrahtBot added the Needs rebase label Oct 28, 2019

This was referenced Nov 5, 2019

WIP: refactor: Use our own integer parsing/formatting everywhere #17385

Closed

test: Add util_ArgParsing test #17390

Merged

ryanofsky mentioned this pull request Nov 14, 2019

refactor: Settings code cleanups #17473

Merged

ryanofsky commented Oct 2, 2024

View reviewed changes

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Oct 4, 2024

common: Grammar / formatting tweaks

98f2edf

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

hodlinator mentioned this pull request Nov 4, 2024

util: Improve documentation and negation of args #31212

Merged

ryanofsky mentioned this pull request Nov 8, 2024

scripted-diff: Type-safe settings retrieval #31260

Open

ryanofsky mentioned this pull request Dec 5, 2024

Args: -noconnect=0 is interpreted as -connect=0.0.0.1 #31426

Open

ryanofsky force-pushed the pr/argcheck branch from b5ef854 to d28a5c8 Compare December 9, 2024 20:27

ryanofsky commented Dec 9, 2024

View reviewed changes

ryanofsky force-pushed the pr/argcheck branch 2 times, most recently from 2057315 to 4090dcf Compare December 9, 2024 21:15

ryanofsky mentioned this pull request Jan 10, 2025

Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior #30529

Merged

coderabbitai bot mentioned this pull request Feb 5, 2025

backport: merge bitcoin#24041, #24375, #24498, merge bitcoin-core/gui#408, #398, #434, #439, #319, #404, #569, #576, #618, #620, #631, #591, partial bitcoin#23757 (qt backports: part 3) dashpay/dash#6541

Merged

5 tasks

coderabbitai bot mentioned this pull request Feb 16, 2025

backport: bitcoin#22766, #22972, #23305, #23332, #23338, #23371, #23410, #23482, partial #23311 dashpay/dash#6580

Merged

5 tasks

hodlinator mentioned this pull request Mar 12, 2025

CLI cleanups #31887

Merged

DrahtBot mentioned this pull request May 7, 2025

test: Add and use ElapseTime helper #32430

Open

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Dec 12, 2025

common: Grammar / formatting tweaks

25d1119

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Dec 12, 2025

common: Grammar / formatting tweaks

57cefac

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

ryanofsky force-pushed the pr/argcheck branch from 4090dcf to 7cb06ef Compare December 12, 2025 12:18

DrahtBot added the CI failed label Dec 12, 2025

ryanofsky and others added 6 commits December 15, 2025 11:42

common: Grammar / formatting tweaks

986fd4d

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

doc: Add detailed ArgsManager type flag documention

053c5b4

This commit just adds documentation for the type flags. The flags are actually implemented in the following two commits.

test: Add tests to demonstrate usage of ArgsManager flags

fb48ff0

Co-authored-by: Hodlinator <[email protected]>

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Dec 16, 2025

common: Grammar / formatting tweaks

6444411

Implement cleanup suggestions from l0rinc: bitcoin#16545 (comment) bitcoin#16545 (comment) bitcoin#16545 (comment)

ryanofsky force-pushed the pr/argcheck branch from 7cb06ef to 68909e1 Compare December 16, 2025 02:10

DrahtBot removed the CI failed label Dec 16, 2025

refactor: Implement missing error checking for ArgsManager flags #16545

Are you sure you want to change the base?

refactor: Implement missing error checking for ArgsManager flags #16545

Conversation

ryanofsky commented Aug 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky commented Aug 4, 2019

Uh oh!

Uh oh!

Uh oh!

DrahtBot commented Aug 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

hebasto commented Aug 4, 2019

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

promag commented Aug 7, 2019

Uh oh!

ryanofsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hebasto commented Aug 10, 2019

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

laanwj commented Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanofsky Oct 2, 2024

Choose a reason for hiding this comment

Uh oh!

ryanofsky Oct 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanofsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

ryanofsky Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

ryanofsky Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DrahtBot commented Dec 12, 2025

ryanofsky commented Aug 4, 2019 •

edited

Loading

DrahtBot commented Aug 4, 2019 •

edited

Loading

ryanofsky left a comment •

edited

Loading

laanwj commented Oct 23, 2019 •

edited

Loading

ryanofsky left a comment •

edited

Loading