Stricter validation for CLI options and syntax #11819

issue unsystemizer opened this issue on December 2, 2017
  1. unsystemizer commented at 7:01 PM on December 2, 2017: contributor

    Using 0.15.1 from BitcoinCore.org:

    $ bitcoin-qt -rescan 1 -wallet otherWallet.dat 
    PaymentServer::ipcSendCommandLine: Payment request file does not exist:  "1"
    "PaymentServer::verifySize: Payment request too large (720896 bytes, allowed 50000 bytes)."
    

    Maybe this pilot error could be handled better. For example, Bitcoin-Qt (or bitcoind) could refuse to start.

    Note that in the above case Bitcoin-Qt silently ends up using a wrong (default) wallet.

  2. promag commented at 12:08 PM on December 6, 2017: member

    -rescan has no value.

    Note that in the above case Bitcoin-Qt silently ends up using a wrong (default) wallet.

    It should be -wallet=....

    Please run bitcoin-qt -help.

  3. promag cross-referenced this on Dec 6, 2017 from issue Problems with command-line options silently ignored by laanwj
  4. unsystemizer commented at 2:10 PM on December 7, 2017: contributor

    I know it’s all wrong, but the problem is despite the both options being syntactically wrong, service still starts, whereas it IMO shouldn’t.

    Thanks for linking to the existing issue. I disagree with the view that it’s worse to fail to start than start with a wrong wallet or with “fake rescan”(or whatever else is passed and appears to work whereas it doesn’t at all).

    The above example illustrates how a fake rescan of a wrong (new) wallet results in no funds found, ie after your daemon starts you conclude that your old otherWallet.dat can be deleted because it’s empty.

    Why put convenience before correctness when it comes to daemon startup options? No one who didn’t mess with their settings will have a problem starting service, and those who did shouldn’t succeed to start it if they passed syntactically wrong options.

    I am okay if the maintainers close this because it’s a duplicate.

  5. promag commented at 2:24 PM on December 7, 2017: member

    I think it's fine to not launch the process in these cases.

  6. meshcollider cross-referenced this on Dec 12, 2017 from issue Add configuration file/argument testing by meshcollider
  7. MarcoFalke cross-referenced this on Dec 14, 2017 from issue Replace atoi64 with ParseInt64 by promag
  8. laanwj referenced this in commit cfd99ddc3c on Dec 20, 2017
  9. Sjors commented at 2:00 PM on March 16, 2018: member

    Concept ACK on more aggressively checking parameters. Also agree that this ticket can be closed as duplicate of #1044.

  10. meshcollider closed this on Mar 17, 2018

  11. virtload referenced this in commit 6ad76a91f9 on Apr 4, 2018
  12. PastaPastaPasta referenced this in commit bdf1e71125 on Feb 13, 2020
  13. PastaPastaPasta referenced this in commit d7dec0fbfb on Feb 27, 2020
  14. PastaPastaPasta referenced this in commit 262bac5213 on Feb 27, 2020
  15. PastaPastaPasta referenced this in commit df30971371 on Feb 27, 2020
  16. akshaynexus referenced this in commit 3180832e3f on May 6, 2020
  17. ckti referenced this in commit 36fc29566d on Mar 28, 2021
  18. gades referenced this in commit 98834cbae2 on Jun 30, 2021
  19. bitcoin locked this on Sep 8, 2021

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: 2026-05-20 06:55 UTC