Commit Graph

26661 Commits

Author SHA1 Message Date
Hennadii Stepanov
d3ef947524
build: Check that Homebrew's berkeley-db4 package is actually installed 2020-12-03 23:39:14 +02:00
Wladimir J. van der Laan
a0489f3472
Merge #20221: net: compat.h related cleanup
cadb77a6ab net: Add compat.h header for htonl function (Hennadii Stepanov)
f796f0057b net: Drop unneeded headers when compat.h included (Hennadii Stepanov)
467c346448 net: Drop unneeded Windows headers in compat.h (Hennadii Stepanov)

Pull request description:

  It is the `compat.h` header's job to provide platform-agnostic interfaces for internet operations.
  No need in `#include <arpa/inet.h>` scattered around.

ACKs for top commit:
  practicalswift:
    re-ACK cadb77a6ab: patch looks even better
  laanwj:
    Code review ACK cadb77a6ab

Tree-SHA512: 625ff90b2806310ab856a6ca1ddb6d9a85aa70f342b323e8525a711dd12219a1ecec8373ec1dca5a0653ffb11f9b421753887b25615d991ba3132c1cca6a3c6e
2020-12-03 14:54:07 +01:00
Wladimir J. van der Laan
3fa6a9fc8c
Merge #20506: ci: AppVeyor fixes for Visual Studio 2019 16.8.1 image
2c69381f3d Removed redundant git pull from appveyor config. (Aaron Clauson)
8b99e609e7 Adjusted msvc compiler and linker settings to remove optimisations that are causing sporadic ABI issues on Visual Studio updates. (Aaron Clauson)

Pull request description:

  The motivation for this PR is twofold:

    1. Update the Qt binaries used by the appveyor CI job after a recent update to Visual Studio 2019 used in the Appveyor build image resulted in ABI incompatibilities,

    2. Remove optimisations and debug information from the Bitcoin Core `Release` msvc build to reduce the chance of future ABI incompatibility issues for future Visual Studio updates.

  The changes made in this PR are:

   - Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.
   - Adjusted msvc compiler and linker settings to remove optimisations and debug information generation to help avoid future ABI issues on Visual Studio updates.
   - Tidied up debug and release configuration blocks in common project file to avoid duplication.
   - Updated appveyor config to use latest Visual Studio 2019 image.
   - Bumped vcpkg version to tag `2020.11-1` for binary caching feature*.

  See #20392 for related discussion.

  *Binary caching is a new [vcpkg feature](https://github.com/microsoft/vcpkg/blob/master/docs/specifications/binarycaching.md) that allows dependency caching. This PR is not using the feature but by updating the vcpkg version it means it can be optionally used by other contributors in their own Appveyor configs. By caching the vcpkg dependencies using this feature my build times are reduced by approx 10 minutes.

ACKs for top commit:
  MarcoFalke:
    Concept ACK 2c69381f3d
  laanwj:
    Concept ACK 2c69381f3d

Tree-SHA512: 372f5b8b3b5f7e56b78045f9fc06a22fd9f5366d06e99e2f1eaad6d741680da74d0e6371e7bc580cb41f4d6696b2101b950167309d33e98242578b458ace813a
2020-12-03 14:36:51 +01:00
Wladimir J. van der Laan
a3186b6da6
Merge #20520: depends: Do not force Precompiled Headers (PCH) for building Qt on Linux
c82d15b6d1 depends: Do not force Precompiled Headers (PCH) for building Qt on Linux (Hennadii Stepanov)

Pull request description:

  On CentOS 8 (Cirrus CI job) the forced `-pch` option breaks Qt build.

  Removing `-pch` option does not affect build time for other systems:

  - master (e2ff5e7b35):
  ```
  $ time make -j 9 -C depends/ qt
  ...
  Caching qt...
  make: Leaving directory '/home/hebasto/guix/GitHub/bitcoin/depends'

  real	4m22,359s
  user	18m3,719s
  sys     1m24,769s
  ```

  - this PR:
  ```
  $ time make -j 9 -C depends/ qt
  ...
  Caching qt...
  make: Leaving directory '/home/hebasto/guix/GitHub/bitcoin/depends'

  real	4m14,862s
  user	18m3,355s
  sys 	1m24,506s
  ```

  Qt docs: https://doc.qt.io/qt-5/qmake-precompiledheaders.html

  Fixes #20423

ACKs for top commit:
  MarcoFalke:
    review ACK c82d15b6d1

Tree-SHA512: 0f2a3712e90de881d00f8e56c363edde33dd4f5c117df5744ab4e51d0a8146331de7236bc8329d68ddd91535cd853e68ee80ef4cceb6a909786abfd8881b01e8
2020-12-03 13:23:09 +01:00
Aaron Clauson
2c69381f3d
Removed redundant git pull from appveyor config. 2020-12-03 09:23:22 +00:00
MarcoFalke
681ce59d0e
Merge #20466: test: Fix intermittent p2p_fingerprint issue
fad7be584f test: Fix intermittent p2p_finerprint issue (MarcoFalke)

Pull request description:

  A single sync_with_ping can't be used to drop a block announcement, as the block might be sent *after* the ping has been responded to.

  Fix that by waiting for the block.

ACKs for top commit:
  theStack:
    ACK fad7be584f

Tree-SHA512: d43ba9d07273486858f65a26326cc6637ef743bf7b400e5048ba7eac266fb1893283e6503dd49f179caa1abab2977315fb70ba9fba34be9a817a74259d8e4034
2020-12-03 10:09:30 +01:00
MarcoFalke
a35b948836
Merge #20545: ci: Adjust cirrus ci task names
fa5c4f12f5 ci: Adjust cirrus ci task names (MarcoFalke)

Pull request description:

  The task names are too long for GitHub to display them properly without truncation in the "checks-view". Fix that by using a new naming scheme:

  * Native builds don't mention "x86_64 Linux", as it is redundant, they do mention the OS (bionic or focal) in the name suffix
  * Cross builds mention the target in the prefix and the OS (always bionic) in the suffix
  * the macos native build simply says "macos native"

ACKs for top commit:
  practicalswift:
    ACK fa5c4f12f5: patch&nbsp;&nbsp;looks&nbsp;&nbsp;correct!
  hebasto:
    ACK fa5c4f12f5, I have reviewed the code and it looks OK, I agree it can be merged.
  RiccardoMasutti:
    ACK fa5c4f1

Tree-SHA512: 856deb0577c97c70069ef1d369991addc49522135c0ad9e382218fd79ba3d55a95d6c601288dcef0510764b92fbd30a9d7de32b08dc5be55482deab14049b892
2020-12-02 17:22:00 +01:00
fanquake
0a13d15c14
Merge #20530: lint, refactor: Update cppcheck linter to c++17 and improve explicit usage
1e62350ca2 refactor: Improve use of explicit keyword (Fabian Jahr)
c502a6dbfb lint: Use c++17 std in cppcheck linter (Fabian Jahr)

Pull request description:

  I found the `extended-lint-cppcheck` linter still uses `std=c++11` when reviewing #20471. The only difference in the output after this change is one line is missing:

  ```
  src/script/descriptor.cpp:159:5: warning: Struct 'PubkeyProvider' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
  ```

  After some digging, I am still not sure why this one is ignored with c++17 when 40 other`noExplicitConstructor` warnings were still appearing.

  In the second commit, I fix these warnings, adding `explicit` where appropriate and adding fixes to ignore otherwise.

ACKs for top commit:
  practicalswift:
    cr ACK 1e62350ca2: patch looks correct!
  MarcoFalke:
    review ACK 1e62350ca2

Tree-SHA512: dff7b324429a57160e217cf38d9ddbb6e70c6cb3d3e3e0bd4013d88e07afc2292c3df94d0acf7122e9d486322821682ecf15c8f2724a78667764c05d47f89a12
2020-12-02 20:52:19 +08:00
Hennadii Stepanov
c82d15b6d1
depends: Do not force Precompiled Headers (PCH) for building Qt on Linux
On CentOS 8 the forced '-pch' option breaks Qt build.
Removing '-pch' option does not affect build time for other Linux
systems.
2020-12-02 13:49:25 +02:00
Aaron Clauson
8b99e609e7
Adjusted msvc compiler and linker settings to remove optimisations that are causing sporadic ABI issues on Visual Studio updates.
Tidied up debug and release configuration blocks in common project file to avoid duplication.

Updated appveyor config to use latest Visual Studio 2019 image.

Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.

Bumped vcpkg version to tag '2020.11-1' for binary caching feature.

See #20392 for related discussion.
2020-12-02 11:39:01 +00:00
MarcoFalke
fa5c4f12f5
ci: Adjust cirrus ci task names 2020-12-02 11:52:03 +01:00
MarcoFalke
3693fccc3a
Merge #20543: ci: no-longer exclude feature_block in TSAN job
2b356117e9 ci: no-longer exclude feature_block in TSAN job (fanquake)

Pull request description:

  The TSAN job is now running on Cirrus.
  Increase the allocated memory to the [maximum allowed](https://cirrus-ci.org/guide/linux/#linux-containers).

ACKs for top commit:
  jonasschnelli:
    utACK 2b356117e9 - checked the CI run and confirmed that the feature_block runs: https://cirrus-ci.com/task/6008403543719936?command=ci#L3249
  MarcoFalke:
    review ACK 2b356117e9

Tree-SHA512: b774995600361c74bc3267b566e12add66a4604bdf88f6e3f69669edbb8d7aff6f20fdbf0ef98187be4730ce4e18b1939bbcecd993a5c5c1ff40b237c7921b71
2020-12-02 10:56:14 +01:00
Jonas Schnelli
c33662a0ea
Merge #18948: qt: Call setParent() in the parent's context
8963b2c71f qt: Improve comments in WalletController::getOrCreateWallet() (Hennadii Stepanov)
5fcfee68af qt: Call setParent() in the parent's context (Hennadii Stepanov)
5659e73493 qt: Add ObjectInvoke template function (Hennadii Stepanov)

Pull request description:

  The `setParent(parent)` internally calls `QCoreApplication::sendEvent(parent, QChildEvent)` that implies running in the thread which created the parent object. That is not the case always, and an internal assertion fails in the debug mode.

  Steps to reproduce this issue on master (007e15dcd7) on Linux Mint 20 (x86_64):

  ```
  $ make -C depends DEBUG=1
  $ CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure
  $ make
  $ QT_FATAL_WARNINGS=1 lldb src/qt/bitcoin-qt -- --regtest -debug=qt
  (lldb) target create "src/qt/bitcoin-qt"
  Current executable set to '/home/hebasto/GitHub/bitcoin/src/qt/bitcoin-qt' (x86_64).
  (lldb) settings set -- target.run-args  "--regtest" "-debug=qt"
  (lldb) run
  Process 431562 launched: '/home/hebasto/GitHub/bitcoin/src/qt/bitcoin-qt' (x86_64)
  # load wallet via GUI
  Process 431562 stopped
  * thread #24, name = 'QThread', stop reason = signal SIGABRT
      frame #0: 0x00007ffff794518b libc.so.6`__GI_raise(sig=2) at raise.c:51:1
  (lldb) bt
  * thread #24, name = 'QThread', stop reason = signal SIGABRT
    * frame #0: 0x00007ffff794518b libc.so.6`__GI_raise(sig=2) at raise.c:51:1
      frame #1: 0x00007ffff7924859 libc.so.6`__GI_abort at abort.c:79:7
      frame #2: 0x0000555556508ec4 bitcoin-qt`::qt_message_fatal((null)=<unavailable>, context=<unavailable>, message=<unavailable>) at qlogging.cpp:1690:15
      frame #3: 0x00005555565099cf bitcoin-qt`QMessageLogger::fatal(this=<unavailable>, msg=<unavailable>) const at qlogging.cpp:796:21
      frame #4: 0x000055555650479d bitcoin-qt`qt_assert_x(where=<unavailable>, what=<unavailable>, file=<unavailable>, line=<unavailable>) at qglobal.cpp:3088:46
      frame #5: 0x0000555556685733 bitcoin-qt`QCoreApplicationPrivate::checkReceiverThread(receiver=0x0000555557b27510) at qcoreapplication.cpp:557:5
      frame #6: 0x00005555567ced86 bitcoin-qt`QApplication::notify(this=0x00007fffffffd4a0, receiver=0x0000555557b27510, e=0x00007fff9a7f8ce0) at qapplication.cpp:2956:27
      frame #7: 0x0000555556685d31 bitcoin-qt`QCoreApplication::notifyInternal2(receiver=0x0000555557b27510, event=0x00007fff9a7f8ce0) at qcoreapplication.cpp:1024:24
      frame #8: 0x00005555566c9224 bitcoin-qt`QObjectPrivate::setParent_helper(QObject*) [inlined] QCoreApplication::sendEvent(event=<unavailable>, receiver=<unavailable>) at qcoreapplication.h:233:59
      frame #9: 0x00005555566c9210 bitcoin-qt`QObjectPrivate::setParent_helper(this=0x00007fff85855260, o=0x0000555557b27510) at qobject.cpp:2036
      frame #10: 0x00005555566c9b41 bitcoin-qt`QObject::setParent(this=<unavailable>, parent=<unavailable>) at qobject.cpp:1980:24
      frame #11: 0x0000555555710be8 bitcoin-qt`WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet, std::default_delete<interfaces::Wallet> >) + 2534

  ...
  ```

  Fixes #18835.

ACKs for top commit:
  ryanofsky:
    Code review ACK 8963b2c71f. No changes since last review, just rebase because of conflict on some adjacent lines
  jonasschnelli:
    utACK 8963b2c71f

Tree-SHA512: fef615904168717df3d8a0bd85eccc3eef990cc3e66c9fa280c8ef08ea009a7cb5a2a4f868ed0be3c0fe5bf683e8465850b5958deb896fdadd22d296186c9586
2020-12-02 10:52:09 +01:00
MarcoFalke
7f0f01f0ab
Merge #20507: sync: print proper lock order location when double lock is detected
db058efeb0 sync: use HasReason() in double lock tests (Vasil Dimov)
a21dc469cc sync: const-qualify the argument of double_lock_detected() (Vasil Dimov)
6d3689fcf6 sync: print proper lock order location when double lock is detected (Vasil Dimov)

Pull request description:

  Before:
  ```
  Assertion failed: detected double lock at src/sync.cpp:153, details in debug log.
  ```

  After:
  ```
  Assertion failed: detected double lock for 'm' in src/test/sync_tests.cpp:40 (in thread ''), details in debug log.
  ```

ACKs for top commit:
  jonasschnelli:
    utACK db058efeb0
  ajtowns:
    ACK db058efeb0
  hebasto:
    ACK db058efeb0, tested on Linux Mint 20 (x86_64).

Tree-SHA512: 452ddb9a14e44bb174135b39f2219c76eadbb8a6c0e80d64a25f995780d6dbc7b570d9902616db94dbfabaee197b5828ba3475171a68240ac0958fb203a7acdb
2020-12-02 09:44:55 +01:00
fanquake
2b356117e9
ci: no-longer exclude feature_block in TSAN job
The TSAN job is now running on Cirrus.
Increase the allocated memory to the maximum allowed.
2020-12-02 16:42:07 +08:00
MarcoFalke
283f22cabb
Merge #20461: rpc: Validate -rpcauth arguments
053b4fbad8 doc: Release note regarding -rpcauth validation (João Barbosa)
46001323b1 rpc: Validate -rpcauth arguments (João Barbosa)
d37c813a43 rpc: Refactor to process -rpcauth once (João Barbosa)

Pull request description:

  Invalid `-rpcauth` arguments are currently silently ignored. This make server initialization fail if any `-rpcauth` is invalid.

ACKs for top commit:
  MarcoFalke:
    review ACK 053b4fbad8
  jonatack:
    ACK 053b4fbad8
  ryanofsky:
    Code review ACK 053b4fbad8. Only changes since last review are moving a variable declaration and adding a comment, release notes, and a `const`.

Tree-SHA512: c99923d4a121f0c9f882b07f5402ea53e9b2d9455ad34468a094ffab1d64df26c82e1279734c0d42bc2e113eae7b581fbc3be52f3ed4a2d7450d11793afcf406
2020-12-02 09:37:37 +01:00
MarcoFalke
607c844f37
Merge #20540: test: Fix wallet_multiwallet issue on windows
fada2dfcac test: Fix wallet_multiwallet issue on windows (MarcoFalke)

Pull request description:

  The error message on windows:

  > 2020-11-30T18:10:47.536032Z ListWalletDir: Error scanning C:\Users\user\AppData\Local\Temp\test_runner_₿_🏃_20201130_181042\wallet_multiwallet_0\node0\regtest\wallets\self_walletdat_symlink: boost::filesystem::status: The name of the file cannot be resolved by the system: "C:\Users\user\AppData\Local\Temp\test_runner_₿_🏃_20201130_181042\wallet_multiwallet_0\node0\regtest\wallets\self_walletdat_symlink\wallet.dat"

ACKs for top commit:
  promag:
    Code review ACK fada2dfcac. Although it could ignore (don't log) directories that lead to no permission error.
  fanquake:
    ACK fada2dfcac

Tree-SHA512: b475162cc3cd1574209d916605b229a79c8089714295f5e16569b71f958f0007d54dc76833938492d931387784588b11b73e3ef00f963540af42c079417f8d72
2020-12-02 08:31:02 +01:00
fanquake
80d4231e16
Merge #19980: refactor: Some wallet cleanups
9b74461fa2 refactor: Assert before dereference in CWallet::GetDatabase (João Barbosa)
021feb3187 refactor: Drop redudant CWallet::GetDBHandle (João Barbosa)

Pull request description:

ACKs for top commit:
  achow101:
    Code Review ACK 9b74461fa2
  meshcollider:
    utACK 9b74461fa2
  ryanofsky:
    Code review ACK 9b74461fa2. Changes since last review: rebasing due to conflict, dropping wallet path commit c6a5cd7a64c78b162f545a3467d0fea7dcaadfcc as suggested in discussion, making GetDatabase() const in the earlier commit. Giving more descriptive title like

Tree-SHA512: 68cf3b5e9fe0acb3a5cd081086629989f213f1904cc344e5775767b56759a7d905b1e1c303afbe40f172ff81bf07f3719b59d8f6ec2de3fdd53cd0e2d220fb25
2020-12-02 08:23:00 +08:00
Fabian Jahr
1e62350ca2
refactor: Improve use of explicit keyword 2020-12-01 18:36:39 +01:00
Fabian Jahr
c502a6dbfb
lint: Use c++17 std in cppcheck linter 2020-12-01 18:32:26 +01:00
MarcoFalke
f17e8ba3a1
Merge #20207: Follow-up extra comments on taproot code and tests
2d8099c713 Mention units of MAX_STANDARD_ policy constants (Pieter Wuille)
84e29c7c01 Mention in validation that IsWitnessStandard tests for P2TR (Pieter Wuille)
f867cbcc26 Clean up assets test minimizer LDFLAGS (Pieter Wuille)
ea0e78677b Document additional IsWitnessStandard behavior (Pieter Wuille)
6040de9a46 Add comments on CPubKey::IsValid (Pieter Wuille)
8dbb7de67c Add comments to VerifyTaprootCommitment (Pieter Wuille)
cdf900cbf2 Document need_vin_vout_mismatch argument to make_spender (Pieter Wuille)
18246ed5f0 Fix and improve taproot_construct comments (Pieter Wuille)

Pull request description:

  Addressing some review comments raised here: https://github.com/bitcoin/bitcoin/pull/19953#pullrequestreview-512238027 and https://github.com/bitcoin/bitcoin/pull/19953#pullrequestreview-513499921

ACKs for top commit:
  jonatack:
    ACK 2d8099c per `git range-diff 5009159 4f10965 2d8099c`
  ariard:
    ACK 2d8099c, only changes are comment light improvements on IsValid/IsWitnessStandard.

Tree-SHA512: c4881546c379ea8efc7ef99a43cbf3b9cd3f9dde5fd97a07ee66f2b593c78aef0bd8784853c5c9c737b66c269241a1048bbbdd6c964a3d872efd8ba0ec410b68
2020-12-01 15:11:51 +01:00
MarcoFalke
dfd0b70088
Merge #20425: fuzz: Make CAddrMan fuzzing harness deterministic
17a5f172fa fuzz: Make addrman fuzzing harness deterministic (practicalswift)

Pull request description:

  Make `CAddrMan` fuzzing harness deterministic.

  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:
  Crypt-iQ:
    utACK 17a5f172fa

Tree-SHA512: 725f983745233e9b616782247fa18847e483c074ca4336a5beea8a9009128c3a74b4d50a12662d8ca2177c2e1fc5fc121834df6b459ac0af43c931d77ef7c4d8
2020-12-01 14:04:10 +01:00
MarcoFalke
fada2dfcac
test: Fix wallet_multiwallet issue on windows 2020-12-01 13:38:23 +01:00
Jonas Schnelli
277c225b84
Merge #20478: Don't set BDB flags when configuring without
982e548a9a Don't set BDB flags when configuring without (Jonas Schnelli)

Pull request description:

  Configuring `--without-bdb` on MacOS leads to a compile error (when BerkeleyDB is not installed). `brew --prefix berkeley-db4` always reports the target directory (even if not installed).

  This PR prevents BDB_CFLAGS (et al) from being populated when configuring `--without-bdb`

  ```
  ld: warning: directory not found for option '-L/Users/user/Documents/homebrew/Cellar/berkeley-db@4/4.8.30/lib'
  ld: warning: directory not found for option '-L/Users/user/Documents/homebrew/Cellar/berkeley-db@4/4.8.30/lib'
  ld: library not found for -ldb_cxx-4.8
  ld: library not found for -ldb_cxx-4.8
  ```

ACKs for top commit:
  promag:
    Tested ACK 982e548a9a.
  hebasto:
    ACK 982e548a9a, tested on macOS 11 Big Sur.

Tree-SHA512: f8ca0adca0e18e2de4c0f99d5332cba70d957a9d31a357483b43dcf61c2ed4749d223eabadd45fdbf3ef0781c6b37217770e9aa935b5207eaf7f87c5bdfe9e95
2020-12-01 11:28:27 +01:00
Jonas Schnelli
dcb7518067
Merge #20496: build: Drop unneeded macOS framework dependencies
ec4a46dd9c build: Drop unneeded IOKit framework dependency (Hennadii Stepanov)
65afe4cb69 build: Drop unneeded ApplicationServices framework dependency (Hennadii Stepanov)

Pull request description:

  Bitcoin Core codebase does not contain direct dependencies on the `ApplicationServices` and `IOKit` frameworks.

ACKs for top commit:
  jonasschnelli:
    utACK ec4a46dd9c
  practicalswift:
    cr ACK ec4a46dd9c: patch looks correct!
  promag:
    Tested ACK ec4a46dd9c (not depends build).

Tree-SHA512: 47b5ad87d761992850133a921f07d485565c70ba2909a3289050f406e6dbd39ad49e1aeeb6cad79c6914385a72ddffd273dfadd69259a35545a13cd17d0e5043
2020-12-01 11:27:06 +01:00
Jonas Schnelli
335d27d081
Merge #20512: doc: Add bash as an OpenBSD dependency
1d578c078f doc: Add bash as an OpenBSD dependency (Emil Engler)

Pull request description:

  If we require Python for the test framework, we should also require
  bash. It is required for the linters and other scripts and does not
  comes in a default OpenBSD installation.

ACKs for top commit:
  practicalswift:
    ACK 1d578c078f
  RiccardoMasutti:
    ACK 1d578c0
  theStack:
    ACK 1d578c078f

Tree-SHA512: ef14e801983093a99d4c28d134adbc589ca06e5437bf1ff34f8e593968c59793e9d99e8a48f577f847acdfc5185e193276526894b4c632c59d66d87939977910
2020-12-01 11:23:23 +01:00
Jonas Schnelli
f2a673f15b
Merge bitcoin-core/gui#137: refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date
86b1ab64b1 refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date (Hennadii Stepanov)

Pull request description:

  As all deprecated warning in Qt 5.15.0 were eliminated in #46, Qt 5.15.1 introduced another one that is fixed in this PR.

  Required for https://github.com/bitcoin/bitcoin/pull/20182.

  Details in Qt docs:
  - https://doc.qt.io/qt-5/qdatetime.html#toString-1
  - https://doc.qt.io/qt-5/qdate.html#toString-1

ACKs for top commit:
  jarolrod:
    Tested ACK 86b1ab6 on MacOS 10.15.7 and Arch Linux both with Qt 5.15.1
  jonasschnelli:
    Tested ACK 86b1ab64b1

Tree-SHA512: 1dbba8ee70c895bf58317172a9901cdbe5503b1d6258f51caaae88d88d332d9fbd4697c995192d31e3618ddfd532c5f5881289b3af1184422e5a9263a1224115
2020-12-01 10:59:09 +01:00
MarcoFalke
cd720337fe
Merge #20222: refactor: CTxMempool constructor clean up
f15e780b9e refactor: Clean up CTxMemPool initializer list (Elle Mouton)
e3310692d0 refactor: Make CTxMemPool::m_check_ratio a const and a constructor argument (Elle Mouton)
9d4b4b2c2c refactor: Avoid double to int cast for nCheckFrequency (Elle Mouton)

Pull request description:

  This PR cleans up the CTxMemPool interface by including the ratio used to determine when a mempool sanity check should run in the constructor of CTxMempool instead of using nCheckFrequency which required a cast from a double to a uint32_t. Since nCheckFrequency (now called m_check_ratio) is set in the constructor and only every read from there after, it can be turned into a const and no longer needs to be guarded by the 'cs' lock.

  Since nCheckFrequency/m_check_ratio no longer needs to lock the 'cs' mutux, mutex lock line in the "CTxMempool::check" function can be moved below where the m_check_ratio variable is checked. Since the variable is 0 by default (meaning that "CTxMempool::check" will most likely not run its logic) this saves us from unnecessarily grabbing the lock.

ACKs for top commit:
  jnewbery:
    utACK f15e780b9e
  MarcoFalke:
    ACK f15e780b9e 👘
  glozow:
    utACK f15e780b9e
  theStack:
    Code Review ACK f15e780b9e

Tree-SHA512: d83f3b5311ca128847b621e5e999c7e1bf0f4e6261d4cc090fb13e229a0f7eecd66ad997f654f50a838baf708d1515740aa3bffc244909a001d01fd5ae398b68
2020-12-01 10:02:56 +01:00
MarcoFalke
24e4857b29
Merge #20494: refactor: Move node and wallet code out of src/interfaces
629a9299b2 Move WalletImpl from interfaces/wallet.cpp to wallet/interfaces.cpp (Russell Yanofsky)
2a26771d81 Move ChainImpl from interfaces/chain.cpp to node/interfaces.cpp (Russell Yanofsky)
12bd0fc9d7 Move NodeImpl from interfaces/node.cpp to node/interfaces.cpp (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  ---

  Move `NodeImpl` from `interfaces/node.cpp` to `node/interfaces.cpp`
  Move `ChainImpl` from `interfaces/chain.cpp` to `node/interfaces.cpp`
  Move `WalletImpl` from `interfaces/wallet.cpp` to `wallet/interfaces.cpp`

  No changes to any classes (can review with `git diff --color-moved=dimmed_zebra`)

  Motivation for this change is to move node and wallet code to respective directories where it might fit in better than `src/interfaces/`, but also to remove all unnecessary code from `src/interfaces/` to unblock #19160 review, which has been hung up partially because of code organization. Building on top of this PR, #19160 should now be able to organize interface implementations more understandably in `src/node/` `src/wallet/` `src/ipc/` and `src/init/` directories instead of having so much functionality all in `src/interfaces/`

ACKs for top commit:
  promag:
    Code review ACK 629a9299b2.
  MarcoFalke:
    review ACK 629a9299b2 🔺

Tree-SHA512: 87c2b8fd51519bbd4e5ad3539a79debcf88c3bf021eb28c63f3f555186538b62a0c4cc1a3f07cfb4ff13aea8b0b2fdde505d81f22a5e5fd12a6e375b55a92ab8
2020-12-01 09:39:56 +01:00
MarcoFalke
ffd5e7a856
Merge #20519: Handle rename failure in DumpMempool(...) by using the RenameOver(...) return value. Add [[nodiscard]] to RenameOver(...).
ce9dd45422 Add [[nodiscard]] to RenameOver(...) (practicalswift)
9429a398e2 Handle rename failure in DumpMempool(...) by using RenameOver(...) return value (practicalswift)

Pull request description:

  Handle rename failure in `DumpMempool(...)` by using the `RenameOver(...)` return value.

  Add `[[nodiscard]]` to `RenameOver(...)` to reduce the risk of similar rename issues in the future.

ACKs for top commit:
  vasild:
    ACK ce9dd454
  theStack:
    ACK ce9dd45422 🏷️

Tree-SHA512: 1e63d7f3061e1f6ea2df5750dbc1547a39bd50b6c529812a0c8a0c11d3100c241afdf14094e69b69a38bade7e54a12b2a42888545874398eaf5d02421b57e874
2020-12-01 08:28:34 +01:00
MarcoFalke
81d5af42f4
Merge #20499: Remove obsolete NODISCARD ifdef forest. Use [[nodiscard]] (C++17).
79bff8e48a Remove NODISCARD (practicalswift)
4848e71107 scripted-diff: Use [[nodiscard]] (C++17) instead of NODISCARD (practicalswift)

Pull request description:

  Remove obsolete `NODISCARD` `ifdef` forest. Use `[[nodiscard]]` (C++17).

ACKs for top commit:
  theStack:
    ACK 79bff8e48a
  fanquake:
    ACK 79bff8e48a

Tree-SHA512: 56dbb8e50ed97ecfbce28cdc688a01146108acae49a943e338a8f983f7168914710d36e38632f6a7c200ba6c6ac35b2519e97d6c985e8e7eb23223f13bf985d6
2020-11-30 15:42:36 +01:00
fanquake
817aeca57a
Merge #20491: refactor: Drop noop gcc version checks
830ddf4139 Drop noop gcc version checks (Hennadii Stepanov)

Pull request description:

  Since #20413 the minimum required GCC version is 7.

ACKs for top commit:
  fanquake:
    ACK 830ddf4139

Tree-SHA512: 36264661d6ced1683a0c907efba7c700502acaf8e9fd50d9066bc9c7b877b25165b0684c2d7fe74bd58e500a77d7702bdbdd53691c274f29e4abccd241c10964
2020-11-30 16:10:41 +08:00
fanquake
2e1336dbfe
Merge #20471: build: use C++17 in depends
2f5dfe4a7f depends: build qt in c++17 mode (fanquake)
104e859c97 builds: don't pass -silent to qt when building in debug mode (fanquake)
e2c500636c depends: build zeromq with -std=c++17 (fanquake)
2374f2fbef depends: build Boost with -std=c++17 (fanquake)
2dde55702d depends: build bdb with -std=c++17 (fanquake)

Pull request description:

  In packages where we are passing `-std=c++11` switch to `-std=c++17`, or, `-std=c++1z` in the case of Qt.

  This PR also contains a [commit](104e859c97) that improves debug output when building Qt for debugging (`DEBUG=1`).

  Now we'll get output like this:
  ```bash
  g++ -c -pipe -ffunction-sections -O2 -fPIC -std=c++11 -fno-exceptions <lots more> ../../corelib/kernel/qcoreapplication.cpp
  ```
  rather than just:
  ```bash
  compiling ../../corelib/kernel/qcoreapplication.cpp
  ```

  Note that when you look at the DEBUG output for these changes when building Qt, you'll see objects being compiled with a mix of C++11 and C++17. The breakdown is roughly:

  1. `qmake` built with `-std=c++11`:
  ```bash
  Creating qmake...
  make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/qmake'
  g++ -c -o project.o   -std=c++11 -ffunction-sections -O2 -g <trim> <trim>/qt/5.9.8-4110fa99945/qtbase/qmake/project.cpp

  # when qmake, Qt also builds some of it's corelib, such as corelib/global/qmalloc.cpp
  g++ -c -o qmalloc.o   -std=c++11 -ffunction-sections -O2 -g <trim> <trim>/qt/5.9.8-4110fa99945/qtbase/src/corelib/global/qmalloc.cpp
  ```

  2. `qmake` is run, and passed our build options, including `-c++std`:
  ```bash
  make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase'
  <trim>qt/5.9.8-4110fa99945/qtbase/bin/qmake -o Makefile qtbase.pro -- -bindir <trim>/native/bin -c++std c++1z -confirm-license <trim>
  ```

  3. After some cleaning and configuring, we actually start to build Qt, as well as it's tools and internal libs:
  ```bash
  Building qt...
  make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/src'

  # build libpng, zlib etc
  gcc -c -m64 -pipe -pipe -O1 <trim> -o .obj/png.o png.c

  # build libQt5Bootstrap, using C++11, which again compiles qmalloc.cpp
  make[2]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/src/tools/bootstrap'
  g++ -c -pipe -ffunction-sections -O2 -fPIC -std=c++11 <trim> -o .obj/qmalloc.o ../../corelib/global/qmalloc.cpp

  # build a bunch of tools like moc, rcc, uic, qfloat16-tables, qdbuscpp2xml, using C++11
  g++ -c -pipe -O2 -std=c++11 -fno-exceptions -Wall -W <trim> -o .obj/rcc.o rcc.cpp

  # from here, Qt is compiled with -std=c++1z, including qmalloc.cpp, for the third and final time:
  g++ -c -include .pch/Qt5Core <trim> -g -Og -fPIC -std=c++1z -fvisibility=hidden <trim> -o .obj/qmalloc.o global/qmalloc.cpp
  ```

  4.  Finally, build tools like `lrelease`, `lupdate`, etc, but back to using -std=c++11
  ```bash
  make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qttools/src/linguist/lrelease'
  g++ -c -pipe -O2 -std=c++11 -fno-exceptions -Wall -W <trim> -o .obj/translator.o ../shared/translator.cpp
  ```

  If you dump the debug info from the built Qt libs, they should also tell you that they were compiled with `C++17`:
  ```bash
  objdump -g bitcoin/depends/x86_64-pc-linux-gnu/lib/libQt5Core.a
  GNU C++17 9.3.0 -m64 -mtune=generic -march=x86-64 -g -O1 -Og -std=c++17 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -fasynchronous-unwind-tables -fstack-protector-strong -fstack-clash-protection -fcf-protection
  ```

ACKs for top commit:
  laanwj:
    Code review ACK 2f5dfe4a7f
  practicalswift:
    cr ACK 2f5dfe4a7f: patch looks correct
  fjahr:
    Code review ACK 2f5dfe4a7f
  hebasto:
    ACK 2f5dfe4a7f, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: fc5e9d7c7518c68349c8228fb1aead829850373efc960c9b8c079096a83d1dad19c62a9730fce5802322bf07e320960fd47851420d429eda0a87c307f4e8b03a
2020-11-30 12:23:23 +08:00
MarcoFalke
7ae86b3c68
Merge #20522: [test] Fix sync issue in disconnect_p2ps
3ebde2143a [test] Fix wait condition in disconnect_p2ps (Amiti Uttarwar)

Pull request description:

  #19315 currently has a [test failure](https://cirrus-ci.com/task/4545582645641216) because of a race. `disconnect_p2ps` is intended to have a `wait_until` clause that prevents this race, but the conditional doesn't match since its comparing two different object types. `MY_SUBVERSION` is defined in messages.py as a byte string, but is compared to the value returned by the RPC. This PR simply converts types to ensure they match, which should prevent the race from occurring.

  HUGE PROPS TO jnewbery for discovering the issue 🔎

ACKs for top commit:
  jnewbery:
    ACK 3ebde2143a
  glozow:
    Code review ACK 3ebde2143a

Tree-SHA512: ca096b80a3e4d757a645f38846d6dc89d6a3d35c3435513a72d278e305faddd4aff9e75a767941b51b2abbf59c82679bac1e9a0140d6f285efe3053e51bcc2a8
2020-11-28 18:06:43 +01:00
MarcoFalke
854b36cfa2
Merge bitcoin-core/gui#138: unlock encrypted wallet "OK" button bugfix
8008ef770f qt: unlock wallet "OK" button bugfix (Michael Dietz)

Pull request description:

  When trying to send a transaction from an encrypted wallet, the ask
  passphrase dialog would not allow the user to click the "OK" button
  and proceed. Therefore it was impossible to send a transaction
  through the gui. It was not enabling the "OK" button after the
  passphrase was entered by the user, because it was using the same
  form validation logic as the "Change passphrase" flow.

  I reported this in a comment in https://github.com/bitcoin-core/gui/issues/136. But then I realized this seems to be a flat out bug.

ACKs for top commit:
  MarcoFalke:
    review ACK 8008ef770f
  hebasto:
    ACK 8008ef770f, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: cc09b34c7f3aea09729e1c7ccccff05dc11fec56fee2ad369f2d862979572b1edd8b7e738ffe6e91d35d071b819b0c3e0f5d48bf5e27427a80af4a28893f8aaf
2020-11-28 10:39:32 +01:00
MarcoFalke
1ae5758981
Merge #20448: RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint wallet
89bdad5b25 RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint (Luke Dashjr)

Pull request description:

  Allow specifying the `wallet_name` param to `unloadwallet` on RPC wallet endpoints, so long as it matches the endpoint wallet.

ACKs for top commit:
  jonatack:
    ACK 89bdad5b25
  MarcoFalke:
    review ACK 89bdad5b25

Tree-SHA512: efb399c33f7b5596870a26a8680f453ca47aa7a6db4e550f9435d13044f1c4bad0ae11e8f0205213409d08b75c4188c3be782e54aafab1f65b97eb8cf5c252a9
2020-11-28 10:14:45 +01:00
Amiti Uttarwar
3ebde2143a [test] Fix wait condition in disconnect_p2ps
MY_SUBVERSION is defined in messages.py as a byte string, but here we were
comparing this value to the value returned by the RPC. Convert to ensure the
types match.
2020-11-27 14:54:55 -08:00
Michael Dietz
8008ef770f qt: unlock wallet "OK" button bugfix
When trying to send a transaction from an encrypted wallet, the ask
passphrase dialog would not allow the user to click the "OK" button
and proceed. Therefore it was impossible to send a transaction
through the gui. It was not enabling the "OK" button after the
passphrase was entered by the user, because it was using the same
form validation logic as the "Change passphrase" flow.
2020-11-27 14:18:50 -06:00
practicalswift
ce9dd45422 Add [[nodiscard]] to RenameOver(...) 2020-11-27 12:41:26 +00:00
practicalswift
9429a398e2 Handle rename failure in DumpMempool(...) by using RenameOver(...) return value 2020-11-27 12:41:07 +00:00
MarcoFalke
e2ff5e7b35
Merge #20497: [Refactor] Add MAX_STANDARD_SCRIPTSIG_SIZE to policy
e416cfc92b Add MAX_STANDARD_SCRIPTSIG_SIZE to policy (sanket1729)

Pull request description:

  Bitcoin core has a standardness rule for max satisfaction script sig size.
  This PR adds to the policy header file so that it is documented along with
  along policy rules. The initial reasoning that 1650 is an implicit
  limit(would not reach assuming all other policy rules are being
  followed) is outdated.

  As we now know, bitcoin transactions can have spend conditions are more than
  just signatures and there may exist p2sh transactions involving 100 byte
  preimages that maybe non-standard because of this rule. Because this
  rule is no longer implicit, we should explicitly document it in policy
  header file

ACKs for top commit:
  sipa:
    utACK e416cfc92b
  practicalswift:
    cr ACK e416cfc92b
  theStack:
    Code Review ACK e416cfc92b

Tree-SHA512: 1a91ee23dfb6085807e04dd0687d7a443e0f3e0f52d0a995a6599dff28533b0b599afba2724735d93948a64a3e25d0bc016ce3e771c0bd453eef78b22dc2369d
2020-11-27 10:59:24 +01:00
Hennadii Stepanov
86b1ab64b1
refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date 2020-11-27 10:04:13 +02:00
Pieter Wuille
2d8099c713 Mention units of MAX_STANDARD_ policy constants 2020-11-26 14:56:25 -08:00
Pieter Wuille
84e29c7c01 Mention in validation that IsWitnessStandard tests for P2TR 2020-11-26 14:56:25 -08:00
Pieter Wuille
f867cbcc26 Clean up assets test minimizer LDFLAGS 2020-11-26 14:56:25 -08:00
Pieter Wuille
ea0e78677b Document additional IsWitnessStandard behavior 2020-11-26 14:56:25 -08:00
Pieter Wuille
6040de9a46 Add comments on CPubKey::IsValid 2020-11-26 14:56:25 -08:00
Pieter Wuille
8dbb7de67c Add comments to VerifyTaprootCommitment 2020-11-26 14:56:25 -08:00
Pieter Wuille
cdf900cbf2 Document need_vin_vout_mismatch argument to make_spender 2020-11-26 14:56:25 -08:00
Pieter Wuille
18246ed5f0 Fix and improve taproot_construct comments 2020-11-26 14:56:25 -08:00