aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-04-19Merge #18695: test: Replace boost::mutex with std::mutexfanquake
27abd1a4f4c7a3d092d59edbbaa1e0f324c8b0ef test: Replace boost::mutex with std::mutex (Hennadii Stepanov) Pull request description: This PR replaces `boost::mutex` with `std::mutex` in the `scheduler_tests` test suite. ACKs for top commit: theStack: ACK https://github.com/bitcoin/bitcoin/pull/18695/commits/27abd1a4f4c7a3d092d59edbbaa1e0f324c8b0ef sipa: utACK 27abd1a4f4c7a3d092d59edbbaa1e0f324c8b0ef Tree-SHA512: 062eed360a68910fb71552fd892bfd097442718a237446cfb8350bfd5d807da7251ead2b9755e1d7022598774ed23fa5432a589ac6f8cadddab404b439883466
2020-04-18Merge #17219: wallet: allow transaction without change if keypool is emptySamuel Dobson
92bcd70808b9cac56b184903aa6d37baf9641b37 [wallet] allow transaction without change if keypool is empty (Sjors Provoost) 709f8685ac37510aa145ac259753583c82280038 [wallet] CreateTransaction: simplify change address check (Sjors Provoost) 5efc25f9638866941028454cfa9bae27f1519cb4 [wallet] translate "Keypool ran out" message (Sjors Provoost) Pull request description: Extracted from #16944 First this PR simplifies the check when generating a change address, by dropping `CanGetAddresses` and just letting `reservedest.GetReservedDestination` do this check. Second, when the keypool is empty, instead of immediately giving up, we create a dummy change address and pass that to coin selection. If we didn't need the change address (e.g. when spending the entire balance), then it's all good. If we did need a change address, we throw the original error. ACKs for top commit: fjahr: Code review ACK 92bcd70808b9cac56b184903aa6d37baf9641b37 jonasschnelli: utACK 92bcd70808b9cac56b184903aa6d37baf9641b37 achow101: ACK 92bcd70808b9cac56b184903aa6d37baf9641b37 meshcollider: Code review ACK 92bcd70808b9cac56b184903aa6d37baf9641b37 Tree-SHA512: 07b8c8251f57061c58a85ebf0359be63583c23bac7a2c4cefdc14820c0cdebcc90a2bb218e5ede0db11d1e204cda149e056dfd18614642070b3d56efe2735006
2020-04-18test: Replace boost::mutex with std::mutexHennadii Stepanov
2020-04-17Merge #18682: fuzz: http_request workaround for libevent < 2.1.1MarcoFalke
6f8b498d186df5aa08dbb9ca8fdeab6652f1db5e fuzz: http_request workaround for libevent < 2.1.1 (Sebastian Falbesoner) Pull request description: The fuzz test `http_request` calls the following two internal libevent functions: * `evhttp_parse_firstline_` * `evhttp_parse_headers_` Before libevent 2.1.1 however, internal functions names didn't end with an underscore (see libevent commit https://github.com/libevent/libevent/commit/8ac3c4c25bea4b9948ab91cd00605bf34fc0bd72 and [Changelog for 2.1.1.-alpha](https://github.com/libevent/libevent/blob/master/ChangeLog#L1830) when the change was first mentioned) hence the build fails with a linking error. This PR adds a preprocessor workaround to the test that checks for the libevent version (via ~`_EVENT_NUMERIC_VERSION`~ `LIBEVENT_VERSION_NUMBER`) and creates wrapper functions mapping to naming scheme without underscore in case the version is older than 2.1.1. Tested with Ubuntu Xenial 16.04.6 LTS and clang-8. ACKs for top commit: hebasto: ACK 6f8b498d186df5aa08dbb9ca8fdeab6652f1db5e, tested on xenial: Tree-SHA512: 3b9e0147b8aea22e417d418e3b6d4905f5be131c2b0ae4b0f8b9411c5606d2e22f1b23e1ecc6980ecab907c61404de09e588aae1ac43cf70cf9e8d006bbdee73
2020-04-17fuzz: http_request workaround for libevent < 2.1.1Sebastian Falbesoner
Before libevent 2.1.1, internal functions names didn't end with an underscore.
2020-04-17Merge #18607: rpc: Fix named arguments in documentationMarcoFalke
fa168d754221a83cab0d2984a02c41cf6819e873 rpc: Document all aliases for first arg of listtransactions (MarcoFalke) fa5b1f067fcf8bebb23455dd8a16cde5068e79cd rpc: Document all aliases for second arg of getblock (MarcoFalke) fa86a4bbfc000593ae4ad9dcdaec3fd0c0406086 rpc: Rename first arg of generateblock RPC to "output" (MarcoFalke) Pull request description: This fixes a bug found with #18531: * Currently the named argument for `generateblock` is documented as `address/descriptor`, but the server only accepts a named argument of `address`. Fix it by changing the name to `output` for both the documentation and the server code. Also, add tests to prove the server understands the new name `output`. * Unrelated to that, there have been a bunch of aliases in the server code that are not present in the source code of the documentation. Fix that by adding the alias to the source code of the documentation. Only the first alias is displayed in the rendered documentation. Also, add tests to prove the server actually understands all aliases. ACKs for top commit: pierreN: Tested ACK fa168d7 tests, help messages Tree-SHA512: 05e15628e3a667b296f3783d20f764b450b959451b5360c7eaf5993156582d47a0f5882330ca2493b851eb46324d504953b90c875bc88a15c9e8c89eb3ef8d92
2020-04-17Merge #18673: scripted-diff: Sort test includesMarcoFalke
fa4632c41714dfaa699bacc6a947d72668a4deef test: Move boost/stdlib includes last (MarcoFalke) fa488f131fd4f5bab0d01376c5a5013306f1abcd scripted-diff: Bump copyright headers (MarcoFalke) fac5c373006a9e4bcbb56843bb85f1aca4d87599 scripted-diff: Sort test includes (MarcoFalke) Pull request description: When writing tests, often includes need to be added or removed. Currently the list of includes is not sorted, so developers that write tests and have `clang-format` installed will either have an unrelated change (sorting) included in their commit or they will have to manually undo the sort. This pull preempts both issues by just sorting all includes in one commit. Please be aware that this is **NOT** a change to policy to enforce clang-format or any other developer guideline or process. Developers are free to use whatever tool they want, see also #18651. Edit: Also includes a commit to bump the copyright headers, so that the touched files don't need to be touched again for that. ACKs for top commit: practicalswift: ACK fa4632c41714dfaa699bacc6a947d72668a4deef jonatack: ACK fa4632c41714dfaa, light review and sanity checks with gcc build and clang fuzz build Tree-SHA512: 130a8d073a379ba556b1e64104d37c46b671425c0aef0ed725fd60156a95e8dc83fb6f0b5330b2f8152cf5daaf3983b4aca5e75812598f2626c39fd12b88b180
2020-04-17Merge #18664: fuzz: fix unused variable compiler warningMarcoFalke
eab7367e25e35688a4d4a6c96701dd7149134df5 fuzz: fix unused variable compiler warning (Jon Atack) Pull request description: Fixes the compiler warning while hopefully not invalidating the existing seeds. Added an explanatory comment. ``` test/fuzz/locale.cpp:59:19: warning: unused variable 'random_int32' [-Wunused-variable] const int32_t random_int32 = fuzzed_data_provider.ConsumeIntegral<int32_t>(); ``` ACKs for top commit: practicalswift: ACK eab7367e25e35688a4d4a6c96701dd7149134df5 Tree-SHA512: 4c90784518027cd3f85acd18030201efe4018f9da46365fef934e9a53a0b923031fec4c884a2da2f14232b6060aeb9016ac09950a18e31395de048548ecbc836
2020-04-17Merge #18467: rpc: Improve documentation and return value of settxfeeMarcoFalke
38677274f931088218eeb1f258077d3387f39c89 rpc: settxfee respects -maxtxfee wallet setting (Fabian Jahr) bda84a08a0ac92dff6cadc99cf9bb8c3fadd7e13 rpc: Add documentation for deactivating settxfee (Fabian Jahr) Pull request description: ~~Closes 18315~~ `settxfee` can be deactivated by passing 0 as the fee. That does not seem to be documented for the user so this PR adds it in the description. The return value of a simple boolean seems also too simplified given the multiple dimensions that this deactivation feature enables. I.e. it does not seem intuitive if the returned boolean shows that the call succeeded or if means that `settxfee` is active. My suggested solution is to change the return value to a JSON object that included the "active" state and the currently set fee rate. Examples: ``` $ src/bitcoin-cli settxfee 0.0000000 { "active": false, "fee_rate": "0.00000000 BTC/kB" } $ src/bitcoin-cli settxfee 0.0001 { "active": true, "fee_rate": "0.00010000 BTC/kB" } ``` ACKs for top commit: MarcoFalke: ACK 38677274f931088218eeb1f258077d3387f39c89, seems useful to error out early instead of later #16257 🕍 jonatack: ACK 38677274f931088218eeb meshcollider: LGTM, utACK 38677274f931088218eeb1f258077d3387f39c89 Tree-SHA512: 642813b5cf6612abb4b6cb63728081a6bd1659d809e0149c8f56060b6da7253fee989b3b202854f3051df3773c966799af30b612648c466b099f00590f356548
2020-04-17fuzz: fix unused variable compiler warningJon Atack
2020-04-17Merge #17824: wallet: Prefer full destination groups in coin selectionSamuel Dobson
a2324e4d3f47f084b07a364c9a360a0bf31e86a0 test: Improve naming and logging of avoid_reuse tests (Fabian Jahr) 1abbdac6777bc5396d17a6772c8176a354730997 wallet: Prefer full destination groups in coin selection (Fabian Jahr) Pull request description: Fixes #17603 (together with #17843) In the case of destination groups of >10 outputs existing in a wallet with `avoid_reuse` enabled, the grouping algorithm is adding left-over outputs as an "incomplete" group to the list of groups even when a full group has already been added. This leads to the strange behavior that if there are >10 outputs for a destination the transaction spending from that will effectively use `len(outputs) % 10` as inputs for that transaction. From the original PR and the code comment I understand the correct behavior should be the usage of 10 outputs. I opted for minimal changes in the current code although there maybe optimizations possible for cases with >20 outputs on a destination this sounds like too much of an edge case right now. ACKs for top commit: jonatack: Re-ACK a2324e4 achow101: ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0 kallewoof: ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0 meshcollider: Tested ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0 (verified the new test fails on master without this change) Tree-SHA512: 4743779c5d469fcd16df5baf166024b1d3c8eaca151df1e8281b71df62b29541cf7bfee3f8ab48d83e3b34c9256e53fd38a7b146a54c79f9caa44cce3636971a
2020-04-17Merge #18670: refactor: Remove unused methods CBloomFilter::reset()/clear()MarcoFalke
69ffddc83e0f3e265bf6cf7ae31489ae629fe6be refactor: Remove unused methods CBloomFilter::reset()/clear() (Sebastian Falbesoner) Pull request description: The method `CBloomFilter::reset()` was introduced by commit d2d7ee0e863b286e1c9f9c54659d494fb0a7712d in 2015, but was never ever used, as far as I could find. As discovered by MarcoFalke, the method `clear()` is also unused outside of unit tests and is hence also removed. ACKs for top commit: MarcoFalke: re-ACK 69ffddc83e0f3e265bf6cf7ae31489ae629fe6be jonatack: ACK 69ffddc83e0f3e2, code review, compiled a fuzz build and started the bloom_filter fuzz test as a sanity check. promag: ACK 69ffddc83e0f3e265bf6cf7ae31489ae629fe6be. Tree-SHA512: 6c53678545ad8e2fa1ffc0a8838e450462f26748a60632f738dc020f0eb494ae2c32841e6256e266ed9140177257a78b707123421942f3819a14ffcb9a99322f
2020-04-17test: Move boost/stdlib includes lastMarcoFalke
2020-04-17Merge #18262: bnb: exit selection when best_waste is 0Samuel Dobson
9b5950db8683f9b4be03f79ee0aae8a780b01a4b bnb: exit selection when best_waste is 0 (Andrew Chow) Pull request description: If we find a solution which has no waste, just use that. This solution is what we would consider to be optimal, and other solutions we find would have to also have 0 waste, so they are equivalent to the first one with 0 waste. Thus we can optimize by just choosing the first one with 0 waste. Closes #18257 ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/18262/commits/9b5950db8683f9b4be03f79ee0aae8a780b01a4b meshcollider: utACK 9b5950db8683f9b4be03f79ee0aae8a780b01a4b Tree-SHA512: 59565ff4a3d8281e7bc0ce87065a34c8d8bf8a95f628ba96b4fe89f1274979165aea6312e5f1f21b418c8c484aafc5166d22d9eff9d127a8192498625d58c557
2020-04-17refactor: Remove unused methods CBloomFilter::reset()/clear()Sebastian Falbesoner
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2020-04-16Merge #18662: test: Replace gArgs with local argsman in benchMarcoFalke
faf989f93695d29099f6e152d5a2e117cd304183 util: Document why ArgsManager (con/de)structor is not inline (MarcoFalke) fae00a77e2589cc784650e3e60e1b99c22ca8a7b bench: Remove unused argsman.ClearArgs (MarcoFalke) fa46aebeb1fc419b227524eab8352d9a7fc1f981 scripted-diff: Replace gArgs with local argsman in bench (MarcoFalke) fa2bc4141df59f2e38fef863723b433250295d20 tools: Add unused argsman to bench_bitcoin (MarcoFalke) Pull request description: All utilities use the same gArgs global that the node uses. This is generally fine and does not lead to issues unless a bench test is going to spin up a NodeContext via the TestingSetup. In that case the two uses of gArgs conflict and currently it needs to be cleared: https://github.com/bitcoin/bitcoin/blob/544709763e1f45148d1926831e07ff03487673ee/src/bench/bench_bitcoin.cpp#L76 One solution would be to do nothing, because the current code works with that workaround. Another solution would be to not use the same global in all binaries. ACKs for top commit: promag: ACK faf989f93695d29099f6e152d5a2e117cd304183. ryanofsky: Code review ACK faf989f93695d29099f6e152d5a2e117cd304183. Just new commit added restoring forward declaration Tree-SHA512: 8ee4b28eee294d41c002f801fa844b0c23c919a3061f5109638701db0947b3b0ea28caa7311ae5f126fc660648bbaa0890853e6b06bdc5868692f52ba8c05f66
2020-04-16scripted-diff: Bump copyright headersMarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
2020-04-16scripted-diff: Sort test includesMarcoFalke
-BEGIN VERIFY SCRIPT- # Mark all lines with #includes sed -i --regexp-extended -e 's/(#include <.*>)/\1 /g' $(git grep -l '#include' ./src/bench/ ./src/test ./src/wallet/test/) # Sort all marked lines git diff -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v -END VERIFY SCRIPT-
2020-04-16Merge #17669: tests: have coins simulation test also use CCoinsViewDBMarcoFalke
bee88b8c5887e6beb75f26f0db97888a48fa7e7c tests: have coins simulation test also use CCoinsViewDB (James O'Beirne) Pull request description: Before this change, the coins simulation test uses a base view of type CCoinsViewTest, which has no relevance outside of the unittest suite. Might as well reuse this testcase with a more realistic configuration that has CCoinsViewDB (i.e. in-memory leveldb) at the bottom of the view structure. This adds explicit use of CCoinsViewDB in the unittest suite. #### Before change ``` ./src/test/test_bitcoin --run_test=coins_tests --catch_system_errors=no 21.99s user 0.04s system 99% cpu 22.057 total ``` #### After change ``` ./src/test/test_bitcoin --run_test=coins_tests --catch_system_errors=no 78.80s user 0.04s system 100% cpu 1:18.82 total ``` ACKs for top commit: ryanofsky: Code review ACK bee88b8c5887e6beb75f26f0db97888a48fa7e7c Tree-SHA512: 75296b2bcbae2f46e780489aafb032592544a15c384d569d016005692fe79fe60d7f05857cf25cc7b0f9ab1c53b47886a6c71cca074a03fb9afec30e1f376858
2020-04-16util: Document why ArgsManager (con/de)structor is not inlineMarcoFalke
2020-04-16Merge #18660: test: Verify findCommonAncestor always initializes outputsMarcoFalke
9986608ba93de040490ee0d5584ea33e605f1df0 test: Verify findCommonAncestor always initializes outputs (Russell Yanofsky) Pull request description: Also add code comment to clarify surprising code noted by practicalswift https://github.com/bitcoin/bitcoin/pull/18657#issuecomment-614278450 ACKs for top commit: MarcoFalke: ACK 9986608ba93de040490ee0d5584ea33e605f1df0 jonatack: ACK 9986608ba93de04 modulo @practicalswift's https://github.com/bitcoin/bitcoin/pull/18660#issuecomment-614487724 Tree-SHA512: d79c910291d68b770ef4b09564d274c0e19a6acf43ef1a6691dc889e3944ab3462b86056eeb794fd0c6f2464cfad6cc00711a833f84b32079c69ef9b3c8da24c
2020-04-16Merge #18650: qt: Make bitcoin.ico non-executableMarcoFalke
a95af77eb264646b4160836e4e9a2c4b45f87bb8 qt: Make bitcoin.ico non-executable (practicalswift) Pull request description: Make `bitcoin.ico` non-executable. No need to execute icons and having +x bits laying around breaks `find … -executable` :) Before this patch: ```sh $ find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable ci/retry/retry contrib/macdeploy/macdeployqtplus depends/config.guess depends/config.sub src/qt/res/icons/bitcoin.ico src/secp256k1/src/modules/recovery/main_impl.h ``` After this patch: ```sh $ find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable ci/retry/retry contrib/macdeploy/macdeployqtplus depends/config.guess depends/config.sub src/secp256k1/src/modules/recovery/main_impl.h ``` FWIW: ``` $ file $(find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable) ci/retry/retry: Bourne-Again shell script, UTF-8 Unicode text executable contrib/macdeploy/macdeployqtplus: Python script, ASCII text executable depends/config.guess: POSIX shell script, ASCII text executable depends/config.sub: POSIX shell script, ASCII text executable src/qt/res/icons/bitcoin.ico: MS Windows icon resource - 10 icons, 48x48, 16 colors, 4 bits/pixel, 32x32, 16 colors, 4 bits/pixel src/secp256k1/src/modules/recovery/main_impl.h: C source, ASCII text ``` ACKs for top commit: MarcoFalke: ACK a95af77eb264646b4160836e4e9a2c4b45f87bb8 gitian build finished, so it doesn't look like the icon used in Windows resource files needs to be executable. Though, I didn't read the documentation. jonatack: ACK a95af77eb264646 Tree-SHA512: ecf9154077824ae4c274b4341e985797f3648c0cb0c31cb25ce382163b923a3acbc7048683720be4ae3663501801129cd0f48c441a36f049cc304ebe9f30994e
2020-04-16Merge #18401: Refactor: Initialize PrecomputedTransactionData in ↵MarcoFalke
CheckInputScripts f63dec189c3c8eee1ab2187681d5d0b2513b1b2e [REFACTOR] Initialize PrecomputedTransactionData in CheckInputScripts (Pieter Wuille) Pull request description: This is a single commit taken from the Schnorr/Taproot PR #17977. Add a default constructor to `PrecomputedTransactionData`, which doesn't initialize the struct's members. Instead they're initialized inside the `CheckInputScripts()` function. This allows a later commit to add the spent UTXOs to that structure. The spent UTXOs are required for the schnorr signature hash, since it commits to the scriptPubKeys. See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#common-signature-message for details. By itself, this isn't really an improvement to the code, but I think it makes sense to separate out the refactor/moveonly commits from PR #17977 so that PR is only the logical changes needed for Schnorr/Taproot. ACKs for top commit: jonatack: Re-ACK f63dec1 `git diff 851908d f63dec1` shows no change since last ACK. sipa: utACK f63dec189c3c8eee1ab2187681d5d0b2513b1b2e theStack: re-ACK f63dec189c3c8eee1ab2187681d5d0b2513b1b2e fjahr: Re-ACK f63dec189c3c8eee1ab2187681d5d0b2513b1b2e ariard: Code Review ACK f63dec1 Tree-SHA512: ecf9154077824ae4c274b4341e985797f3648c0cb0c31cb25ce382163b923a3acbc7048683720be4ae3663501801129cd0f48c441a36f049cc304ebe9f30994e
2020-04-16rpc: Document all aliases for first arg of listtransactionsMarcoFalke
2020-04-16rpc: Document all aliases for second arg of getblockMarcoFalke
2020-04-15bench: Remove unused argsman.ClearArgsMarcoFalke
2020-04-15scripted-diff: Replace gArgs with local argsman in benchMarcoFalke
-BEGIN VERIFY SCRIPT- sed -i -e 's/gArgs/argsman/g' src/bench/bench_bitcoin.cpp -END VERIFY SCRIPT-
2020-04-15tools: Add unused argsman to bench_bitcoinMarcoFalke
2020-04-15test: Verify findCommonAncestor always initializes outputsRussell Yanofsky
Also add code comment to clarify surprising code noted by practicalswift https://github.com/bitcoin/bitcoin/pull/18657#issuecomment-614278450
2020-04-15fuzz: Disable debug log fileMarcoFalke
2020-04-15test: Add optional extra_args to testing setupMarcoFalke
2020-04-15node: Add args alias for gArgs globalMarcoFalke
2020-04-15Merge #18615: test: Avoid accessing free'd memory in ↵MarcoFalke
validation_chainstatemanager_tests fa176e253fb473767c61d4d8cd2d93e87d71a015 test: Avoid accessing free'd memory in validation_chainstatemanager_tests (MarcoFalke) Pull request description: ACKs for top commit: ryanofsky: Code review ACK fa176e253fb473767c61d4d8cd2d93e87d71a015, though if you have to update this again, would suggest separating txindex test cleanup and the chainstatemanager test fix in separate commits, or identifying which part of the change is the bugfix fix in the commit description. Also to clean up the txindex test it might make sense to call SyncWithValidationInterfaceQueue in the test destructor to prevent nondeterminism in other tests Tree-SHA512: 34c5dca283a7c205cd42b6aa59f00a71fd1bd980bc3d6640a18b280be11470bfabb2fd8c93fadde6fb8e084bcf96c80ec3aa72bbccccfde8a8260d173eaad08f
2020-04-15test: Avoid accessing free'd memory in validation_chainstatemanager_testsMarcoFalke
2020-04-15qt: Make bitcoin.ico non-executablepracticalswift
2020-04-15gui: use PACKAGE_NAME in exception messagefanquake
2020-04-15Merge #18621: script: Disallow silent bool -> CScript conversionfanquake
88884ee8d8dcd5303b20e54801b03f9631959c76 script: Disallow silent bool -> CScript conversion (MarcoFalke) Pull request description: Makes nonsensical stuff like `ScriptToAsmStr(false,false);` a compile failure ACKs for top commit: practicalswift: ACK 88884ee8d8dcd5303b20e54801b03f9631959c76 laanwj: ACK 88884ee8d8dcd5303b20e54801b03f9631959c76 promag: ACK 88884ee8d8dcd5303b20e54801b03f9631959c76. instagibbs: utACK 88884ee8d8dcd5303b20e54801b03f9631959c76 jb55: ACK 88884ee8d8dcd5303b20e54801b03f9631959c76 ryanofsky: Code review ACK 88884ee8d8dcd5303b20e54801b03f9631959c76 Tree-SHA512: 419d79c03b44a979c061b0540662928251ad68d53e65996bf370bb55ed1526ac7a22710cb7536c9954db5fec07bc312884bf8828f97a4ba180a5b07969a17f54
2020-04-14rpc: settxfee respects -maxtxfee wallet settingFabian Jahr
2020-04-14wallet: Prefer full destination groups in coin selectionFabian Jahr
When a wallet uses avoid_reuse and has a large number of outputs in a single destination, it groups these outputs in OutputGroups that are no larger than OUTPUT_GROUP_MAX_ENTRIES. The goal is to spend as many outputs as possible from the destination while not breaking consensus due to a huge number of inputs and also not surprise the use with high fees. If there are n outputs in a destination and n > OUTPUT_GROUP_MAX_ENTRIES then this results in one or many groups of size OUTPUT_GROUP_MAX_ENTRIES and possibly one group of size < OUTPUT_GROUP_MAX_ENTRIES. Prior to this commit the coin selection in the case where n > OUTPUT_GROUP_MAX_ENTRIES was skewed towards the one group of size < OUTPUT_GROUP_MAX_ENTRIES if it exists and the amount to be spent by the transaction is smaller than the aggregate of those of the group size < OUTPUT_GROUP_MAX_ENTRIES. The reason is that the coin selection decides between the different groups based on fees and mostly the smaller group will cause smaller fees. The behavior that users of the avoid_reuse flag seek is that the full groups of size OUTPUT_GROUP_MAX_ENTRIES get used first. This commit implements this by pretending that the small group has a large number of ancestors (one smallet than the maximum allowed for this wallet). This dumps the small group to the bottom of the list of priorities in the coin selection algorithm.
2020-04-14Merge #17954: wallet: Remove calls to Chain::Lock methodsMarcoFalke
48973402d8bccb673eaeb68b7aa86faa39d3cb8a wallet: Avoid use of Chain::Lock in CWallet::GetKeyBirthTimes (Russell Yanofsky) e958ff9ab5607da2cd321f29fc785a6d359e44f4 wallet: Avoid use of Chain::Lock in CWallet::CreateTransaction (Russell Yanofsky) c0d07dc4cba7634cde4e8bf586557772f3248a42 wallet: Avoid use of Chain::Lock in CWallet::ScanForWalletTransactions (Russell Yanofsky) 1be8ff280c78c30baabae9429c53c0bebb89c44d wallet: Avoid use of Chain::Lock in rescanblockchain (Russell Yanofsky) 3cb85ac594f115db99f96b0a0f4bfdcd69ef0590 wallet refactor: Avoid use of Chain::Lock in CWallet::RescanFromTime (Russell Yanofsky) f7ba881bc669451a60fedac58a449794702a3e23 wallet: Avoid use of Chain::Lock in listsinceblock (Russell Yanofsky) bc96a9bfc61afdb696fb92cb644ed5fc3d1793f1 wallet: Avoid use of Chain::Lock in importmulti (Russell Yanofsky) 25a9fcf9e53bfa94e8f8b19a4abfda0f444f6b2a wallet: Avoid use of Chain::Lock in importwallet and dumpwallet (Russell Yanofsky) c1694ce6bb7e19a8722d5583cd85ad17da40bb67 wallet: Avoid use of Chain::Lock in importprunedfunds (Russell Yanofsky) ade5f87971211bc67753f14a0d49e020142efc7c wallet refactor: Avoid use of Chain::Lock in qt wallettests (Russell Yanofsky) f6da44ccce4cfff53433e665305a6fe0a01364e4 wallet: Avoid use of Chain::Lock in tryGetTxStatus and tryGetBalances (Russell Yanofsky) bf30cd4922ea62577d7bf63f5029e8be62665d45 refactor: Add interfaces::FoundBlock class to selectively return block data (Russell Yanofsky) Pull request description: This is a set of changes updating wallet code to make fewer calls to `Chain::Lock` methods, so the `Chain::Lock` class will be easier to remove in #16426 with fewer code changes and small changes to behavior. ACKs for top commit: MarcoFalke: re-ACK 48973402d8, only change is fixing bug 📀 fjahr: re-ACK 48973402d8bccb673eaeb68b7aa86faa39d3cb8a, reviewed rebase and changes since last review, built and ran tests locally ariard: Coce Review ACK 4897340, only changes are one suggested by last review on more accurate variable naming, human-readable output, args comments in `findCommonAncestor` Tree-SHA512: cfd2f559f976b6faaa032794c40c9659191d5597b013abcb6c7968d36b2abb2b14d4e596f8ed8b9a077e96522365261299a241a939b3111eaf729ba0c3ef519b
2020-04-13Merge #18493: rpc: Remove deprecated "size" from mempool txsMarcoFalke
0753efd9dc8f2e756955a726afbb602d904e1e92 rpc: Remove deprecated "size" from mempool txs (Vasil Dimov) Pull request description: Remove the "size" property of a mempool transaction from RPC replies. Deprecated in e16b6a718 in 0.19, about 1 year ago. ACKs for top commit: kristapsk: ACK 0753efd9dc8f2e756955a726afbb602d904e1e92 Tree-SHA512: 392ced6764dd6a1d47c6d1dc9de78990cf3384910d801253f8f620bd1751b2676a6b52bee8a30835d28e845d84bfb37431c1d2370f48c9d4dc8e6a48a5ae8b9e
2020-04-13script: Disallow silent bool -> CScript conversionMarcoFalke
2020-04-13Merge #18502: doc: Update docs for getbalance (default minconf should be 0)MarcoFalke
c0af173da20cc4560523205bc268677a808a43df doc: default minconf for getbalance should be 0 (U-Zyn Chua) Pull request description: - Default `minconf` for `getbalance` is `0` but example in doc was showing as `1`. - `at least 6 blocks confirmed` now updated to be `at least 6 confirmations` to be more consistent with the terminology used elsewhere in the codebase and documentations. ACKs for top commit: theStack: re-ACK https://github.com/bitcoin/bitcoin/pull/18502/commits/c0af173da20cc4560523205bc268677a808a43df Tree-SHA512: 8f67af78a222a4bd2957658b37fae2224783274f355af84f39a5ce0da90b21f03dc798a6408d44a724c353ff5ed7dfec943fb28726ec423028b64fc579f937ad
2020-04-13doc: default minconf for getbalance should be 0U-Zyn Chua
2020-04-12rpc: Rename first arg of generateblock RPC to "output"MarcoFalke
2020-04-12Merge #18495: rpc: Remove deprecated migration codeMarcoFalke
2599d13c9417dc8c5107535521173687ec5e6c2f rpc: Remove deprecated migration code (Vasil Dimov) Pull request description: Don't accept a second argument to `sendrawtransaction` and `testmempoolaccept` of type `bool`. Actually even the code before this change would not accept `bool`, but it would print a long explanatory message when rejecting it: "Second argument must be numeric (maxfeerate) and no longer supports a boolean. To allow a transaction with high fees, set maxfeerate to 0." This was scheduled for removal in 6c0a6f73e. ACKs for top commit: MarcoFalke: ACK 2599d13c9417dc8c5107535521173687ec5e6c2f 📅 Tree-SHA512: e2c74c0bde88e20149d0deab0845851bb3979143530a6bae4f46769d61b607ad2e2347f8969093c2461a80c47661732dc0b3def140f8ce84081719adda3b3811
2020-04-11[REFACTOR] Initialize PrecomputedTransactionData in CheckInputScriptsPieter Wuille
Add a default constructor to `PrecomputedTransactionData`, which doesn't initialize the struct's members. Instead they're initialized inside the `CheckInputScripts()` function. This allows a later commit to add the spent UTXOs to that structure.
2020-04-11Merge #18588: Revert "Merge #16367: Multiprocess build support"MarcoFalke
f29bd546ec169dd9af2ca6265c76353e68db92be Revert "Merge #16367: Multiprocess build support" (MarcoFalke) Pull request description: Reverting the changes temporarily is going to help with the following: * Discussion about the next steps for the multiprocess concept and the experimental libmultiprocess library without having code already commited in the master branch, potentially influencing the discussion * Allowing for more conceptual as well as code review ACKs to accumulate, since the pull only had one ACK (two if I count mine, which didn't make it to GitHub) Can be reviewed with `git diff HEAD HEAD~2 | wc` or `git diff 1b307613604883daea4913a65da30ae073c9dc4d~ | wc` (should be all zeros) Context here: https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-612260496 ACKs for top commit: ryanofsky: Code review ACK f29bd546ec169dd9af2ca6265c76353e68db92be. Confirmed revert with fanquake: ACK f29bd546ec169dd9af2ca6265c76353e68db92be Tree-SHA512: 3ce06c30de23c81c2d69cfb3ada20b3458c48efda1a5ba96aee678e946c499f701bc83e9eae91580f0156c0f30a90e5d015ef8b1806ad611d433c482fa55723e
2020-04-10Revert "Merge #16367: Multiprocess build support"MarcoFalke
This reverts the changes made in merge commit 1b307613604883daea4913a65da30ae073c9dc4d: This reverts commit b919efadff3d0393f4a8c3c1dc735f7ac5c665bb. This reverts commit d54f64c6c700be0604190f52c84fc5f1cdd9f02f. This reverts commit 787f40668dc15980c3d6de028d7950c08175d84a. This reverts commit d6306466626635e6fee44385e6a688c8dc118eb5. This reverts commit e6e44eedd56ecaf59f3fabf8e07ab7dee0ddb1b6.
2020-04-10Merge #17905: gui: Avoid redundant tx status updatesMarcoFalke
96cb597325f64cadb3cf43e2cdb3d7c1e2e49891 gui: Avoid redundant tx status updates (Russell Yanofsky) Pull request description: This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). In `TransactionTablePriv::index`, avoid calling `interfaces::Wallet::tryGetTxStatus` if the status is up to date as of the most recent `NotifyBlockTip` notification. Store height from the most recent notification in a new `ClientModel::cachedNumBlocks` variable in order to check this. This avoids floods of IPC traffic from `tryGetTxStatus` with #10102 when there are a lot of transactions. It might also make the GUI a little more efficient even when there is no IPC. ACKs for top commit: promag: Code review ACK 96cb597325f64cadb3cf43e2cdb3d7c1e2e49891. hebasto: ACK 96cb597325f64cadb3cf43e2cdb3d7c1e2e49891 Tree-SHA512: fce597bf52a813ad4923110d0a39229ea09e1631e0d580ea18cffb09e58cdbb4b111a40a9a9270ff16d8163cd47b0bd9f1fe7e3a6c7ebb19198f049f8dd1aa46