Commit graph

25021 commits

Author SHA1 Message Date
Wladimir J. van der Laan 3864219d40
Merge #19360: net: improve encapsulation of CNetAddr
bc74a40a56 net: improve encapsulation of CNetAddr (Vasil Dimov)

Pull request description:

  Do not access `CNetAddr::ip` directly from `CService` methods.

  This improvement will help later when we change the type of
  `CNetAddr::ip` (in the BIP155 implementation).

  (chopped off from https://github.com/bitcoin/bitcoin/pull/19031 to ease review)

ACKs for top commit:
  dongcarl:
    ACK bc74a40a56
  naumenkogs:
    ACK bc74a40
  fjahr:
    Code review ACK bc74a40
  laanwj:
    code review ACK bc74a40a56
  jonatack:
    ACK bc74a40a56
  jnewbery:
    ACK bc74a40a5

Tree-SHA512: 29a203905538e8311e3249b78565abe69ce36dc4ec239bec85c726c30e1a7b55b0aaf5c6659b676935008e068cfa53d716f7a598469064108daf130f94329a5d
2020-07-15 21:17:18 +02:00
Wladimir J. van der Laan 31bdd86631
Merge #19353: Fix mistakenly swapped "previous" and "current" lock orders
0ecff9dd34 Improve "detected inconsistent lock order" error message (Hennadii Stepanov)
bbe9cf4fe4 test: Improve "potential deadlock detected" exception message (Hennadii Stepanov)
35599344c8 Fix mistakenly swapped "previous" and "current" lock orders (Hennadii Stepanov)

Pull request description:

  In master (8ef15e8a86) the "previous" and "current" lock orders are mistakenly swapped.

  This PR:
  - fixes printed lock orders
  - improves the `sync_tests` unit test
  - makes the "detected inconsistent lock order" error message pointing to the lock location rather `tfm::format()` location.

  Debugger output example with this PR (with modified code, of course):
  ```
  2020-06-22T15:46:56Z [msghand] POTENTIAL DEADLOCK DETECTED
  2020-06-22T15:46:56Z [msghand] Previous lock order was:
  2020-06-22T15:46:56Z [msghand]  (2) 'cs_main' in net_processing.cpp:2545 (in thread 'msghand')
  2020-06-22T15:46:56Z [msghand]  (1) 'g_cs_orphans' in net_processing.cpp:1400 (in thread 'msghand')
  2020-06-22T15:46:56Z [msghand] Current lock order is:
  2020-06-22T15:46:56Z [msghand]  (1) 'g_cs_orphans' in net_processing.cpp:2816 (in thread 'msghand')
  2020-06-22T15:46:56Z [msghand]  (2) 'cs_main' in net_processing.cpp:2816 (in thread 'msghand')
  Assertion failed: detected inconsistent lock order for 'cs_main' in net_processing.cpp:2816 (in thread 'msghand'), details in debug log.
  Process 131393 stopped
  * thread #15, name = 'b-msghand', stop reason = signal SIGABRT
      frame #0: 0x00007ffff775c18b libc.so.6`__GI_raise(sig=2) at raise.c:51:1
  (lldb) bt
  * thread #15, name = 'b-msghand', stop reason = signal SIGABRT
    * frame #0: 0x00007ffff775c18b libc.so.6`__GI_raise(sig=2) at raise.c:51:1
      frame #1: 0x00007ffff773b859 libc.so.6`__GI_abort at abort.c:79:7
      frame #2: 0x0000555555e5b196 bitcoind`(anonymous namespace)::potential_deadlock_detected(mismatch=0x00007fff99ff6f30, s1=size=2, s2=size=2, lock_location=0x00007fff99ff7010) at sync.cpp:134:9
      frame #3: 0x0000555555e5a1b1 bitcoind`(anonymous namespace)::push_lock(c=0x0000555556379220, locklocation=0x00007fff99ff7010) at sync.cpp:158:13
      frame #4: 0x0000555555e59e8a bitcoind`EnterCritical(pszName="cs_main", pszFile="net_processing.cpp", nLine=2816, cs=0x0000555556379220, fTry=false) at sync.cpp:177:5
      frame #5: 0x00005555555b0500 bitcoind`UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::Enter(this=0x00007fff99ff8c20, pszName="cs_main", pszFile="net_processing.cpp", nLine=2816) at sync.h:134:9
      frame #6: 0x00005555555b017f bitcoind`UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::UniqueLock(this=0x00007fff99ff8c20, mutexIn=0x0000555556379220, pszName="cs_main", pszFile="net_processing.cpp", nLine=2816, fTry=false) at sync.h:160:13
      frame #7: 0x00005555556aa57e bitcoind`ProcessMessage(pfrom=0x00007fff90001180, msg_type=error: summary string parsing error, vRecv=0x00007fff9c005ac0, nTimeReceived=1592840815980751, chainparams=0x00005555564b7110, chainman=0x0000555556380880, mempool=0x0000555556380ae0, connman=0x000055555657aa20, banman=0x00005555565167b0, interruptMsgProc=0x00005555565cae90) at net_processing.cpp:2816:9
  ```

ACKs for top commit:
  laanwj:
    ACK 0ecff9dd34
  vasild:
    ACK 0ecff9dd

Tree-SHA512: ff285de8dd3198b5b33c4bfbdadf9b1448189c96143b9696bc4f41c07e784c00851ec169cf3ed45cc325f3617ba6783620803234f57fcce28bf6bc3d6a7234fb
2020-07-15 20:53:40 +02:00
MarcoFalke 804ca26629
Merge #19386: rpc: Assert that RPCArg names are equal to CRPCCommand ones (server)
fa7592bfa8 rpc: Update server to use new RPCHelpMan (MarcoFalke)
aaaaad5627 rpc: Add option to hide RPCArg (MarcoFalke)
fa9708f94c rpc: Assert that passed arg names are equal to hardcoded ones (MarcoFalke)
faaeb2b0b3 rpc: Add CRPCCommand constructor which takes RPCHelpMan (MarcoFalke)
fa8ec00061 rpc: Check that left section is not multiline (MarcoFalke)

Pull request description:

  This is split out from #18531 to just touch the RPC methods in server. Description from the main pr:

  ### Motivation

  RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here.

  ### Changes

  The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`.

  ### Future work

  > Here or follow up, makes sense to also assert type of returned UniValue?

  Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including:

  * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant
  * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table
  * Auto-formatting and sanity checking the RPCExamples with RPCMan
  * Checking passed-in json in self-check. Removing redundant checks
  * Checking returned json against documentation to avoid regressions or false documentation
  * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static

  ### Bugs found

  * The assert identified issue #18607
  * The changes itself fixed bug #19250

ACKs for top commit:
  laanwj:
    ACK fa7592bfa8
  ryanofsky:
    Code review ACK fa7592bfa8. Looks great! Just some hidden arg and Check() and comment cleanups since last review

Tree-SHA512: e64b6a212f4a3aeedeee47557559bde104d5fd40cdc1746b27eb2f3d4c8885d5e6e4dd287595ea11cdbc6a939654fe103cae765fd505875444d851f0abb11308
2020-07-15 19:20:21 +02:00
Wladimir J. van der Laan d626a3be31
Merge #19210: qt: Get rid of cursor in out-of-focus labels
bd315eb5e2 qt: Get rid of cursor in out-of-focus labels (Hennadii Stepanov)

Pull request description:

  After clicking on `QLabel` with selectable text the cursor remains forever:

  ![47532924-65e7b200-d8ba-11e8-9254-7bde658961cb](https://user-images.githubusercontent.com/32963518/84038485-ad945200-a9a8-11ea-89e3-c7c17d02a611.png)

  This PR fixes this visual bug.

  Earlier attempts to fix this issue:
  - #14577
  - #14810 (combined with other UX feature)

ACKs for top commit:
  promag:
    Code review ACK bd315eb5e2.
  laanwj:
    Tested ACK bd315eb5e2

Tree-SHA512: 6bf89362412e5ce9a4dec6944b62fe44fc31ca49cda7f6e2eb37e847fac9dccb68bca7ac6877b19e42add2333e40d0b4265757ead105ac0a5d28f8ab43b322c3
2020-07-15 16:48:03 +02:00
Wladimir J. van der Laan 21209c9cce
Merge #19512: p2p: banscore updates to gui, tests, release notes
fa108d6a75 test: update tests for peer discouragement (Jon Atack)
1a9f462caa gui, doc: rm Ban Score in GUI Peers window/release notes updates (Jon Atack)

Pull request description:

  This is the third `-banscore` PR in the mini-series described in #19464. See that PR for the intention and reasoning.

  - no longer display "Ban Score" in the GUI peers window and add a release note, plus release note fixups per https://github.com/bitcoin/bitcoin/pull/19464#pullrequestreview-447452052
  - update tests (`src/test/denialofservice_tests.cpp` and `test/functional/p2p_leak.py`) from banning to discouragement and per https://github.com/bitcoin/bitcoin/pull/19464#issuecomment-658052518

ACKs for top commit:
  jnewbery:
    ACK fa108d6a75
  laanwj:
    ACK fa108d6a75

Tree-SHA512: 58a449b3f47b8cb5490b34e4442ee8675bfad1ce48af4e4fd5c67715b0c1a596fb8e731d42e576b4c3b64627f76e0a68cbb1da9ea9f588a5932fe119baf40d50
2020-07-15 16:32:27 +02:00
Wladimir J. van der Laan 7ebc365047
Merge #19214: Auto-detect SHA256 implementation in benchmarks
addf18da95 Call SHA256AutoDetect in benchmark setup (Pieter Wuille)

Pull request description:

  It seems `SHA256AutoDetect()` was not being called in benchmarks, making the numbers only reflect the naive implementation. Fix this by calling it in bench_bitcoin's setup.

ACKs for top commit:
  fjahr:
    tested ACK addf18da95
  pstratem:
    ACK addf18da95
  laanwj:
    ACK addf18da95

Tree-SHA512: 3ba4b068145942df1429bf5913e3f685511e6ebeae2c1a3f9b8ac0144f6db1c7df456f88f480a2129f3e1602e3bf6a39530bb96e2c74c03ddb19324cec6799c7
2020-07-15 15:35:11 +02:00
Wladimir J. van der Laan 43125596ce
Merge #19296: tests: Add fuzzing harness for AES{CBC,}256{Encrypt,Decrypt}, poly1305_auth, CHKDF_HMAC_SHA256_L32, ChaCha20 and ChaCha20Poly1305AEAD
cca7c577d5 tests: Add fuzzing harness for ChaCha20Poly1305AEAD (practicalswift)
2fc4e5916c tests: Add fuzzing harness for ChaCha20 (practicalswift)
e9e8aac029 tests: Add fuzzing harness for CHKDF_HMAC_SHA256_L32 (practicalswift)
ec86ca1aaa tests: Add fuzzing harness for poly1305_auth(...) (practicalswift)
4cee53bba7 tests: Add fuzzing harness for AES256CBCEncrypt/AES256CBCDecrypt (practicalswift)
9352c32325 tests: Add fuzzing harness for AES256Encrypt/AES256Decrypt (practicalswift)

Pull request description:

  Add fuzzing harness for `AES{CBC,}256{Encrypt,Decrypt}`, `poly1305_auth`, `CHKDF_HMAC_SHA256_L32`, `ChaCha20` and `ChaCha20Poly1305AEAD`.

  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:
  laanwj:
    ACK cca7c577d5

Tree-SHA512: cff9acefe370c12a3663aa55145371df835479c6ab8f6d81bbf84e0f81a9d6b0d94e45ec545f9dd5e1702744eaa7947a1f4ffed0171f446fc080369161afd740
2020-07-15 14:45:12 +02:00
Wladimir J. van der Laan 2c4093ebc3
Merge #18882: build: fix -Wformat-security check when compiling with GCC
6cef3652d1 build: fix -Wformat-security check when compiling with GCC (fanquake)

Pull request description:

  GCC expects `-Wformat` to be passed with [`-Wformat-security`](https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html), which means
  when we test for it in configure it currently fails:

  ```bash
  checking whether C++ compiler accepts -Wformat-security... no
  ...
  configure:15907: checking whether C++ compiler accepts -Wformat-security
  configure:15926: g++ -std=c++11 -c -g -O2 -Werror -Wformat-security  conftest.cpp >&5
  cc1plus: error: '-Wformat-security' ignored without '-Wformat' [-Werror=format-security]
  cc1plus: all warnings being treated as errors
  ```

  and never gets added to our CXX flags. Note that Clang does not have this requirement and the check is working correctly there.

  The change in this PR is the simple fix, however we might want to consider using something like `-Wformat=2` in future, which in GCC is equivalent to `-Wformat -Wformat-nonliteral -Wformat-security -Wformat-y2k.` and similar [in Clang](https://clang.llvm.org/docs/DiagnosticsReference.html#wformat-2).

ACKs for top commit:
  practicalswift:
    ACK 6cef3652d1
  laanwj:
    ACK 6cef3652d1

Tree-SHA512: f9230d42af39f85ea9d2f55dbbebd2bae4740fe59b0da2e092af3ac9ef7e6799d3a4cf83eb64574c63982e5f6b14e226d44c84fa0335255d65c9947d86a1ea38
2020-07-15 13:57:41 +02:00
fanquake 6cef3652d1
build: fix -Wformat-security check when compiling with GCC
Debian GCC ignores -Wformat-security, without -Wformat, which
means when we test for it, it currently fails:

```bash
checking whether C++ compiler accepts -Wformat-security... no
...
configure:15907: checking whether C++ compiler accepts -Wformat-security
configure:15926: g++ -std=c++11 -c -g -O2 -Werror -Wformat-security  conftest.cpp >&5
cc1plus: error: '-Wformat-security' ignored without '-Wformat' [-Werror=format-security]
cc1plus: all warnings being treated as errors
```

Fix this by just combining the -Wformat and -Wformat-security checks
together.
2020-07-15 17:50:01 +08:00
MarcoFalke 3d57015aa2
Merge #19491: util: Make Assert work with any value
fa53635381 util: Make Assert work with any value (MarcoFalke)

Pull request description:

  Goal is to avoid compile failures

ACKs for top commit:
  jonatack:
    ACK fa53635381
  ryanofsky:
    Code review ACK fa53635381. Looks like if argument is an lvalue this effectively does:

Tree-SHA512: a5cf47a8bb2fa1bd8b8895774f33de50ad803165d6f7b520351be1cfcd5612d5d97c51d118461331d30640186c470879e5ad19e3333e09e72685c5e4e4f23079
2020-07-15 09:03:41 +02:00
fanquake 032cfdc9a2
Merge #19517: psbt: Increment input value sum only once per UTXO in decodepsbt
75122780e2 Increment input value sum only once per UTXO in decodepsbt (Andrew Chow)

Pull request description:

  Refactors the UTXO processing of `decodepsbt` to extract the relevant `CTxOut` and handle the input amounts from that. This avoids double counting the input value.

  Fixes #19516

ACKs for top commit:
  sipa:
    utACK 75122780e2
  ryanofsky:
    Code review ACK 75122780e2

Tree-SHA512: 004ec1597790a88a98098f1a26534d10ab0130a438dec0913522a529a8d7f18ad679948617dbcad6e541fbab5bcb2682aeed386b67746807c03b64d76ce5441d
2020-07-15 12:01:06 +08:00
Andrew Chow 75122780e2 Increment input value sum only once per UTXO in decodepsbt 2020-07-14 12:20:06 -04:00
MarcoFalke f4de89edfa
Merge #19429: test: Fix intermittent failure in wallet_encryption
fabd33b541 test: Fix intermittent failure in wallet_encryption (MarcoFalke)

Pull request description:

  Iterating all crypted keys might take time.

  E.g.

  ```
   node0 2020-07-01T14:41:19.227367Z [httpworker.0] ThreadRPCServer method=walletpassphrase user=__cookie__
   node0 2020-07-01T14:41:24.377142Z [httpworker.0] queue run of timer lockwallet() in 100000000 seconds (using HTTP)
  ...
   test  2020-07-01T14:41:24.379000Z TestFramework (ERROR): Assertion failed
                                     Traceback (most recent call last):
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 117, in main
                                         self.run_test()
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_encryption.py", line 88, in run_test
                                         assert_greater_than(expected_time + 5, actual_time) # 5 second buffer
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 54, in assert_greater_than
                                         raise AssertionError("%s <= %s" % (str(thing1), str(thing2)))
                                     AssertionError: 1693614483 <= 1693614484
  ```

  https://cirrus-ci.com/task/5322429885054976?command=ci#L4517

ACKs for top commit:
  achow101:
    ACK fabd33b541

Tree-SHA512: 7a3ccdfc0cdc05fef1f942d3167d100ed63422eb54c05405c884ed91162b7bdb5ce54cb5a981b99a6df2e4af1ea834ccd7d5156531c8c14ea13e735becd6b377
2020-07-14 17:08:19 +02:00
MarcoFalke 834ac4c0f5
Merge #19323: gui: Fix regression in *txoutset* in GUI console
314b49bd50 gui: Fix regression in GUI console (Hennadii Stepanov)

Pull request description:

  The regression was introduced in #19056: if the GUI is running without `-server=1`, the `*txoutset*` call in the console returns "Shutting down".

  Fix #19255.

ACKs for top commit:
  ryanofsky:
    Code review ACK 314b49bd50. Only change since last review is rebase

Tree-SHA512: 8ff85641a5c249858fecb1ab69c7a1b2850af651ff2a94aa41ce352b5b5bc95bc45c41e1767e871b51e647612d09e4d54ede3e20c313488afef5678826c51b62
2020-07-14 16:33:01 +02:00
MarcoFalke a924f61cae
Merge #19325: wallet: Refactor BerkeleyDatabase to introduce DatabaseBatch abstract class
b82f0ca4d5 walletdb: Add MakeBatch function to BerkeleyDatabase and use it (Andrew Chow)
eac9200814 walletdb: Refactor DatabaseBatch abstract class from BerkeleyBatch (Andrew Chow)

Pull request description:

  In order to support alternative database systems, we need to have a generic `Batch` class. This PR adds a `DatabaseBatch` abstract class which is implemented by `BerkeleyBatch`. `DatabaseBatch` is now the class that is used by `WalletBatch` to interact with the database. To be able to get the correct type of `DatabaseBatch`, `BerkeleyDatabase` now has a `MakeBatch` function which returns a newly constructed `std::unique_ptr<DatabaseBatch>`. For `BerkeleyDatabase`, that will be `std::unique_ptr<BerkeleyBatch>`.

  The `Read`, `Write`, `Erase`, and `Exists` template functions are moved from `BerkeleyBatch`.

  Part of #18971

  Requires #19308 and #19324

ACKs for top commit:
  Sjors:
    re-utACK b82f0ca4d5
  MarcoFalke:
    ACK b82f0ca4d5 🌘
  meshcollider:
    LGTM, utACK b82f0ca4d5

Tree-SHA512: 6d2d41631c0983391dbecd702e881c6775b155c90b275df97f7157e42608ed251744f9d7ce5173d02a6c5cc38d90b611880fac7fa635d3d8c4d590681f56ac6a
2020-07-14 16:21:01 +02:00
MarcoFalke b26d62c49a
Merge #18990: log: Properly log txs rejected from mempool
fa9f20b647 log: Properly log txs rejected from mempool (MarcoFalke)

Pull request description:

  Currently `CheckTxInputs` rejections from the mempool are the only rejections that log directly and unconditionally to debug.log instead of leaving it to the caller. This has multiple issues:

  * A rejected RPC transaction will log a redundant failure reason to debug log. All other failures are merely reported to the RPC user.
  * A rejected p2p transaction will log the failure twice. Once with the `MEMPOOLREJ` flag, and once unconditionally.
  * A rejected orphan transaction will log no failure.

  Fix all issues by simply returning the state to the caller, like it is done for all other rejections.

  The patch includes whitespace fixups to highlight relevant parts of the codebase and simplify review.

ACKs for top commit:
  naumenkogs:
    utACK fa9f20b647
  rajarshimaitra:
    Concept ACK. Compiled and ran tests. `fa9f20b`
  jnewbery:
    code review ACK fa9f20b647

Tree-SHA512: 86cc17b2a9239c01c4fc3f254ad48ee1d3883266966b9811030176338b9ac3deaea7ea5babfb8bbf739d7440154e30011fede8f9313175f199d4a062af6494f7
2020-07-14 16:15:07 +02:00
MarcoFalke 1a655e89cb
Merge #19514: [net/net processing] check banman pointer before dereferencing
ca3585a483 [net/net processing] check banman pointer before dereferencing (John Newbery)

Pull request description:

  Although we currently don't do this, it should be possible to create a
  CConnman or PeerLogicValidation without a Banman instance. Therefore
  always check that banman exists before dereferencing the pointer.

  Also add comments to the m_banman members of CConnman and
  PeerLogicValidation to document that these may be nullptr.

ACKs for top commit:
  jonatack:
    ACK ca3585a
  theStack:
    ACK ca3585a483

Tree-SHA512: 726401c8921b9a502029ead34ae797473a1bc359d6e4e58dcbe3e25b70dde40bb100723be467fd3e2bf418892c493911998226de19c9d529d72034e3be26be48
2020-07-14 15:26:40 +02:00
Jon Atack fa108d6a75
test: update tests for peer discouragement 2020-07-14 14:15:01 +02:00
John Newbery ca3585a483 [net/net processing] check banman pointer before dereferencing
Although we currently don't do this, it should be possible to create a
CConnman or PeerLogicValidation without a Banman instance. Therefore
always check that banman exists before dereferencing the pointer.

Also add comments to the m_banman members of CConnman and
PeerLogicValidation to document that these may be nullptr.
2020-07-14 10:24:43 +01:00
Jon Atack 1a9f462caa
gui, doc: rm Ban Score in GUI Peers window/release notes updates 2020-07-14 10:18:17 +02:00
MarcoFalke 07c83ce039
Merge #19268: doc: Add non-thread-safe note to FeeFilterRounder::round()
d842e6ac96 doc: Add non-thread-safe note to FeeFilterRounder::round() (Hennadii Stepanov)

Pull request description:

  The `FastRandomContext` class is documented as not thread-safe.
  This PR adds a relevant note to the `FeeFilterRounder::round()` function declaration.

  Close #19254

ACKs for top commit:
  MarcoFalke:
    self ACK d842e6ac96
  practicalswift:
    ACK d842e6ac96: explicit is better than implicit
  naumenkogs:
    ACK d842e6a

Tree-SHA512: 538508f24b9cb29baece6a64108e2c5fc3960768c6475c4f2baf48a4a7bdb96dcef1a74d21a4822e1f8635e1375362986da4e3a20f5644129046a354c4b0a8a0
2020-07-14 09:30:02 +02:00
MarcoFalke 90b5fc9ce4
Merge #19495: ci: Disable macOS functional tests on forked repos to avoid timeouts
60824b3c3a ci: Fix configure options for macOS builds (Hennadii Stepanov)
687939e3d2 ci: Drop Homebrew caching while using Homebrew addon on Travis (Hennadii Stepanov)
557d3f1cc0 ci: Do not activate Travis ccache caching strategy (Hennadii Stepanov)
2d747428e2 ci: Disable functional tests on forked repos to avoid timeouts for macOS (Hennadii Stepanov)

Pull request description:

  See: https://github.com/bitcoin-core/gui/issues/5#issuecomment-656819184

  Additionally, this PR:
  - updates macOS image to the recent 10.15.5 version
  - drops Homebrew caching as the Travis Homebrew addon have been used since #18438

  My forked repo build: https://travis-ci.org/github/hebasto/bitcoin/jobs/707200431

Top commit has no ACKs.

Tree-SHA512: 398e935f965a04babeb10e7b26d2341562f21a1ef671c2e7cc97c9ec79d5c31643f81ca18561ab7714b5c52e19df2e4bffe4223eadbab984daa9418ffbf8c2a8
2020-07-14 08:40:32 +02:00
MarcoFalke b93c4244b9
Merge #19464: net: remove -banscore configuration option
06059b0c2a net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD (Jon Atack)
1d4024bca8 net: remove -banscore configuration option (Jon Atack)

Pull request description:

  per https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652684340, https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592. Edit: now split into 3 straightforward PRs:
  - net: remove -banscore configuration option (this PR)
  - rpc: deprecate banscore field in getpeerinfo (#19469, *merged*)
  - gui: no longer display banscores (TBA in the gui repo)

ACKs for top commit:
  MarcoFalke:
    review ACK 06059b0c2a 📙
  vasild:
    ACK 06059b0c

Tree-SHA512: 03fad249986e0896697033fbb8ba2cbfaae7d7603b1fb2a38b3d41db697630d238623f4d732b9098c82af249ce5a1767dd432b7ca0fec10544e23d24fbd57c50
2020-07-14 08:13:25 +02:00
fanquake 5550fa5983
Merge #19109: Only allow getdata of recently announced invs
f32c408f3a Make sure unconfirmed parents are requestable (Pieter Wuille)
c4626bcd21 Drop setInventoryTxToSend based filtering (Pieter Wuille)
43f02ccbff Only respond to requests for recently announced transactions (Pieter Wuille)
b24a17f039 Introduce constant for mempool-based relay separate from mapRelay caching (Pieter Wuille)
a9bc563803 Swap relay pool and mempool lookup (Pieter Wuille)

Pull request description:

  This implements the follow-up suggested here: https://github.com/bitcoin/bitcoin/pull/18861#issuecomment-627630111 . Instead of checking `setInventoryTxToSend`, maintain an explicit bloom filter with the 3500 most recently announced invs, and permit fetching any of these as long as they're in the relay pool or the mempool. In addition, permit relay from the mempool after just 2 minutes instead of 15.

  This:

  * Fixes the brief opportunity an attacker has to request unannounced invs just after the connection is established (pointed out by naumenkogs, see https://github.com/bitcoin/bitcoin/pull/18861#issuecomment-627627010).
  * Guarantees that locally resubmitted invs after `filterInventoryKnown` rolls over can still be requested (pointed out by luke-jr, see https://github.com/bitcoin/bitcoin/pull/18861#discussion_r419695831).

  It adds 37 KiB of filter per peer.

  This is also a step towards dropping the relay pool entirely and always relaying from the mempool directly (see #17303), but that is still blocked by dealing properly with NOTFOUNDs (see #18238).

ACKs for top commit:
  jnewbery:
    reACK f32c408f3
  jonatack:
    re-ACK f32c408 per `git range-diff f7c19e8 2da7ee3 f32c408` and redid the following: code review, thought about motivation, DoS and privacy aspects, debug build to check for warnings after updating Clang from 6 to 11 since last review.
  ajtowns:
    re-ACK f32c408f3a

Tree-SHA512: aa05b9fd01bad59581c4ec91836a52d7415dc933fa49d4c4adced79aa25aaad51e11166357e8c8b29fbf6021a7401b98c21b850b5d8e8ad773fdb5d6608e1e85
2020-07-14 08:40:35 +08:00
Wladimir J. van der Laan 89fa736783
Merge #19494: doc: CONTRIBUTING.md improvements
b03697b68e doc: CONTRIBUTING.md improvements (Jon Atack)

Pull request description:

  The motivation here was to add a mention of hygienic commits following a discussion today, e.g. something along the lines of:

  *Make sure each individual commit is hygienic, building successfully on its own without warnings, errors, or regressions, and that all tests pass.*

  While here, made various fixups. They are optional and can be omitted.

ACKs for top commit:
  harding:
    ACK b03697b68e Locally reviewed the word diff.
  MarcoFalke:
    ACK b03697b68e 🚌
  practicalswift:
    ACK b03697b68e
  hebasto:
    ACK b03697b68e, I have reviewed the changes and they look OK, I agree they can be merged.

Tree-SHA512: 6fb56219c311d914ec18fcf5d50fdbe3a51e4743a8cace93e348cb4a10c83b6fce631518f1455a1804d1fc81558b235bef58a8be6ccb1a010f46aa4143b1ebf5
2020-07-14 02:11:09 +02:00
Hennadii Stepanov 60824b3c3a
ci: Fix configure options for macOS builds 2020-07-14 02:37:15 +03:00
Hennadii Stepanov 687939e3d2
ci: Drop Homebrew caching while using Homebrew addon on Travis 2020-07-14 02:35:02 +03:00
Hennadii Stepanov 557d3f1cc0
ci: Do not activate Travis ccache caching strategy
It is sufficient to specify CCACHE_DIR to cache.
Also this change fixes ccache on native macOS build.
2020-07-14 02:35:02 +03:00
Hennadii Stepanov 2d747428e2
ci: Disable functional tests on forked repos to avoid timeouts for macOS 2020-07-14 02:34:37 +03:00
Wladimir J. van der Laan 37485baa27
Merge #19483: doc: Merge release notes
fa740d4cea doc: move-only release notes (MarcoFalke)
faa3d2eee1 doc: Remove release notes of backports (MarcoFalke)

Pull request description:

ACKs for top commit:
  laanwj:
    ACK fa740d4cea

Tree-SHA512: 86e7077f9050c65b0b33829892aafabbb783a603a1a77932893659bc1071fb2d2a0515662e7e9e6fdfffb59f86bfc41ad74ba6e86d618bf71a6616d9f7a18fa2
2020-07-13 18:33:00 +02:00
MarcoFalke d52bfc4916
Merge bitcoin-core/gui#30: Disable the main window toolbar when the modal overlay is shown
d0cc1f6df7 qt: Disable toolbar when overlay is shown (Hennadii Stepanov)
e74cd2083d qt, refactor: Cleanup ModalOverlay slots (Hennadii Stepanov)

Pull request description:

  Keeping the main window toolbar activated while the modal overlay is shown could create the appearance of the non-responsive GUI.

  Fixes #22.

  ---

  On master (ca055885c6):

  ![Screenshot from 2020-07-11 13-07-00](https://user-images.githubusercontent.com/32963518/87221791-7504e100-c377-11ea-9689-ddd4b21b98f9.png)

  With this PR:

  ![Screenshot from 2020-07-11 13-07-39](https://user-images.githubusercontent.com/32963518/87221803-8817b100-c377-11ea-92c8-3602dc4d2451.png)

ACKs for top commit:
  harding:
    Tested ACK d0cc1f6df7.  Tested on Linux/X11 as much as I could given it's a pretty small change; seems like a nice improvement.  I'm not experienced in Qt, but I don't see anything obviously problematic about the code.
  jonatack:
    ACK d0cc1f6 tested on Debian 5.7.6-1 (2020-06-24) x86_64 GNU/Linux
  LarryRuane:
    ACK d0cc1f6df7 tested on Ubuntu 18.04.4 LTS

Tree-SHA512: e371b34231c01e77118deb100e0f280ba1cdef54e317f7f7d6ac322598bda811bd1bfe3035e90d87f8267f4f5d2095d34a8136911159db63694fd1b1b11335a1
2020-07-13 10:34:27 +02:00
MarcoFalke 631284f09a
Merge #19486: Remove unused constants CADDR_TIME_VERSION and GETHEADERS_VERSION
7bb6f9bfdb [protocol] Remove unused GETHEADERS_VERSION (John Newbery)
37a934e6b3 [protocol] Remove unused CADDR_TIME_VERSION (John Newbery)

Pull request description:

  These constants are no longer required and can be removed.

  Additional code comments are added to explain CAddress serialization.

ACKs for top commit:
  MarcoFalke:
    ACK 7bb6f9bfdb already an improvement, but maybe getting rid of INIT_PROTO_VERSION here would be an even stronger improvement (can be done later)
  jonatack:
    ACK 7bb6f9bfdb
  vasild:
    ACK 7bb6f9bf

Tree-SHA512: 5382562c60fd677c86583754eca11aad3719064efe2e5ef4f307d693b583422ca8d385926c2582aaab899f502b151f2eb87a7ac23363b15f4fceaa06296f98e3
2020-07-13 10:32:19 +02:00
Jon Atack b03697b68e
doc: CONTRIBUTING.md improvements 2020-07-12 07:52:28 +02:00
MarcoFalke fa740d4cea
doc: move-only release notes
Review suggestion:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2020-07-12 07:15:50 +02:00
MarcoFalke faa3d2eee1
doc: Remove release notes of backports 2020-07-12 07:15:42 +02:00
Samuel Dobson 4db44acf2d
Merge #18202: refactor: consolidate sendmany and sendtoaddress code
08fc6f6cfc [rpc] refactor: consolidate sendmany and sendtoaddress code (Sjors Provoost)

Pull request description:

  I consolidated code between these two RPC calls, since `sendtoaddress` is essentially `sendmany` with 1 destination.

  Unless I overlooked something, the only behaviour change is that some `sendtoaddress` error codes changed from `-4` to `-6`. The release note mentions this.

  Salvaged from #18201.

ACKs for top commit:
  fjahr:
    Code review ACK 08fc6f6cfc
  jonatack:
    ACK 08fc6f6cfc
  meshcollider:
    Code review & functional test run ACK 08fc6f6cfc

Tree-SHA512: 7b66c52fa0444a4d02fc3f81d9c2a386794d447616026a30111eda35fb46510475eea6506a9ceda00bb4e0230ebb758da5d236b3ac05c954c044fa68a1e3e909
2020-07-12 14:42:35 +12:00
Samuel Dobson 32302e5c88
Merge #19490: wallet: Fix typo in comments; Simplify assert
facd7dd3d1 wallet: Fix typo in comments; Simplify assert (MarcoFalke)

Pull request description:

  Follow up to https://github.com/bitcoin/bitcoin/pull/19046#discussion_r443783101 and https://github.com/bitcoin/bitcoin/pull/19046#discussion_r443793690

ACKs for top commit:
  practicalswift:
    ACK facd7dd3d1
  jonatack:
    ACK facd7dd3d1
  hebasto:
    ACK facd7dd3d1, spelling verified with `test/lint/lint-spelling.sh`: all remaining warnings are false positive.

Tree-SHA512: 2b185d138058840db56726bb6bcc42e5288a954e2a410c49e04806a047fbbdaf0bb2decc70ecf7613c69caa766655705ca44151613e7ea5015b386d1e726d870
2020-07-12 13:34:37 +12:00
Jon Atack 06059b0c2a
net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD
and move it from validation to net processing.
2020-07-11 19:41:24 +02:00
Jon Atack 1d4024bca8
net: remove -banscore configuration option 2020-07-11 19:41:21 +02:00
MarcoFalke 42fe6aad32
Merge #19493: wallet: Fix clang build in Mac
1e58bcc9af wallet: Fix clang build in Mac (Anthony Fieroni)

Pull request description:

  Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>

Top commit has no ACKs.

Tree-SHA512: 19312929af14dab97c37cf4547fbd6589a6de960f1a499c2118bb684240639af4b127cf8dc4d201b41d253cfbb645614a0606d4ecce29f300b10c210d38a961b
2020-07-11 19:12:43 +02:00
Anthony Fieroni 1e58bcc9af wallet: Fix clang build in Mac
Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>
2020-07-11 19:33:57 +03:00
MarcoFalke fa53635381
util: Make Assert work with any value 2020-07-11 15:02:07 +02:00
MarcoFalke facd7dd3d1
wallet: Fix typo in comments; Simplify assert 2020-07-11 14:24:36 +02:00
Samuel Dobson 160800ac10
Merge #19441: walletdb: don't reinitialize desc cache with multiple cache entries
a66a7a1a70 walletdb: don't reinitialize desc cache with multiple cache entries (Andrew Chow)

Pull request description:

  When loading descriptor caches, we would accidentally reinitialize the descriptor cache when seeing that one already exists. This should have only been initializing the cache when one does not exist. However this code itself is unnecessary as the act of looking up the cache to add to it will initialize it if it didn't already exist.

  This issue could be hit by trying to load a wallet that had imported a multisig descriptor. The wallet would fail to load.

  A test has been added to wallet_importdescriptors.py to catch this case. Another test case has also been added to check that loading a wallet with only single key descriptors works.

ACKs for top commit:
  hugohn:
    tACK [a66a7a1](a66a7a1a70)
  jonatack:
    ACK a66a7a1a70
  meshcollider:
    Code review ACK a66a7a1a70

Tree-SHA512: 3df746421a008708eaa3bbbdd12b9ddd3e2ec111d54625a212dca7414b971cc1f6e2b1757b3232c31a2f637d1b1ef43bf3ffa4ac4216646cf1e92db5f79954f1
2020-07-12 00:14:27 +12:00
Samuel Dobson 5f96bce9b7
Merge #18923: wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off
fa73493930 refactor: Use C++11 range-based for loop (MarcoFalke)
fa7b164d62 wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off (MarcoFalke)
faf8401c19 wallet: Pass unused args to StartWallets (MarcoFalke)
fa6c186436 gui tests: Limit life-time of dummy testing setup (MarcoFalke)
fa28a61897 test: Add smoke test to check that wallets are flushed by default (MarcoFalke)

Pull request description:

  User-facing, this is a refactor. Internally, the scheduler does not have to call a mostly empty function every half a second.

ACKs for top commit:
  jnewbery:
    utACK fa73493930
  meshcollider:
    utACK fa73493930
  ryanofsky:
    Code review ACK fa73493930. Just rebased since last review

Tree-SHA512: 99e1fe1b2c22a3f4b19de3e566241d38693f4fd8d5a68ba1838d86740aa6c08e3325c11a072e30fd262a8861af4278bed52eb9374c85179b8f536477f528247c
2020-07-11 23:23:28 +12:00
Samuel Dobson 89899a3448
Merge #19046: Replace CWallet::Set* functions that use memonly with Add/Load variants
3a9aba21a4 Split SetWalletFlags into Add/LoadWalletFlags (Andrew Chow)
d9cd095b59 Split SetActiveScriptPubKeyMan into Add/LoadActiveScriptPubKeyMan (Andrew Chow)
0122fbab4c Split SetHDChain into AddHDChain and LoadHDChain (Andrew Chow)

Pull request description:

  `SetHDChaiin`, `SetActiveScriptPubKeyMan`, and `SetWalletFlags` have a `memonly` argument which is kind of confusing, as noted in https://github.com/bitcoin/bitcoin/pull/17681#discussion_r427633081. This PR replaces those functions with `Add*` and `Load*` variants so that they follow the pattern used elsewhere in the wallet.

  `AddHDChain`, `AddActiveScriptPubKeyMan`, and `AddWalletFlags` both set their respective variables in `CWallet` and writes them to disk. These functions are used by the actions which modify the wallet such as `sethdseed`, `importdescriptors`, and creating a new wallet.

  `LoadHDChain`, `LoadActiveScriptPubKeyMan`, and `LoadWalletFlags` just set the `CWallet` variables. These functions are used by `LoadWallet` when loading the wallet from disk.

ACKs for top commit:
  jnewbery:
    Code review ACK 3a9aba21a4
  ryanofsky:
    Code review ACK 3a9aba21a4. Only changes since last review tweaks making m_wallet_flags updates more safe
  meshcollider:
    utACK 3a9aba21a4

Tree-SHA512: 365aeaafc5ba42879c0eb797ec3beb29ab70e27f917dc880763f743420b3be6ddf797240996beed8a9ad70fb212c2590253c6b44c9dc244529c3939d9538983f
2020-07-11 23:08:54 +12:00
Samuel Dobson 4fc9224ee7
Merge #18850: wallet: Fix ZapSelectTx to sync wallet spends
9c59f9c285 Fix ZapSelectTx to sync wallet spends (Anthony Fieroni)

Pull request description:

  Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>

ACKs for top commit:
  achow101:
    ACK 9c59f9c285
  ryanofsky:
    Code review ACK 9c59f9c285. Only change since last review tweaking the for loop as suggested
  jonatack:
    ACK 9c59f9c285 tested rebased on current master b33136b6ba and the new unit test does indeed fail without the change.
  meshcollider:
    utACK 9c59f9c285

Tree-SHA512: 71672a5ab0c659550c3a40577614ea896412b79566b5672636ab18765e4c71b9d0a990d94dc6b6e623b03a05737022b04026b5699438809c7c54782d0fd0a5d2
2020-07-11 22:20:43 +12:00
Hennadii Stepanov d0cc1f6df7
qt: Disable toolbar when overlay is shown 2020-07-11 12:59:58 +03:00
MarcoFalke a42631775a
Merge #19222: tests: Add fuzzing harness for BanMan
97846d7f5b tests: Add fuzzing harness for BanMan (practicalswift)
deba199f1c tests: Add ConsumeSubNet(...). Move and increase coverage in ConsumeNetAddr(...). (practicalswift)

Pull request description:

  Add fuzzing harness for `BanMan`.

  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 :)

Top commit has no ACKs.

Tree-SHA512: f4126c15bbb77638833367d73f58193c8f05d16bed0b1d6c33b39387d5b610ff34af78cd721adb51778062ce3ac5e79756d1c3895ef54c6c80c61dcf056e94ff
2020-07-11 11:41:12 +02:00
Hennadii Stepanov e74cd2083d
qt, refactor: Cleanup ModalOverlay slots 2020-07-11 12:16:29 +03:00