Fixes #741
Starting bitcoin-qt with non-hyphen ("-") arguments causes it to silently ignore any later valid options. For instance, invoking bitcoin-qt -server=1 foo -regtest on a fresh install will run mainnet instead of regtest.
This change makes the client exit with an error message if any such “loose” arguments are encountered. This mirrors how bitcoind handles it:
However, BIP-21 bitcoin: payment URIs are still allowed, but only if they’re not followed by any additional options.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | hernanmarino, pablomartin4btc, maflcko, hebasto |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
I’m not entirely sure why they weren’t the same. In the beginning, bitcoind would handle RPC commands. Then QT got special-cased so that you couldn’t do that for bitcoin-qt – though it wouldn’t give any errors. It would just silently ignore the subsequent options.
Then the argument-handling code split entirely here. Now bitcoind and bitcoin-qt mostly did their own thing.
Subsequently, the ability to call RPC methods from bitcoind’s command line was removed, and “loose” (non-hyphen) arguments would result in errors.
I can’t see any good reason for “loose” arguments to remain in bitcoin-qt.
The “ignored options” issue was actually first raised here. The fix didn’t address the specific issue exactly, as it just caught unrecognized options (ie - arguments with a hyphen). But by then, as I mentioned, the “loose” argument problem had already been fixed in bitcoind.
Ah, thanks @achow101. I think this may still be salvageable by making an exception for just those arguments.
Will revisit tomorrow.
bitcoin-qt -help?
I could modify this PR to enforce that behavior and add the documentation to bitcoin-qt -help. Right now, the PR only allows non-option arguments which must start with bitcoin: (case insensitive). It will still ignore any options after that.
I think leaving the current master behavior and just adding documentation to reflect it would be insufficient. My worry is that a user could be fooled by something like:
Scammer: “Can you send me some testnet coins? Start your client with bitcoin-qt select-chain -testnet. You should have the same number of testnet coins as you have on the real network. Just send me some, please.”
It’s also just an annoyance. I spent an embarrassingly long time trying to figure out why bitcoin-qt datadir=bitcoindatadir -regtest kept opening mainnet before I realized I omitted the hyphen on datadir.
Starting bitcoin-qt with non-dash ("-") arguments causes it to
silently ignore any later valid options. This change makes the
client exit with an error message if any such "loose" arguments
are encountered.
However, allow BIP-21 'bitcoin:' URIs only if no other options
follow.bitcoin: URIs if no other options follow.
tACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1
I like both features the error shown when invalid args are passed and the error for options that are passed following a BIP21.
For the last one and regarding the -regtest being ignored in your original PR description, I detected a similar scenario in bitcoin-cli a few months ago (https://github.com/bitcoin/bitcoin/pull/26990 - still in progress) where [options] passed after a command that receives arguments are not detected by the ArgsManager, they are being interpreted as another arg of the last command.
Last thing, not part of the scope of this PR, I think we could validate the BIP21 URI grammar before opening bitcoin-qt.
556+ }
557+#endif
558+ if (payment_server_token_seen && arg.startsWith("-")) {
559+ InitError(Untranslated(strprintf("Options ('%s') cannot follow a BIP-21 payment URI", argv[i])));
560+ QMessageBox::critical(nullptr, PACKAGE_NAME,
561+ // message cannot be translated because translations have not been initialized
554+ invalid_token &= false;
555+ payment_server_token_seen = true;
556+ }
557+#endif
558+ if (payment_server_token_seen && arg.startsWith("-")) {
559+ InitError(Untranslated(strprintf("Options ('%s') cannot follow a BIP-21 payment URI", argv[i])));
0 InitError(Untranslated(strprintf("Options (eg, '%s') cannot follow a URI", argv[i])));