b2a1e477444bfb90328b353e89967ace6ef10918 net_processing: simplify logging statement (Martin Zumsande)
6548ba68e85298c01b983c2392aab86e88e447d3 test: fix intermittent errors in p2p_ibd_stalling.py (Martin Zumsande)
Pull request description:
Two small fixups to #25880:
- Use `is_connected` instead of `num_test_p2p_connections` to avoid intermittent failures where the p2p MiniNode got disconnected but this info hasn't made it to python yet, so it fails a ping. (https://github.com/bitcoin/bitcoin/pull/25880#discussion_r1089217720)
- Simplify a logging statement (suggested in https://github.com/bitcoin/bitcoin/pull/25880#discussion_r1013738635)
ACKs for top commit:
MarcoFalke:
review ACK b2a1e477444bfb90328b353e89967ace6ef10918 🕧
Tree-SHA512: 337f0883bf1c94cc26301a80dfa649093ed1e211ddda1acad8449a2add5be44e5c12d6073c209df9c7aa1edb9da33ec1cfdcb0deafd76178ed78785843e80bc7
faf7b4f1fc35f1488567e0e4a57ecb348596b992 Add BlockManager::IsPruneMode() (MarcoFalke)
fae71fe27ec021583aaeac09aa924522bb63db05 Add BlockManager::GetPruneTarget() (MarcoFalke)
fa0f0436d83288262d7d764b1d239c1a6de6146f Add BlockManager::LoadingBlocks() (MarcoFalke)
Pull request description:
Requested in https://github.com/bitcoin/bitcoin/pull/25781#discussion_r1061323795, but adding getters seems unrelated from removing globals, so I split it out for now.
ACKs for top commit:
dergoegge:
Code review ACK faf7b4f1fc35f1488567e0e4a57ecb348596b992
brunoerg:
crACK faf7b4f1fc35f1488567e0e4a57ecb348596b992
Tree-SHA512: 204d0e9a0e8b78175482f89b4ce620fba0e65d8e49ad845d187af44d3843f4c733a01bac1ffe5a5319f524d8346123693a456778b69d6c75268c447eb8839642
39b93649c4b98cd82c64b957fd9f6a6fd3c2a359 test: add functional test for IBD stalling logic (Martin Zumsande)
0565951f34e6d155dc825964c5d8b1dd00931682 p2p: Make block stalling timeout adaptive (Martin Zumsande)
Pull request description:
During IBD, there is the following stalling mechanism if we can't proceed with assigning blocks from a 1024 lookahead window because all of these blocks are either already downloaded or in-flight: We'll mark the peer from which we expect the current block that would allow us to advance our tip (and thereby move the 1024 window ahead) as a possible staller. We then give this peer 2 more seconds to deliver a block (`BLOCK_STALLING_TIMEOUT`) and if it doesn't, disconnect it and assign the critical block we need to another peer.
Now the problem is that this second peer is immediately marked as a potential staller using the same mechanism and given 2 seconds as well - if our own connection is so slow that it simply takes us more than 2 seconds to download this block, that peer will also be disconnected (and so on...), leading to repeated disconnections and no progress in IBD. This has been described in #9213, and I have observed this when doing IBD on slower connections or with Tor - sometimes there would be several minutes without progress, where all we did was disconnect peers and find new ones.
The `2s` stalling timeout was introduced in #4468, when blocks weren't full and before Segwit increased the maximum possible physical size of blocks - so I think it made a lot of sense back then.
But it would be good to revisit this timeout now.
This PR makes the timout adaptive (idea by sipa):
If we disconnect a peer for stalling, we now double the timeout for the next peer (up to a maximum of 64s). If we connect a block, we half it again up to the old value of 2 seconds. That way, peers that are comparatively slower will still get disconnected, but long phases of disconnecting all peers shouldn't happen anymore.
Fixes#9213
ACKs for top commit:
achow101:
ACK 39b93649c4b98cd82c64b957fd9f6a6fd3c2a359
RandyMcMillan:
Strong Concept ACK 39b93649c4b98cd82c64b957fd9f6a6fd3c2a359
vasild:
ACK 39b93649c4b98cd82c64b957fd9f6a6fd3c2a359
naumenkogs:
ACK 39b93649c4b98cd82c64b957fd9f6a6fd3c2a359
Tree-SHA512: 85bc57093b2fb1d28d7409ed8df5a91543909405907bc129de7c6285d0810dd79bc05219e4d5aefcb55c85512b0ad5bed43a4114a17e46c35b9a3f9a983d5754
To directly return a CRIPEMD160 hash from data.
Incidentally, decoding this acronym:
* RIPEMD -> RIPE Message Digest
* RIPE -> RACE Integrity Primitives Evaluation
* RACE -> Research and Development in Advanced Communications Technologies in Europe
fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4 Remove unused CDataStream::SetType (MarcoFalke)
fa29e73cdab82f98682821322cda89b1084ba887 Use DataStream where possible (MarcoFalke)
fa9becfe1cea5040e7cea36324d1b0789cbbd25d streams: Add DataStream without ser-type and ser-version (MarcoFalke)
Pull request description:
This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone.
The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version.
So do this here for `DataStream`. `CDataStream` remains in places where it is not yet possible.
ACKs for top commit:
stickies-v:
re-ACK [fa035fe](fa035fe2d6)
aureleoules:
diff re-ACK fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4 fa0e6640ba..fa035fe2d6
Tree-SHA512: cb5e53d0df7c94319ffadc6ea1d887fc38516decaf43f0673396d79cc62d450a1a61173654a91b8c2b52d2cecea53fe4a500b8f6466596f35731471163fb051c
c58c249a5b694c88122589fedbef4e2f13f08bb4 net_processing: indicate more work to do when orphans are ready to reconsider (Anthony Towns)
ecb0a3e4259b81d6bb74d59a58eb65552c17d8d8 net_processing: Don't process tx after processing orphans (Anthony Towns)
c5837757068bf8ea3e5b6fdad82f69d1deb81545 net_processing: only process orphans before messages (Anthony Towns)
be2304676bedcd15debcdc694549fdd2b255ba62 txorphange: Drop redundant originator arg from GetTxToReconsider (Anthony Towns)
a4fe09973aa82210b98dcb4e4e9f11ef59780f9b txorphanage: index workset by originating peer (Anthony Towns)
Pull request description:
We currently process orphans by assigning them to the peer that provided a missing parent; instead assign them to the peer that provided the orphan in the first place. This prevents a peer from being able to marginally delay another peer's transactions and also simplifies the internal API slightly. Because we're now associating orphan processing with the peer that provided the orphan originally, we no longer process orphans immediately after receiving the parent, but defer until a future call to `ProcessMessage`.
Based on #26295
ACKs for top commit:
naumenkogs:
utACK c58c249a5b694c88122589fedbef4e2f13f08bb4
glozow:
ACK c58c249a5b694c88122589fedbef4e2f13f08bb4
mzumsande:
Code Review ACK c58c249a5b694c88122589fedbef4e2f13f08bb4
Tree-SHA512: 3186c346f21e60440266a2a80a9d23d7b96071414e14b2b3bfe50457c04c18b1eab109c3d8c2a7726a6b10a2eda1f0512510a52c102da112820a26f5d96f12de
fab958290b84037fad1d78ffb2bce34d2b0e8c36 refactor: Remove c_str from util/check (MarcoFalke)
Pull request description:
Seems confusing and fragile to require calling code to call `c_str()` when passing a read-only view of a std::string.
Fix that by using std::string_view, which can be constructed from string literals and std::string.
Also, remove the now unused `c_str()` from `src/wallet/bdb.cpp`.
ACKs for top commit:
stickies-v:
ACK fab958290b84037fad1d78ffb2bce34d2b0e8c36
aureleoules:
ACK fab958290b84037fad1d78ffb2bce34d2b0e8c36
theStack:
ACK fab958290b84037fad1d78ffb2bce34d2b0e8c36
Tree-SHA512: ae39812c6bb8e2ef095e1b843774af2718f48404cb848c3e43b16d3c22240557d69d54a13a038a4a9c48b3ba0e4523e1f87abdd60f91486092f50fd43c0e8483
fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac rpc: Throw more user friendly arg type check error (MarcoFalke)
Pull request description:
The arg type check error doesn't list which arg (position or name) failed. Fix that.
ACKs for top commit:
stickies-v:
ACK fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac - although I think the functional test isn't in a logical place (but not blocking)
Tree-SHA512: 17425aa145aab5045940ec74fff28f0e3b2b17ae55f91c4bb5cbcdff0ef13732f8e31621d85964dc2c04333ea37dbe528296ac61be27541384b44e37957555c8
b9d567454159f062ce84353f5821d6e6daf433bd init: Remove sensitive flag from rpcbind (Andrew Chow)
Pull request description:
`-rpcbind` is currently flagged as a sensitive option which means that its value will be masked when the command line args are written to the debug.log file. However this is not useful as if `rpcbind` is actually activated, the bound IP addresses will be written to the log anyways. The test `feature_config_args.py` did not catch this contradiction as the test node was not started with `rpcallowip` and so `rpcbind` was not acted upon.
This also brings `rpcbind` inline with `bind` as that is not flagged as sensitive either.
ACKs for top commit:
Sjors:
re-utACK b9d567454159f062ce84353f5821d6e6daf433bd
willcl-ark:
ACK b9d5674
theStack:
ACK b9d567454159f062ce84353f5821d6e6daf433bd
Tree-SHA512: 50ab5ad2e18ae70649deb1ac429d404b5f5c41f32a4943b2041480580152df22e72d4aae493379d0b23fcb649ab342376a82119760fbf6dfdcda659ffd3e244a
When PR#15644 made orphan processing interruptible, it also introduced a
potential 100ms delay between processing of the first and second newly
reconsiderable orphan, because it didn't check if the orphan work set
was non-empty after invoking ProcessMessage(). This adds that check, so
that ProcessMessages() will return true if there are orphans to process,
usually avoiding the 100ms delay in CConnman::ThreadMessageHandler().
If we made progress on orphans, consider that enough work for this peer
for this round of ProcessMessages. This also allows cleaning up the api
for TxOrphange:GetTxToReconsider().
Previously, when we processed a new tx we would attempt to ATMP any
orphans that considered the new tx a parent immediately, but would only
accept at most one such tx, leaving any others to be considered on a
future run of ProcessMessages(). With this patch, we don't attempt any
orphan processing immediately after receiving a tx, instead deferring
all of them until the next call to ProcessMessages().
9ab62d71fb1a54430ff5071bdb1120a414061288 [fuzz] Actually use mocked mempool in tx_pool target (dergoegge)
Pull request description:
The current tx_pool target uses the default mempool, making the target non-deterministic. This PR replaces the active chainstate's mempool (i.e. the node's default mempool) with the already present mocked mempool in the target.
ACKs for top commit:
fanquake:
ACK 9ab62d71fb1a54430ff5071bdb1120a414061288
Tree-SHA512: fe9af3dbdd13cb569fdc2ddbb4290b5ce94206ae83d94267c6365ed0ee9bbe072fcfe7fd632a1a8522dce44608e89aba2f398c1e20bd250484bbadb78143320c
a1c36275b5a27ae685f49ff02dabff0adbf51aa1 [fuzz] Assert that omitting missing transactions always fails block reconstruction (dergoegge)
a8ac61ab5e1805df61f4dc94ded44a874725484c [fuzz] Add PartiallyDownloadedBlock target (dergoegge)
42bd4c746824e3b2adf2c696cf4a705fa43d1fa8 [block encodings] Avoid fuzz blocking asserts in PartiallyDownloadedBlock (dergoegge)
1429f8377017c0029cb87c4d355c37b796432611 [block encodings] Make CheckBlock mockable for PartiallyDownloadedBlock (dergoegge)
Pull request description:
This PR adds a fuzz target for `PartiallyDownloadedBlock`, which we currently do not have any coverage for.
ACKs for top commit:
mzumsande:
Code Review ACK a1c36275b5a27ae685f49ff02dabff0adbf51aa1
MarcoFalke:
re-ACK a1c36275b5a27ae685f49ff02dabff0adbf51aa1 🎼
Tree-SHA512: 01ae452fe457da0c8f2b28c72091d40807c56a9e5d0f80b55f166b67be50baf80a02f53d4cbe9736bb22424cca1758b87e4e471b8a24e756c22563a2640e9a5f
4aebd832a405090c2608e4b60bb4f34501bcea61 db: Change DatabaseCursor::Next to return status enum (Andrew Chow)
d79e8dcf2981ef1964a2fde8c472b5de1ca1c963 wallet: Have cursor users use DatabaseCursor directly (Andrew Chow)
7a198bba0a1d0a0f0fd4ca947955cb52b84bdd4b wallet: Introduce DatabaseCursor RAII class for managing cursor (Andrew Chow)
69efbc011bb74fcd8dd9ed2a8a5d31bc9e323c10 Move SafeDbt out of BerkeleyBatch (Andrew Chow)
Pull request description:
Instead of having database cursors be tied to a particular `DatabaseBatch` object and requiring its setup and teardown be separate functions in that batch, we can have cursors be separate RAII classes. This makes it easier to create and destroy cursors as well as having cursors that have slightly different behaviors.
Additionally, since reading data from a cursor is a tri-state, this PR changes the return value of the `Next` function (formerly `ReadAtCursor`) to return an Enum rather than the current system of 2 booleans. This greatly simplifies and unifies the code that deals with cursors as now there is no confusion as to what the function returns when there are no records left to be read.
Extracted from #24914
ACKs for top commit:
furszy:
diff ACK 4aebd83
theStack:
Code-review ACK 4aebd832a405090c2608e4b60bb4f34501bcea61
Tree-SHA512: 5d0be56a18de5b08c777dd5a73ba5a6ef1e696fdb07d1dca952a88ded07887b7c5c04342f9a76feb2f6fe24a45dc31f094f1f5d9500e6bdf4a44f4edb66dcaa1
b358bde020e2b9f6249fdbc500db6cb30f500f19 randomenv: consolidate WIN32 #ifdefs (fanquake)
fff80cd2488899a322f9e673930a00eb9ab5b165 random: remove windows-only compat.h include in randomenv (fanquake)
Pull request description:
Similar to #26814.
Having a windows-only include of compat.h is confusing, not-only because it's already included globally via util/time.h, but also because it's unclear why compat.h is included (neither of the required headers are included there).
This change is related to removing the use of compat.h as a miscellaneous catch-all for unclear/platform specific includes. Somewhat prompted by IWYU-related discussion here: https://github.com/bitcoin/bitcoin/pull/26763/files#r1058861693.
ACKs for top commit:
hebasto:
ACK b358bde020e2b9f6249fdbc500db6cb30f500f19.
TheCharlatan:
ACK b358bde020e2b9f6249fdbc500db6cb30f500f19
Tree-SHA512: d46dffe36a17ad0f9374a55e0ecaf2d60d0b473c8fc9ad6f3005142014c08a7c10bae4948856531abb443f5e0bd6062958fe574197e282dad22ae50134d71e5f
-BEGIN VERIFY SCRIPT-
sed -i -e "/Deprecated alias for OMITTED, can be removed/d" src/rpc/util.h src/rpc/util.cpp
sed -i -e "s/OMITTED_NAMED_ARG/OMITTED/g" $(git grep -l "OMITTED_NAMED_ARG" src/)
-END VERIFY SCRIPT-
8e85164e7d12be324ea1af2e288ebcf689c930b7 doc: release note on mempool size in -blocksonly (willcl-ark)
ae797463dc7c72d990afa3ca53eeced7563ccd29 doc: Update blocksonly behaviour in reduce-memory (willcl-ark)
1134686ef92fb622ac32dc7463d3763cf18c85ad mempool: Don't share mempool with dbcache in blocksonly (willcl-ark)
Pull request description:
Fixes#9526
When `-blocksonly` has been set reduce default mempool size to avoid surprising resource usage via sharing un-used mempool cache space with dbcache.
In comparison to https://github.com/bitcoin/bitcoin/pull/9569 which either set `maxmempool` size to 0 when `-blocksonly` was set or else errored on startup, this change will permit `maxmempool` options being set.
This preserves the current (surprising?) behaviour of having a functional mempool in `-blocksonly` mode, to permit whitelisted peer transaction relay, whilst reducing average runtime memory usage for blocksonly nodes which either use the default settings or have otherwise configured a `maxmempool` size.
To use the previous old defaults node operators can configure their node with: `-blocksonly -maxmempool=300`.
ACKs for top commit:
ajtowns:
ACK 8e85164e7d12be324ea1af2e288ebcf689c930b7
stickies-v:
re-ACK 8e85164e7d
Tree-SHA512: 1c461c24b6f14ba02cfe4e2cde60dc629e47485db5701bca3003b8df79e3aa311c0c967979f6a1dca3ba69f5b1e45fa2db6ff83352fdf2d4349d5f8d120e740d
fadeb6b103cb441e0e91ef506ef29febabb10715 Add missing includes to fix gcc-13 compile error (MarcoFalke)
Pull request description:
On current master:
```
CXX support/libbitcoin_util_a-lockedpool.o
support/lockedpool.cpp: In member function ‘void Arena::free(void*)’:
support/lockedpool.cpp:99:20: error: ‘runtime_error’ is not a member of ‘std’
99 | throw std::runtime_error("Arena: invalid or double free");
| ^~~~~~~~~~~~~
support/lockedpool.cpp:22:1: note: ‘std::runtime_error’ is defined in header ‘<stdexcept>’; did you forget to ‘#include <stdexcept>’?
21 | #include <algorithm>
+++ |+#include <stdexcept>
22 | #ifdef ARENA_DEBUG
support/lockedpool.cpp: In member function ‘void LockedPool::free(void*)’:
support/lockedpool.cpp:320:16: error: ‘runtime_error’ is not a member of ‘std’
320 | throw std::runtime_error("LockedPool: invalid address not pointing to any arena");
| ^~~~~~~~~~~~~
support/lockedpool.cpp:320:16: note: ‘std::runtime_error’ is defined in header ‘<stdexcept>’; did you forget to ‘#include <stdexcept>’?
ACKs for top commit:
hebasto:
ACK fadeb6b103cb441e0e91ef506ef29febabb10715.
fanquake:
ACK fadeb6b103cb441e0e91ef506ef29febabb10715 - tested this fixes compilation with GCC 13. I don't think theres a need to do anything else here, and that'd also just potentially complicate backporting.
Tree-SHA512: 99f79cf385c913138a9cf9fc23be0a3a067b0a28518b8bdc033a7220b85bbc5d18f5356c5bdad2f628c22abb87c18b232724f606eba6326c031518559054be31
3d1a4d8a45cd91bdfe0ef107c2e9c5e882b34155 RPC: make RPCResult::MatchesType return useful errors (Anthony Towns)
Pull request description:
Currently if you don't correctly update the description of the return value for an RPC call, you essentially just get an assertion failure with no useful information; this generates a description of the problems instead.
ACKs for top commit:
MarcoFalke:
re-ACK 3d1a4d8a45cd91bdfe0ef107c2e9c5e882b34155 🌷
Tree-SHA512: cf0580b7046faab0128672a74f8cc5a1655dfdca6646a2e38b51f0fb5f672c98aad6cd4c5769454a2d644a67da639ccb1c8ff5d24d3d6b4446a082398a643722
dee89438b82e94474ebaa31367035f98b4636dac Abstract out ComputeTapbranchHash (Russell O'Connor)
8e3fc9942729716e95907008fcf36eee758c3a6a Do not use CScript for tapleaf scripts until the tapleaf version is known (Russell O'Connor)
Pull request description:
While BIP-341 calls the contents of tapleaf a "script", only in the case that the tapleaf version is `0xc0` is this script known to be a tapscript. Otherwise the tapleaf "script" is simply an uninterpreted string of bytes.
This PR corrects the issue where the type `CScript` is used prior to the tapleaf version being known to be a tapscript. This prevents `CScript` methods from erroneously being called on non-tapscript data.
A second commit abstracts out the TapBranch hash computation in the same manner that the TapLeaf computation is already abstracted. These two abstractions ensure that the TapLeaf and TapBranch tagged hashes are always constructed properly.
ACKs for top commit:
ajtowns:
ACK dee89438b82e94474ebaa31367035f98b4636dac
instagibbs:
ACK dee89438b82e94474ebaa31367035f98b4636dac
achow101:
ACK dee89438b82e94474ebaa31367035f98b4636dac
sipa:
ACK dee89438b82e94474ebaa31367035f98b4636dac
aureleoules:
reACK dee89438b82e94474ebaa31367035f98b4636dac - I verified that there is no behavior change.
Tree-SHA512: 4a1d37f3e9a1890e7f5eadcf65562688cc451389581fe6e2da0feb2368708edacdd95392578d8afff05270d88fc61dce732d83d1063d84d12cf47b5f4633ec7e
5eabb61b2386d00e93e6bbb2f493a56d1b326ad9 addrdb: Only call Serialize() once (Martin Zumsande)
da6c7aeca38e1d0ab5839a374c26af0504d603fc hash: add HashedSourceWriter (Martin Zumsande)
Pull request description:
There have been various reports of corruption of `peers.dat` recently, see #26599.
As explained in [this post](https://github.com/bitcoin/bitcoin/issues/26599#issuecomment-1381082886) in more detail, the underlying issue is likely that we currently serialize `AddrMan` twice - once for the file stream, once for the hasher that helps create the checksum - and if `AddrMan` changes in between these two calls, the checksum doesn't match the data and the resulting `peers.dat` is corrupted.
This PR attempts to fix this by introducing and using `HashedSourceWriter` - a class that keeps a running hash while serializing data, similar to the existing `CHashVerifier` which does the analogous thing while unserializing data. Something like this was suggested before, see https://github.com/bitcoin/bitcoin/pull/10248#discussion_r120694343.
Fixes#26599 (not by changing the behavior in case of a crash, but by hopefully fixing the underlying cause of these corruptions).
ACKs for top commit:
sipa:
utACK 5eabb61b2386d00e93e6bbb2f493a56d1b326ad9
naumenkogs:
utACK 5eabb61b2386d00e93e6bbb2f493a56d1b326ad9
Tree-SHA512: f19ad37c37088d3a9825c60de2efe85cc2b7a21b79b9156024d33439e021443ef977a5f8424a7981bcc13d73d11e30eaa82de60e14d88b3568a67b03e02b153b
58c2bbdb55b97e350baf51cfd7f4692b681a8acb [fuzz] Enable erlay in process_message(s) targets (dergoegge)
Pull request description:
The process_message(s) targets can't exercise the Erlay logic at the moment as the config setting is off by default and not switched on in the fuzz targets.
This PR enables the `-txreconciliation` setting in both targets.
ACKs for top commit:
fanquake:
ACK 58c2bbdb55b97e350baf51cfd7f4692b681a8acb
Tree-SHA512: a2754fd04549bdcac94d8225244c5c83fe4c26114c0c2fdf316257480625e05e4e6b1b791974e1f1021451d3f81cb59a109261fb73178ad03911f0a3db963077
d96d97ad30c4a079450bc2bf02e3e2a45f7efd2d doc: Add release note for shutdownnotify. (klementtan)
0bd73e2c453e8a88312edf43d184c33109f12ad6 util: Add -shutdownnotify option. (klementtan)
Pull request description:
**Description**: Similar to `-startupnotify`, this PR adds a new option to allow users to specify a command to be executed when Bitcoin Core shuts down.
**Note**: The `shutdownnotify` commands will not be executed if bitcoind shut down due to *unexpected* reasons (ie `killall -9 bitcoind`).
### Testing:
**Normal shutdown commands**
```
# start bitcoind with shutdownnotify optioin
./src/bitcoind -signet -shutdownnotify="touch foo.txt"
# shutdown bitcoind
./src/bitcoin-cli -signet stop
# check that foo.txt has been created
```
**Final RPC call**
Commands:
```
$ ./src/bitcoind -signet -nolisten -noconnect -shutdownnotify="./src/bitcoin-cli -signet getblockchaininfo > tmp.txt"
$ ./src/bitcoin-cli stop
$ cat tmp.txt
```
<details>
<summary>Screen Shot</summary>
</details>
ACKs for top commit:
achow101:
ACK d96d97ad30c4a079450bc2bf02e3e2a45f7efd2d
1440000bytes:
ACK d96d97ad30
theStack:
re-ACK d96d97ad30c4a079450bc2bf02e3e2a45f7efd2d
Tree-SHA512: 16f7406fd232e8b97aea5e58854c84755b0c35c88cb3ef9ee123b29a1475a376122b1e100da860cc336d4d657e6046a70e915fdb9b70c9fd097c6eef1b028161
a2ac6f9582c4c996fa36e4801fa0aac756235754 wallet: unify FindNonChangeParentOutput functions (furszy)
b3f4e827378e010cd2a5d1b876d01db52c054d26 wallet: simplify ListCoins implementation (furszy)
Pull request description:
Focused on the following changes:
1) Removed the entire locked coins lookup that was inside `ListCoins` by including them directly on the `AvailableCoins` result (where we were skipping them before).
2) Unified both `FindNonChangeParentOutput` functions (only called from `ListCoins`)
ACKs for top commit:
achow101:
ACK a2ac6f9582c4c996fa36e4801fa0aac756235754
aureleoules:
ACK a2ac6f9582c4c996fa36e4801fa0aac756235754, LGTM
theStack:
Code-review ACK a2ac6f9582c4c996fa36e4801fa0aac756235754
Tree-SHA512: f72b21ee1600c5992559b5dcd8ff260527afac2ec696737f998343a0850b84d439e7f86ea52a14cc0cddabf132a30bf5b52fb34950578ac323083d4375b937f1
