Commit Graph

18073 Commits

Author SHA1 Message Date
Jonas Schnelli
a0dac26647 Merge #12029: Build: Add a makefile target for Doxygen documentation
a777244e4 Build: Add a makefile target for Doxygen documentation (Andrea Comand)

Pull request description:

  You can now build the doxygen documentation with `make docs` and clean it with `make clean-docs`.

  Fixes: #11949

Tree-SHA512: f2361ec7f771227367dd04bba1a444b44e59f13901463a678a5f2f579a10a56d67db2e29552e754e312a1c472a31593b6af189cbaac5cd351a428c57baf5ace7
2020-06-17 14:29:55 -05:00
Wladimir J. van der Laan
128e14640e Merge #12425: Add some script tests
be45a67 Add some script tests related to BOOL ops and odd values like negative 0. (Richard Kiss)

Pull request description:

  Add some script tests related to BOOL ops and odd values like negative 0.

Tree-SHA512: 8e633f7ea5eea39e31016994baf60f295fa1dc8cae27aa5fcfc741ea97136bfb3ddc57bb62b9c6bf9fe256fc09cdd184906ba8e611e297cf8d2d363da2bbf1d4
2020-06-17 14:29:55 -05:00
MarcoFalke
10a27192b1 Merge #12442: devtools: Exclude patches from lint-whitespace
fafbf7f74e devtools: Exclude patches from lint-whitespace (MarcoFalke)

Pull request description:

  By default, unified patches have trailing whitespace in all context lines. Thus, exclude patches from linting.

Tree-SHA512: 8f89f1584581e94dd4e34bd522cba21602bafe7933b4631a3abc5da5a8f25568810862d696618fe63c15edf3e046869ad5077d09373f09792985503c6a415538
2020-06-17 14:29:55 -05:00
Wladimir J. van der Laan
145f6798a3 Merge #12426: qt: Initialize members in WalletModel
fa27623 qt: Initialize members in WalletModel (MarcoFalke)

Pull request description:

  This prevents segfaults (or errors when running qt in valgrind)

  ```
  Conditional jump or move depends on uninitialised value(s)
      WalletModel::checkBalanceChanged() (walletmodel.cpp:156)

Tree-SHA512: 38c8c03c7fa947edb3f1c13eab2ac7a62ef8f8141603c2329a7dc5821a887a349af8014dc739b762e046f410f44a9c6653b6930f08b53496cf66381cadc06246
2020-06-17 14:29:55 -05:00
Wladimir J. van der Laan
2a24d84bd2 Merge #12409: rpc: Reject deprecated reserveChangeKey in fundrawtransaction
fa5f518 rpc: Reject deprecated reserveChangeKey in fundrawtransaction (MarcoFalke)

Pull request description:

Tree-SHA512: 8506d1494b13c4582b1379e3b8c3906016f1980ebe847727a43a90e7bb9f71b896a1792bc97a8dc7320ccce0534050eb04f92a6f82f811d08efa74a98b3e43f0
2020-06-17 14:29:55 -05:00
Wladimir J. van der Laan
9b0c1f7733 Merge #12393: Fix a-vs-an typos
11376b5 Fix a-vs-an typos (practicalswift)

Pull request description:

  Fix a-vs-an typos.

Tree-SHA512: 2cf74c15656a20ec13d2da7d86a39d14e634db368833d92da06a78d1266950accfc4fcc89cfecdaadd46e6b48b17e6fad29080428e564871e78482c53f3e855c
2020-06-17 14:29:55 -05:00
Wladimir J. van der Laan
4025a5b9af Merge #12282: wallet: Disallow abandon of conflicted txes
fa795cf wallet: Disallow abandon of conflicted txes (MarcoFalke)

Pull request description:

  Abandon transactions that are already conflicted is a noop, so don't try and return false/throw instead.

Tree-SHA512: fd2af4149bd2323f7f31fe18685c763790b8589319b4e467b464ab456d5e8971501ab16d124e57a22693666b06ae433ac3e59f0fd6dfbd2be2c6cae8be5bcbd8
2020-06-17 14:29:55 -05:00
Pasta
b40dc8f1ec fix lint-python.sh after 11835 and 12295
Signed-off-by: Pasta <pasta@dashboost.org>
2020-06-17 14:29:55 -05:00
MarcoFalke
89cf527e57 Merge #12295: Enable flake8 warnings for all currently non-violated rules
a9d0ebc262 Enable flake8 warnings for all currently non-violated rules (practicalswift)
4cbab15e75 tests: Fix accidental redefinition of previously defined variable via list comprehension (practicalswift)
0b9207efbe Enable flake8 warning for "list comprehension redefines 'foo' from line N" (F812) (practicalswift)

Pull request description:

  * Enable `flake8` warnings for all currently non-violated rules
  * Fix accidental redefinition via list comprehension

Tree-SHA512: 738b87789e99d02abb2c6b8ff58f65c0cbfeb93e3bf320763e033e510ebd0a4f72861bc8faaf42c14a056a5d4659c33dc70a63730a32cc15159559427bf21193
2020-06-17 14:29:55 -05:00
MarcoFalke
cd1fdb5b01 Partial Merge #11835: Add Travis check for unused Python imports
d60b32074 Add Travis check for unused Python imports (practicalswift)
c7399e708 Remove unused Python imports (practicalswift)

Pull request description:

  Add Travis check for unused Python imports.

  ```
  $ contrib/devtools/lint-python.sh
  ./test/functional/example_test.py:18:1: F401 'test_framework.mininode.NODE_NETWORK' imported but unused
  ./test/functional/test_framework/messages.py:27:1: F401 'test_framework.util.wait_until' imported but unused
  ./test/functional/test_framework/test_framework.py:16:1: F401 'traceback' imported but unused
  ```

Tree-SHA512: 78e50fb1488abe3ebe365e766cb8d6d448cf1bd16c8691e102cb9bf7c202988bdf6e10b25ff772c62e05c72568168462e88cdc7ad98069d9eb3be727735b2d56
2020-06-17 14:29:55 -05:00
MarcoFalke
9e71390d39 Merge #12438: [Tests] Fix trivial typo in test_runner.py causing error
ada1af6d8f Fix typo in test_runner.py causing error (MeshCollider)

Pull request description:

  In the case that a test fails, the typo in run_tests() (introduced in #11858) will cause an error rather than printing out the combined logs, hiding the cause of the failure.

Tree-SHA512: 7d7aa406d92750320ed20610cc5f174cdc94086f630af8c0c4db2003497132e0c56d59b94312fb42ad4507904a2fa858226a4a9337450930bf206183fc35c0a0
2020-06-17 14:29:54 -05:00
Wladimir J. van der Laan
a8da932ff0 Merge #11858: qa: Prepare tests for Windows
faefd29 qa: Prepare functional tests for Windows (MarcoFalke)

Pull request description:

  * Pass `sys.executable` when calling a python script via the subprocess
    module
  * Don't remove the log file while it is still open and written to
  * Properly use os.pathsep and os.path.sep when modifying the PATH
    environment variable
  * util-tests: Use os.path.join for Windows compatibility

  Ref:  #8227

Tree-SHA512: c507a536af104b3bde4366b6634099db826532bd3e7c35d694b5883c550920643b3eab79c76703ca67e1044ed09979e855088f7324321c8d52112514e334d614
2020-06-17 14:29:54 -05:00
John Newbery
219ff3901f Partial Merge #11789 2020-06-17 14:29:54 -05:00
UdjinM6
f12c592b1f
Merge pull request #3416 from PastaPastaPasta/backport-12254
Backport 12254 and 14073 (BIP158)
2020-06-16 12:20:23 +03:00
UdjinM6
593e8b2e2a
Merge pull request #3527 from PastaPastaPasta/backports-0.17-pr4
Backports 0.17 pr4
2020-06-16 12:19:57 +03:00
UdjinM6
085bd7fd0a
Merge pull request #3533 from PastaPastaPasta/backport-13352
Merge #13352: qa: Avoid checking reject code for now
2020-06-16 12:19:20 +03:00
UdjinM6
8a3d82dd3c
Merge pull request #3519 from PastaPastaPasta/backports-0.17-pr3
Backports 0.17 pr3
2020-06-16 11:25:02 +03:00
Wladimir J. van der Laan
329c56721b
Merge #13352: qa: Avoid checking reject code for now
faac7a2db4f9f511c901cb1b4d4e7c599b92884f qa: Avoid checking reject code for now (MarcoFalke)

Pull request description:

  The node will often disconnect before sending a reject code. A more
  robust solution would be to read from the debug log. See  #13006

Tree-SHA512: 1dabf8a43dabbc722f4ffe4fbc1f870090253a66290b2d1a95e7a24e14c6442b493c314480c0314587164eb65e5d468aa9eb5e107ad90bb3ca821a97ea4d373c
2020-06-14 12:22:23 -05:00
Wladimir J. van der Laan
78ef3fdd3a
Merge #12373: Build: Add build support for profiling.
cfaac2a60 Add build support for 'gprof' profiling. (murrayn)

Pull request description:

  Support for profiling build: `./configure --enable-profiling`

Tree-SHA512: ea983cfce385f1893bb4ab7f94ac141b7d620951dc430da3bbc92ae1357fb05521eac689216e66dc87040171a8a57e76dd7ad98036e12a2896cfe5ab544347f0
2020-06-14 11:41:08 -05:00
Wladimir J. van der Laan
a00da7fb0e
Merge #10271: Use std:🧵:hardware_concurrency, instead of Boost, to determine available cores
937bf4335 Use std:🧵:hardware_concurrency, instead of Boost, to determine available cores (fanquake)

Pull request description:

  Following discussion on IRC about replacing Boost usage for detecting available system cores, I've opened this to collect some benchmarks + further discussion.

  The current method for detecting available cores was introduced in #6361.

  Recap of the IRC chat:
  ```
  21:14:08 fanquake: Since we seem to be giving Boost removal a good shot for 0.15, does anyone have suggestions for replacing GetNumCores?
  21:14:26 fanquake: There is std:🧵:hardware_concurrency(), but that seems to count virtual cores, which I don't think we want.
  21:14:51 BlueMatt: fanquake: I doubt we'll do boost removal for 0.15
  21:14:58 BlueMatt: shit like BOOST_FOREACH, sure
  21:15:07 BlueMatt: but all of boost? doubtful, there are still things we need
  21:16:36 fanquake: Yea sorry, not the whole lot, but we can remove a decent chunk. Just looking into what else needs to be done to replace some of the less involved Boost usage.
  21:16:43 BlueMatt: fair
  21:17:14 wumpus: yes, it makes sense to plan ahead a bit, without immediately doing it
  21:18:12 wumpus: right, don't count virtual cores, that used to be the case but it makes no sense for our usage
  21:19:15 wumpus: it'd create a swarm of threads overwhelming any machine with hyperthreading (+accompanying thread stack overhead), for script validation, and there was no gain at all for that
  21:20:03 sipa: BlueMatt: don't worry, there is no hurry
  21:59:10 morcos: wumpus: i don't think that is correct
  21:59:24 morcos: suppose you have 4 cores (8 virtual cores)
  21:59:24 wumpus: fanquake: indeed seems that std has no equivalent to physical_concurrency, on any standard. That's annoying as it is non-trivial to implement
  21:59:35 morcos: i think running par=8 (if it let you) would be notably faster
  21:59:59 morcos: jeremyrubin and i discussed this at length a while back... i think i commented about it on irc at the time
  22:00:21 wumpus: morcos: I think the conclusion at the time was that it made no difference, but sure would make sense to benchmark
  22:00:39 morcos: perhaps historical testing on the virtual vs actual cores was polluted by concurrency issues that have now improved
  22:00:47 wumpus: I think there are not more ALUs, so there is not really a point in having more threads
  22:01:40 wumpus: hyperthreads are basically just a stored register state right?
  22:02:23 sipa: wumpus: yes but it helps the scheduler
  22:02:27 wumpus: in which case the only speedup using "number of cores" threads would give you is, possibly, excluding other software from running on the cores on the same time
  22:02:37 morcos: well this is where i get out of my depth
  22:02:50 sipa: if one of the threads is waiting on a read from ram, the other can use the arithmetic unit for example
  22:02:54 morcos: wumpus: i'm pretty sure though that the speed up is considerably more than what you might expect from that
  22:02:59 wumpus: sipa: ok, I back down, I didn't want to argue this at all
  22:03:35 morcos: the reason i haven't tested it myself, is the machine i usually use has 16 cores... so not easy due to remaining concurrency issues to get much more speedup
  22:03:36 wumpus: I'm fine with restoring it to number of virtual threads if that's faster
  22:03:54 morcos: we should have somene with 4 cores (and  actually test it though, i agree
  22:03:58 sipa: i would expect (but we should benchmark...) that if 8 scriot validation threads instead of 4 on a quadcore hyperthreading is not faster, it's due to lock contention
  22:04:20 morcos: sipa: yeah thats my point, i think lock contention isn't that bad with 8 now
  22:04:22 wumpus: on 64-bit systems the additional thread overhead wouldn't be important at least
  22:04:23 gmaxwell: I previously benchmarked, a long time ago, it was faster.
  22:04:33 gmaxwell: (to use the HT core count)
  22:04:44 wumpus: why was this changed at all then?
  22:04:47 wumpus: I'm confused
  22:05:04 sipa: good question!
  22:05:06 gmaxwell: I had no idea we changed it.
  22:05:25 wumpus: sigh 
  22:05:54 gmaxwell: What PR changed it?
  22:06:51 gmaxwell: In any case, on 32-bit it's probably a good tradeoff... the extra ram overhead is worth avoiding.
  22:07:22 wumpus: https://github.com/bitcoin/bitcoin/pull/6361
  22:07:28 gmaxwell: PR 6461 btw.
  22:07:37 gmaxwell: er lol at least you got it right.
  22:07:45 wumpus: the complaint was that systems became unsuably slow when using that many thread
  22:07:51 wumpus: so at least I got one thing right, woohoo
  22:07:55 sipa: seems i even acked it!
  22:07:57 BlueMatt: wumpus: there are more alus
  22:08:38 BlueMatt: but we need to improve lock contention first
  22:08:40 morcos: anywya, i think in the past the lock contention made 8 threads regardless of cores a bit dicey.. now that is much better (although more still to be done)
  22:09:01 BlueMatt: or we can just merge #10192, thats fee
  22:09:04 gribble: https://github.com/bitcoin/bitcoin/issues/10192 | Cache full script execution results in addition to signatures by TheBlueMatt · Pull Request #10192 · bitcoin/bitcoin · GitHub
  22:09:11 BlueMatt: s/fee/free/
  22:09:21 morcos: no, we do not need to improve lock contention first.   but we should probably do that before we increase the max beyond 16
  22:09:26 BlueMatt: then we can toss concurrency issues out the window and get more speedup anyway
  22:09:35 gmaxwell: wumpus: yea, well in QT I thought we also diminished the count by 1 or something?  but yes, if the motivation was to reduce how heavily the machine was used, thats fair.
  22:09:56 sipa: the benefit of using HT cores is certainly not a factor 2
  22:09:58 wumpus: gmaxwell: for the default I think this makes a lot of sense, yes
  22:10:10 gmaxwell: morcos: right now on my 24/28 physical core hosts going beyond 16 still reduces performance.
  22:10:11 wumpus: gmaxwell: do we also restrict the maximum par using this? that'd make less sense
  22:10:51 wumpus: if someone *wants* to use the virtual cores they should be able to by setting -par=
  22:10:51 sipa: *flies to US*
  22:10:52 BlueMatt: sipa: sure, but the shared cache helps us get more out of it than some others, as morcos points out
  22:11:30 BlueMatt: (because it means our thread contention issues are less)
  22:12:05 morcos: gmaxwell: yeah i've been bogged down in fee estimation as well (and the rest of life) for a while now.. otherwise i would have put more effort into jeremy's checkqueue
  22:12:36 BlueMatt: morcos: heh, well now you can do other stuff while the rest of us get bogged down in understanding fee estimation enough to review it 
  22:12:37 wumpus: [to answer my own question: no, the limit for par is MAX_SCRIPTCHECK_THREADS, or 16]
  22:12:54 morcos: but to me optimizing for more than 16 cores is pretty valuable as miners could use beefy machines and be less concerned by block validation time
  22:14:38 BlueMatt: morcos: i think you may be surprised by the number of mining pools that are on VPSes that do not have 16 cores 
  22:15:34 gmaxwell: I assume right now most of the time block validation is bogged in the parts that are not as concurrent. simple because caching makes the concurrent parts so fast. (and soon to hopefully increase with bluematt's patch)
  22:17:55 gmaxwell: improving sha2 speed, or transaction malloc overhead are probably bigger wins now for connection at the tip than parallelism beyond 16 (though I'd like that too).
  22:18:21 BlueMatt: sha2 speed is big
  22:18:27 morcos: yeah lots of things to do actually...
  22:18:57 gmaxwell: BlueMatt: might be a tiny bit less big if we didn't hash the block header 8 times for every block. 
  22:21:27 BlueMatt: ehh, probably, but I'm less rushed there
  22:21:43 BlueMatt: my new cache thing is about to add a bunch of hashing
  22:21:50 BlueMatt: 1 sha round per tx
  22:22:25 BlueMatt: and sigcache is obviously a ton
  ```

Tree-SHA512: a594430e2a77d8cc741ea8c664a2867b1e1693e5050a4bbc8511e8d66a2bffe241a9965f6dff1e7fbb99f21dd1fdeb95b826365da8bd8f9fab2d0ffd80d5059c
2020-06-14 11:41:08 -05:00
Wladimir J. van der Laan
f52fa3465e
Merge #12616: Set modal overlay hide button as default
cfdd89589 qt: Set modal overlay hide button as default (João Barbosa)

Pull request description:

  Without this change the only way to close the modal overlay is to click the hide button. Setting the button to default allows to activate it with the ENTER key.

  Before:
  <img width="849" alt="screen shot 2018-03-06 at 15 14 23" src="https://user-images.githubusercontent.com/3534524/37040276-58af9ce0-2151-11e8-8c55-50acdea669d9.png">

  After:
  <img width="848" alt="screen shot 2018-03-06 at 15 12 41" src="https://user-images.githubusercontent.com/3534524/37040294-650d1c9c-2151-11e8-8245-2da250a71b3d.png">

Tree-SHA512: a93ef440a507843ed7870fd07a693af93dd97c8fce2fb6824c69a227b5dee258f340bf1ae344da32a9dd6e6cb2330f72db9dac9635bbd34184c3e7f8476a472e
2020-06-14 11:41:07 -05:00
Wladimir J. van der Laan
9b825c8bb7
Merge #12604: Add DynamicMemoryUsage() to CDBWrapper to estimate LevelDB memory use
741f0177c Add DynamicMemoryUsage() to LevelDB (Evan Klitzke)

Pull request description:

  This adds a new method `CDBWrapper::DynamicMemoryUsage()` similar to Bitcoin's existing methods of the same name. It's implemented by asking LevelDB for the information, and then parsing the string response. I've also added logging to `CDBWrapper::WriteBatch()` to track this information:

  ```
  $ tail -f ~/.bitcoin/testnet3/debug.log | grep WriteBatch
  2018-03-05 19:34:55 WriteBatch memory usage: db=chainstate, before=0.0MiB, after=0.0MiB
  2018-03-05 19:35:17 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
  2018-03-05 19:35:17 WriteBatch memory usage: db=chainstate, before=0.0MiB, after=8.0MiB
  2018-03-05 19:35:22 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
  2018-03-05 19:35:22 WriteBatch memory usage: db=chainstate, before=8.0MiB, after=17.0MiB
  2018-03-05 19:35:26 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
  2018-03-05 19:35:27 WriteBatch memory usage: db=chainstate, before=9.0MiB, after=18.0MiB
  2018-03-05 19:35:40 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
  2018-03-05 19:35:41 WriteBatch memory usage: db=chainstate, before=9.0MiB, after=7.0MiB
  2018-03-05 19:35:52 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
  2018-03-05 19:35:52 WriteBatch memory usage: db=chainstate, before=7.0MiB, after=9.0MiB
  ^C
  ```

  As LevelDB doesn't seem to provide a way to get the database name, I've also added a new `m_name` field to the `CDBWrapper`. This is necessary because we have multiple LevelDB databases (two now, and possibly more later, e.g. #11857).

  I am using this information in other branches where I'm experimenting with changing LevelDB buffer sizes.

Tree-SHA512: 7ea8ff5484bb07ef806af17d000c74ccca27d2e0f6c3229e12d93818f00874553335d87428482bd8acbcae81ea35aef2a243326f9fccbfac25989323d24391b4
2020-06-14 11:41:07 -05:00
Wladimir J. van der Laan
1b4d89f319
Merge #12568: Allow dustrelayfee to be set to zero
874e81808 Allow dustrelayfee to be set to zero (Luke Dashjr)

Pull request description:

  I don't see and can't think of any rationale for forbidding this configuration.

Tree-SHA512: df09441f4aec63e79bea94838b7f8e336cebaeb0a22b5e58d27937bbeb1377f229921aeae43674e0b63fc40a39ae51a264d48aa1cdb4cbd0e3339d32856698bf
2020-06-14 11:41:07 -05:00
Wladimir J. van der Laan
4e26e4e72c
Merge #11880: Stop special-casing phashBlock handling in validation for TBV
9c5a4a6ed Stop special-casing phashBlock handling in validation for TBV (Matt Corallo)

Pull request description:

  There is no reason to do this, really, we already have "ignore PoW" flags. Motivated by https://github.com/bitcoin/bitcoin/pull/11739#discussion_r155841721

Tree-SHA512: 37cb1ae5b11c9e8ed7a679bb07ad3b119a2a014744b26d197d67ba21beb19fe6815271df935e40f7c7bd5f2e4d7ae4dad7bd4d00fa230a8d789f37e9de31a769
2020-06-14 11:41:06 -05:00
Wladimir J. van der Laan
9dd01a17a3
Merge #12260: [Trivial] link mentioned scripted-diff-commit (developer-doc)
7eb665fc8 [Trivial] link mentioned scripted-diff-commit (Felix Wolfsteller)

Pull request description:

  Make it easier for people who do not operate on a cloned repository to access the example mentioned.

Tree-SHA512: 1c06e551c68cad03e6bd541bf0e0076cdf0b48ef9b8b4e4a61435367c3435e2e4ccb934112e8dc29d3d70217d8834924704aaf839e25d1133312df86848ca1a1
2020-06-14 11:41:06 -05:00
Wladimir J. van der Laan
28879fc052
Merge #12434: [doc] dev-notes: Members should be initialized
fa9461473 [doc] dev-notes: Members should be initialized (MarcoFalke)

Pull request description:

  Also, remove mention of threads that were removed long ago.

  Motivation:
  Make it easier to spot bugs such as #11654 and  #12426

Tree-SHA512: 8ca1cb54e830e9368803bd98a8b08c39bf2d46f079094ed7e070b32ae15a6e611ce98d7a614f897803309f4728575e6bc9357fab1157c53d2536417eb8271653
2020-06-14 11:41:05 -05:00
Wladimir J. van der Laan
c939de0256
Merge #12570: Add test cases for HexStr (std::reverse_iterator and corner cases)
ac48861 Add tests for HexStr std::reverse_iterator cases (Kosta Zertsekel)
90eac8c Add tests for HexStr corner cases (Kosta Zertsekel)

Pull request description:

Tree-SHA512: 6298d6fdc344e67a9ea6dc74eadb04e68f4f49fc4511d4a8765cafce7eeb8603f96ebedd82c13811326bcaf1ee511946419b651ca411f711baca91bec51947d6
2020-06-14 11:41:05 -05:00
Wladimir J. van der Laan
eaf79feda0
Merge #12421: [qt] navigate to transaction history page after send
e7d9fc5 [qt] navigate to  transaction history page after send (Sjors Provoost)

Pull request description:

  Before this change QT just remained on the Send tab, which I found confusing. Now it switches to the Transactions tab. This makes it more clear to the user that the send actually succeeded, and here they can monitor progress.

  Ideally I would like to highlight the transaction, e.g. by refactoring `TransactionView::focusTransaction(const QModelIndex &idx)` to accept a transaction hash, but I'm not sure how to do that.

Tree-SHA512: 8aa93e03874de8434e18951f8aec47377814c0bcaf7eda4766fc41d5a4e32806346e12e4139e4d45468dfdf0b786f5a7faa393a31b8cd6c65ccac21fb3782c33
2020-06-14 11:41:04 -05:00
Wladimir J. van der Laan
169db16be5
Merge #12083: Improve getchaintxstats test coverage
57e6786 qa: Improve getchaintxstats functional test (João Barbosa)
501b439 rpc: Refactor blockhash parse in getchaintxstats (João Barbosa)

Pull request description:

Tree-SHA512: 61dec5cb68122998df7ec7b5239830f3caf0fe7185c107a66f27653ab2531a800db19a09050671b6fa8dbb5b53181da861eb31199c79d8635f246ccfa0d10efd
2020-06-14 11:41:04 -05:00
Wladimir J. van der Laan
82f04481f1
Merge #11733: qt: Remove redundant locks
d6f3a73 Remove redundant locks (practicalswift)

Pull request description:

  Remove redundant locks:
  * ~~`FindNode(...)` is locking `cs_vNodes` internally~~
  * `SetAddressBook(...)` is locking `cs_wallet` internally
  * `DelAddressBook(...)` is locking `cs_wallet` internally

  **Note to reviewers:** From what I can tell these locks are redundantly held from a data integrity perspective (guarding specific variables), and they do not appear to be needed from a data consistency perspective (ensuring a consistent state at the right points). Review thoroughly and please let me know if I'm mistaken :-)

Tree-SHA512: 7e3ca2d52fecb16385dc65051b5b20d81b502c0025d70b0c489eb3881866bdd57947a9c96931f7b213f5a8a76b6d2c7b084dff0ef2028a1e9ca9ccfd83e5b91e
2020-06-14 11:41:03 -05:00
Wladimir J. van der Laan
7dc8c4e687
Merge #12351: Libraries: Use correct type ; avoid compiler warnings.
a25cb0f Use ptrdiff_t type to more precisely indicate usage and avoid compiler warnings. (murrayn)

Pull request description:

  ptrdiff_t is a more strictly correct type, and gets rid of compiler warnings.

Tree-SHA512: 39718a5cdc10e698f14185f4622a9b439728bce619bd8b3a86f2b99ed5b056cf5a8545a3e5c4bc8a6a01b845fb73510036cee5e6d2629c58df26be692a957fba
2020-06-14 11:41:03 -05:00
UdjinM6
436872aaec
Dashify blockfilter tests
Drop segwit txes and witness test case. Update block hashes (x11 instead of sha256d), block filters and block filter headers.
2020-06-14 11:26:15 -05:00
PastaPastaPasta
658a580e44
fix semicolon
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2020-06-14 16:24:14 +00:00
Wladimir J. van der Laan
5258682efc
Merge #12797: init: Fix help message for checkblockindex
4ae7d15 init: Fix help message for checkblockindex (MarcoFalke)

Pull request description:

  Minor fixup for my commit fa6ab96799.

Tree-SHA512: 18f9255bf1342007be2bdc26d6f688bcd27ba8eebfc709bd9ee31dfd2e4d955d2b699686492ccf59e94eb4b1cc7bf3332376aa151a68cb0b21695b3f67d4a940
2020-06-14 11:23:13 -05:00
Pieter Wuille
a173453de5 Merge #12750: Replace boost::call_once with std::call_once
57dae3fc4a Replace boost::call_once with std::call_once (donaloconnor)

Pull request description:

  This replaces boost::call_once with the C++11 std::call_once. The aim is to remove unnecessary boost code.

  Tested on Windows/MSVC

Tree-SHA512: 5e98ea6e5052fffeaf29f845f4ecf1078b38cbb27671c5b7b6167e7f074a391e10020445107979d9e220d029bc9464fb8b2ccb0bea664eeb7af59a789c988b10
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
4f3a0974fc Merge #12652: bitcoin-cli: Provide a better error message when bitcoind is not running
8b2ef27 tests: Test connecting with non-existing RPC cookie file (practicalswift)
a2b2476 tests: Test connecting to a non-existing server (practicalswift)
de04fde bitcoin-cli: Provide a better error message when bitcoind is not running (practicalswift)

Pull request description:

  Provide a better `bitcoin-cli` error message when `bitcoind` is not running.

  Before this patch:

  ```
  $ killall -9 bitcoind
  $ bitcoin-cli -testnet echo 'hello world'
  error: Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set.  See -rpcpassword and -stdinrpcpass.  Configuration file: (/root/.bitcoin/bitcoin.conf)
  ```

  After this patch:

  ```
  $ killall -9 bitcoind
  $ bitcoin-cli -testnet echo 'hello world'
  error: Could not connect to the server 127.0.0.1:18332

  Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
  ```

Tree-SHA512: bb16e1a9a1ac110ee202c3cb99b5d7c5c1e5487a17e6cd101e12dc69e9525c14dc71f37b128c26ad615369a57547f15d0f1e29b207c1b2f2ee4b4ba7105f3433
Signed-off-by: Pasta <pasta@dashboost.org>
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
6e74920366 Merge #12721: Qt: remove "new" button during receive-mode in addressbook
d843db7 Qt: remove "new" button during receive-mode in addressbook (Jonas Schnelli)

Pull request description:

  There are currently two ways how to generate new receiving addresses in the GUI (which leads to code duplication or required refactoring, see #12520).

  Since the address-book is probably something that should be removed in the long run, suppressing the new-button in receive-mode could be a first step in deprecating the address book.

  With this PR, users can still edit existing receiving address book entries and they can still create new sending address book entries.

Tree-SHA512: abe8d1b44bc3e1b53826ccf9d2b3f764264337758d95ca1fe1ef1bac72d47608cf454055fce3720e06634f0a5841a752ce643b4505b47d6e322b6fc71296e961
2020-06-13 14:50:11 -05:00
Pieter Wuille
4fd7ef0b98 Merge #12693: Remove unused variable in SortForBlock
bb079a0e2c Remove unused variable in SortForBlock (Drew Rasmussen)

Pull request description:

  Although txiter is passed to BlockAssembler::SortForBlock, it is never used. Other than BlockAssembler::addPackageTxs, no other method ever makes a call to SortForBlock, thus making this change harmless.

Tree-SHA512: c7df948c5f75f7371844200e0227a26476437f300148d29020e01041b382f5bda31d9c520c9c5425aee88ce8f4a52cd0e594985d69ed8a081b878cda2e4de8c5
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
a22d90420c Merge #12431: Only call NotifyBlockTip when chainActive changes
f98b54352 Only call NotifyBlockTip when the active chain changes (James O'Beirne)
152b7fb25 [tests] Add a (failing) test for waitforblockheight (James O'Beirne)

Pull request description:

  This is a subset of the more controversial https://github.com/bitcoin/bitcoin/pull/12407, but this also adds a test demonstrating the bug.

  In InvalidateBlock, we're calling NotifyBlockTip with the now-invalid block's prev regardless of what chain the ancestor block is on. This could create numerous issues, but it at least screws up `waitforblockheight` (or anything else relying on `rpc/blockchain.cpp:latestblock`) when InvalidateBlock is called on a block not in chainActive, which can happen via RPC.

  Only call NotifyBlockTip when the block being marked invalid is on the active chain.

Tree-SHA512: 9a54fe5e8c7eb489daf5df4483c0986129e871e2ca931a456ba869ecb5d5a8d4f7bd27ccc9e711e9292c9ed79ddef896c85d0e81fc76883503e327995b0e914f
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
d3a19c5f42 Merge #12080: Add support to search the address book
c316fdf [qt] Add support to search the address book (João Barbosa)

Pull request description:

  This PR adds support to search the address book for both receiving and sending addresses.

  A specialisation of the `QSortFilterProxyModel` is added to implement the custom filtering.

  <img width="757" alt="screen shot 2018-01-03 at 16 05 57" src="https://user-images.githubusercontent.com/3534524/34528196-0347d61e-f0a0-11e7-9bd3-535e9e34ceb8.png">
  <img width="759" alt="screen shot 2018-01-03 at 16 00 58" src="https://user-images.githubusercontent.com/3534524/34528202-07c99f24-f0a0-11e7-8e34-cff6a1ba2364.png">

  Closes #623.

Tree-SHA512: 316e646015c858fc70db6be72dc7922d5bb10a3399e7fa327c992e184cc37a124f11cffefab2dbe0d16bda790c7c0437db364686e66c40b4054b8250b4be15d0
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
2e9bccee4f Merge #12666: configure: UniValue 1.0.4 is required for pushKV(, bool)
8172d3a configure: UniValue 1.0.4 is required for pushKV(, bool) (Luke Dashjr)

Pull request description:

  The breaking changes (#12193) are already merged, so this blocks 0.17.0.

  It depends on jgarzik/univalue#42 or jgarzik/univalue#50 being merged and released in UniValue 1.0.4.

Tree-SHA512: 3a21bbc72d6632bd07ee60ad7780b9ee95908357bcf59b4795b693d8a5d8c88943d6451482f11916ff5417e3bdbb9916062f87d0d73e79f50eb95ddabe21f943
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
337105e00f Merge #12102: Apply hardening measures in bitcoind systemd service file
79ddfad Apply hardening measurements in bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Adds typical systemd hardening measurements for network services.

Tree-SHA512: 63e54d5a2e3e625c123c91e4392474226ec26c48709f2627f4d9d257a59f6960dd53ba4faa10cd355a89cad37fe351e2dbe8db79e681645b59081cf83e940438
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
87798aba80 Merge #11872: [rpc] createrawtransaction: Accept sorted outputs
fac70134a rpc: Update createrawtransaction examples (MarcoFalke)
fa06dfce0 [rpc] createrawtransaction: Accept sorted outputs (MarcoFalke)
8acd25d85 rpc: Allow typeAny in RPCTypeCheck (MarcoFalke)

Pull request description:

  The second parameter of the `createrawtransaction` is a dictionary of the outputs. This comes with at least two drawbacks:

  * In case of duplicate keys, either of them might silently disappear, with no user feedback at all. A user needs to make other mistakes, but this could eventually lead to abnormal tx fees.
  * A dictionary does not guarantee that keys are sorted. Again, a user needs to keep this in mind, as it could eventually lead to excessive tx fees.

  Even though my scenario of loss-of-funds is unlikely to happen, I see it as a inconvenience that should be fixed.

Tree-SHA512: cd562f34f7f9f79c7d3433805971325c388c2035611be283980f4049066a622df4f0afdc11d7ac96662260ec0115147cb65e1ab5268f5a1b063242f3fe425f77
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
a2a37735d9 Merge #12622: net: Correct addrman logging
b4db76c55 net: Correct addrman logging (Wladimir J. van der Laan)

Pull request description:

  These were introduced in #9037.

  Found by @theuni (https://github.com/bitcoin/bitcoin/pull/9037#pullrequestreview-101704656).

Tree-SHA512: 9b5153da8a8e5d4ddf9513a5c453f9609cffd4df2924fd48c7b36c1b1055748c7077d4fc0e70be62ca36af87df7f621a744bb374a234baba271ce4982a240825
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
e3341ba245 Merge #12479: RPC: Add child transactions to getrawmempool verbose output
1dfb4e7d7 [Tests] Check output of parent/child tx list from getrawmempool, getmempooldescendants, getmempoolancestors, and REST interface (Conor Scott)
fc44cb108 [RPC] Add list of child transactions to verbose output of getrawmempool (Conor Scott)

Pull request description:

  `bitcoin-cli getrawmempool true` only lists a transaction's parents in the `depends` field. This change adds a `spentby` field to the json response, which lists the transaction's children in the mempool.

  Currently the only way to find child transactions is to use `getrawmempool` or make another call to `getmempooldescendants` and search the response for transactions that list the parent_txid in the `depends` list, which is inefficient.

  This change allows direct lookup of children.

  Example Output
  ```
    "9a9b5733c0d89f207908cfa3fe17809bee71f629aa095c9f8754524e29e98ba4": {
      ...other geterawmempool data...
      "wtxid": "9a9b5733c0d89f207908cfa3fe17809bee71f629aa095c9f8754524e29e98ba4",
      "depends": [
        "bdd92851d5766a42aeb62af667bb422a116cab4e032bba5e3dd6efe5b4b40aa0"
      ],
      "spentby": [
        "dc5d3ec388a9121421208738a041ac30a22163bc2e17758f2275b6c51a15ba7b"
      ]
    },
  ```

Tree-SHA512: 83da7d421c9799a40ef65af3b7fdb586d6d87385f3f2ede3afd2c311725444b858f9d91cc110422a0fa31905779934fee07211ca6fe6b746792b83692c94b3ce
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
9c1e5bc2b7 Merge #12564: [arith_uint256] Avoid unnecessary this-copy using prefix operator
22b4aae02 [arith_uint256] Avoid unnecessary this-copy using prefix operator (Karl-Johan Alm)

Pull request description:

  I noticed while profiling a related project that `operator-()` actually calls the `base_uint` constructor, which is because the postfix operator version of `operator++` (used in `operator-()`) creates a copy of `this` and returns it.

Tree-SHA512: d9a2665caa3d93f064cdeaf1c6fada101b9943bb53d93ccac6d9a0edac20279d2e921349e30239039c71e0a9629e45c29ec9f10d8d7499e936cdba6cb7c3c3eb
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
8b9012d292 Merge #9037: net: Add test-before-evict discipline to addrman
e68172ed9 Add test-before-evict discipline to addrman (Ethan Heilman)

Pull request description:

  This change implement countermeasures 3 (test-before-evict) suggested in our paper: ["Eclipse Attacks on Bitcoin’s Peer-to-Peer Network"](http://cs-people.bu.edu/heilman/eclipse/).
  # Design:

  A collision occurs when an address, addr1, is being moved to the tried table from the new table, but maps to a position in the tried table which already contains an address (addr2). The current behavior is that addr1 would evict addr2 from the tried table.

  This change ensures that during a collision, addr1 is not inserted into tried but instead inserted into a buffer (setTriedCollisions). The to-be-evicted address, addr2, is then tested by [a feeler connection](https://github.com/bitcoin/bitcoin/pull/8282). If addr2 is found to be online, we remove addr1 from the buffer and addr2 is not evicted, on the other hand if addr2 is found be offline it is replaced by addr1.

  An additional small advantage of this change is that, as no more than ten addresses can be in the test buffer at once, and addresses are only cleared one at a time from the test buffer (at 2 minute intervals), thus an attacker is forced to wait at least two minutes to insert a new address into tried after filling up the test buffer. This rate limits an attacker attempting to launch an eclipse attack.
  # Risk mitigation:
  - To prevent this functionality from being used as a DoS vector, we limit the number of addresses which are to be tested to ten. If we have more than ten addresses to test, we drop new addresses being added to tried if they would evict an address. Since the feeler thread only creates one new connection every 2 minutes the additional network overhead is limited.
  - An address in tried gains immunity from tests for 4 hours after it has been tested or successfully connected to.
  # Tests:

  This change includes additional addrman unittests which test this behavior.

  I ran an instance of this change with a much smaller tried table (2 buckets of 64 addresses) so that collisions were much more likely and observed evictions.

  ```
  2016-10-27 07:20:26 Swapping 208.12.64.252:8333 for 68.62.95.247:8333 in tried table
  2016-10-27 07:20:26 Moving 208.12.64.252:8333 to tried
  ```

  I documented tests we ran against similar earlier versions of this change in #6355.
  # Security Benefit

  This is was originally posted in PR #8282 see [this comment for full details](https://github.com/bitcoin/bitcoin/pull/8282#issuecomment-237255215).

  To determine the security benefit of these larger numbers of IPs in the tried table I modeled the attack presented in [Eclipse Attacks on Bitcoin’s Peer-to-Peer Network](https://eprint.iacr.org/2015/263).

  ![attackergraph40000-10-1000short-line](https://cloud.githubusercontent.com/assets/274814/17366828/372af458-595b-11e6-81e5-2c9f97282305.png)

  **Default node:** 595 attacker IPs for ~50% attack success.
  **Default node + test-before-evict:** 620 attacker IPs for ~50% attack success.
  **Feeler node:** 5540 attacker IPs for ~50% attack success.
  **Feeler node + test-before-evict:** 8600 attacker IPs for ~50% attack success.

  The node running feeler connections has 10 times as many online IP addresses in its tried table making an attack 10 times harder (i.e. requiring the an attacker require 10 times as many IP addresses in different /16s). Adding test-before-evict increases resistance of the node by an additional 3000 attacker IP addresses.

  Below I graph the attack over even greater attacker resources (i.e. more attacker controled IP addresses). Note that test-before-evict maintains some security far longer even against an attacker with 50,000 IPs. If this node had a larger tried table test-before-evict could greatly boost a nodes resistance to eclipse attacks.

  ![attacker graph long view](https://cloud.githubusercontent.com/assets/274814/17367108/96f46d64-595c-11e6-91cd-edba160598e7.png)

Tree-SHA512: fdad4d26aadeaad9bcdc71929b3eb4e1f855b3ee3541fbfbe25dca8d7d0a1667815402db0cb4319db6bd3fcd32d67b5bbc0e12045c4252d62d6239b7d77c4395
2020-06-13 14:50:11 -05:00
Wladimir J. van der Laan
ea90d1b6c1 Merge #14073: blockfilter: Avoid out-of-bounds script access.
f05599557a8305d16bd5965921583af9d012fc27 blockfilter: Omit empty scripts from filter contents. (Jim Posen)

Pull request description:

  Caught during review of #12254 by @TheBlueMatt. https://github.com/bitcoin/bitcoin/pull/12254#discussion_r212830981

Tree-SHA512: cfc9e3eeaba12a14fd3d2e1ccce1a1f89e8cf44cc340ceec05d2d5fa61d27ff64e355603f4ad2184ff73c0ed23dfdab6e2103bddc48f3b76cb13b88d428770ac
2020-06-13 14:08:18 -05:00
Pasta
223dfc94dc Rename BlockFilterType::BASIC to BASIC_FILTER
bls dependency defines a macro BASIC as 1 in relic_conf.h.
This caused blockfilter.h to not compile after macro expansion when it says BASIC = 0.

Maybe there is a fancy C++ way to solve this, but renaming it seemed good to me :)

Signed-off-by: Pasta <pasta@dashboost.org>
2020-06-13 14:08:18 -05:00
Pasta
27a43511f6 test_bitcoin -> test_dash
Signed-off-by: Pasta <pasta@dashboost.org>
2020-06-13 14:08:18 -05:00