From 4afbaf2ea107795ed4135bbae4fb4a2bdf1c7014 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 10 Nov 2020 09:20:48 +0100 Subject: [PATCH 01/10] Merge #20322: test: Fix intermittent issue in wallet_listsinceblock 444412821e5349ce0e0b039d884583edb70c4399 test: Fix intermittent issue in wallet_listsinceblock (MarcoFalke) Pull request description: ACKs for top commit: Empact: Code Review ACK https://github.com/bitcoin/bitcoin/pull/20322/commits/444412821e5349ce0e0b039d884583edb70c4399 Tree-SHA512: 86d47b1e3c8681dd479654589c894016ac81a3c96a34c3b4a75278b2af85054ea8c6f768e518a5322a4928d82d5e99105bbce0f4fa6a7a18c40e3e0799f9ab54 --- test/functional/wallet_listsinceblock.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index eb68ab5815..d1b21f5810 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -192,6 +192,7 @@ class ListSinceBlockTest(BitcoinTestFramework): address = key_to_p2pkh(eckey.get_pubkey().get_bytes()) self.nodes[2].sendtoaddress(address, 10) self.nodes[2].generate(6) + self.sync_all() self.nodes[2].importprivkey(privkey) utxos = self.nodes[2].listunspent() utxo = [u for u in utxos if u["address"] == address][0] From 1fcc5f1101393c0ce0a33df22bea4f6163200de0 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 2 Dec 2020 08:30:57 +0100 Subject: [PATCH 02/10] Merge #20540: test: Fix wallet_multiwallet issue on windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fada2dfcac1c4b47ee76b877d91d515cf1d36410 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 fada2dfcac1c4b47ee76b877d91d515cf1d36410. Although it could ignore (don't log) directories that lead to no permission error. fanquake: ACK fada2dfcac1c4b47ee76b877d91d515cf1d36410 Tree-SHA512: b475162cc3cd1574209d916605b229a79c8089714295f5e16569b71f958f0007d54dc76833938492d931387784588b11b73e3ef00f963540af42c079417f8d72 --- test/functional/wallet_multiwallet.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 9fb15b365f..dc095fcaf1 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -130,7 +130,7 @@ class MultiWalletTest(BitcoinTestFramework): os.mkdir(wallet_dir('no_access')) os.chmod(wallet_dir('no_access'), 0) try: - with self.nodes[0].assert_debug_log(expected_msgs=['Too many levels of symbolic links', 'Error scanning']): + with self.nodes[0].assert_debug_log(expected_msgs=['Error scanning']): walletlist = self.nodes[0].listwalletdir()['wallets'] finally: # Need to ensure access is restored for cleanup From 267f42fd6a80547ad0b98d3648196cc260c9b277 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 Mar 2021 15:46:26 +0800 Subject: [PATCH 03/10] Merge #21382: build: Clean remnants of QTBUG-34748 fix 173ef8980d921c9c0e46257747ab1165965e3ced build: Small libxcb.mk improvements (Hennadii Stepanov) 5129b36573cb07d8bcbac493506c46e26d7239eb build: Clean remnants of QTBUG-34748 fix (Hennadii Stepanov) Pull request description: Hope, this PR will make [transit](https://github.com/bitcoin/bitcoin/pull/21376) to Qt 5.12.10 neater. A fix for [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was introduced in #5915 (v0.11.0, Qt 5.2.1). [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was [fixed](https://github.com/qt/qtbase/commit/b19b0808940c8c54b102012be134a370b26e348e) in Qt 5.3.0. The separated [`fix-xcb-include-order.patch`](https://github.com/theuni/bitcoin/blob/bb44d9e7546e6118cd91db5bbe471a3ce2ee7fcd/depends/patches/qt/fix-xcb-include-order.patch), provided by #5915, was dropped in #12971 while bumping Qt to 5.9.4 (5.9.6). But `libxcb.mk` remained unchanged. This PR reverts #5915 for `libxcb.mk` as well. ACKs for top commit: practicalswift: cr ACK 173ef8980d921c9c0e46257747ab1165965e3ced: patch looks correct fanquake: ACK 173ef8980d921c9c0e46257747ab1165965e3ced Tree-SHA512: 9815a7e532ff4aa08f9623ded8d5708eca1c9c73ac7a2684419a18c125da7627b44ac3191f2e7978946942c8d0580e73b1a93df624986fb2a13791a68ce1e025 --- depends/packages/libxcb.mk | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/depends/packages/libxcb.mk b/depends/packages/libxcb.mk index 82edcc2be0..fa30e80f5c 100644 --- a/depends/packages/libxcb.mk +++ b/depends/packages/libxcb.mk @@ -19,17 +19,12 @@ $(package)_config_opts += --disable-xtest --disable-xv --disable-xvmc endef define $(package)_preprocess_cmds - cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub build-aux &&\ + cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub build-aux && \ sed "s/pthread-stubs//" -i configure endef -# Don't install xcb headers to the default path in order to work around a qt -# build issue: https://bugreports.qt.io/browse/QTBUG-34748 -# When using qt's internal libxcb, it may end up finding the real headers in -# depends staging. Use a non-default path to avoid that. - define $(package)_config_cmds - $($(package)_autoconf) --includedir=$(host_prefix)/include/xcb-shared + $($(package)_autoconf) endef define $(package)_build_cmds @@ -41,5 +36,5 @@ define $(package)_stage_cmds endef define $(package)_postprocess_cmds - rm -rf share/man share/doc lib/*.la + rm -rf share lib/*.la endef From c326830f484821739b6bc7264e9263df3a140934 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 26 Mar 2021 17:30:18 +0100 Subject: [PATCH 04/10] Merge bitcoin-core/gui#243: fix issue when disabling the auto-enabled blank wallet checkbox 915e34112b5a4c2ef391d7e3706603bcd6f62a2a qt: fix issue when disabling the auto-enabled blank wallet checkbox (Jarol Rodriguez) Pull request description: As detailed by #151, On `master` a user can create the confusing scenario where you have a disabled `Encrypt Wallet` checkbox and a selected `Disable Private Keys` checkbox after unselecting the auto-enabled `Blank Wallet` checkbox. This commit makes it so that when the `Blank Wallet` checkbox is auto-selected after the user selects `Disable Private keys`, unselecting it will also unselect the `Disable Private Keys` checkbox, which in turn re-enables the `Encrypt Wallet` checkbox. Below are screenshots comparing the behavior of selecting `Disable Private Keys` then unselecting the `Blank Wallet` between `master` and this `PR`: **Master:** | Select `Disable Private Keys` | Unselect `Blank Wallet` | | ----------------------------- | ------------------------ | | ![Screen Shot 2021-03-09 at 7 57 14 PM](https://user-images.githubusercontent.com/23396902/110560141-77405a80-8113-11eb-9285-5acba6241dcf.png) | ![Screen Shot 2021-03-09 at 7 57 31 PM](https://user-images.githubusercontent.com/23396902/110560159-81faef80-8113-11eb-9b37-086aa39ecb9f.png) | **PR:** | Select `Disable Private Keys` | Unselect `Blank Wallet` | | ----------------------------- | ------------------------ | | ![Screen Shot 2021-03-09 at 7 34 12 PM](https://user-images.githubusercontent.com/23396902/110560379-e3bb5980-8113-11eb-899a-3a4c6a1bc115.png) | ![Screen Shot 2021-03-09 at 7 34 20 PM](https://user-images.githubusercontent.com/23396902/110560412-f170df00-8113-11eb-8bd0-f7fe6fc0d739.png) | ACKs for top commit: hebasto: ACK 915e34112b5a4c2ef391d7e3706603bcd6f62a2a Talkless: ACK 915e34112b5a4c2ef391d7e3706603bcd6f62a2a Tree-SHA512: ce6ecbc35b94a08cabf0b8a24dbdfc874d82cc8918cc8623dce8172c7fc9c75d63a13b036bae5f7ab2c090f8d020574a542285d1651600813faf5d91e2506a8d --- src/qt/createwalletdialog.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qt/createwalletdialog.cpp b/src/qt/createwalletdialog.cpp index 37ccd17edb..450d50dc12 100644 --- a/src/qt/createwalletdialog.cpp +++ b/src/qt/createwalletdialog.cpp @@ -52,6 +52,12 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) : } }); + connect(ui->blank_wallet_checkbox, &QCheckBox::toggled, [this](bool checked) { + if (!checked) { + ui->disable_privkeys_checkbox->setChecked(false); + } + }); + #ifndef USE_SQLITE ui->descriptor_checkbox->setToolTip(tr("Compiled without sqlite support (required for descriptor wallets)")); ui->descriptor_checkbox->setEnabled(false); From 44f91cbc9a1eb04e0631b8c41c575cb5b3db7891 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 6 Apr 2021 10:32:57 +0800 Subject: [PATCH 05/10] Merge #21597: test: Document race:validation_chainstatemanager_tests suppression fab19871bad1cbe15ec2193f01152eacbf14aeb1 test: Document race:validation_chainstatemanager_tests suppression (MarcoFalke) Pull request description: ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/21597/commits/fab19871bad1cbe15ec2193f01152eacbf14aeb1 practicalswift: ACK fab19871bad1cbe15ec2193f01152eacbf14aeb1 Tree-SHA512: 3f1838b4cf11eba768ce06826cd4b57c9065669b61a5530af44216fc96535ebf37124b47a8de8f72aedf32345157a72d2208cd63214481a9cb56c063f05db5dd --- test/sanitizer_suppressions/tsan | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index 7d9f96dc96..53ab7e6c2b 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -6,9 +6,6 @@ # Data races from zmq namespace race:zmq::* -# race (TODO fix) -race:validation_chainstatemanager_tests - # double locks (TODO fix) mutex:g_genesis_wait_mutex mutex:Interrupt @@ -58,6 +55,10 @@ race:InterruptRPC race:src/qt/test/* deadlock:src/qt/test/* +# Race in src/test/main.cpp +# Can be removed once upgraded to boost test 1.74 in depends +race:validation_chainstatemanager_tests + # External libraries deadlock:libdb race:libzmq From 1dc97c767949038180bf3ad82524df9d68376fa6 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Mon, 7 Jun 2021 16:01:38 +0200 Subject: [PATCH 06/10] Merge bitcoin/bitcoin#22149: test: Add temporary logging to debug #20975 faa94961d6e38392ba068381726ed4e033367b03 test: Add temporary logging to debug #20975 (MarcoFalke) Pull request description: to be reverted after a fix ACKs for top commit: laanwj: Code review ACK https://github.com/bitcoin/bitcoin/pull/22149/commits/faa94961d6e38392ba068381726ed4e033367b03 Tree-SHA512: 1f3103fcf4cad0af54e26c4d257bd824b128b5f2d2b81c302e861a829fd55d6a099fa476b79b30a71fe98975ae604b9e3ff31fd48a51d442389a9bd515e60ba0 --- test/functional/test_framework/blocktools.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index d0eb93ec3b..f797c4b6b8 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -223,6 +223,11 @@ def create_raw_transaction(node, txid, to_address, *, amount): signed_psbt = wrpc.walletprocesspsbt(psbt) psbt = signed_psbt['psbt'] final_psbt = node.finalizepsbt(psbt) + if not final_psbt["complete"]: + node.log.info(f'final_psbt={final_psbt}') + for w in node.listwallets(): + wrpc = node.get_wallet_rpc(w) + node.log.info(f'listunspent={wrpc.listunspent()}') assert_equal(final_psbt["complete"], True) return final_psbt['hex'] From 0505229c8946fe52369474290cdd79fdd9952aed Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 24 Jun 2021 16:01:18 +0200 Subject: [PATCH 07/10] Merge bitcoin/bitcoin#22327: cli: Avoid truncating -rpcwaittimeout fa34cb80248cc39a73fc393f65c3cfc62e849556 cli: Avoid truncating -rpcwaittimeout (MarcoFalke) Pull request description: `seconds` is not enough precision to "exactly" store a timestamp n seconds into the future. Improve the precision by using `microseconds`. Fixes #22325 Also, use chrono literals. ACKs for top commit: jonatack: ACK fa34cb80248cc39a73fc393f65c3cfc62e849556 review, debug-built, tested theStack: Tested ACK fa34cb80248cc39a73fc393f65c3cfc62e849556 Tree-SHA512: 7158da8545f9998a82bcc8636e04564efdb1e1be43b4288298c151b4df29ad47a2760259eefadd4a01db92ea18a1e017f3febc1cd8c69a4b28c86180229d8c90 --- src/bitcoin-cli.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 8d6f793832..b1631e410b 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -735,7 +735,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str // Execute and handle connection failures with -rpcwait. const bool fWait = gArgs.GetBoolArg("-rpcwait", false); const int timeout = gArgs.GetArg("-rpcwaittimeout", DEFAULT_WAIT_CLIENT_TIMEOUT); - const int64_t deadline = GetTime().count() + timeout; + const auto deadline{GetTime() + 1s * timeout}; do { try { @@ -748,9 +748,9 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str } break; // Connection succeeded, no need to retry. } catch (const CConnectionFailed& e) { - const int64_t now = GetTime().count(); + const auto now{GetTime()}; if (fWait && (timeout <= 0 || now < deadline)) { - UninterruptibleSleep(std::chrono::seconds{1}); + UninterruptibleSleep(1s); } else { throw CConnectionFailed(strprintf("timeout on transient error: %s", e.what())); } From a9b1575fe83aadaba4e1a75e81e17195a01d5fe4 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Sun, 22 Aug 2021 12:48:12 +1200 Subject: [PATCH 08/10] Merge bitcoin/bitcoin#22781: wallet: fix the behavior of IsHDEnabled, return false in case of a blank hd wallet. 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e the result of CWallet::IsHDEnabled() was initialized with true. (Saibato) Pull request description: the result of CWallet::IsHDEnabled() was initialized with true. But in case of no keys or a blank hd wallet the iterator would be skipped and not set to false but true, since the loop would be not entered. That had resulted in a wrong return and subsequent false HD and watch-only icon display in GUi when reloading a wallet after closing. ACKs for top commit: achow101: ACK 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e hebasto: ACK 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e theStack: utACK 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e meshcollider: utACK 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e Tree-SHA512: 79b976594f7174d05c29fe3819037ead59aaef27498d95415ceba74d633a8e035f6b03b521000ac3370684a8cb09319d8be1a443ce2d29b3ff4089e399f6b719 --- src/wallet/wallet.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e106d5ef72..6e50dcdfd9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1617,9 +1617,10 @@ CAmount CWallet::GetChange(const CTransaction& tx) const bool CWallet::IsHDEnabled() const { // All Active ScriptPubKeyMans must be HD for this to be true - bool result = true; + bool result = false; for (const auto& spk_man : GetActiveScriptPubKeyMans()) { - result &= spk_man->IsHDEnabled(); + if (!spk_man->IsHDEnabled()) return false; + result = true; } return result; } From 51630d2e5ebaa533ed3035761e18c287e30bfa32 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 30 Aug 2021 10:07:28 +0200 Subject: [PATCH 09/10] Merge bitcoin/bitcoin#22824: refactor: remove RecursiveMutex cs_nBlockSequenceId 0bd882b7405414b5355e69a9fdcd7a533e504b6b refactor: remove RecursiveMutex cs_nBlockSequenceId (Sebastian Falbesoner) Pull request description: The RecursiveMutex `cs_nBlockSequenceId` is only used at one place in `CChainState::ReceivedBlockTransactions()` to atomically read-and-increment the nBlockSequenceId member: https://github.com/bitcoin/bitcoin/blob/83daf47898f8a79cb20d20316c64becd564cf54c/src/validation.cpp#L2973-L2976 ~~For this simple use-case, we can make the member `std::atomic` instead to achieve the same result (see https://en.cppreference.com/w/cpp/atomic/atomic/operator_arith).~~ ~~This is related to #19303. As suggested in the issue, I first planned to change the `RecursiveMutex` to `Mutex` (still possible if the change doesn't get Concept ACKs), but using a Mutex for this simple operation seems to be overkill. Note that at the time when this mutex was introduced (PR #3370, commit 75f51f2a63e0ebe34ab290c2b7141dd240b98c3b) `std::atomic` were not used in the codebase yet -- according to `git log -S std::atomic` they have first appeared in 2016 (commit 7e908c7b826cedbf29560ce7a668af809ee71524), probably also because the compilers didn't support them properly earlier.~~ At this point, the cs_main lock is set, hence we can use a plain int for the member and mark it as guarded by cs_main. ACKs for top commit: Zero-1729: ACK 0bd882b promag: Code review ACK 0bd882b7405414b5355e69a9fdcd7a533e504b6b. hebasto: ACK 0bd882b7405414b5355e69a9fdcd7a533e504b6b Tree-SHA512: 435271ac8f877074099ddb31436665b500e555f7cab899e5c8414af299b154d1249996be500e8fdeff64e4639bcaf7386e12510b738ec6f20e415e7e35afaea9 --- src/validation.cpp | 5 +---- src/validation.h | 5 ++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 3387f746b9..9a7336915f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3688,10 +3688,7 @@ void CChainState::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pi CBlockIndex *pindex = queue.front(); queue.pop_front(); pindex->nChainTx = (pindex->pprev ? pindex->pprev->nChainTx : 0) + pindex->nTx; - { - LOCK(cs_nBlockSequenceId); - pindex->nSequenceId = nBlockSequenceId++; - } + pindex->nSequenceId = nBlockSequenceId++; if (m_chain.Tip() == nullptr || !setBlockIndexCandidates.value_comp()(pindex, m_chain.Tip())) { if (!(pindex->nStatus & BLOCK_CONFLICT_CHAINLOCK)) { setBlockIndexCandidates.insert(pindex); diff --git a/src/validation.h b/src/validation.h index 3bfe6b872d..1b6b1bb28c 100644 --- a/src/validation.h +++ b/src/validation.h @@ -629,9 +629,8 @@ protected: * Every received block is assigned a unique and increasing identifier, so we * know which one to give priority in case of a fork. */ - RecursiveMutex cs_nBlockSequenceId; /** Blocks loaded from disk are assigned id 0, so start the counter at 1. */ - int32_t nBlockSequenceId = 1; + int32_t nBlockSequenceId GUARDED_BY(::cs_main) = 1; /** Decreasing counter (used by subsequent preciousblock calls). */ int32_t nBlockReverseSequenceId = -1; /** chainwork for the last block that preciousblock has been applied to. */ @@ -828,7 +827,7 @@ public: void PruneBlockIndexCandidates(); - void UnloadBlockIndex(); + void UnloadBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Check whether we are doing an initial block download (synchronizing from disk or network) */ bool IsInitialBlockDownload() const; From d5d1a714fb5f66b534f19703b7de73dd910bce6c Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 19 Feb 2022 19:52:56 +0000 Subject: [PATCH 10/10] Merge bitcoin/bitcoin#24390: test: Remove suppression no longer needed with headers-only Boost.Test 81738d2881253f28b69666ada2a01ebb353f503a test: Remove suppression no longer needed with headers-only Boost.Test (Hennadii Stepanov) Pull request description: It appears, that moving to [headers-only](https://github.com/bitcoin/bitcoin/pull/24301) Boost.Test makes the removed suppression unneeded even without [bumping](https://github.com/bitcoin/bitcoin/pull/24383) boost version. ACKs for top commit: MarcoFalke: cr ACK 81738d2881253f28b69666ada2a01ebb353f503a Tree-SHA512: e60443f79a2e38cc78fceeff5c2956d622e8a10730129f9c27c14aef59bc6fa0894b8011e6191530443bf3165f78da978bc08ad04248ddb65e2da373264afa6a --- test/sanitizer_suppressions/tsan | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index 53ab7e6c2b..b76ee223b3 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -55,10 +55,6 @@ race:InterruptRPC race:src/qt/test/* deadlock:src/qt/test/* -# Race in src/test/main.cpp -# Can be removed once upgraded to boost test 1.74 in depends -race:validation_chainstatemanager_tests - # External libraries deadlock:libdb race:libzmq