CLI binary improvements #3086
CLI binary improvements #3086
Conversation
str4d
added
C-upstream-port
|
These are some generally useful changes, that also remove merge conflicts for bitcoin/bitcoin#8883, which itself will remove merge conflicts for pulling in the upstream Bech32 PRs for Sapling. |
str4d
added
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #3098) made this pull request unmergeable. Please resolve the merge conflicts. |
str4d
force-pushed
the
cli-binary-improvements-1
branch
from
5c2bf90 to
23959f5
Compare
|
Rebased on master to fix merge conflict. |
daira
left a comment
daira
left a comment
There was a problem hiding this comment.
Some non-blocking comments.
doc/release-notes.md
Outdated
There was a problem hiding this comment.
This happens to avoid the process table issue because echo is (usually) a shell builtin, but I think this would more clearly avoid it:
$ src/zcash-cli -stdin walletpassphrase
mysecretcode
120
^D
src/bitcoin-cli.cpp
Outdated
There was a problem hiding this comment.
Add
static_assert(CONTINUE_EXECUTION != EXIT_FAILURE,
"CONTINUE_EXECUTION should be different from EXIT_FAILURE");
static_assert(CONTINUE_EXECUTION != EXIT_SUCCESS,
"CONTINUE_EXECUTION should be different from EXIT_SUCCESS");
(This problem will never occur on POSIX or Windows systems, but could technically occur on other platforms. Note that although EXIT_SUCCESS is equivalent to 0 as the return value from main, it is not necessarily defined to be 0.)
src/bitcoind.cpp
Outdated
There was a problem hiding this comment.
There's an exit(1) on line 141 that should probably be return false;.
There was a problem hiding this comment.
Oh, looks like that got changed to exit(EXIT_FAILURE). But I still think it should probably be return false, for consistency with other returns in that function.
There was a problem hiding this comment.
This will be addressed when we pull in bitcoin/bitcoin#10447 and bitcoin/bitcoin#11511 (which we can do in a subsequent PR).
src/bitcoin-tx.cpp
Outdated
There was a problem hiding this comment.
Another case of atoi being used on untrusted input; see #2075.
|
☔ The latest upstream changes (presumably #2814) made this pull request unmergeable. Please resolve the merge conflicts. |
A non-zero locktime also causes input sequences to be set to non-max, activating the locktime.
Implements #7442 by adding an option `-stdin` which reads additional arguments from stdin, one per line. For example ```bash echo -e "mysecretcode\n120" | src/bitcoin-cli -stdin walletpassphrase echo -e "walletpassphrase\nmysecretcode\n120" | src/bitcoin-cli -stdin ```
- `--help`, `--version` etc should exit with `0` i.e. no error ("not enough args" case should still trigger an error)
- error reading config file should exit with `1`
Slightly refactor AppInitRPC/AppInitRawTx to return standard exit codes (EXIT_FAILURE/EXIT_SUCCESS) or CONTINUE_EXECUTION (-1)
…ppropriate places.
str4d
force-pushed
the
cli-binary-improvements-1
branch
from
23959f5 to
abf5dd3
Compare
|
Rebased on master to fix merge conflict, and address some of the non-blocking comments. @zkbot r+ |
|
📌 Commit abf5dd3 has been approved by |
|
⌛ Testing commit abf5dd3 with merge 3b0a5bcd249d98355ee57c197c9017ac4cf3eedc... |
|
|
|
@zkbot force |
CLI binary improvements Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#5936 - bitcoin/bitcoin#7550 - bitcoin/bitcoin#7989 - bitcoin/bitcoin#7957 - bitcoin/bitcoin#9067 - bitcoin/bitcoin#9220 Excludes any changes that affected the QT code.
10 participants
Cherry-picked from the following upstream PRs:
Excludes any changes that affected the QT code.