17531 Commits

Author SHA1 Message Date
MarcoFalke
03b1db6114
Merge #18766: Disable fee estimation in blocksonly mode (by removing the fee estimates global)
4e28753f60613ecd35cdef87bef5f99c302c3fbd feestimator: encapsulate estimation file logic (Antoine Poinsot)
e8ea6ad9c16997bdc7e22a20eca16e234290b7ff init: don't create a CBlockPolicyEstimator if we don't relay transactions (Antoine Poinsot)
86ff2cf202bfb9d9b50800b8ffe3fead3f77f5fa Remove the remaining fee estimation globals (Antoine Poinsot)
03bfeee957ab7e3b6aece82b9561774648094f54 interface: remove unused estimateSmartFee method from node (Antoine Poinsot)

Pull request description:

  If the `blocksonly` mode is turned on after running with transaction
  relay enabled for a while, the fee estimation will serve outdated data
  to both the internal wallet and to external applications that might be
  feerate-sensitive and make use of `estimatesmartfee` (for example a
  Lightning Network node).

  This has already caused issues (for example https://github.com/bitcoin/bitcoin/issues/16840 (C-lightning), or https://github.com/lightningnetwork/lnd/issues/2562 (LND)) and it seems prudent to fail rather than to give inaccurate values.

  This fixes #16840, and closes #16890 which tried to fix the symptoms (RPC) but not the cause as mentioned by sdaftuar :
  > If this is a substantial problem, then I would think we should take action to protect our own wallet users as well (rather than hide the results of what our fee estimation would do!).

ACKs for top commit:
  MarcoFalke:
    re-ACK 4e28753f60 👋
  jnewbery:
    utACK 4e28753f60613ecd35cdef87bef5f99c302c3fbd

Tree-SHA512: c869cf03b86d8194002970bbc84662dae76874967949b9be0d9a4511a1eabcb1627c38aca3154da9dcece1a4c49ec02bd4f9fcca2ec310986e07904559e63ba8
2020-12-07 12:59:48 +01:00
John Newbery
3025ca9e77 [net processing] Add RemovePeer()
This allows us to avoid repeated locking in FinalizeNode()
2020-12-07 11:59:24 +00:00
John Newbery
a20ab22786 [net processing] Make GetPeerRef const 2020-12-07 11:57:12 +00:00
John Newbery
ed7e469cee [net_processing] Move peer_map to PeerManager 2020-12-07 11:55:28 +00:00
MarcoFalke
00f4dcd552
Merge #20138: net: Assume that SetCommonVersion is called at most once per peer
fa0f4157098ea68169ced44730986d0ed2c3a5aa net: Assume that SetCommonVersion is called at most once per peer (MarcoFalke)

Pull request description:

  This restores the check removed in https://github.com/bitcoin/bitcoin/pull/17785#discussion_r503224381

  Instead of using `error`, which was used previously, it uses a newly introduced `Assume()`. `error` had several issues:
  * It logs unconditionally to the debug log
  * It doesn't abort the program when the error is hit in tests

ACKs for top commit:
  practicalswift:
    cr ACK fa0f4157098ea68169ced44730986d0ed2c3a5aa: patch looks correct
  jnewbery:
    utACK fa0f4157098ea68169ced44730986d0ed2c3a5aa

Tree-SHA512: cd7424a9485775e8c7093b725f8f52a90d47485185e79bac80f7810e450d0b3fda608d8805e9239094929f7bad2dca3fe772fb78ae606c2399d15405521e136b
2020-12-07 12:48:55 +01:00
Wladimir J. van der Laan
f3e17686b3
Merge #20468: build: warn when generating man pages for binaries built from a dirty branch
6690adba08006739da0060eb4937126bdfa1181a Warn when binaries are built from a dirty branch. (Tyler Chambers)

Pull request description:

  - Adjusted `--version` flag behavior in bitcoind and bitcoin-wallet to have the same behavior.
  - Added `--version` flag to bitcoin-tx to match.
  - Added functionality in gen-manpages.sh to error when attempting to generate man pages for binaries built from a dirty branch.

  mitigates problem with  issue #20412

ACKs for top commit:
  laanwj:
    Tested ACK 6690adba08006739da0060eb4937126bdfa1181a

Tree-SHA512: b5ca509f1a57f66808c2bebc4b710ca00c6fec7b5ebd7eef58018e28e716f5f2358e36551b8a4df571bf3204baed565a297aeefb93990e7a99add502b97ee1b8
2020-12-07 10:51:45 +01:00
MarcoFalke
fa8abdc995
rpc: Use FeeModes doc helper in estimatesmartfee
Can be reviewed with --ignore-all-space
2020-12-07 09:28:47 +01:00
Jonas Schnelli
eab63b971d
Merge #19847: rpc, refactor: Avoid duplicate set lookup in gettxoutproof
52fc39917fc52c2ff279fe434431e18900b347bd rpc: Reject empty txids in gettxoutproof (João Barbosa)
73dc19a330f8cb063af46e6c4246f2e64a04bdc1 rpc, refactor: Avoid duplicate set lookup in gettxoutproof (João Barbosa)

Pull request description:

ACKs for top commit:
  jonasschnelli:
    code review ACK 52fc39917fc52c2ff279fe434431e18900b347bd

Tree-SHA512: 76b18e5235e8b2d394685515a4a60335666eeb0f6b31c1d397f7db2fbe681bc817b8cd3e8f6708b9dacd6113e4e1d94837072cae27834b8a1a22d2717db8191e
2020-12-07 09:17:08 +01:00
MarcoFalke
1a04f45fe9
Merge #19832: p2p: Put disconnecting logs into BCLog::NET category
1816327e533d359c237c53eb6440b2f3a7cbf4fa p2p: Put disconnecting logs into BCLog::NET category (Hennadii Stepanov)

Pull request description:

  It's too noisy:
  ```
  $ cat debug.log | wc -l
  28529
  $ cat debug.log | grep "Disconnecting and discouraging peer" | wc -l
  10177
  ```

ACKs for top commit:
  MarcoFalke:
     noban, addnode and local peers are still unconditionally logged (as they should), but this one can go into a category, so cr-ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa
  practicalswift:
    ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa for the reasons MarcoFalke gave above.
  ajtowns:
    ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa

Tree-SHA512: c312c1009090840659b2cb1364d8ad9b6ab8e742fc462aef169996d93c76c248507639a00257ed9d73a6916c01176b1793491b2305e92fdded5f9de0935b6ba6
2020-12-07 09:07:57 +01:00
practicalswift
5021810650 Make CanFlushToDisk a const member function 2020-12-06 00:54:10 +00:00
practicalswift
281cf99554 Do not run functions with necessary side-effects in assert() 2020-12-06 00:48:09 +00:00
practicalswift
12dcdaaa54 Don't make "in" parameters look like "out"/"in-out" parameters: pass by ref to const instead of ref to non-const 2020-12-06 00:22:40 +00:00
Jon Atack
ce207d6b93
wallet, bugfix: allow send to take string fee rate values 2020-12-04 22:12:36 +01:00
João Barbosa
52fc39917f rpc: Reject empty txids in gettxoutproof 2020-12-04 18:38:23 +00:00
João Barbosa
73dc19a330 rpc, refactor: Avoid duplicate set lookup in gettxoutproof 2020-12-04 18:38:23 +00:00
Russell Yanofsky
f3d870fc22 wallet: List all wallets in non-SQLite or non-BDB builds
This commit does not change behavior when bitcoin is built normally with both
the SQLite and BDB libraries. It just makes non-SQLite and non-BDB builds
more similar to the normal build. Specifically:

- It makes wallet directory lists always include all wallets so wallets don't
  appear missing depending on the build.

- It now triggers specific "Build does not support SQLite database format" and
  "Build does not support Berkeley DB database format" errors if a wallet can't
  be loaded instead of the more ambiguous and scary "Data is not in recognized
  format" error.
2020-12-04 11:03:28 -04:00
Russell Yanofsky
d70dc89e78 refactor: Consolidate redundant wallet database path and exists functions
No change in behavior. Just remove a little bit of code, reduce macro usage,
remove duplicative functions, and make BDB and SQLite implementations more
consistent with each other.
2020-12-04 11:03:28 -04:00
Russell Yanofsky
6a7a63644c refactor: Drop call to GetWalletEnv in wallet salvage code
No observable change in behavior. This just avoids a redundant environment
lookup. Motivation is to be able to simplify the GetWalletEnv implementation in
an upcoming commit.
2020-12-04 11:03:28 -04:00
Russell Yanofsky
6ee9cbdd18 refactor: Replace ListWalletDir() function with ListDatabases()
No change to behavior. This is just cleanup after previous MOVEONLY commit to
make db.h list function fit conventions of surrounding functions.
2020-12-04 11:03:28 -04:00
Russell Yanofsky
5aaeb6cf87 MOVEONLY: Move IsBDBFile, IsSQLiteFile, and ListWalletDir
This commit does not change to any code and behavior. It it is easily reviewed
with the --color-moved=dimmed_zebra git diff option.

Motivation for this change is to:

- Consolidate redundant functions
  IsBDBFile /ExistsBerkeleyDatabase / SplitWalletPath, and
  IsSQLiteFile / ExistsSQLiteDatabase in the next commits

- Detect SQLite wallets consistently regardless whether bitcoin is built with
  SQLite support in the next commits

- Avoid attempting to open SQLite databases with the BDB library when bitcoin
  is built without SQLite support in the next commits
2020-12-04 11:03:28 -04:00
MarcoFalke
c1604483d3
Merge #20566: refactor: Use C++17 std::array where possible
fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2 refactor: Use C++17 std::array where possible (MarcoFalke)

Pull request description:

  Using the C++11 std::array with explicit template parameters is problematic because overshooting the size will fill the memory with default constructed types.

  For example,

  ```cpp
  #include <array>
  #include <iostream>

  int main()
  {
      std::array<int, 3> a{1, 2};
      for (const auto& i : a) {
          std::cout << i << std::endl;  // prints "1 2 0"
      }
  }
  ```

ACKs for top commit:
  jonasschnelli:
    Code Review ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2
  practicalswift:
    cr ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2
  vasild:
    ACK fac7ab1d
  promag:
    Code review ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2.

Tree-SHA512: ef7e872340226e0d6160e6fd66c6ca78b2ef9c245fa0ab27fe4777aac9fba8d5aaa154da3d27b65dec39a6a63d07f1063c3a8ffb667a98ab137756a1a0af2656
2020-12-04 15:18:48 +01:00
John Newbery
a529fd3e3f [net processing] Move GetNodeStateStats into PeerManager 2020-12-04 11:37:45 +00:00
MarcoFalke
fa0f415709
net: Assume that SetCommonVersion is called at most once per peer 2020-12-04 11:19:15 +01:00
MarcoFalke
dca80ffb45
Merge #20255: util: Add Assume() identity function
faa05854f832405231c9198787a4eafe2cd4c5f0 util: Remove probably misleading TODO (MarcoFalke)
fac5efe730ab5b8694920547203deefc5252d294 util: Add Assume() identity function (MarcoFalke)
fa861569dcfff9e72686dc5f30b1faa645b4d54e util: Allow Assert(...) to be used in all contexts (practicalswift)

Pull request description:

  This is needed for #20138. Please refer to the added documentation for motivation.

ACKs for top commit:
  practicalswift:
    cr ACK faa05854f832405231c9198787a4eafe2cd4c5f0
  jnewbery:
    utACK faa05854f832405231c9198787a4eafe2cd4c5f0
  hebasto:
    ACK faa05854f832405231c9198787a4eafe2cd4c5f0, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 72165fbd898b92ab9a79b070993fa1faa86c2e3545b6645e72c652bda295d5107bc298d0482bf3aaf0926fc0c3e6418a445c0e073b08568c44231f547f76a688
2020-12-04 11:07:28 +01:00
MarcoFalke
fac7ab1d5b
refactor: Use C++17 std::array where possible 2020-12-04 08:22:45 +01:00
Suhas Daftuar
65273fa0e7 Clear m_addr_known before our periodic self-advertisement
This behavior was apparently inadvertently broken in 5400ef6; without this
change our daily self-announcements frequently go unsent, because our
address is still in the peer's rolling bloom filter (for potentially many
days, depending on addr traffic).
2020-12-03 13:10:37 -05:00
Wladimir J. van der Laan
a0489f3472
Merge #20221: net: compat.h related cleanup
cadb77a6ab8a3e6f56062cfaec4dd8168c71b39d net: Add compat.h header for htonl function (Hennadii Stepanov)
f796f0057bc7dad8e7065831b07f432fc0fb9f08 net: Drop unneeded headers when compat.h included (Hennadii Stepanov)
467c34644861a5267601255650e27c7aadab31dc 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 cadb77a6ab8a3e6f56062cfaec4dd8168c71b39d: patch looks even better
  laanwj:
    Code review ACK cadb77a6ab8a3e6f56062cfaec4dd8168c71b39d

Tree-SHA512: 625ff90b2806310ab856a6ca1ddb6d9a85aa70f342b323e8525a711dd12219a1ecec8373ec1dca5a0653ffb11f9b421753887b25615d991ba3132c1cca6a3c6e
2020-12-03 14:54:07 +01:00
Antoine Poinsot
4e28753f60
feestimator: encapsulate estimation file logic
This moves the fee_estimates file management to the CBlockPolicyEstimator
Flush() method.

Co-authored-by: John Newbery <john@johnnewbery.com>
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-12-03 12:56:37 +01:00
Antoine Poinsot
e8ea6ad9c1
init: don't create a CBlockPolicyEstimator if we don't relay transactions
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-12-03 12:56:37 +01:00
Antoine Poinsot
86ff2cf202
Remove the remaining fee estimation globals
This moves the CBlockPolicyEstimator to the NodeContext, which get rids
of two globals and allows us to conditionally create the
CBlockPolicyEstimator (and to remove a circular dep).

Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-12-03 12:56:37 +01:00
Antoine Poinsot
03bfeee957
interface: remove unused estimateSmartFee method from node
Co-Authored-by: MarcoFalke <falke.marco@gmail.com>
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-12-03 12:56:36 +01:00
fanquake
0a13d15c14
Merge #20530: lint, refactor: Update cppcheck linter to c++17 and improve explicit usage
1e62350ca20898189904a88dfef9ea11ddcd8626 refactor: Improve use of explicit keyword (Fabian Jahr)
c502a6dbfb854ca827a5a3925394f9e09d29b898 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 1e62350ca20898189904a88dfef9ea11ddcd8626: patch looks correct!
  MarcoFalke:
    review ACK 1e62350ca20898189904a88dfef9ea11ddcd8626

Tree-SHA512: dff7b324429a57160e217cf38d9ddbb6e70c6cb3d3e3e0bd4013d88e07afc2292c3df94d0acf7122e9d486322821682ecf15c8f2724a78667764c05d47f89a12
2020-12-02 20:52:19 +08:00
Jonas Schnelli
c33662a0ea
Merge #18948: qt: Call setParent() in the parent's context
8963b2c71f120b2746396c4987392f0105c8dd60 qt: Improve comments in WalletController::getOrCreateWallet() (Hennadii Stepanov)
5fcfee68af47d4a891ae9c9964d73886f0f01d7d qt: Call setParent() in the parent's context (Hennadii Stepanov)
5659e73493fcdfb5d0cb9d686c24c4fbe1c217ed 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 (007e15dcd7f8b42501e31cc36343655c53027077) 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 8963b2c71f120b2746396c4987392f0105c8dd60. No changes since last review, just rebase because of conflict on some adjacent lines
  jonasschnelli:
    utACK 8963b2c71f120b2746396c4987392f0105c8dd60

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
db058efeb0821cb5022e3b29e0aff3627d7aaf83 sync: use HasReason() in double lock tests (Vasil Dimov)
a21dc469ccf076ca3b07b1adbd8bf667145f1c44 sync: const-qualify the argument of double_lock_detected() (Vasil Dimov)
6d3689fcf6cff397187028344570489db3e6ecf4 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 db058efeb0821cb5022e3b29e0aff3627d7aaf83
  ajtowns:
    ACK db058efeb0821cb5022e3b29e0aff3627d7aaf83
  hebasto:
    ACK db058efeb0821cb5022e3b29e0aff3627d7aaf83, tested on Linux Mint 20 (x86_64).

Tree-SHA512: 452ddb9a14e44bb174135b39f2219c76eadbb8a6c0e80d64a25f995780d6dbc7b570d9902616db94dbfabaee197b5828ba3475171a68240ac0958fb203a7acdb
2020-12-02 09:44:55 +01:00
MarcoFalke
283f22cabb
Merge #20461: rpc: Validate -rpcauth arguments
053b4fbad8729308774d5e7b53f53c12627fa88b doc: Release note regarding -rpcauth validation (João Barbosa)
46001323b1f4a57d8d6805f1bc39a5b8d401f0c5 rpc: Validate -rpcauth arguments (João Barbosa)
d37c813a43166f559a4e2d1c22e7243f70301291 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 053b4fbad8729308774d5e7b53f53c12627fa88b
  jonatack:
    ACK 053b4fbad8729308774d5e7b53f53c12627fa88b
  ryanofsky:
    Code review ACK 053b4fbad8729308774d5e7b53f53c12627fa88b. 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
fanquake
80d4231e16
Merge #19980: refactor: Some wallet cleanups
9b74461fa293453a9eb0b1717b30b3f7fa778d91 refactor: Assert before dereference in CWallet::GetDatabase (João Barbosa)
021feb3187b207d511561c1f0ffd7f9e5e0c9c1d refactor: Drop redudant CWallet::GetDBHandle (João Barbosa)

Pull request description:

ACKs for top commit:
  achow101:
    Code Review ACK 9b74461fa293453a9eb0b1717b30b3f7fa778d91
  meshcollider:
    utACK 9b74461fa293453a9eb0b1717b30b3f7fa778d91
  ryanofsky:
    Code review ACK 9b74461fa293453a9eb0b1717b30b3f7fa778d91. 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
MarcoFalke
f17e8ba3a1
Merge #20207: Follow-up extra comments on taproot code and tests
2d8099c713dfd4b546150fd53c2e4f364b9009f4 Mention units of MAX_STANDARD_ policy constants (Pieter Wuille)
84e29c7c0141b52044020ec0c5dfa8a462b7e97f Mention in validation that IsWitnessStandard tests for P2TR (Pieter Wuille)
f867cbcc268a3bfaeef5510a7e40e6d3c0818b6d Clean up assets test minimizer LDFLAGS (Pieter Wuille)
ea0e78677bdbe3313f594118c500cf7784c56970 Document additional IsWitnessStandard behavior (Pieter Wuille)
6040de9a46725826330cd63cdf76e2121a18e728 Add comments on CPubKey::IsValid (Pieter Wuille)
8dbb7de67ce0a71f5fc54289c0ff048ac8dd0acc Add comments to VerifyTaprootCommitment (Pieter Wuille)
cdf900cbf26db05c7edb398ea645f1d23049d810 Document need_vin_vout_mismatch argument to make_spender (Pieter Wuille)
18246ed5f09dd078fa1410b7ec2ba4379cc5e032 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
practicalswift
8c09c0c1d1 fuzz: Avoid time-based "non-determinism" in fuzzing harnesses by using mocked GetTime() 2020-12-01 13:18:34 +00:00
MarcoFalke
dfd0b70088
Merge #20425: fuzz: Make CAddrMan fuzzing harness deterministic
17a5f172fa9ec509b1c3f950ee8dfb6f025534d2 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 17a5f172fa9ec509b1c3f950ee8dfb6f025534d2

Tree-SHA512: 725f983745233e9b616782247fa18847e483c074ca4336a5beea8a9009128c3a74b4d50a12662d8ca2177c2e1fc5fc121834df6b459ac0af43c931d77ef7c4d8
2020-12-01 14:04:10 +01:00
Jonas Schnelli
dcb7518067
Merge #20496: build: Drop unneeded macOS framework dependencies
ec4a46dd9c158e1a0de144e9abb260da941aa702 build: Drop unneeded IOKit framework dependency (Hennadii Stepanov)
65afe4cb69e80278de0f584b6eeb6741f02a31eb 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 ec4a46dd9c158e1a0de144e9abb260da941aa702
  practicalswift:
    cr ACK ec4a46dd9c158e1a0de144e9abb260da941aa702: patch looks correct!
  promag:
    Tested ACK ec4a46dd9c158e1a0de144e9abb260da941aa702 (not depends build).

Tree-SHA512: 47b5ad87d761992850133a921f07d485565c70ba2909a3289050f406e6dbd39ad49e1aeeb6cad79c6914385a72ddffd273dfadd69259a35545a13cd17d0e5043
2020-12-01 11:27:06 +01:00
Jonas Schnelli
f2a673f15b
Merge bitcoin-core/gui#137: refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date
86b1ab64b1a5b56518787ef16ea54ddbbc97d83e 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 86b1ab64b1a5b56518787ef16ea54ddbbc97d83e

Tree-SHA512: 1dbba8ee70c895bf58317172a9901cdbe5503b1d6258f51caaae88d88d332d9fbd4697c995192d31e3618ddfd532c5f5881289b3af1184422e5a9263a1224115
2020-12-01 10:59:09 +01:00
MarcoFalke
cd720337fe
Merge #20222: refactor: CTxMempool constructor clean up
f15e780b9e57554c723bc02aa41150ecf3e3a8c9 refactor: Clean up CTxMemPool initializer list (Elle Mouton)
e3310692d0e9720e960b9785274ce1f0b58b4cd7 refactor: Make CTxMemPool::m_check_ratio a const and a constructor argument (Elle Mouton)
9d4b4b2c2c49774523de740d6492ee5b1ee15e74 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 f15e780b9e57554c723bc02aa41150ecf3e3a8c9
  MarcoFalke:
    ACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9 👘
  glozow:
    utACK f15e780b9e
  theStack:
    Code Review ACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9

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
629a9299b2a7241a3fa7d597cb34abcbe1af9255 Move WalletImpl from interfaces/wallet.cpp to wallet/interfaces.cpp (Russell Yanofsky)
2a26771d8161d30be1853a35acfee588cce03634 Move ChainImpl from interfaces/chain.cpp to node/interfaces.cpp (Russell Yanofsky)
12bd0fc9d70333c54c83ebb08c734272dbd330c2 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 629a9299b2a7241a3fa7d597cb34abcbe1af9255.
  MarcoFalke:
    review ACK 629a9299b2a7241a3fa7d597cb34abcbe1af9255 🔺

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(...).
ce9dd45422e1f4ecce6df68da086b8bfc2100756 Add [[nodiscard]] to RenameOver(...) (practicalswift)
9429a398e291a1b5edcfc657b94fcaf52cd1d8f9 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 ce9dd45422e1f4ecce6df68da086b8bfc2100756 🏷️

Tree-SHA512: 1e63d7f3061e1f6ea2df5750dbc1547a39bd50b6c529812a0c8a0c11d3100c241afdf14094e69b69a38bade7e54a12b2a42888545874398eaf5d02421b57e874
2020-12-01 08:28:34 +01:00
MarcoFalke
81d5af42f4
Merge #20499: Remove obsolete NODISCARD ifdef forest. Use [[nodiscard]] (C++17).
79bff8e48aca961ec271b0d592aca9278b981e2f Remove NODISCARD (practicalswift)
4848e711076c6ebc5d841feb83baeb6d2bc76c94 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 79bff8e48aca961ec271b0d592aca9278b981e2f
  fanquake:
    ACK 79bff8e48aca961ec271b0d592aca9278b981e2f

Tree-SHA512: 56dbb8e50ed97ecfbce28cdc688a01146108acae49a943e338a8f983f7168914710d36e38632f6a7c200ba6c6ac35b2519e97d6c985e8e7eb23223f13bf985d6
2020-11-30 15:42:36 +01:00
fanquake
817aeca57a
Merge #20491: refactor: Drop noop gcc version checks
830ddf413934226d0b6ca99165916790cc52ca18 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 830ddf413934226d0b6ca99165916790cc52ca18

Tree-SHA512: 36264661d6ced1683a0c907efba7c700502acaf8e9fd50d9066bc9c7b877b25165b0684c2d7fe74bd58e500a77d7702bdbdd53691c274f29e4abccd241c10964
2020-11-30 16:10:41 +08:00
Tyler Chambers
6690adba08
Warn when binaries are built from a dirty branch.
Adjusted version flag behavior in bitcoin-tx, bitcoin-wallet, and
bitcoind to match. Added functionality in gen-manpages.sh to warning when
attempting to generate man pages for binaries built from a dirty
branch.
2020-11-28 18:43:55 -05:00
MarcoFalke
854b36cfa2
Merge bitcoin-core/gui#138: unlock encrypted wallet "OK" button bugfix
8008ef770f3d0b14d03e22371314500373732143 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 8008ef770f3d0b14d03e22371314500373732143
  hebasto:
    ACK 8008ef770f3d0b14d03e22371314500373732143, 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
89bdad5b25ae4ac03a486f729a5b58ae6f21946d 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 89bdad5b25ae4ac03a486f729a5b58ae6f21946d
  MarcoFalke:
    review ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d

Tree-SHA512: efb399c33f7b5596870a26a8680f453ca47aa7a6db4e550f9435d13044f1c4bad0ae11e8f0205213409d08b75c4188c3be782e54aafab1f65b97eb8cf5c252a9
2020-11-28 10:14:45 +01:00