aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-12-09Merge #20323: tests: Create or use existing properly initialized NodeContextsMarcoFalke
81137c60fe6234569e1c5e6760f3a6f016956944 test: Add new ChainTestingSetup and use it (Carl Dong) 7e9e7fe56734d729ed7de39e880577b135dfd368 qt/test: [FIX] Add forgotten Context setting in RPCNestedTests (Carl Dong) Pull request description: This is part 1/n of the effort to [de-globalize `ChainstateManager`](https://github.com/bitcoin/bitcoin/pull/20158) Reviewers: Looking for tested/Code-Review/plain-ACKs ### Context In many of our tests, we manually instantiate `NodeContext`s or `ChainstateManager`s in the test code, which is error prone. Instead, we should create or use existing references because: 1. Before we [de-globalize `ChainstateManager`](https://github.com/bitcoin/bitcoin/pull/20158), much of our code still acts on `g_chainman` (our global `ChainstateManager`), sometimes even when you're calling a method on a specific instance of `ChainstateManager`! This means that we may act on two instances of `ChainstateManager`, which is most likely not what we want. 2. Using existing references (initialized by the `{Basic,}TestingSetup` constructors) means that you're acting on objects which are properly initialized, instead of "just initialized enough for this dang test to pass". Also, they're already there! It's free! 3. By acting on the right object, we also allow the review-only assertions in future commits of [de-globalize `ChainstateManager`](https://github.com/bitcoin/bitcoin/pull/20158) to work and demonstrate correctness. Some more detailed debugging notes can be found in the first commit, reproduced below: ``` Previously, the validation_chainstatemanager_tests test suite instantiated its own duplicate ChainstateManager on which tests were performed. This wasn't a problem for the specific actions performed in that suite. However, the existence of this duplicate ChainstateManager and the fact that many of our validation static functions reach for g_chainman, ::Chain(state|)Active means we may end up acting on two different CChainStates should we write more extensive tests in the future. This change adds a new ChainTestingSetup which performs all initialization previously done by TestingSetup except: 1. Mempool sanity check frequency setting 2. ChainState initialization 3. Genesis Activation 4. {Ban,Conn,Peer}Man initialization Means that we will no longer need to initialize a duplicate ChainstateManger in order to test the initialization codepaths of CChainState and ChainstateManager. Lastly, this change has the additional benefit of allowing for review-only assertions meant to show correctness to work in future work de-globalizing g_chainman. In the test chainstatemanager_rebalance_caches, an additional LoadGenesisBlock call is added as MaybeReblanaceCaches eventually calls FlushBlockFile, which tries to access vinfoBlockFile[nLastBlockFile], which is out of bounds when LoadGenesisBlock hasn't been called yet. ----- Note for the future: In a previous version of this change, I put ChainTestingSetup between BasicTestingSetup and TestingSetup such that TestingSetup inherited from ChainTestingSetup. This was suboptimal, and showed how the class con/destructor inheritance structure we have for these TestingSetup classes is probably not the most suitable abstraction. In particular, for both TestingSetup and ChainTestingSetup, we need to stop the scheduler first before anything else. Otherwise classes depending on the scheduler may be referenced by the scheduler after said classes are freed. This means that there's no clear parallel between our teardown code and C++'s destructuring order for class hierarchies. Future work should strive to coalesce (as much as possible) test and non-test init codepaths and perhaps structure it in a more fail-proof way. ``` ACKs for top commit: MarcoFalke: ACK 81137c60fe looking excellent now 🐩 jnewbery: ACK 81137c60fe6234569e1c5e6760f3a6f016956944 ryanofsky: Code review ACK 81137c60fe6234569e1c5e6760f3a6f016956944. This change is simpler after the rebase because wallet & bench commits are dropped. Tree-SHA512: a8d84f08f2db6428b0b88449bdc814c9db35b7559156d536dfebd3225c2707dba65959e76d2152e3f8c96eacbf1e0b0000f745edf1e196deddb97ff1ef360953
2020-12-09Merge #20564: Don't send 'sendaddrv2' to pre-70016 software, and send before ↵MarcoFalke
'verack' 1583498fb6781c01ca2f33c09319ed793964c574 Send and require SENDADDRV2 before VERACK (Pieter Wuille) c5a89196602e43ebb1cdc9cd4f08d153419c13e1 Don't send 'sendaddrv2' to pre-70016 software (Pieter Wuille) Pull request description: BIP155 defines addrv2 and sendaddrv2 for all protocol versions, but some implementations reject messages they don't know. As a courtesy, don't send it to nodes with a version before 70016, as no software is known to support BIP155 that doesn't announce at least that protocol version number. Also move the sending of sendaddrv2 earlier (before sending verack), as proposed in https://github.com/bitcoin/bips/pull/1043. This has the side effect that local address broadcast of torv3 will work (as it'll only trigger after we know whether or not the peer supports addrv2). ACKs for top commit: MarcoFalke: ACK 1583498fb6781c01ca2f33c09319ed793964c574 jnewbery: ACK 1583498fb6781c01ca2f33c09319ed793964c574 jonatack: ACK 1583498fb6781c01ca2f33c09319ed793964c574 vasild: ACK 1583498 Tree-SHA512: 3bd5833fa8c8567b6dedd99e4a9b6bb71c127aa66d5284b217503c86d597dc59aa7382c41f3a4bf561bb658b89db81d1a7703a700eef4ffc17cb916660e23a82
2020-12-08test: Add new ChainTestingSetup and use itCarl Dong
Previously, the validation_chainstatemanager_tests test suite instantiated its own duplicate ChainstateManager on which tests were performed. This wasn't a problem for the specific actions performed in that suite. However, the existence of this duplicate ChainstateManager and the fact that many of our validation static functions reach for g_chainman, ::Chain(state|)Active means we may end up acting on two different CChainStates should we write more extensive tests in the future. This change adds a new ChainTestingSetup which performs all initialization previously done by TestingSetup except: 1. RPC command registration 2. ChainState initialization 3. Genesis Activation 4. {Ban,Conn,Peer}Man initialization Means that we will no longer need to initialize a duplicate ChainstateManger in order to test the initialization codepaths of CChainState and ChainstateManager. Lastly, this change has the additional benefit of allowing for review-only assertions meant to show correctness to work in future work de-globalizing g_chainman. In the test chainstatemanager_rebalance_caches, an additional LoadGenesisBlock call is added as MaybeReblanaceCaches eventually calls FlushBlockFile, which tries to access vinfoBlockFile[nLastBlockFile], which is out of bounds when LoadGenesisBlock hasn't been called yet. ----- Note for the future: The class con/destructor inheritance structure we have for these TestingSetup classes is probably not the most suitable abstraction. In particular, for both TestingSetup and ChainTestingSetup, we need to stop the scheduler first before anything else. Otherwise classes depending on the scheduler may be referenced by the scheduler after said classes are freed. This means that there's no clear parallel between our teardown code and C++'s destructuring order for class hierarchies. Future work should strive to coalesce (as much as possible) test and non-test init codepaths and perhaps structure it in a more fail-proof way.
2020-12-08qt/test: [FIX] Add forgotten Context setting in RPCNestedTestsCarl Dong
2020-12-08Merge #19425: refactor: Get rid of more redundant chain methodsMarcoFalke
5baa88fd38c8efa0e361636bb2c60af89d27b5d5 test: Remove no longer needed MakeChain calls (Russell Yanofsky) 6965f1352d2d7086d308750ce83a84f191a17755 refactor: Replace uses ChainActive() in interfaces/chain.cpp (Russell Yanofsky) 3fbbb9a6403a86fbed3d5d9f7939998922593377 refactor: Get rid of more redundant chain methods (Russell Yanofsky) Pull request description: This just drops three interfaces::Chain methods replacing them with other calls. Motivation for removing these chain methods: - Need to get rid of findFirstBlockWithTimeAndHeight for #10102, which doesn't support overloaded methods - Followup from https://github.com/bitcoin/bitcoin/pull/16426#discussion_r412487403 - phantomcircuit comments about findNextBlock test http://www.erisian.com.au/bitcoin-core-dev/log-2020-06-06.html#l-214 Behavior is not changing in any way here. A TODO comment in ScanForWalletTransactions was removed, but just because it was invalid (see https://github.com/bitcoin/bitcoin/pull/19195#discussion_r448020762), not because it was implemented. ACKs for top commit: MarcoFalke: re-ACK 5baa88fd38c8efa0e361636bb2c60af89d27b5d5 🕶 dongcarl: ACK 5baa88fd38c8efa0e361636bb2c60af89d27b5d5 Tree-SHA512: d20a2a8cf8742e6100c6d3a4871ed67f184925712cf9e55d301abee60353bf3f74cd0d46a13be1715d1c2faef72102bb321654d08a128c2ef6880f72b72a6687
2020-12-08Send and require SENDADDRV2 before VERACKPieter Wuille
See the corresponding BIP change: https://github.com/bitcoin/bips/pull/1043
2020-12-07test: Remove no longer needed MakeChain callsRussell Yanofsky
These calls are no longer needed after edc316020e8270dafc5e31465d532baebdafd3dd from #19098 which started instantiating BasicTestingSetup.m_node.chain Patch from MarcoFalke <falke.marco@gmail.com> in https://github.com/bitcoin/bitcoin/pull/19425#discussion_r526701954 Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2020-12-07Merge #20561: p2p: periodically clear m_addr_knownWladimir J. van der Laan
65273fa0e74f0c11dfbf0645dd962bdc779ea558 Clear m_addr_known before our periodic self-advertisement (Suhas Daftuar) Pull request description: We use a rolling bloom filter to track which addresses we've previously sent a peer, but after #7125 we no longer clear it every day before our own announcement. This looks to me like an oversight which has the effect of reducing the frequency with which we actually self-announce our own address, so this reintroduces resetting that filter. ACKs for top commit: naumenkogs: ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558 laanwj: Code review ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558 sipa: utACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558 Tree-SHA512: 602c155fb6d2249b054fcb6f1c0dd17143605ceb87132286bbd90babf26d258ff6c41f9925482c17e2be41805d33f9b83926cb447f394969ffecd4bccfa0a64f
2020-12-07Don't send 'sendaddrv2' to pre-70016 softwarePieter Wuille
2020-12-07refactor: Replace uses ChainActive() in interfaces/chain.cppRussell Yanofsky
Suggested https://github.com/bitcoin/bitcoin/pull/19425#discussion_r456236407
2020-12-07Merge #20568: doc: Use FeeModes doc helper in estimatesmartfeeWladimir J. van der Laan
fa8abdc9953e381715493b259908e246914793b0 rpc: Use FeeModes doc helper in estimatesmartfee (MarcoFalke) Pull request description: Not sure why this doesn't use the doc helper, probably an oversight? ACKs for top commit: laanwj: Code review ACK fa8abdc9953e381715493b259908e246914793b0 Tree-SHA512: 1f2dc8356e3476ddcf9cafafa7f9865ad95bed1e3067c0edab8e3c483e374bdbdbecc066167554b4a1b479e28f6a52c4ae6a75a70c67ee4e1ff4f3ba36b04001
2020-12-07refactor: Get rid of more redundant chain methodsRussell Yanofsky
This just drops three interfaces::Chain methods replacing them with other calls. Motivation for removing these chain methods: - Need to get rid of findFirstBlockWithTimeAndHeight for #10102, which doesn't support overloaded methods - Followup from https://github.com/bitcoin/bitcoin/pull/16426#discussion_r412487403 - phantomcircuit comments about findNextBlock test http://www.erisian.com.au/bitcoin-core-dev/log-2020-06-06.html#l-214 Behavior is not changing in any way here. A TODO comment in ScanForWalletTransactions was removed, but just because it was invalid (see https://github.com/bitcoin/bitcoin/pull/19195#discussion_r448020762), not because it was implemented.
2020-12-07Merge #18766: Disable fee estimation in blocksonly mode (by removing the fee ↵MarcoFalke
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-07Merge #20138: net: Assume that SetCommonVersion is called at most once per peerMarcoFalke
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-07Merge #20468: build: warn when generating man pages for binaries built from ↵Wladimir J. van der Laan
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-07rpc: Use FeeModes doc helper in estimatesmartfeeMarcoFalke
Can be reviewed with --ignore-all-space
2020-12-07Merge #19847: rpc, refactor: Avoid duplicate set lookup in gettxoutproofJonas Schnelli
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-07Merge #19832: p2p: Put disconnecting logs into BCLog::NET categoryMarcoFalke
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-06Don't make "in" parameters look like "out"/"in-out" parameters: pass by ref ↵practicalswift
to const instead of ref to non-const
2020-12-04rpc: Reject empty txids in gettxoutproofJoão Barbosa
2020-12-04rpc, refactor: Avoid duplicate set lookup in gettxoutproofJoão Barbosa
2020-12-04Merge #20566: refactor: Use C++17 std::array where possibleMarcoFalke
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-04net: Assume that SetCommonVersion is called at most once per peerMarcoFalke
2020-12-04Merge #20255: util: Add Assume() identity functionMarcoFalke
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-04refactor: Use C++17 std::array where possibleMarcoFalke
2020-12-03Clear m_addr_known before our periodic self-advertisementSuhas Daftuar
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-03Merge #20221: net: compat.h related cleanupWladimir J. van der Laan
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-03feestimator: encapsulate estimation file logicAntoine Poinsot
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-03init: don't create a CBlockPolicyEstimator if we don't relay transactionsAntoine Poinsot
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-12-03Remove the remaining fee estimation globalsAntoine Poinsot
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-03interface: remove unused estimateSmartFee method from nodeAntoine Poinsot
Co-Authored-by: MarcoFalke <falke.marco@gmail.com> Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-12-02Merge #20530: lint, refactor: Update cppcheck linter to c++17 and improve ↵fanquake
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-02Merge #18948: qt: Call setParent() in the parent's contextJonas Schnelli
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-02Merge #20507: sync: print proper lock order location when double lock is ↵MarcoFalke
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-02Merge #20461: rpc: Validate -rpcauth argumentsMarcoFalke
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-02Merge #19980: refactor: Some wallet cleanupsfanquake
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-01refactor: Improve use of explicit keywordFabian Jahr
2020-12-01Merge #20207: Follow-up extra comments on taproot code and testsMarcoFalke
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-01Merge #20425: fuzz: Make CAddrMan fuzzing harness deterministicMarcoFalke
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-01Merge #20496: build: Drop unneeded macOS framework dependenciesJonas Schnelli
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-01Merge bitcoin-core/gui#137: refactor: Replace deprecated ↵Jonas Schnelli
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-01Merge #20222: refactor: CTxMempool constructor clean upMarcoFalke
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 https://github.com/bitcoin/bitcoin/pull/20222/commits/f15e780b9e57554c723bc02aa41150ecf3e3a8c9 theStack: Code Review ACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9 Tree-SHA512: d83f3b5311ca128847b621e5e999c7e1bf0f4e6261d4cc090fb13e229a0f7eecd66ad997f654f50a838baf708d1515740aa3bffc244909a001d01fd5ae398b68
2020-12-01Merge #20494: refactor: Move node and wallet code out of src/interfacesMarcoFalke
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-01Merge #20519: Handle rename failure in DumpMempool(...) by using the ↵MarcoFalke
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-11-30Merge #20499: Remove obsolete NODISCARD ifdef forest. Use [[nodiscard]] (C++17).MarcoFalke
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-30Merge #20491: refactor: Drop noop gcc version checksfanquake
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-28Warn when binaries are built from a dirty branch.Tyler Chambers
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-28Merge bitcoin-core/gui#138: unlock encrypted wallet "OK" button bugfixMarcoFalke
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-28Merge #20448: RPC/Wallet: unloadwallet: Allow specifying wallet_name param ↵MarcoFalke
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-27qt: unlock wallet "OK" button bugfixMichael Dietz
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.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-10 09:30:22 +0000