Stricter validation for CLI options and syntax #11819

issue unsystemizer openend 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:

    0$ bitcoin-qt -rescan 1 -wallet otherWallet.dat 
    1PaymentServer::ipcSendCommandLine: Payment request file does not exist:  "1"
    2"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. 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.

  4. promag commented at 2:24 pm on December 7, 2017: member
    I think it’s fine to not launch the process in these cases.
  5. laanwj referenced this in commit cfd99ddc3c on Dec 20, 2017
  6. 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.
  7. meshcollider closed this on Mar 17, 2018

  8. virtload referenced this in commit 6ad76a91f9 on Apr 4, 2018
  9. PastaPastaPasta referenced this in commit bdf1e71125 on Feb 13, 2020
  10. PastaPastaPasta referenced this in commit d7dec0fbfb on Feb 27, 2020
  11. PastaPastaPasta referenced this in commit 262bac5213 on Feb 27, 2020
  12. PastaPastaPasta referenced this in commit df30971371 on Feb 27, 2020
  13. akshaynexus referenced this in commit 3180832e3f on May 6, 2020
  14. ckti referenced this in commit 36fc29566d on Mar 28, 2021
  15. gades referenced this in commit 98834cbae2 on Jun 30, 2021
  16. DrahtBot 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: 2024-11-17 12:12 UTC

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