Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge #17154,16569,17351,13716,17084,18391,16821,17140 #4742

Merged
merged 9 commits into from
Apr 3, 2022

Conversation

vijaydasmp
Copy link

9e95931 [wallet] Remove state argument from CWallet::CommitTransaction (John Newbery)
d1734f9 [wallet] Remove return value from CommitTransaction() (John Newbery)
b6f486a [wallet] Add doxygen comment to CWallet::CommitTransaction() (John Newbery)
8bba91b [wallet] Fix whitespace in CWallet::CommitTransaction() (John Newbery)

Pull request description:

CommitTransaction() returns a bool to indicate success, but since commit
b3a7410 (bitcoin#9302) it only returns true, even if the transaction was not
successfully broadcast. This commit changes CommitTransaction() to return
void.

All dead code in if (!CommitTransaction()) branches has been removed.

Two additional commits fix up the idiosyncratic whitespace in CommitTransaction and add a doxygen comment for the function.

ACKs for top commit:
laanwj:
ACK 9e95931

Tree-SHA512: a55a2c20369a45222fc0e02d0891495655a926e71c4f52cb72624768dd7b9c1dca716ea67d38420afb90f40c6e0fd448caa60c18fd693bb10ecb110b641820e6

@vijaydasmp vijaydasmp force-pushed the bp2002 branch 2 times, most recently from 88e59ee to 9d8e369 Compare March 31, 2022 01:53
@vijaydasmp vijaydasmp changed the title Merge #17154: wallet: Remove return value from CommitTransaction Merge #17154,16569,17351,13716,17084,18391,16821,17140 Mar 31, 2022
@vijaydasmp vijaydasmp force-pushed the bp2002 branch 3 times, most recently from 3a8d676 to 95393ed Compare March 31, 2022 04:51
@vijaydasmp vijaydasmp marked this pull request as ready for review March 31, 2022 06:23
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

laanwj and others added 8 commits April 2, 2022 16:30
9e95931 [wallet] Remove `state` argument from CWallet::CommitTransaction (John Newbery)
d1734f9 [wallet] Remove return value from CommitTransaction() (John Newbery)
b6f486a [wallet] Add doxygen comment to CWallet::CommitTransaction() (John Newbery)
8bba91b [wallet] Fix whitespace in CWallet::CommitTransaction() (John Newbery)

Pull request description:

  `CommitTransaction()` returns a bool to indicate success, but since commit
  b3a7410 (bitcoin#9302) it only returns true, even if the transaction was not
  successfully broadcast. This commit changes CommitTransaction() to return
  void.

  All dead code in `if (!CommitTransaction())` branches has been removed.

  Two additional commits fix up the idiosyncratic whitespace in `CommitTransaction` and add a doxygen comment for the function.

ACKs for top commit:
  laanwj:
    ACK 9e95931

Tree-SHA512: a55a2c20369a45222fc0e02d0891495655a926e71c4f52cb72624768dd7b9c1dca716ea67d38420afb90f40c6e0fd448caa60c18fd693bb10ecb110b641820e6
7fb7acf Set init stop timeout to 10 min (setpill)

Pull request description:

  `bitcoind` can take a long time to flush its db cache to disk upon
  shutdown. Systemd sends a `SIGKILL` after a timeout, causing unclean
  shutdowns and triggering a long "Rolling forward" at the next startup.
  Disabling the timeout should prevent this from happening, and does not
  break systemd's `restart` logic.

  Addresses bitcoin#13736.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@7fb7acf

Tree-SHA512: 16e0ce5a9ecf0628f8d93d68db3f5a78ab36021d9bede05a90c84f144db2e87e17707a6eb910cb7c018c265ce2c81d43de2988bd79e4a2d8554515db8fb5aa36
…e at compile time

fac5225 rpc: Document an RPCResult for all calls; Enforce at compile time (MarcoFalke)
fadd99f rpc: Add missing newline in RPCResult description (MarcoFalke)

Pull request description:

  This documents the RPC Result (type and description, if applicable) everywhere it was missing. The patch can be reviewed with the `git diff` option `-W`/`--function-context`.

  Also, code won't compile without having an RPCResult documented.

ACKs for top commit:
  laanwj:
    Lightly tested ACK fac5225
  promag:
    Tested ACK fac5225, built and verified listunspent help output.

Tree-SHA512: af2c1af1432beb944993776026c320814bfaecaf202f47359f5758849096ca7051ec6560395a2cc6678dcc111e7c9cf4917d0f0b221bdcf3ed1642e14d0e5b3c
… stdin passwords

50c4afa add newline after -stdin* (Karl-Johan Alm)
7f11fba cli: add -stdinwalletpassphrase for (slightly more) secure CLI (Karl-Johan Alm)
0da503e add stdin helpers for password input support (Karl-Johan Alm)

Pull request description:

  This PR
  * adds `-stdinwalletpassphrase` for use with `walletpasshprase(change)`
  * adds no-echo for passwords (`-stdinrpcpass` and above)

  It may not be ideal, but it's better than having to clear the screen whenever you unlock the wallet.

ACKs for top commit:
  laanwj:
    code review ACK 50c4afa

Tree-SHA512: 473db8a303ff360ffaa36ac81a2f82be2136fa82696df0bc4f33cb44033a3ae258b5aa5bbcc1f101f88ae9abe9598ed564ce52877ab139bd5d709833f5275ec6
…ithout sys/)

4de0bde build: Fix #include sys/poll.h to just poll.h (without sys/) (Daki Carnhof)

Pull request description:

  http://pubs.opengroup.org/onlinepubs/009695399/functions/poll.html
  http://man7.org/linux/man-pages/man2/poll.2.html

ACKs for top commit:
  Empact:
    ACK 4de0bde

Tree-SHA512: 01c22a62b5bc327b3a46f721312af2283f4e09cb314bc7a3f82dbc1f4eda6f018a394559c4370fee895532e768b8a9152783979d61cb8a0ed86de069f7c150fb
…ocksonly

621e86e Update -blocksonly documentation (glowang)

Pull request description:

  When -blocksonly is set to 1, it interacts with the -walletbroadcast
  parameter and sets it to 0.

  This behavior is not captured by the current documentation, which
  claims that -blocksonly does not impact any wallet transactions at
  all.

  Fixes bitcoin#17294

ACKs for top commit:
  MarcoFalke:
    ACK 621e86e

Tree-SHA512: f47bfb40a196c23e62505e1d4f79094011ac7c21fc9b920fad60cdadb5c4f48e993be1f015e26e568ce329967c24848fd7b665a6cffd3881f4cfcd2fd0081ed8
9743432 Fix bug where duplicate PSBT keys are accepted (John L. Jegutanis)

Pull request description:

  As per the BIP 174 spec a PSBT key cannot be duplicated,
  however the current code accepts key duplication.
  The PSBT key/value entries can be duplicated when the value
  is `empty()` or `IsNull()` for `CScript` or `CTxOut` respectively
  and if those key/value entries are serialized before the non-empty ones.

  For example, the following PSBT, included in the test vectors,
  contains a duplicate field:

  ```
  // magic
  70736274ff

  // global tx
  //// key
  0100
  //// value
  2a02000000000140420f000000000017a9146e91b72d5593e7d4391e2ff44e91e985c31641f08700000000
  //// separator
  00

  // no inputs

  // outputs
  //// key PSBT_OUT_WITNESSSCRIPT
  0101
  //// value (empty script)
  00
  //// key PSBT_OUT_WITNESSSCRIPT (same as the above)
  0101
  //// value (an OP_RETURN script)
  016a
  //// separator
  00
  ```

ACKs for top commit:
  achow101:
    ACK 9743432
  instagibbs:
    code review ACK bitcoin@9743432

Tree-SHA512: 34f4b34c8e6561c6a6ab745cdd319f6687eac6f7cecc735c94035eeca8c5157e17a27f2ae853dbaa6634fcd5a8f4e1c6cc13d1ebd7e563459665d72bb147cc1e
f59bbb6 test: Fix bug in blockfilter_index_tests. (Jim Posen)

Pull request description:

  The test case tests a chain reorganization, however the two chains were generated in the same manner and thus produced the same blocks.

  This issue was [pointed out](bitcoin#14121 (comment)) by MarcoFalke.

ACKs for top commit:
  MarcoFalke:
    Thanks! ACK f59bbb6 (looked at the diff on GitHub, didn't compile, nor run tests)

Tree-SHA512: a2f063ae9312051ffc2a3fcc1116a6a8ac09beeef261bc40aa3ff7270ff4de22a790eb19fec6b15ba1eb46e78f1f317bfd91472d8581b95bb9441a56b102554e
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK for merging via merge commit

@PastaPastaPasta PastaPastaPasta merged commit 279686f into dashpay:develop Apr 3, 2022
@UdjinM6 UdjinM6 added this to the 18 milestone Apr 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants