23688 Commits

Author SHA1 Message Date
MarcoFalke
a998c5185b
Merge #18675: tests: Don't initialize PrecomputedTransactionData in txvalidationcache tests
3718ae2ef8dd2559e435bf8d7f5ed5217611ce81 [tests] Don't initialize PrecomputedTransactionData in txvalidationcache tests (John Newbery)

Pull request description:

  PrecomputedTransactionData is initialized inside CheckInputScripts(). No need to pre-initialize it before calling into CheckInputScripts().

  Normally, I wouldn't bother, but we're making changes to `PrecomputedTransactionData` in #17977 which would break these tests without removing these constructions. Might as well get these changes out of the way here.

ACKs for top commit:
  robot-visions:
    ACK 3718ae2ef8dd2559e435bf8d7f5ed5217611ce81
  sipa:
    utACK 3718ae2ef8dd2559e435bf8d7f5ed5217611ce81

Tree-SHA512: bc9c095035a7072a2a91941df38cdbb969e817264efbaa6dcb88cc3ab132d9264aa0751fa588d1a5e45f37b4d2bb1903cda078765f0bbcc87d9cc47cbec5356a
2020-04-19 06:18:21 -04:00
MarcoFalke
b690b24eb2
Merge #18633: test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2)
fa03713e133e3017112fdd5c278e0c8643054578 test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) (MarcoFalke)

Pull request description:

  actually (?) fix #18561

  See most recent traceback https://travis-ci.org/github/bitcoin/bitcoin/jobs/674668692#L7062

  I believe the reason the error is still there is that ConnectionResetError is derived from OSError:

  ConnectionResetError(ConnectionError(OSError))

  And IOError is an alias for OSError since python 3.3, see https://docs.python.org/3/library/exceptions.html#IOError

  So fix that by renaming IOError to the alias OSError and move the less specific catch clause down a few lines.

ACKs for top commit:
  jonatack:
    ACK fa03713e133e3017112fdd5c278e0c8643054578

Tree-SHA512: 6e5b214ed9101bf8ebe7472dcc1f9e9d128e2575c93ec00c8d0774ae1a9b52a8c2a653a45a0eab8d881570b08dd5ffeddf5aca88a10438c366e1f633253cb0b5
2020-04-19 06:10:06 -04:00
fanquake
d65631171c
Merge #18695: test: Replace boost::mutex with std::mutex
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 27abd1a4f4
  sipa:
    utACK 27abd1a4f4c7a3d092d59edbbaa1e0f324c8b0ef

Tree-SHA512: 062eed360a68910fb71552fd892bfd097442718a237446cfb8350bfd5d807da7251ead2b9755e1d7022598774ed23fa5432a589ac6f8cadddab404b439883466
2020-04-19 11:53:25 +08:00
MarcoFalke
fa03713e13
test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) 2020-04-18 14:58:58 -04:00
MarcoFalke
6ae99aab5d
Merge #18692: test: Bump timeout in wallet_import_rescan
fabfcad8764bb8f807b0ac5f3482b414278a4525 test: Bump timeout in wallet_import_rescan (MarcoFalke)

Pull request description:

  Avoid timeouts when starting the node, also make error message more verbose

ACKs for top commit:
  practicalswift:
    ACK fabfcad8764bb8f807b0ac5f3482b414278a4525 -- patch looks correct

Tree-SHA512: 8fd60a05380349f521d0e814d2f268702dfbe57c7567a4f6e94435498dfdd32909179d75fded44757ecb1a93a4045842bc6d00bfd6cd18ba751513461359c7b0
2020-04-18 12:57:16 -04:00
Samuel Dobson
bbb1ba1814
Merge #17219: wallet: allow transaction without change if keypool is empty
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-18 22:00:26 +12:00
Hennadii Stepanov
27abd1a4f4
test: Replace boost::mutex with std::mutex 2020-04-18 01:51:05 +03:00
MarcoFalke
895c71e535
Merge #18682: fuzz: http_request workaround for libevent < 2.1.1
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 8ac3c4c25b 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-17 17:17:11 -04:00
MarcoFalke
fabfcad876
test: Bump timeout in wallet_import_rescan 2020-04-17 17:09:27 -04:00
MarcoFalke
c54295c1a2
Merge #18641: test: Create cached blocks not in the future
fa320975411af4f0e41771d89958a77fd7a2284b test: Create cached blocks not in the future (MarcoFalke)

Pull request description:

  This avoids test failures when tests assume blocks are not from the future, like in wallet_dump: https://cirrus-ci.com/task/6607130193035264?command=ci#L3306

ACKs for top commit:
  jonatack:
    ACK fa320975411af4f0e41771d89958a77fd7a2284b

Tree-SHA512: 60b6882e0e1df8c5d67f034533407a45d3685983891b67ff4631072bfd0a93a325c7ca18758d7a2df252e4fcdb7c87321cb1e84458b22782e57e719eec634c22
2020-04-17 14:04:14 -04:00
MarcoFalke
d7bdba460c
Merge #18683: ci: Disable valgrind functionl tests on forked repos to avoid timeouts
faceeae49ae85fb644996fedb970ad9d0e297ca5 ci: Disable valgrind functionl tests on forked repos to avoid timeouts (MarcoFalke)

Pull request description:

  Allows people to fork our repo and run the tests again

  Also print more cache stats

ACKs for top commit:
  hebasto:
    ACK faceeae49ae85fb644996fedb970ad9d0e297ca5, tested on my own repo: https://travis-ci.org/github/hebasto/bitcoin/jobs/676257500

Tree-SHA512: 50e44edf94fcb997438eeaf7308b2b58a0854141ecb1fbb0ba7bf5ed662f19b60899f966f579cca90ad5e789234d0e90122d8c2c854da70339058adc3a475fa6
2020-04-17 14:00:04 -04:00
Sebastian Falbesoner
6f8b498d18 fuzz: http_request workaround for libevent < 2.1.1
Before libevent 2.1.1, internal functions names didn't end with an underscore.
2020-04-17 19:00:19 +02:00
MarcoFalke
244daa4821
Merge #18607: rpc: Fix named arguments in documentation
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-17 12:16:42 -04:00
MarcoFalke
54f812d9d2
Merge #18673: scripted-diff: Sort test includes
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-17 10:12:13 -04:00
MarcoFalke
ecc2e4e363
Merge #18664: fuzz: fix unused variable compiler warning
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-17 09:09:59 -04:00
MarcoFalke
faceeae49a
ci: Disable valgrind functionl tests on forked repos to avoid timeouts 2020-04-17 09:00:43 -04:00
MarcoFalke
c2e53ff064
Merge #18467: rpc: Improve documentation and return value of settxfee
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-17 07:55:55 -04:00
Jon Atack
eab7367e25
fuzz: fix unused variable compiler warning 2020-04-17 13:45:43 +02:00
Samuel Dobson
c189bfd260
Merge #17824: wallet: Prefer full destination groups in coin selection
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-17 23:05:48 +12:00
MarcoFalke
4a71c46905
Merge #18670: refactor: Remove unused methods CBloomFilter::reset()/clear()
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-17 06:49:00 -04:00
MarcoFalke
fa4632c417
test: Move boost/stdlib includes last 2020-04-17 06:36:04 -04:00
Samuel Dobson
0856c15706
Merge #18262: bnb: exit selection when best_waste is 0
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 9b5950db86
  meshcollider:
    utACK 9b5950db8683f9b4be03f79ee0aae8a780b01a4b

Tree-SHA512: 59565ff4a3d8281e7bc0ce87065a34c8d8bf8a95f628ba96b4fe89f1274979165aea6312e5f1f21b418c8c484aafc5166d22d9eff9d127a8192498625d58c557
2020-04-17 22:32:13 +12:00
Sebastian Falbesoner
69ffddc83e refactor: Remove unused methods CBloomFilter::reset()/clear()
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2020-04-17 01:09:39 +02:00
MarcoFalke
969ee85494
Merge #18662: test: Replace gArgs with local argsman in bench
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:

  544709763e/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-16 16:37:27 -04:00
Wladimir J. van der Laan
8f2497941e
Merge #18598: gitian: Add missing automake package to gitian-win-signer.yml
e44aeefaaed8d698d1b9004b66f85384397b1a75 gitian: Add missing automake package to gitian-win-signer.yml (Andrew Chow)

Pull request description:

  automake is needed to build osslsigncode otherwise autogen.sh fails with the docker virtualization method.

ACKs for top commit:
  hebasto:
    ACK e44aeefaaed8d698d1b9004b66f85384397b1a75, for `osslsigncode-1.7.1` we did not run `autogen.sh` in the past.
  fanquake:
    ACK e44aeefaaed8d698d1b9004b66f85384397b1a75
  jonatack:
    ACK e44aeef

Tree-SHA512: a0e615c1b099ee1c469ce41f886f2ece6746234a5a800743a4e8be671e4114fd30e1c35bc0ddcb75778409564129d0fde7ac4e3d70b0f7691f97f729f34c8e0c
2020-04-16 22:27:46 +02:00
MarcoFalke
447f8676b2
Merge #18667: ci: Limit cache size regardless of NO_DEPENDS
0c6318788beaf1a31aeba5a21f3f8bb5c07cea6c ci: Limit cache size regardless of NO_DEPENDS (Hennadii Stepanov)

Pull request description:

  Close #18666.

ACKs for top commit:
  MarcoFalke:
    ACK 0c6318788beaf1a31aeba5a21f3f8bb5c07cea6c . Depends has ccache disabled anyway and is cached regardless of whether ccache is there or not, see #17248

Tree-SHA512: b1bf98be0f844b4704abd177841b014f3900be8160496f0d12596310db607b4f544547e8c3cbfcf17c086a78afd251653363f3dd467b769ac0062bc19adc8144
2020-04-16 16:12:30 -04:00
John Newbery
3718ae2ef8 [tests] Don't initialize PrecomputedTransactionData in txvalidationcache tests
PrecomputedTransactionData is initialized inside CheckInputScripts(). No need
to pre-initialize it before calling into CheckInputScripts().
2020-04-16 15:20:57 -04:00
MarcoFalke
fa488f131f
scripted-diff: Bump copyright headers
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
2020-04-16 13:33:09 -04:00
MarcoFalke
fac5c37300
scripted-diff: Sort test includes
-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-16 13:32:36 -04:00
MarcoFalke
d8dfcea5d9
Merge #17669: tests: have coins simulation test also use CCoinsViewDB
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-16 12:56:36 -04:00
MarcoFalke
faf989f936
util: Document why ArgsManager (con/de)structor is not inline 2020-04-16 12:26:09 -04:00
MarcoFalke
f4c0ad4aef
Merge #18660: test: Verify findCommonAncestor always initializes outputs
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-16 11:44:03 -04:00
Hennadii Stepanov
0c6318788b
ci: Limit cache size regardless of NO_DEPENDS 2020-04-16 17:56:21 +03:00
MarcoFalke
661e8df1b6
Merge #18653: test: add coverage for bitcoin-cli -rpcwait
c28c7b882b40cc75989dd102414de3b3dad69706 test: add -getinfo "unlocked_until" and "headers" coverage (Jon Atack)
bb13f46fb15c360c68e370d0106463afd590dc43 test: verify cli.getwalletinfo in wallet section (Jon Atack)
becc8b9747c963b0fb1b927ced60d5bebed60be4 test: verify bitcoin-cli -version with node stopped (Jon Atack)
727b67e7b86567adff81d945044989de3b5d2d5f test: add coverage for bitcoin-cli -rpcwait (Jon Atack)

Pull request description:

  and other coverage improvements in `interface-bitcoin_cli.py`

ACKs for top commit:
  kristapsk:
    ACK c28c7b882b40cc75989dd102414de3b3dad69706

Tree-SHA512: c3ac73b673872cba05496e3f9debc2c4dfa4291c6426bf0be9bb0242035613b25c0aecebb39abb14a1d87b13e1d28a5f34b21d4b77d93de56c679a8be92bbe82
2020-04-16 10:11:12 -04:00
MarcoFalke
79b0459648
Merge #18650: qt: Make bitcoin.ico non-executable
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-16 09:59:23 -04:00
MarcoFalke
e16718a8b3
Merge #18401: Refactor: Initialize PrecomputedTransactionData in 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-16 08:51:54 -04:00
MarcoFalke
fa168d7542
rpc: Document all aliases for first arg of listtransactions 2020-04-16 08:45:33 -04:00
MarcoFalke
fa5b1f067f
rpc: Document all aliases for second arg of getblock 2020-04-16 08:45:05 -04:00
MarcoFalke
fae00a77e2
bench: Remove unused argsman.ClearArgs 2020-04-15 20:33:36 -04:00
MarcoFalke
fa46aebeb1
scripted-diff: Replace gArgs with local argsman in bench
-BEGIN VERIFY SCRIPT-
sed -i -e 's/gArgs/argsman/g' src/bench/bench_bitcoin.cpp
-END VERIFY SCRIPT-
2020-04-15 20:32:30 -04:00
MarcoFalke
fa2bc4141d
tools: Add unused argsman to bench_bitcoin 2020-04-15 20:22:18 -04:00
MarcoFalke
544709763e
Merge #18571: fuzz: Disable debug log file
fa69f88486e900aacf3fc768671f947927173226 fuzz: Disable debug log file (MarcoFalke)
fa0cbd48c418ec14e1d91bffea206bce20bd1e56 test: Add optional extra_args to testing setup (MarcoFalke)
fad4fa7e2fb95b7ced9007060ebfd0e8f181f5d8 node: Add args alias for gArgs global (MarcoFalke)

Pull request description:

  There are several issues with writing to a debug log file when fuzzing:

  * Disk access is slow, but fuzzing should be fast (Note: I could not verify this claim with data)
  * Disks have a limited size and will eventually run out of space, but fuzzing should run continuous

  Fix both issues by disabling the debug log file for fuzz tests

ACKs for top commit:
  practicalswift:
    ACK fa69f88486e900aacf3fc768671f947927173226 -- patch looks correct

Tree-SHA512: f61beb6c94a9ab664deb191685fcad601e228b77bb1c43db6ec40616ae393c9dd35c51474f1b0759ac0bc29b5ca8456a329906a3695bd0f18fa4372210c8b54a
2020-04-15 17:41:56 -04:00
Russell Yanofsky
9986608ba9 test: Verify findCommonAncestor always initializes outputs
Also add code comment to clarify surprising code noted by practicalswift
https://github.com/bitcoin/bitcoin/pull/18657#issuecomment-614278450
2020-04-15 17:07:44 -04:00
MarcoFalke
e84a5f0004
Merge #18645: [doc] Update thread information in developer docs
808ef36b89ea9ce72116bbd7ee479b984367dc60 [doc] Update thread information in developer docs (John Newbery)

Pull request description:

  - DumpAddresses thread was removed in #5964
  - Shutdown thread was removed in #5679
  - Add new threads (scheduler, RPC worker, indexer, tor control)
  - Small changes to documentation of other threads

ACKs for top commit:
  MarcoFalke:
    ACK 808ef36b89
  hebasto:
    ACK 808ef36b89ea9ce72116bbd7ee479b984367dc60.

Tree-SHA512: 85b6ace7bcc4dee030c63461bef1ded1a9581d4fa249c59f6fcd5d33d89c4357a6b8b35888ce0960f276d397b5e38a21e6c5d4b7b79544827a28c950e097b36d
2020-04-15 16:30:11 -04:00
MarcoFalke
6db8ef2431
Merge #18640: appveyor: Remove clcache
fac0c8db9f634ea7231fb0b6dc139ae0bda4a051 appveyor: Remove clcache (MarcoFalke)

Pull request description:

  The build time without cache seems to be 47 minutes and with cache 46 minutes. Maybe we can save more time by not installing clcache.

Top commit has no ACKs.

Tree-SHA512: ce503641a465f5e49deb7aa6b566aaab5e567148c001704451891c49ba7ade3219ca788bc0d8e55565febb9aadd579e26894f6c03b99755eb8b5d1498acf4dc9
2020-04-15 16:19:52 -04:00
MarcoFalke
fa69f88486
fuzz: Disable debug log file 2020-04-15 15:13:11 -04:00
MarcoFalke
fa0cbd48c4
test: Add optional extra_args to testing setup 2020-04-15 15:13:04 -04:00
MarcoFalke
fad4fa7e2f
node: Add args alias for gArgs global 2020-04-15 15:05:18 -04:00
MarcoFalke
4bd6bc5cb4
Merge #18615: test: Avoid accessing free'd memory in 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-15 14:38:02 -04:00
John Newbery
808ef36b89 [doc] Update thread information in developer docs 2020-04-15 12:02:39 -04:00