e7b02b54ccfb6b2e119a67799220f8d8d8b5cccd Add roundtrip and more tests to ParseISO8601DateTime and FormatISO8601DateTime (Elichai Turkel)
9e2c623be50ee7e586a411923b9ed136acfa2b3f Rename DecodeDumpTime to ParseISO8601DateTime and move to time.cpp (Elichai Turkel)
Pull request description:
As discussed in #17245.
1. Renamed the function.
2. Moved it from `rpcdump.cpp` to `time.cpp`.
3. Added a check if the time is less then epoch return 0 to prevent an overflow.
4. Added more edge cases tests and a roundtrip test.
ACKs for top commit:
laanwj:
ACK e7b02b54ccfb6b2e119a67799220f8d8d8b5cccd
MarcoFalke:
ACK e7b02b54ccfb6b2e119a67799220f8d8d8b5cccd
promag:
Code review ACK e7b02b54ccfb6b2e119a67799220f8d8d8b5cccd. Moved code is correct, left a comment regarding the test change.
Tree-SHA512: 703c21e09b2aabc992235149e67acba63d9d77a593ec8f6d2fec3eb63a7e5c406d56cbce6c6513ab32fba43367d073d2345f3b589843e3c5fe4f55ea3e00bf29
ed2dc5e48abed1cde6ab98025dc8212917d47d21 Add override/final modifiers to V1TransportDeserializer (Pieter Wuille)
f342a5e61a73e1edf389b662d265d20cf26a1d51 Make resetting implicit in TransportDeserializer::Read() (Pieter Wuille)
6a91499496d76c2b3e84489e9723b60514fb08db Remove oversized message detection from log and interface (Pieter Wuille)
b0e10ff4df3d4c70fb172ea8c3128c82e6e368bb Force CNetMessage::m_recv to use std::move (Jonas Schnelli)
efecb74677222f6c70adf7f860c315f430d39ec4 Use adapter pattern for the network deserializer (Jonas Schnelli)
1a5c656c3169ba525f84145d19ce8c64f2cf1efb Remove transport protocol knowhow from CNetMessage / net processing (Jonas Schnelli)
6294ecdb8bb4eb7049a18c721ee8cb4a53d80a06 Refactor: split network transport deserializing from message container (Jonas Schnelli)
Pull request description:
**This refactors the network message deserialization.**
* It transforms the `CNetMessage` into a transport protocol agnostic message container.
* A new class `TransportDeserializer` (unique pointer of `CNode`) is introduced, handling the network buffer reading and the decomposing to a `CNetMessage`
* **No behavioral changes** (in terms of disconnecting, punishing)
* Moves the checksum finalizing into the `SocketHandler` thread (finalizing was in `ProcessMessages` before)
The **optional last commit** makes the `TransportDeserializer` following an adapter pattern (polymorphic interface) to make it easier to later add a V2 transport protocol deserializer.
Intentionally not touching the sending part.
Pre-Requirement for BIP324 (v2 message transport protocol).
Replacement for #14046 and inspired by a [comment](https://github.com/bitcoin/bitcoin/pull/14046#issuecomment-431528330) from sipa
ACKs for top commit:
promag:
Code review ACK ed2dc5e48abed1cde6ab98025dc8212917d47d21.
marcinja:
Code review ACK ed2dc5e48abed1cde6ab98025dc8212917d47d21
ryanofsky:
Code review ACK ed2dc5e48abed1cde6ab98025dc8212917d47d21. 4 cleanup commits added since last review. Unaddressed comments:
ariard:
Code review and tested ACK ed2dc5e.
Tree-SHA512: bab8d87464e2e8742529e488ddcdc8650f0c2025c9130913df00a0b17ecdb9a525061cbbbd0de0251b76bf75a8edb72e3ad0dbf5b79e26f2ad05d61b4e4ded6d
3bb0a4674f74d22043c7911ea76ab8a4d93fed62 bench: Fix negative values and zero for -evals flag (nijynot)
Pull request description:
This PR makes `bench_bitcoin -evals=0` evaluate at once and throws when `-evals` is a negative integer.
---
Currently when you run `bench_bitcoin -evals=0`, it'll get stuck at
```
# Benchmark, evals, iterations, total, min, max, median
```
. This is not intuitively expected and should instead evaluate instantly as it's set to zero. Negative integers for `-evals` does not make sense either and should throw if set.
ACKs for top commit:
laanwj:
ACK 3bb0a4674f74d22043c7911ea76ab8a4d93fed62
Tree-SHA512: 03cd4c7c55134c7ffd8cdb6ee993551ce41061a73e13c3c047247af9df1fd7ed07d798272b643ec864099036922aaadbdcd2b798d710406f48df60b9d5448c26
fa398091b7ad683dfd3cd3c2cd030eaf9f336737 Avoid unused call to GuessVerificationProgress in NotifyHeaderTip (MarcoFalke)
Pull request description:
`GuessVerificationProgress` for a header (not a block) is always 0 because the number of txs in the block can not be determined from the header alone. Anyway, this result was never used, so we can optimize this call by hardcoding 0.
This is the next commit in a series of changes toward removing nChainTx (see #14863, #13875)
ACKs for top commit:
promag:
Code review ACK fa398091b7ad683dfd3cd3c2cd030eaf9f336737, missed that.
laanwj:
ACK fa398091b7ad683dfd3cd3c2cd030eaf9f336737
Tree-SHA512: 11016f8dbb1af1cf75241948d1ad35eac0c79d1311cd0db8c6ec806df2a9e3dc5f998dbd66ccbad5d84564e6cec7fe21ce7a2a13c2b34c746e2d3b31aa1db53a
faeb6665362e35f573ad715ade0ef2db62d71839 util: Add CHECK_NONFATAL and use it in src/rpc (MarcoFalke)
Pull request description:
Fixes#17181
Currently, we use `assert` in RPC code to document logic and code assumptions. However, it seems a bit extreme to abort all of Bitcoin Core on an assert failure in one of the RPC threads. I suggest to replace all `assert`s with a macro `CHECK_NONFATAL(condition)` that throws a runtime error when the condition evaluates to `false`. That runtime error will then be returned to the rpc caller and will include instructions to report the error to our issue tracker.
ACKs for top commit:
practicalswift:
ACK faeb6665362e35f573ad715ade0ef2db62d71839
laanwj:
ACK faeb6665362e35f573ad715ade0ef2db62d71839
ryanofsky:
Code review ACK faeb6665362e35f573ad715ade0ef2db62d71839
Tree-SHA512: 9b748715a5e0767ac11f1324a95a3a6ec672a0e0658013492219223bda83ce4b1b447fd8183bbb235f7df5ef7dddda7666ad569544b4d61cc65f232ca7a800ec
e156b9d8b974f57253306b693a03aa80322ebc6c gui: disable font antialiasing for QR image address (fanquake)
Pull request description:
The address text inside the QR code is currently fairly blurry / unreadable. Explicitly disabling font antialiasing improves that somewhat.
master (693e40090ae7af52585ce1a6136a4bd56318fac7):
PR (e156b9d8b974f57253306b693a03aa80322ebc6c):
ACKs for top commit:
laanwj:
ACK e156b9d8b974f57253306b693a03aa80322ebc6c
Tree-SHA512: 32aeb2ffe8164a1006f80e76c6e413fcb88e32ced42d2b2af69cca908bd32673f3e379184be917f1870864b940db943e7f46a7ecb0779343d5d129b381660c38
6b6be41c36e4fe9a74bed50e7f0a06532ab1260b gui: Make polling in ClientModel asynchronous (João Barbosa)
Pull request description:
After #14193 `ClientModel::updateTimer` can take some time, as such the GUI hangs, like #17112.
Fixes this by polling in a background thread and updating the GUI asynchronously.
ACKs for top commit:
laanwj:
ACK 6b6be41c36e4fe9a74bed50e7f0a06532ab1260b
Sjors:
Code review re-ACK 6b6be41; only replaced the scary cast with `{ timer->start(); }`
Tree-SHA512: fd98b0c6535441aee3ee03c48b58b4b1f9bdd172ec6b8150da883022f719df34cabfd4c133412bf410e7f709f7bf1e9ef16dca05ef1f3689d526ceaeee51de38
8c6081a884cd0969160955ce8687d4d4ed074db3 compat: remove bswap_* check on macOS (fanquake)
2cba35ab38b492768e85bd77442a566dda169466 build: skip building OpenSSL lib_ssl (fanquake)
45a2d3c5526551c279b624e111fe83d1f30b10db build: remove OpenSSL from Qt build (fanquake)
befbc40eb5928ac20a85fd3446946895cd2e66cb build: remove EVP_MD_CTX_new detection (fanquake)
fcee10c2d028cba11416d902f5abf13fea7a65f4 build: remove SSL lib detection (fanquake)
c7f30dbca8034f70488cb494b4d1736c961301b9 gui: Update BIP70 support message (fanquake)
a3e810326d7e70a774ece9e9aa0c571cc9216aab build: remove BIP70 entries from macOS Info.plist (fanquake)
72fe13a58d5894afbaf1bb6789ee9b9b6576f20f gui: remove payment request file handling from OpenURI dialog (fanquake)
3548e4aac72e9148d41b440052a89d89328f7661 Remove BIP70 Support (fanquake)
1cb9a4e28cccd3056e2b8d9cdabc45bf3e4dd2a6 docs: remove protobuf from docs (fanquake)
67328bb7ca57fa0df867ec73dbeeb97d61450a4e build: remove protobuf from depends (fanquake)
Pull request description:
This removes [BIP70](https://github.com/bitcoin/bips/blob/master/bip-0070.mediawiki) support. It also removes OpenSSL linking from Qt and building OpenSSLs `lib_ssl` in depends, as well as SSL lib detection from the build system. It's something that I'd optimistically like to do for `0.20.0`.
ACKs for top commit:
laanwj:
Code review ACK 8c6081a884cd0969160955ce8687d4d4ed074db3
MarcoFalke:
ACK 8c6081a884cd0969160955ce8687d4d4ed074db3
fjahr:
ACK 8c6081a
Tree-SHA512: 9dd9153afa4eca1a795f983e5b31f5fee9fa9a064c2a95d2f98810689add3ad0bf221c4608282299e66e4d1ec31cd556d4b16eea55de7912c3b9931f64735883
04dbdd613fe5c0a742de915e07553614cd4cbf46 [net] SocketHandler: log peer id for close and disconnect (Sjors Provoost)
Pull request description:
When combined with `-logips` this makes it easier to diagnose disconnects.
To test on macOS, find a connection you want to disrupt:
```
lsof -nP -iTCP:8333 -sTCP:ESTABLISHED
```
To shut it down gracefully you can use tcpkill or this Python script: https://github.com/google/tcp_killer
The log should say:
```
2019-10-25T13:26:55Z socket closed for peer=1
2019-10-25T13:26:55Z disconnecting peer=1
2019-10-25T13:26:55Z Cleared nodestate for peer=1
```
To shut it down ungracefully I made a patch to the above script, adding a `-force` argument. _Careful, this may result in data corruption_. Then the log should say:
```
2019-10-25T13:39:57Z socket select error Bad file descriptor (9)
2019-10-25T13:39:57Z socket recv error for peer=0: Bad file descriptor (9)
2019-10-25T13:39:57Z disconnecting peer=0
2019-10-25T13:39:57Z Socket close failed: 35. Error: Bad file descriptor (9)
2019-10-25T13:39:57Z Cleared nodestate for peer=0
```
ACKs for top commit:
MarcoFalke:
unsigned ACK 04dbdd613fe5c0a742de915e07553614cd4cbf46
TheBlueMatt:
unsigned ACK 04dbdd613fe5c0a742de915e07553614cd4cbf46 LGTM!
theuni:
unsigned ACK 04dbdd613fe5c0a742de915e07553614cd4cbf46.
Tree-SHA512: 415313908484f97ffe11a48b4ed6afab3ab0be660c788adb9ad975f88b69aa1cfd5ccbe5859350cdf19ef8fde191fd530fb22cef34e70638defdc9f3d761c71d
dc2fdb99072b87d34620084b82a494a5e698c279 tests: Add fuzzing harness for various CScript related functions (practicalswift)
Pull request description:
Add fuzzing harness for various `CScript` related functions.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/script
…
# And to to quickly verify that the relevant code regions are triggered, that the
# fuzzing throughput seems reasonable, etc.
$ contrib/devtools/test_fuzzing_harnesses.sh '^script$'
```
`test_fuzzing_harnesses.sh` can be found in PR #17000.
Top commit has no ACKs.
Tree-SHA512: a0c5dca3b64ae177020b2ca299a29015d70755231b6bf01edbfc67c8aac90c44b1b4d57350c3aebef6e031108e6ae8e5fa0987c67707831c314f5d3090e0cee8
0a433fc876d82df1005f175c1254fff62f0f36f8 [validation] Remove unused cacheSigStore from CheckInputsFromMempoolAndCache (John Newbery)
Pull request description:
CheckInputsFromMempoolAndCache() is only called in one place, and
cacheSigStore is set to true in that call site. Remove the argument
entirely.
Also improve commenting.
ACKs for top commit:
MarcoFalke:
unsigned ACK 0a433fc876d82df1005f175c1254fff62f0f36f8 Comment looks good
jamesob:
ACK 0a433fc876
laanwj:
ACK 0a433fc876d82df1005f175c1254fff62f0f36f8
fanquake:
ACK 0a433fc876d82df1005f175c1254fff62f0f36f8. Checked that `CheckInputsFromMempoolAndCache` is only called once, in `MemPoolAccept::ConsensusScriptChecks`, and that `cacheSigStore` is true.
Tree-SHA512: e4b4d2550e35df55c8f8fa4c539174cc2d3728112ddb937cb2ff759d8630a01566b5ec42a70a82e33994e6586f5a457a75a59f64b15d27c65331c723cbb097af
b05ec410f2d9f209796a5df31860e23efd729dfe Add unit testing for the CompressScript functions (marcaiaf)
Pull request description:
Salvaging #15104 which adds unit tests for CompressScript function in `compressor.cpp`
Tested following cases for the CScript:
- CKeyID
- CScriptID
- Uncompressed CPubKey (of size: 65)
- Compressed CPubKey (of size: 32)
ACKs for top commit:
theStack:
ACK b05ec410f2
Tree-SHA512: 7e23ace39383122802dfe5f7d38190d772f5db4045a67b7a9bd4c06797a17e0cdc41d6fac92d448057eb7df50172155dc824587c16c68c79fd1a4de37b772001
fa928134075220254a15107c1d9702f4e66271f8 consensus: Explain why fCheckDuplicateInputs can not be skipped and remove it (MarcoFalke)
Pull request description:
As a follow up to CVE-2018-17144, this removes the unused `fCheckDuplicateInputs` parameter and explains why the test can not be disabled. Apart from protecting against a dumb accident in the future, this should document the logic in the code. There is a technical write-up that explains how the underlying coins database behaves if this test is skipped: https://bitcoincore.org/en/2018/09/20/notice/#technical-details. However, it does not explicitly mention why the test can not be skipped. I hope my code comment does that.
ACKs for top commit:
jnewbery:
ACK fa928134075220254a15107c1d9702f4e66271f8
amitiuttarwar:
utACK fa928134075220254a15107c1d9702f4e66271f8
Empact:
Code review ACK fa92813407
promag:
ACK fa928134075220254a15107c1d9702f4e66271f8.
Tree-SHA512: fc1ef670f1a467c543b84f704b9bd8cc7a59a9f707be048bd9b4e85fe70830702aa560a880efa2c840bb43818ab44dfdc611104df04db2ddc14ff92f46bfb28e
50037e97d11356218c4b36767232e47b74742b0b depends: fix boost mac cross build with clang 9+ (Cory Fields)
Pull request description:
The ancient "darwin-4.9.1" profile has long been used to match against clang, which prior to version 9, reported 4.9.1 as its version when invoking "clang++ -dumpversion". Presumably this was a historical compatibility quirk related to Apple's switch from gcc to clang.
This was "fixed" in clang 9.0, so that -dumpversion reports the real version. Unfortunately that had the side-effect of breaking the (brittle) boost compiler detection.
Move to the seemingly more-correct "clang-darwin" profile, which passes the checks and builds correctly.
Also switch to using ar rather than libtool for archiving, as it's what the clang-darwin profile expects to be using.
Note that because this is using a different profile, some of the final command-line arguments end up changing. Those changes look sane at a glance.
ACKs for top commit:
fanquake:
ACK 50037e97d11356218c4b36767232e47b74742b0b - tested on on macOS, will wait for the gitian build.
Tree-SHA512: eac1f353513a445add6fbece7fc78dd3dbdde5e2219bfb7739b82f40bb14de449667a94d2e303d43c67d9b38e7ceb0ba5f0d8fe20b40be2017b1ca0875467c2c
168b781fe7f3f13b24c52a151f36de4cdd0a340a Continue relaying transactions after they expire from mapRelay (Anthony Towns)
Pull request description:
This change allows peers to request transactions even after they've expired from mapRelay and even if they're not doing mempool requests. This is intended to allow for CPFP of old transactions -- if parent tx P wasn't relayed due to low fees, then a higher fee rate child C is relayed, peers will currently request the parent P, but we prior to this patch, we will not relay it due to it not being in mapRelay.
ACKs for top commit:
MarcoFalke:
re-ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a (only change is comment fixup)
sdaftuar:
re-ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a
sipa:
ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a
Tree-SHA512: b206666dd1450cd0a161ae55fd1a7eda2c3d226842ba27d91fe463b551fd924b65b92551b14d6786692e15cf9a9a989666550dfc980b48ab0f8d4ca305bc7762
5b44a75493a1a098404d5e21dc384e74eae1892e refactor: Remove unused CExt{Pub,}Key (de)serialization methods (Sebastian Falbesoner)
Pull request description:
As pointed out in issue #17130, the serialization/deserialization methods for the classes `CExtKey` and
`CExtPubKey` are only used in the BIP32 unit tests and hence can be removed (see comments https://github.com/bitcoin/bitcoin/issues/17130#issuecomment-543750290, https://github.com/bitcoin/bitcoin/issues/17130#issuecomment-543794408 and https://github.com/bitcoin/bitcoin/issues/17130#issuecomment-543814727).
ACKs for top commit:
practicalswift:
ACK 5b44a75493a1a098404d5e21dc384e74eae1892e -- -60 LOC diff looks correct :)
promag:
ACK 5b44a75493a1a098404d5e21dc384e74eae1892e.
MarcoFalke:
unsigned ACK 5b44a75493a1a098404d5e21dc384e74eae1892e
fjahr:
ACK 5b44a75
jonatack:
Light ACK 5b44a75493a1a098404d5e21dc384e74eae1892e. Built, ran tests and bitcoind. `git blame` shows most of the last changes are from commit 90604f16af63ec066d6561337f476ccd8acec326 in 2015 to add bip32 pubkey serialization.
Tree-SHA512: 6887573b76b9e54e117a076557407b6f7908719b2202fb9eea498522baf9f30198b3f78b87a62efcd17ad1ab0886196f099239992ce7cbbaee79979ffe9e5f2c
This was originally added in #9366 to fix the gui build, as
Protobuf would also define these macros. Now that we're no-longer
using Protobuf, remove the additional check.
fa710066b9c992ccde7149cfa8da923bcc08bc22 ci: Disable functional tests on mac host (MarcoFalke)
Pull request description:
Judging from the lack of responses to https://github.com/bitcoin/bitcoin/issues/15400#issuecomment-543745053, no one can reproduce the failures locally. Thus, disable the tests on the ci mac host. Otherwise they cause ci failures to be ignored or overwritten by a blind re-run.
ACKs for top commit:
laanwj:
ok, ACK fa710066b9c992ccde7149cfa8da923bcc08bc22 in that case
ryanofsky:
Code review ACK fa710066b9c992ccde7149cfa8da923bcc08bc22. I've been blindly restarting these but can confirm it seemed like random test failures.
Tree-SHA512: 805f552124409f6e79b16b419826d334307202b8e3636edc8a91dadffe9949554d924c00cb16bb09473ec1aaaf2d8eaf7a97c7c2b8c11bf6372135d39bba802b
CheckInputsFromMempoolAndCache() is only called in one place, and
cacheSigStore is set to true in that call site. Remove the argument
entirely.
Also improve commenting.
6f6852335fc236dd6ec161162f49bfd813036acc doc: Changed miniupnp links to https (Marius Kjærstad)
Pull request description:
doc: Changed miniupnp links to https
ACKs for top commit:
fjahr:
ACK 6f68523
fanquake:
ACK 6f6852335fc236dd6ec161162f49bfd813036acc - as mentioned in this [comment](https://github.com/bitcoin/bitcoin/pull/17166#issuecomment-545906845), in future, there's no need to open a new PR instead than squashing. One of the requirements of contributing to Bitcoin Core is some basic `git` usage, and we have some documentation on that in our [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits) (which also mentions not to open multiple PRs).
Tree-SHA512: 1e806326552b94dc1c2f105bed822a1dd56127d42bbb0c15a16623ff084b2047331f2f1ad5303c99b0d68c70ad2caf8e8028eb354ba4379973b9e1198676ce72
c2f964a6745be085f2891c909d6c998687de9080 tests: Remove Cygwin WinMain workaround (practicalswift)
db4bd32cc31789fc017f5db0b86a69ee43e41575 tests: Skip unnecessary fuzzer initialisation. Hold ECCVerifyHandle only when needed. (practicalswift)
Pull request description:
Skip unnecessary fuzzer initialisation. Hold `ECCVerifyHandle` only when needed.
As suggested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/17018#discussion_r336645391.
Top commit has no ACKs.
Tree-SHA512: 598da44859d736e3fdc143b93e07f444d8ad19dfdab0cfe7c6ccff8644e862664d869337dfe6b49416ed09a0024e4a5f2220ca6246de568f9e9227d721baa28e
9075d13153ce06cd59a45644831ecc43126e1e82 [docs] Add release notes for removal of REJECT reasons (John Newbery)
04a2f326ec0f06fb4fce1c4f93500752f05dede8 [validation] Fix REJECT message comments (John Newbery)
e9d5a59e34ff2d538d8f5315efd9908bf24d0fdc [validation] Remove REJECT code from CValidationState (John Newbery)
0053e16714323c1694c834fdca74f064a1a33529 [logging] Don't log REJECT code when transaction is rejected (John Newbery)
a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab [validation] Fix peer punishment for bad blocks (John Newbery)
Pull request description:
We no longer send BIP 61 REJECT messages, so there's no need to set
a REJECT code in the CValidationState object.
Note that there is a minor bug fix in p2p behaviour here. Because the
call to `MaybePunishNode()` in `PeerLogicValidation::BlockChecked()` only
previously happened if the REJECT code was > 0 and < `REJECT_INTERNAL`,
then there are cases were `MaybePunishNode()` can get called where it
wasn't previously:
- when `AcceptBlockHeader()` fails with `CACHED_INVALID`.
- when `AcceptBlockHeader()` fails with `BLOCK_MISSING_PREV`.
Note that `BlockChecked()` cannot fail with an 'internal' reject code. The
only internal reject code was `REJECT_HIGHFEE`, which was only set in
ATMP.
This reverts a minor bug introduced in 5d08c9c579ba8cc7b684105c6a08263992b08d52.
ACKs for top commit:
ariard:
ACK 9075d13, changes since last reviewed are splitting them in separate commits to ease understanding and fix nits
fjahr:
ACK 9075d13153ce06cd59a45644831ecc43126e1e82, confirmed diff to last review was fixing nits in docs/comments.
ryanofsky:
Code review ACK 9075d13153ce06cd59a45644831ecc43126e1e82. Only changes since last review are splitting the main commit and updating comments
Tree-SHA512: 58e8a1a4d4e6f156da5d29fb6ad6a62fc9c594bbfc6432b3252e962d0e9e10149bf3035185dc5320c46c09f3e49662bc2973ec759679c0f3412232087cb8a3a7
9e95931865186d7a9a6dc54b64bd96507e9fea4b [wallet] Remove `state` argument from CWallet::CommitTransaction (John Newbery)
d1734f9a3b138ab046f38ee44a09bc3847bf938a [wallet] Remove return value from CommitTransaction() (John Newbery)
b6f486a02b463ffeaf82ec11fc6f74f439c037ae [wallet] Add doxygen comment to CWallet::CommitTransaction() (John Newbery)
8bba91b22d22a8dfea7c947b542b1022bfc1c0ea [wallet] Fix whitespace in CWallet::CommitTransaction() (John Newbery)
Pull request description:
`CommitTransaction()` returns a bool to indicate success, but since commit
b3a7410 (#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 9e95931865186d7a9a6dc54b64bd96507e9fea4b
Tree-SHA512: a55a2c20369a45222fc0e02d0891495655a926e71c4f52cb72624768dd7b9c1dca716ea67d38420afb90f40c6e0fd448caa60c18fd693bb10ecb110b641820e6
