24538 Commits

Author SHA1 Message Date
João Barbosa
ccf1f6ea24 refactor: Drop ::HasWallets() 2020-06-13 01:09:15 +01:00
MarcoFalke
f154071ec8
Merge #19177: test: Fix and clean p2p_invalid_messages functional tests
af2a145e575f23c64909e6cf1fb323c603bda7ca Refactor resource exhaustion test (Troy Giorshev)
5c4648d17ba18e4194959963994cc6b37053f127 Fix "invalid message size" test (Troy Giorshev)
ff1e7b884447a5ba10553b2d964625f94e255bdc Move size limits to module-global (Troy Giorshev)
57890abf2c7919eddfec36178b1136cd44ffe883 Remove two unneeded tests (Troy Giorshev)

Pull request description:

  This PR touches only the p2p_invalid_messages.py functional test module.  There are two main goals accomplished here.  First, it fixes the "invalid message size" test, which previously made a message that was invalid for multiple reasons.  Second, it refactors the file into a single consistent style.  This file appears to have originally had two authors, with different styles and some test duplication.

  It should now be easier and quicker to understand this module, anticipating the upcoming [BIP324](https://github.com/bitcoin/bitcoin/pull/18242) and [AltNet](https://github.com/bitcoin/bitcoin/issues/18989) changes.

  This should probably go in ahead of #19107, but the two are not strictly related.

ACKs for top commit:
  jnewbery:
    ACK af2a145e575f23c64909e6cf1fb323c603bda7ca
  MarcoFalke:
    re-ACK af2a145e57 🍦

Tree-SHA512: 9b57561e142c5eaefac5665f7355c8651670400b4db1a89525d2dfdd20e872d6873c4f6175c4222b6f5a8e5210cf5d6a52da69b925b673a2e2ac30a15d670d1c
2020-06-12 15:29:53 -04:00
MarcoFalke
5af16a4db7
Merge #16756: test: Connection eviction logic tests
45eff751c6d07007dabc365dc4c0e6c63e3fe5cf Add functional test for P2P eviction logic of inbound peers (Martin Zumsande)

Pull request description:

  This adds a functional test for the eviction logic for inbound peers, which is triggered when the number of maximum connections is exceeded.

  The functional test covers eviction protection for peers that  have sent us blocks or txns recently, or that have faster pings. I couldn't find a way to test the logic of `CConnman::AttemptToEvictConnection` that is based on netgroup (see #14210 for related discussion)

  Fixes #16660 (at least partially).

  [Edit: Earlier, this PR also contained a unit test, which was removed after the discussion]

ACKs for top commit:
  jonatack:
    ACK 45eff751c6d07007dabc365dc4c0e6c63e3fe5cf
  naumenkogs:
    Tested ACK 45eff75
  fjahr:
    re-ACK 45eff751c6d07007dabc365dc4c0e6c63e3fe5cf
  andrewtoth:
    re-ACK 45eff751c6d07007dabc365dc4c0e6c63e3fe5cf

Tree-SHA512: 177208ab6f30dc62da1cc5f51e654f7c9770d8c6b42aca6ae7ecb30e29d3096e04d75739578e7d149a0f29dd92652b4a707e93c0f1be8aa7ed315e6ec3ab07a4
2020-06-12 14:48:20 -04:00
MarcoFalke
19e919217e
Merge #19250: wallet: Make RPC help compile-time static
fadf6bd04f002d05aaff8eba74015e25a41966bc refactor: Remove unused request.fHelp (MarcoFalke)
fad889cbf0b6c46da2e110b73cbea55e4ff7951e wallet: Make RPC help compile-time static (MarcoFalke)

Pull request description:

  Currently calling `help` on a wallet RPC method will either return `help: unknown command: getnewaddress` or the actual help. This runtime dependency of the help is a bug that complicates any tool that relies on documentation. Also, the code that enables the bug is overly complicated and confusing.

  The fix is split into two commits:

  * First, a commit that can be reviewed with the `--color-moved=dimmed-zebra` option and tested with the included test.
  * Second, a commit that removes the complicated and confusing code.

ACKs for top commit:
  achow101:
    re-ACK fadf6bd04f002d05aaff8eba74015e25a41966bc
  promag:
    Tested ACK fadf6bd04f002d05aaff8eba74015e25a41966bc.

Tree-SHA512: 65d4ff400467f57cb8415c30ce30f814dc76c5c157308b7a7409c59ac9db629e65dfba31cd9c389cfe60a008d3d87787ea0a0e0f2671fd65fd190543c915493d
2020-06-12 13:04:37 -04:00
MarcoFalke
b33136b6ba
Merge #19083: test: msg_mempool, fRelay, and other bloomfilter tests
dca73941eb0f0a4c9b68efed3870b536f7dd6cfe scripted-diff: rename node to peer for mininodes (gzhao408)
0474ea25afc65546cbfe5f822c0212bf3e211023 [test] fix race conditions and test in p2p_filter (gzhao408)
4ef80f0827392a1310ca5a29cc1f8f5ca5d16f95 [test] sending invalid msgs to node with bloomfilters=0 causes disconnect (gzhao408)
497a619386008dfaec0db15ecaebcdfaf75f5011 [test] add BIP 37 test for node with fRelay=false (gzhao408)
e8acc6015695c8439fc971a12709468995b96dcf [test] add mempool msg test for node with bloomfilter enabled (gzhao408)

Pull request description:

  This PR adds a few tests that are bloomfilter-related, including behavior for when bloomfilters are turned _off_:
  1. Tests p2p message `msg_mempool`: a node that has `peerbloomfilters` enabled should send its mempool (disabled behavior already tested [here](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_mempool.py)).
  2. Tests that bloomfilter peers with [`fRelay=False`](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki#extensions-to-existing-messages) in the `version` message should not receive any invs until they set the filter. The rest is the same as what’s already tested in `p2p_filter.py`.
  3. Tests that peers get disconnected if they send `filterload` or `filteradd` p2p messages to a node with bloom filters disabled.
  4. Refactor: renames p2p_mempool.py to p2p_nobloomfilter_messages.py.
  5. Fixes race conditions in p2p_filter.py

ACKs for top commit:
  MarcoFalke:
    ACK dca73941eb only changes is restoring accidentally deleted test 🍮
  jonatack:
    ACK dca73941eb0f0a4c9b68efed3870b536f7dd6cfe modulo a few nits if you retouch, happy to re-ACK if you take any of them but don't feel obliged to.

Tree-SHA512: 442aeab0755cb8b830251ea170d1d5e6da8ac9029b3276d407a20ee3d588cc61b77b8842368de18c244056316b8c63b911776d6e106bc7c023439ab915b27ad3
2020-06-11 14:35:08 -04:00
MarcoFalke
8682414d02
Merge #19247: tests: Add fuzzing harness for {Read,Write}{LE,BE}{16,32,64} (crypto/common.h)
cf5b8f64b3fef053035fa11231601b79bfa53aff tests: Add fuzzing harness for {Read,Write}{LE,BE}{16,32,64} (crypto/common.h) (practicalswift)
4a8181b303218683d014e8e79a172ea8ccccc4dd tests: Add std::vector<uint8_t> ConsumeFixedLengthByteVector(FuzzedDataProvider& fuzzed_data_provider, const size_t length) (practicalswift)

Pull request description:

  Add fuzzing harness for `{Read,Write}{LE,BE}{16,32,64}` (`crypto/common.h`).

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  MarcoFalke:
    ACK cf5b8f64b3fef053035fa11231601b79bfa53aff

Tree-SHA512: 26412daa6987add1c721ad0348a5a894d68a646e724f328f2db6d9c9358a533481d8888b89d4b0743e9d1c11aa4e0e5341eb4c0d05a4da77b15ab75489327749
2020-06-11 12:59:09 -04:00
MarcoFalke
85f7db2284
Merge #19239: tests: move generate_wif_key to wallet_util.py
3a83a01694160f2e722e1bc90a328bd569b8e109 [tests] move generate_wif_key to wallet_util.py (John Newbery)
b216b0b71f7853d747af8b712fc250c699f3c320 [tests] sort imports in rpc_createmultisig.py (John Newbery)
e38081846d889fcbbbc6ca4a4d3bca26807fde2f Revert "[TESTS] Move base58 to own module to break circular dependency" (John Newbery)

Pull request description:

  generate_wif_key is a wallet utility function. Move it from the EC key module to the wallet util module.

  This fixes the circular dependency issue in #17977

ACKs for top commit:
  MarcoFalke:
    ACK 3a83a01694160f2e722e1bc90a328bd569b8e109 🍪

Tree-SHA512: 24985dffb75202721ccc0c6c5b52f1fa5d1ce7963bccde24389feb913cab4dad0c265274ca67892c46c8b64e6a065a0f23263a89be4fb9134dfefbdbe5c7238a
2020-06-11 12:49:38 -04:00
MarcoFalke
fadf6bd04f
refactor: Remove unused request.fHelp 2020-06-11 12:39:22 -04:00
MarcoFalke
fad889cbf0
wallet: Make RPC help compile-time static 2020-06-11 12:38:36 -04:00
practicalswift
cf5b8f64b3 tests: Add fuzzing harness for {Read,Write}{LE,BE}{16,32,64} (crypto/common.h) 2020-06-11 14:05:54 +00:00
practicalswift
4a8181b303 tests: Add std::vector<uint8_t> ConsumeFixedLengthByteVector(FuzzedDataProvider& fuzzed_data_provider, const size_t length) 2020-06-11 14:05:54 +00:00
MarcoFalke
7a24cca829
Merge #19100: refactor: Combine GetWalletForJSONRPCRequest and EnsureWalletIsAvailable functions
f42f5e58f5fd063d5feec3eadf4a4040a941d4af refactor: Combine GetWalletForJSONRPCRequest and EnsureWalletIsAvailable functions (Russell Yanofsky)

Pull request description:

  This simplifies control flow and also helps get rid of the ::vpwallets variable in #19101 since EnsureWalletIsAvailable doesn't have access to the request context.

ACKs for top commit:
  MarcoFalke:
    ACK f42f5e58f5fd063d5feec3eadf4a4040a941d4af (reviewed code to check that this is a refactor) 💢
  promag:
    Tested ACK f42f5e58f5fd063d5feec3eadf4a4040a941d4af.

Tree-SHA512: eb10685de3db3c1d10c3a797d8da5c8c731e4a8c9024bbb7245929ba767a77a52783a739b8cb1fa7af6fcd233dcf9c8ebbe414eb8b902e2542601aac18625997
2020-06-11 09:52:16 -04:00
fanquake
45a6811d36
Merge #19206: test: Remove leftover comment in mining_basic
fa98e10d5efcd965ee224ec21c9e79ebb123f055 test: Remove leftover comment in mining_basic (MarcoFalke)
faedb50d89cf113084adfa50c280c295c95571a8 test: pep-8 mining_basic (MarcoFalke)

Pull request description:

  Remove an accidental leftover comment from #19082, which no longer applies and thus might be confusing

ACKs for top commit:
  adamjonas:
    code review ACK fa98e10

Tree-SHA512: c7f7f8f579b3c6e92f45769be0a7af1a421438a3f5524db5278b2269511a9e0e08f44e3836afb26727644035897ee51ff8296d13ce23030549e7403f57b40e40
2020-06-11 16:38:11 +08:00
John Newbery
3a83a01694 [tests] move generate_wif_key to wallet_util.py
generate_wif_key is a wallet utility function. Move
it from the EC key module to the wallet util module.
2020-06-10 12:10:02 -04:00
John Newbery
b216b0b71f [tests] sort imports in rpc_createmultisig.py 2020-06-10 11:56:44 -04:00
John Newbery
e38081846d Revert "[TESTS] Move base58 to own module to break circular dependency"
This reverts commit c75de5da5fdd3a304f9da3d8a2e0370d1723ddd0.
2020-06-10 11:54:25 -04:00
gzhao408
dca73941eb scripted-diff: rename node to peer for mininodes
-BEGIN VERIFY SCRIPT-
sed -i 's/FilterNode/P2PBloomFilter/g' test/functional/p2p_filter.py;
sed -i 's/filter_node/filter_peer/g' test/functional/p2p_filter.py;
-END VERIFY SCRIPT-
2020-06-10 07:28:45 -07:00
gzhao408
0474ea25af [test] fix race conditions and test in p2p_filter
-grab mininode_lock for every access to mininode attributes,
otherwise there are race conditions
2020-06-10 07:28:04 -07:00
gzhao408
4ef80f0827 [test] sending invalid msgs to node with bloomfilters=0 causes disconnect
-A node with bloomfilters disabled should disconnect peers that send
msg_mempool, msg_filterload, or msg_filteradd.
-Renamed the test because it now has a wider scope and msg_mempool's
actual functionality makes more sense for p2p_filter.py.
2020-06-10 07:28:04 -07:00
gzhao408
497a619386 [test] add BIP 37 test for node with fRelay=false
A node with fRelay=False should not receive any invs until filter is set using
filterload; all other behavior should be identical.
2020-06-10 07:28:04 -07:00
gzhao408
e8acc60156 [test] add mempool msg test for node with bloomfilter enabled
-msg_mempool is currently only tested with bloomfilter disabled
(node is disconnected) in p2p_mempool.py
-msg_mempool should get mempool txns in response when bloomfilter
is enabled
-edit test that doesn't test msg_mempool as intended
2020-06-10 07:27:58 -07:00
MarcoFalke
6762a627ec
Merge #19230: [TESTS] Move base58 to own module to break circular dependency
c75de5da5fdd3a304f9da3d8a2e0370d1723ddd0 [TESTS] Move base58 to own module to break circular dependency (Pieter Wuille)

Pull request description:

  I encountered difficulties with the test framework in #17977. This fixes them, and I think the change is generally useful.

ACKs for top commit:
  laanwj:
    Code review ACK c75de5da5fdd3a304f9da3d8a2e0370d1723ddd0
  MarcoFalke:
    ACK c75de5da5fdd3a304f9da3d8a2e0370d1723ddd0 according to --color-moved=dimmed-zebra this is a move-only apart from the imports 👒

Tree-SHA512: 9e0493de3e279074f0c70e92c959b73ae30479ad6f2083a3c6bbf4b0191d65ef94854559a5b7c904f5dadc5e93129ed00f6dc0a8ccce6ba7921cd45f7119f74b
2020-06-10 10:23:58 -04:00
MarcoFalke
371a73e940
Merge #19233: Make SetMiscWarning() accept bilingual_str argument
d49612f98add29066817b7c808b76c2d728948e5 Make SetMiscWarning() accept bilingual_str argument (Hennadii Stepanov)
d1ae7c0355662481a7d181a0a458284936d53eb1 Make GetWarnings() return bilingual_str (Hennadii Stepanov)
38e33aa481cefbe12c50f344bae190c0d95fb489 refactor: Make GetWarnings() bilingual_str aware internally (Hennadii Stepanov)

Pull request description:

  This is one more step for consistent usage of `bilingual_str`.

  No new translation messages are defined.

ACKs for top commit:
  laanwj:
    Code review ACK d49612f98add29066817b7c808b76c2d728948e5
  MarcoFalke:
    ACK d49612f98add29066817b7c808b76c2d728948e5 🌂

Tree-SHA512: 7413cb94a85291209c182845f6873350bb9e9ce940647d416c462a136603832fec8a63d792341bf634f07629767c78bc206d3a318cf10c7e87241c114c2496e9
2020-06-10 09:50:57 -04:00
Wladimir J. van der Laan
bc933aeaf0
Merge #19226: test: Add BerkeleyDatabase tsan suppression
fa7b46cc915d048d8e6bc7c074334e631fceb7ec test: Add BerkeleyDatabase tsan suppression (MarcoFalke)

Pull request description:

  Suppresses/Fixes #19211 for now

ACKs for top commit:
  laanwj:
    ACK fa7b46cc915d048d8e6bc7c074334e631fceb7ec
  practicalswift:
    ACK fa7b46cc915d048d8e6bc7c074334e631fceb7ec -- patch looks correct

Tree-SHA512: 749e606caf0f140c1a190e3273ff81d220daa3eb004ba2b2078e6b3c5b6ac196bd5fc91ef42841412cfd4fe1e2a8694fc2a28268fde8485db90076593fc51dc7
2020-06-10 15:25:13 +02:00
Hennadii Stepanov
d49612f98a
Make SetMiscWarning() accept bilingual_str argument 2020-06-10 15:01:20 +03:00
Hennadii Stepanov
d1ae7c0355
Make GetWarnings() return bilingual_str 2020-06-10 15:01:20 +03:00
Hennadii Stepanov
38e33aa481
refactor: Make GetWarnings() bilingual_str aware internally 2020-06-10 15:01:09 +03:00
fanquake
20e9531379
Merge #19231: gui: add missing translation.h include to fix build
948f1134bc7b8ea1fb3c6b55f0cda16cc8dec126 gui: add missing translation.h include to fix build (fanquake)

Pull request description:

  After #19176, building the gui on Bionic is failing with:

  ```bash
    CXX      qt/qt_libbitcoinqt_a-guiutil.o
  qt/bitcoin.cpp: In function 'int GuiMain(int, char**)':
  qt/bitcoin.cpp:460:35: error: 'Untranslated' was not declared in this scope
             node->initError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error));
  ```

  The merge commit also failed to compile with the same error:
  https://travis-ci.org/github/bitcoin/bitcoin/jobs/696627543

ACKs for top commit:
  hebasto:
    ACK 948f1134bc7b8ea1fb3c6b55f0cda16cc8dec126, tested on Linux Mint 19.3 (x86_64): it fixes compiling error with the `--disable-wallet` configure option.

Tree-SHA512: db0197b110b3a7d05af2ceb29fbe9eeb6521d28f53b6267aa6d07a975886adb5c6485af79506ab6c66ed101e32292feeaff3707cdbc11432e5b97400953d5631
2020-06-10 17:12:40 +08:00
fanquake
948f1134bc
gui: add missing translation.h include to fix build
After #19176, building the gui on Bionic is failing with:

```bash
  CXX      qt/qt_libbitcoinqt_a-guiutil.o
qt/bitcoin.cpp: In function 'int GuiMain(int, char**)':
qt/bitcoin.cpp:460:35: error: 'Untranslated' was not declared in this scope
node->initError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error));
```

The merge commit also failed to compile with the same error:
https://travis-ci.org/github/bitcoin/bitcoin/jobs/696627543
2020-06-10 10:08:16 +08:00
Pieter Wuille
c75de5da5f [TESTS] Move base58 to own module to break circular dependency
This breaks the script->key->address->script dependency cycle.
2020-06-09 17:50:50 -07:00
MarcoFalke
f8364df250
Merge #19176: refactor: Error message bilingual_str consistency
6fe989054f0ad9308e8a25f7123d9e5dd67f1164 refactor: Change Node::initError to take bilingual_str (Wladimir J. van der Laan)
425e7cb8cf6140e03802a96d2be9a8b4aa2e244a refactor: Put`TryParsePermissionFlags` in anonymous namespace (Wladimir J. van der Laan)
77b79fa6ef60d363ca720cef5473f1a2c45099a3 refactor: Error message bilingual_str consistency (Wladimir J. van der Laan)

Pull request description:

  A straightforward and hopefully uncontroversial refactor to improve consistency.

  - Move the decision whether to translate an individual error message to where it is defined. This simplifies call sites: no more `InitError(Untranslated(SomeFunction(...)))`.

  - Make all functions in `util/error.h` consistently return a   `bilingual_str`. We've decided to use this as error message type so let's roll with it.

  This has no functional changes: no messages are changed, no new translation messages are defined.

  Also make a function static that can be static.

ACKs for top commit:
  MarcoFalke:
    ACK 6fe989054f0ad9308e8a25f7123d9e5dd67f1164 🔣
  hebasto:
    ACK 6fe989054f0ad9308e8a25f7123d9e5dd67f1164, tested on Linux Mint 19.3 (x86_64).

Tree-SHA512: 1dd123ef285c4b50bbc429b2f11c9a63aaa669a84955a0a9b8134e9dc141bc38f863f798e8982ac68bbe83170e1067a87d1a87fe7f791928b7914e10bbc2ef8d
2020-06-09 17:18:04 -04:00
MarcoFalke
221873e158
Merge #19227: test: change blacklist to blocklist
6fc641644f7193365cf2b40f5cf20374ec871943 change blacklist to blocklist (TrentZ)

Pull request description:

  Let's use a more appropriate and clear word and discard the usage of the blacklist. Blocklist is clear and shall make everyone happy.

ACKs for top commit:
  amitiuttarwar:
    ACK 6fc641644f7193365cf2b40f5cf20374ec871943
  jonatack:
    ACK 6fc641644f7193365cf2b40f5cf20374ec871943 git grep shows these two lines to be the only uses of the word in the codebase other than for specifying colors for the GUI.
  sipsorcery:
    ACK 6fc641644f7193365cf2b40f5cf20374ec871943 due to easy change.

Tree-SHA512: 12fd55ad5c79f1a227da90c7fa730972aae6b74ab1f9df79ec1e7d0eca05c383ef7d6ef5f353620a01da344db915005339b62ca0884179d0f47fbefb084c9efc
2020-06-09 17:15:52 -04:00
TrentZ
6fc641644f
change blacklist to blocklist
Let's use a more appropriate and clear word and discard the usage of the blacklist. Blocklist is clear. Happy for everyone.
2020-06-09 15:08:41 -05:00
MarcoFalke
fa7b46cc91
test: Add BerkeleyDatabase tsan suppression 2020-06-09 14:15:04 -04:00
Wladimir J. van der Laan
6fe989054f refactor: Change Node::initError to take bilingual_str
Make it consistent with `Chain::initError`.
2020-06-09 15:40:02 +02:00
Wladimir J. van der Laan
425e7cb8cf refactor: PutTryParsePermissionFlags in anonymous namespace
It's only used inside `net_permissions.cpp`.
2020-06-09 15:39:44 +02:00
Wladimir J. van der Laan
77b79fa6ef refactor: Error message bilingual_str consistency
- Move the decision whether to translate an error message to where it is
  defined. This simplifies call sites: no more `InitError(Untranslated(...))`.

- Make all functions in `util/error.h` consistently return a
  `bilingual_str`. We've decided to use this as error message type so
  let's roll with it.

This has no functional changes: no messages are changed, no new
translation messages are defined.
2020-06-09 15:39:44 +02:00
MarcoFalke
9ad6f14175
Merge #19220: refactor: Replace RecursiveMutex with Mutex in warnings.cpp
bacbfb61eee6d3c32de3db4dea3f585c7159b643 refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov)

Pull request description:

  The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex.

  Related to #19180.

ACKs for top commit:
  laanwj:
    Code review ACK bacbfb61eee6d3c32de3db4dea3f585c7159b643
  MarcoFalke:
    ACK bacbfb61eee6d3c32de3db4dea3f585c7159b643 , reviewed with -W  --word-diff-regex=. 🎿

Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
2020-06-09 08:54:57 -04:00
Hennadii Stepanov
bacbfb61ee
refactor: Replace RecursiveMutex with Mutex in warnings.cpp 2020-06-09 09:35:10 +03:00
MarcoFalke
a79bca2f1f
Merge #19069: refactor: replace pointers by references within tx_verify.{h,cpp}
b00266fe0cf05fe6044f471105ce2bfed4349626 refactor: replace pointers by references within tx_verify.{h,cpp} (Sebastian Falbesoner)

Pull request description:

  This PR gets rid of another unnecessary use of raw pointers, similar to PR #19053 (see also issue #19062 where useful commands for finding potential candidates are listed) but in the tx verification module.

  For the functions `CalculateSequenceLocks()` and `SequenceLocks()`, the  `prevHeights` vector parameter type is changed to be passed as a reference. Note that there were no checks for null pointers -- if one would pass `nullptr` to one of the functions, the following line would immediately lead to a crash:
  dcacea096e/src/consensus/tx_verify.cpp (L32)

ACKs for top commit:
  Empact:
    Code Review ACK b00266fe0c

Tree-SHA512: 0eb71591467905434082029128bdca4df94988c372af40dca325654f6c002c72a00c73776cb5e72d6de2b2f218649211a5dbf19300a2e01f1841d6034e0f01e0
2020-06-08 10:36:57 -04:00
MarcoFalke
297466b49f
Merge #18826: Expose txinwitness for coinbase in JSON form from RPC
34645c4dd04f1e9bc199fb722de0bb397ec0e131 Test txinwitness is accessible on coinbase vin (Rod Vagg)
3e4421070af01374cd3daf77b28a2abc223c6f83 Expose txinwitness for coinbase in JSON form (Rod Vagg)

Pull request description:

  ## Rationale

  The CLI can provide you with everything about transactions and blocks that you need to reconstruct the block structure and raw block itself **except** for the witness commitment nonce which is stored in the `scriptWitness` of the coinbase and is not printed. You could manually parse the raw `"hex"` fields for transactions if you really wanted to, but this seems to defeat the point of having a JSONification of the raw block/transaction data.

  Without the nonce you can't:

  1. calculate and validate the witness commitment yourself, you can generate the witness tx merkle root but you don't have the nonce to combine it with
  2. reconstruct the raw block form because you don't have `scriptWitness` stack associated with the coinbase (although you know how big it will be and can guess the common case of `[0x000...000]`)

  I'm building some archiving tooling for block data and being able to do a validated two-way conversion is very helpful.

  ## What

  This PR simply makes the `txinwitness` field not dependent on whether we are working with the coinbase or not. So you get it for the coinbase as well as the rest.

  ## Examples

  Common case of a `[0x000...000]` nonce: 00000000000000000000140a7289f3aada855dfd23b0bb13bb5502b0ca60cdd7

  ```json
        "vin": [
          {
            "coinbase": "0368890904c1fe8d5e2f706f6f6c696e2e636f6d2ffabe6d6d5565843a681160cf7b08b1b74ac90a719e6d6ab28c16d336b924f0dc2fcabdc6010000000000000051bf2ad74af345dbe642154b2658931612a70d195e007add0100ffffffff",
            "txinwitness": [
              "0000000000000000000000000000000000000000000000000000000000000000"
            ],
            "sequence": 4294967295
          }
        ],
  ...
  ```

  Novel nonce value: 000000000000000000008c31945b2012258366cc600a3e9a3ee0598e8f797731

  ```json
        "vin": [
          {
            "coinbase": "031862082cfabe6d6d80c099b5e21f4c186d54eb292e17026932e52b1b807fa1380574c5adc1c843450200000000000000",
            "txinwitness": [
              "5b5032506f6f6c5d5b5032506f6f6c5d5b5032506f6f6c5d5b5032506f6f6c5d"
            ],
            "sequence": 4294967295
          }
        ],
  ...
  ```

  ## Alternatives

  This field could be renamed for the coinbase, `"witnessnonce"` perhaps. It could also be omitted when null/zero (`0x000...000`).

  ## Tests

  This didn't break any tests and I couldn't find an obvious way to include a test for this. If this is desired I'd apreicate some pointers.

ACKs for top commit:
  MarcoFalke:
    ACK 34645c4dd04f1e9bc199fb722de0bb397ec0e131

Tree-SHA512: b192facc1dfd210a5ec3f0d5d1ac6d0cae81eb35be15eaa71f60009a538dd6a79ab396f218434e7e998563f7f0df2c396cc925cb91619f6841c5a67806148c85
2020-06-08 10:18:42 -04:00
MarcoFalke
3e58734e55
Merge #18898: gui: Display warnings as rich text
a9d28afe23a94efdccc53f9f10716f3a0c9337eb qt: Display warnings as rich text (Hennadii Stepanov)

Pull request description:

  On master (6621be53517d69ab855cee4a5978a44d6a133ba3), warnings that contain `<hr />` HTML tag are not displayed correctly:

  ![Screenshot from 2020-05-06 11-30-10](https://user-images.githubusercontent.com/32963518/81177281-0e49fc80-8faf-11ea-8cac-8847aa517e86.png)

  Fixed:

  ![Screenshot from 2020-05-07 07-30-48](https://user-images.githubusercontent.com/32963518/81255618-ca9ad580-9036-11ea-90ad-7f4d89c1880d.png)

ACKs for top commit:
  jonasschnelli:
    utACK a9d28afe23a94efdccc53f9f10716f3a0c9337eb
  promag:
    Code review ACK a9d28afe23a94efdccc53f9f10716f3a0c9337eb.

Tree-SHA512: ba5b3837d5f6ea15c3255a3120c9753fc58ee67a370c388556214048ab993c45be720af7cb8d43bb0f12088956cb78abc77546ed1fc691082880438072fe774b
2020-06-08 09:54:38 -04:00
MarcoFalke
b3ec1fe811
Merge #18890: test: disconnect_nodes should warn if nodes were already disconnected
34e641a564531853342b03db2d9f0bf52b6e439e test: Remove unnecessary disconnect_nodes call in rpc_psbt.py (Danny Lee)
e6e7abd51a9a6027acac7a9964e36357f25e242c test: remove redundant two-way disconnect_nodes calls (Danny Lee)
a9bd1f9adf869a95f70b3a40615a2f8e8e52db1d test: warn if nodes not connected before disconnect_nodes (Danny Lee)

Pull request description:

  There's no harm in calling `disconnect_nodes` for nodes that weren't connected (in this case it's a no-op).  However, detecting this case and logging a warning can help ensure that tests are behaving as expected.

  In addition, since `disconnect_nodes` works bidirectionally, I removed all instances of this pattern:

  ```
  disconnect_nodes(self.nodes[0], 1)
  disconnect_nodes(self.nodes[1], 0)
  ```

ACKs for top commit:
  MarcoFalke:
    review ACK 34e641a564531853342b03db2d9f0bf52b6e439e 👔
  amitiuttarwar:
    ACK 34e641a564. Thanks for this test improvement!

Tree-SHA512: 344855ceb46c012d43c13d7c09f44d32dcb7645706d10ae1e4645d9edca54c6c6c13fee26b79480755cdfcdf39b4b5770b36bb03ce71ba002d5be8a27fe008af
2020-06-08 09:13:22 -04:00
Troy Giorshev
af2a145e57 Refactor resource exhaustion test
This is a simple refactor of the specified test.  It is now brought in
line with the rest of the tests in the module.  This should make things
easier to debug, as all of the tests are now grouped together at the
top.
2020-06-08 08:52:39 -04:00
MarcoFalke
fa98e10d5e
test: Remove leftover comment in mining_basic 2020-06-08 08:10:37 -04:00
MarcoFalke
faedb50d89
test: pep-8 mining_basic
Can be reviewed with the git options
--word-diff-regex=. --ignore-all-space  -U0
2020-06-08 08:10:23 -04:00
fanquake
9573d2b55b
Merge #19194: util: Don't reference errno when pthread fails.
cb38b069b0f41b1a26264784b1c1303c8ac6ab08 util: Don't reference errno when pthread fails. (MIZUTA Takeshi)

Pull request description:

  Pthread library does not set errno.
  Pthread library's errno is returned by return value.

ACKs for top commit:
  practicalswift:
    ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08 -- patch looks correct
  MarcoFalke:
    review ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08
  hebasto:
    ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08, only squashed commits since the [previous](https://github.com/bitcoin/bitcoin/pull/19194#pullrequestreview-425831739) review.

Tree-SHA512: e6c950e30726e5031db97a7b84c8a9215da5ad3e5d233bcc349f812ad15957ddfe378e26d18339b9e0a5dcac2f50b47a687b87a6a6beaf6139df84f31531321e
2020-06-08 19:36:28 +08:00
MarcoFalke
41fb69404c
Merge #19192: doc: Extract net permissions doc
fa2c2b50d895ff3402b82ce3db69bfc43053b519 doc: Extract net permissions doc (MarcoFalke)

Pull request description:

  Moving the documentation of each flag form the already over-large init.cpp into the net permissions module should clean up the code a bit. Moreover, making the documentation available is also required for an (currently imaginary) `setnetpermissions` RPC.

ACKs for top commit:
  Sjors:
    re-utACK fa2c2b50d895ff3402b82ce3db69bfc43053b519

Tree-SHA512: c0a75facc9768913c28d2ffcdfaad8d60f7604d5584ee546adaf77d270563558d361aeaf354e49e349aca7e2e80814b27ffc24247e7b4f045c63cbdc079b449f
2020-06-08 07:27:58 -04:00
MarcoFalke
374fd6fc8b
Merge #19189: refactor: Replace RecursiveMutex with Mutex in timedata.cpp
cc5c0d2299b09c58cd9962ca5075ffa53f2633c0 refactor: Fix formatting of timedata.cpp (Hennadii Stepanov)
c2410ceb844a443caf6dd8c6df976b9e24724d06 refactor: Replace RecursiveMutex with Mutex in timedata.cpp (Hennadii Stepanov)

Pull request description:

  Only `GetTimeOffset()` and `AddTimeData()` functions lock this mutex. They do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_timeoffset_mutex` could be a non-recursive mutex.

  Related to #19180.

ACKs for top commit:
  MarcoFalke:
    ACK cc5c0d2299b09c58cd9962ca5075ffa53f2633c0 , checked the second commit with  --word-diff-regex=. --ignore-all-space -U0 🦉
  vasild:
    ACK cc5c0d22 verified that recursion is not happening

Tree-SHA512: 38f6df689374d4a1a0e9aedb3ed5e885d8285c4da6b75f9bc84ae036936a159ef8276462db33b4f4dd5c71c6312fa9b45380f7a5726959665bc71dc39031be88
2020-06-08 07:04:27 -04:00
MarcoFalke
8496dbeba6
Merge #19190: refactor: Replace RecursiveMutex with Mutex in netbase.cpp
78c8f4fe11706cf5c165777c2ca122bd933b8b6a refactor: Replace RecursiveMutex with Mutex in netbase.cpp (Hennadii Stepanov)

Pull request description:

  The functions that could lock this mutex, i.e., `{S,G}etProxy()`, `{S,G}etNameProxy()`, `HaveNameProxy()`, `IsProxy()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_proxyinfo_mutex` could be a non-recursive mutex.

  Related to #19180.

ACKs for top commit:
  MarcoFalke:
    ACK 78c8f4fe11706cf5c165777c2ca122bd933b8b6a , reviewed with the -W git option 👮
  vasild:
    ACK 78c8f4fe verified that recursion does not happen

Tree-SHA512: fc077fb371f38af5d05f1383c6bebf9926167c257892936fefd2d4fe6f679ca40124d25099e09f645d8ec266df222f96c5d0f9fd39eddcad15cbde0b427bc205
2020-06-08 07:01:14 -04:00