From 61a01403629d5756bb479814dd10c063495c61e9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 2 Dec 2021 13:56:51 +0100 Subject: [PATCH 01/55] Merge bitcoin/bitcoin#23642: refactor: Call type-solver earlier in decodescript 33330702081f67cb05fd86e00b252f6355249513 refactor: Call type-solver earlier in decodescript (MarcoFalke) fab0d998f4bf0f3f09afa51845d91408dd484408 style: Remove whitespace (MarcoFalke) Pull request description: The current logic is a bit confusing. First creating the `UniValue` return dict, then parsing it again to get the type as `std::string`. Clean this up by using a strong type `TxoutType`. Also, remove whitespace. ACKs for top commit: shaavan: ACK 33330702081f67cb05fd86e00b252f6355249513 theStack: Code-review ACK 33330702081f67cb05fd86e00b252f6355249513 Tree-SHA512: 49db7bc614d2491cd3ec0177d21ad1e9924dbece1eb5635290cd7fd18cb30adf4711b891daf522e7c4f6baab3033b66393bbfcd1d4726f24f90a433124f925d6 --- src/rpc/rawtransaction.cpp | 52 +++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 09e7c7d862..b4aab57602 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -869,29 +869,30 @@ static RPCHelpMan decoderawtransaction() static RPCHelpMan decodescript() { - return RPCHelpMan{"decodescript", - "\nDecode a hex-encoded script.\n", - { - {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "the hex-encoded script"}, - }, - RPCResult{ - RPCResult::Type::OBJ, "", "", + return RPCHelpMan{ + "decodescript", + "\nDecode a hex-encoded script.\n", + { + {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "the hex-encoded script"}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR, "asm", "Script public key"}, + {RPCResult::Type::STR, "type", "The output type (e.g. " + GetAllOutputTypes() + ")"}, + {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"}, + {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, + {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses", { - {RPCResult::Type::STR, "asm", "Script public key"}, - {RPCResult::Type::STR, "type", "The output type (e.g. "+GetAllOutputTypes()+")"}, - {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses", - { - {RPCResult::Type::STR, "address", "Dash address"}, - }}, - {RPCResult::Type::STR, "p2sh", "address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH)"}, - } - }, - RPCExamples{ - HelpExampleCli("decodescript", "\"hexstring\"") - + HelpExampleRpc("decodescript", "\"hexstring\"") - }, + {RPCResult::Type::STR, "address", "Dash address"}, + }}, + {RPCResult::Type::STR, "p2sh", "address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH)"}, + }, + }, + RPCExamples{ + HelpExampleCli("decodescript", "\"hexstring\"") + + HelpExampleRpc("decodescript", "\"hexstring\"") + }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { RPCTypeCheck(request.params, {UniValue::VSTR}); @@ -906,11 +907,10 @@ static RPCHelpMan decodescript() } ScriptPubKeyToUniv(script, r, /* fIncludeHex */ false); - UniValue type; - - type = find_value(r, "type"); + std::vector> solutions_data; + const TxoutType which_type{Solver(script, solutions_data)}; - if (type.isStr() && type.get_str() != "scripthash") { + if (which_type != TxoutType::SCRIPTHASH) { // P2SH cannot be wrapped in a P2SH. If this script is already a P2SH, // don't return the address for a P2SH of the P2SH. r.pushKV("p2sh", EncodeDestination(ScriptHash(script))); From 995cae46af3e0360dea678ab1fdd58b8e7f68b17 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 11 Oct 2021 11:27:34 +0200 Subject: [PATCH 02/55] Merge bitcoin/bitcoin#22794: test: Verify if wallet is compiled in rpc_invalid_address_message.py test c2fbdca54915e85ffafe1a88858d3c70c2b1afe8 Add BECH32_INVALID_VERSION test (lsilva01) b142f79ddb91a44f29fcb2afb7f2edf3ca17e168 skip test_getaddressinfo() if wallet is disabled (lsilva01) Pull request description: Most of `test/functional/rpc_invalid_address_message.py` does not requires wallet. But if the project is compiled in disable-wallet mode, the entire test will be skipped. This PR changes the test to run the RPC tests first and then checks if the wallet is compiled. ACKs for top commit: stratospher: tested ACK c2fbdca Tree-SHA512: 11fa2fedf4a15aa45e3f12490df8e22290a867d5de594247211499533c32289c68c0b60bd42dbf8305e43dbcc042789e7139317ef5c9f8cf386f2d84c91b9ac2 --- test/functional/rpc_invalid_address_message.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py index bb0180a0d3..59d2f55fc1 100755 --- a/test/functional/rpc_invalid_address_message.py +++ b/test/functional/rpc_invalid_address_message.py @@ -22,9 +22,6 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - def test_validateaddress(self): node = self.nodes[0] @@ -50,7 +47,10 @@ def test_getaddressinfo(self): def run_test(self): self.test_validateaddress() - self.test_getaddressinfo() + + if self.is_wallet_compiled(): + self.init_wallet(0) + self.test_getaddressinfo() if __name__ == '__main__': From 66e77f78790b2a301133a4fa01cf9d6847b00438 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 20 Oct 2021 17:43:51 +0200 Subject: [PATCH 03/55] Merge bitcoin/bitcoin#23316: test: make the node param explicit in init_wallet() 7b3c9e4ee8feb552dc0fc4347db2d06e60894a9f Make explicit the node param in init_wallet() (lsilva01) Pull request description: This PR changes the definition of `def init_wallet(self, i)` to `def init_wallet(self, *, node)` to make the node parameter explicit, as suggested in https://github.com/bitcoin/bitcoin/pull/22794#discussion_r713287448 . ACKs for top commit: stratospher: tested ACK 7b3c9e4. Tree-SHA512: 2ef036f4c2110b2f7dc893dc6eea8faa0a18edd7f8f59b25460a6c544df7238175ddd6a0d766e2bb206326b1c9afc84238c75613a0f01eeda89a8ccb7d86a4f1 --- test/functional/rpc_invalid_address_message.py | 2 +- test/functional/test_framework/test_framework.py | 8 ++++---- test/functional/wallet_backup.py | 6 +++--- test/functional/wallet_listdescriptors.py | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py index 59d2f55fc1..18d85d6770 100755 --- a/test/functional/rpc_invalid_address_message.py +++ b/test/functional/rpc_invalid_address_message.py @@ -49,7 +49,7 @@ def run_test(self): self.test_validateaddress() if self.is_wallet_compiled(): - self.init_wallet(0) + self.init_wallet(node=0) self.test_getaddressinfo() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 6aa7a4a53c..76d7a5bfed 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -469,12 +469,12 @@ def setup_nodes(self): def import_deterministic_coinbase_privkeys(self): for i in range(len(self.nodes)): - self.init_wallet(i) + self.init_wallet(node=i) - def init_wallet(self, i): - wallet_name = self.default_wallet_name if self.wallet_names is None else self.wallet_names[i] if i < len(self.wallet_names) else False + def init_wallet(self, *, node): + wallet_name = self.default_wallet_name if self.wallet_names is None else self.wallet_names[node] if node < len(self.wallet_names) else False if wallet_name is not False: - n = self.nodes[i] + n = self.nodes[node] if wallet_name is not None: n.createwallet(wallet_name=wallet_name, descriptors=self.options.descriptors, load_on_startup=True) n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase', rescan=True) diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 9371cee0f5..5e188dce4c 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -135,9 +135,9 @@ def restore_wallet_existent_name(self): assert os.path.exists(wallet_file) def init_three(self): - self.init_wallet(0) - self.init_wallet(1) - self.init_wallet(2) + self.init_wallet(node=0) + self.init_wallet(node=1) + self.init_wallet(node=2) def run_test(self): self.log.info("Generating initial blockchain") diff --git a/test/functional/wallet_listdescriptors.py b/test/functional/wallet_listdescriptors.py index b6c82e64e9..166443c525 100755 --- a/test/functional/wallet_listdescriptors.py +++ b/test/functional/wallet_listdescriptors.py @@ -23,7 +23,7 @@ def skip_test_if_missing_module(self): self.skip_if_no_sqlite() # do not create any wallet by default - def init_wallet(self, i): + def init_wallet(self, *, node): return def run_test(self): From b212ca051542ba3c4a336b1d31c92a51342d32a2 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Mon, 28 Feb 2022 13:00:14 +0100 Subject: [PATCH 04/55] Merge bitcoin/bitcoin#24365: wallet: Don't generate keys for wallets with private keys disabled during upgradewallet c7376cc8d728f3a7c40f79bd57e7cef685def723 tests: Test upgrading wallet with privkeys disabled (Andrew Chow) 3d985d4f43b5344f998bcf6db22d02782e647a2a wallet: Don't generate keys when privkeys disabled when upgrading (Andrew Chow) Pull request description: When we're upgrading a wallet, we shouldn't be trying to generate new keys for wallets where private keys are disabled. Fixes #23610 ACKs for top commit: laanwj: Code review ACK c7376cc8d728f3a7c40f79bd57e7cef685def723 benthecarman: tACK c7376cc8d728f3a7c40f79bd57e7cef685def723 this fixed the issue for me Tree-SHA512: fa07cf37df9196ff98671bb1ce5c9aa0bab46495066b4dab796d7e8e5d5c7adb414ff56adae4fd3e15658a610995bd19a9e1edb00c46144b0df635c5b343f3a6 --- test/functional/wallet_upgradewallet.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index cc2b4168c2..41547353ec 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -239,5 +239,16 @@ def copy_non_hd(): desc_wallet = self.nodes[0].get_wallet_rpc("desc_upgrade") self.test_upgradewallet(desc_wallet, previous_version=120200, expected_version=120200) + self.log.info("Checking that descriptor wallets without privkeys do nothing, successfully") + self.nodes[0].createwallet(wallet_name="desc_upgrade_nopriv", descriptors=True, disable_private_keys=True) + desc_wallet = self.nodes[0].get_wallet_rpc("desc_upgrade_nopriv") + self.test_upgradewallet(desc_wallet, previous_version=120200, expected_version=120200) + + if self.is_bdb_compiled(): + self.log.info("Upgrading a wallet with private keys disabled") + self.nodes[0].createwallet(wallet_name="privkeys_disabled_upgrade", disable_private_keys=True, descriptors=False) + disabled_wallet = self.nodes[0].get_wallet_rpc("privkeys_disabled_upgrade") + self.test_upgradewallet(disabled_wallet, previous_version=120200, expected_version=120200) + if __name__ == '__main__': UpgradeWalletTest().main() From f16265dd50ffc46998d082fd4fa397f34da93406 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 12 Jan 2022 14:56:52 +0200 Subject: [PATCH 05/55] Merge bitcoin-core/gui#517: refactor, qt: Use std::chrono for parameters of QTimer methods 51250b0906e56b39488304208ad119c951b4ae7d refactor, qt: Use std::chrono for input_filter_delay constant (Hennadii Stepanov) f3bdc143b67e8a5e763071a0774f6d994ca35c57 refactor, qt: Add SHUTDOWN_POLLING_DELAY constant (Hennadii Stepanov) 0e193deb523a4fa04e0ee69bd66f917895802ac9 refactor, qt: Use std::chrono for non-zero arguments in QTimer methods (Hennadii Stepanov) 6f0da958116ecc0e06332fad2f490e37b6884166 refactor, qt: Use std::chrono in ConfirmMessage parameter (Hennadii Stepanov) 33d520ac538fcd6285fd958578f1bd26295592e4 refactor, qt: Use std::chrono for MODEL_UPDATE_DELAY constant (Hennadii Stepanov) Pull request description: Since Qt 5.8 `QTimer` methods have overloads that accept `std::chrono::milliseconds` arguments: - [`QTimer::singleShot`](https://doc.qt.io/archives/qt-5.9/qtimer.html#singleShot-8) - [`QTimer::start`](https://doc.qt.io/archives/qt-5.9/qtimer.html#start-2) ACKs for top commit: promag: Code review ACK 51250b0906e56b39488304208ad119c951b4ae7d. shaavan: reACK 51250b0906e56b39488304208ad119c951b4ae7d Tree-SHA512: aa843bb2322a84c0c2bb113d3b48d7bf02d7f09a770779dcde312c32887f973ef9445cdef42f39edaa599ff0f3d0457454f6153aa130efadd989e413d39c6062 --- src/qt/bitcoin.cpp | 5 +++-- src/qt/clientmodel.cpp | 3 ++- src/qt/guiconstants.h | 10 ++++++++-- src/qt/optionsdialog.cpp | 4 +++- src/qt/sendcoinsdialog.cpp | 5 +++-- src/qt/test/addressbooktests.cpp | 4 +++- src/qt/test/util.cpp | 4 +++- src/qt/test/util.h | 8 ++++++-- src/qt/test/wallettests.cpp | 1 + src/qt/transactionview.cpp | 5 +++-- src/qt/walletcontroller.cpp | 5 +++-- 11 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index debab9e2a2..b251da5d02 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -44,6 +44,7 @@ #endif // ENABLE_WALLET #include +#include #include #include @@ -398,10 +399,10 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead connect(paymentServer, &PaymentServer::message, [this](const QString& title, const QString& message, unsigned int style) { window->message(title, message, style); }); - QTimer::singleShot(100, paymentServer, &PaymentServer::uiReady); + QTimer::singleShot(100ms, paymentServer, &PaymentServer::uiReady); } #endif - pollShutdownTimer->start(200); + pollShutdownTimer->start(SHUTDOWN_POLLING_DELAY); } else { Q_EMIT splashFinished(); // Make sure splash screen doesn't stick around during shutdown quit(); // Exit first main loop invocation diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index d604e3ac79..5dd3e8198a 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -323,7 +324,7 @@ static void BlockTipChanged(ClientModel* clientmodel, SynchronizationState sync_ const bool throttle = (sync_state != SynchronizationState::POST_INIT && !fHeader) || sync_state == SynchronizationState::INIT_REINDEX; const int64_t now = throttle ? GetTimeMillis() : 0; int64_t& nLastUpdateNotification = fHeader ? nLastHeaderTipUpdateNotification : nLastBlockTipUpdateNotification; - if (throttle && now < nLastUpdateNotification + MODEL_UPDATE_DELAY) { + if (throttle && now < nLastUpdateNotification + count_milliseconds(MODEL_UPDATE_DELAY)) { return; } diff --git a/src/qt/guiconstants.h b/src/qt/guiconstants.h index 87f47b023f..4dd734b4d4 100644 --- a/src/qt/guiconstants.h +++ b/src/qt/guiconstants.h @@ -6,10 +6,16 @@ #ifndef BITCOIN_QT_GUICONSTANTS_H #define BITCOIN_QT_GUICONSTANTS_H +#include #include -/* Milliseconds between model updates */ -static const int MODEL_UPDATE_DELAY = 250; +using namespace std::chrono_literals; + +/* A delay between model updates */ +static constexpr auto MODEL_UPDATE_DELAY{250ms}; + +/* A delay between shutdown pollings */ +static constexpr auto SHUTDOWN_POLLING_DELAY{200ms}; /* AskPassphraseDialog -- Maximum passphrase length */ static const int MAX_PASSPHRASE_SIZE = 1024; diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 08da36cff8..1b537a9411 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -24,6 +24,8 @@ #include #include +#include + #include #include #include @@ -457,7 +459,7 @@ void OptionsDialog::showRestartWarning(bool fPersistent) ui->statusLabel->setText(tr("This change would require a client restart.")); // clear non-persistent status label after 10 seconds // Todo: should perhaps be a class attribute, if we extend the use of statusLabel - QTimer::singleShot(10000, this, &OptionsDialog::clearStatusLabel); + QTimer::singleShot(10s, this, &OptionsDialog::clearStatusLabel); } } diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 1319d1d836..79526f9381 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -24,10 +24,11 @@ #include #include #include +#include #include #include #include -#include +#include #include #include @@ -1080,7 +1081,7 @@ SendConfirmationDialog::SendConfirmationDialog(const QString& title, const QStri int SendConfirmationDialog::exec() { updateYesButton(); - countDownTimer.start(1000); + countDownTimer.start(1s); return QMessageBox::exec(); } diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 5eb00a1c32..3e0be6da11 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -19,6 +19,8 @@ #include #include +#include + #include #include #include @@ -39,7 +41,7 @@ void EditAddressAndSubmit( dialog->findChild("labelEdit")->setText(label); dialog->findChild("addressEdit")->setText(address); - ConfirmMessage(&warning_text, 5); + ConfirmMessage(&warning_text, 5ms); dialog->accept(); QCOMPARE(warning_text, expected_msg); } diff --git a/src/qt/test/util.cpp b/src/qt/test/util.cpp index 987d921f03..635dbcd1c5 100644 --- a/src/qt/test/util.cpp +++ b/src/qt/test/util.cpp @@ -2,6 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include + #include #include #include @@ -9,7 +11,7 @@ #include #include -void ConfirmMessage(QString* text, int msec) +void ConfirmMessage(QString* text, std::chrono::milliseconds msec) { QTimer::singleShot(msec, [text]() { for (QWidget* widget : QApplication::topLevelWidgets()) { diff --git a/src/qt/test/util.h b/src/qt/test/util.h index df5931a032..f50a6b6c61 100644 --- a/src/qt/test/util.h +++ b/src/qt/test/util.h @@ -5,7 +5,11 @@ #ifndef BITCOIN_QT_TEST_UTIL_H #define BITCOIN_QT_TEST_UTIL_H -#include +#include + +QT_BEGIN_NAMESPACE +class QString; +QT_END_NAMESPACE /** * Press "Ok" button in message box dialog. @@ -13,6 +17,6 @@ * @param text - Optionally store dialog text. * @param msec - Number of milliseconds to pause before triggering the callback. */ -void ConfirmMessage(QString* text = nullptr, int msec = 0); +void ConfirmMessage(QString* text, std::chrono::milliseconds msec); #endif // BITCOIN_QT_TEST_UTIL_H diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 3a8b297037..fe16db6c4e 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 55760147c5..fdc2e5cb85 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -114,8 +115,8 @@ TransactionView::TransactionView(QWidget* parent) : amountWidget->setObjectName("amountWidget"); hlayout->addWidget(amountWidget); - // Delay before filtering transactions in ms - static const int input_filter_delay = 200; + // Delay before filtering transactions + static constexpr auto input_filter_delay{200ms}; QTimer* amount_typing_delay = new QTimer(this); amount_typing_delay->setSingleShot(true); diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 2133999b9b..b39f630ccb 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -271,12 +272,12 @@ void CreateWalletActivity::createWallet() flags |= WALLET_FLAG_DESCRIPTORS; } - QTimer::singleShot(500, worker(), [this, name, flags] { + QTimer::singleShot(500ms, worker(), [this, name, flags] { std::unique_ptr wallet = node().walletLoader().createWallet(name, m_passphrase, flags, m_error_message, m_warning_message); if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet)); - QTimer::singleShot(500, this, &CreateWalletActivity::finish); + QTimer::singleShot(500ms, this, &CreateWalletActivity::finish); }); } From 11eeae2ab9fcf97f355bd73d9194708d6e79a136 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 2 Feb 2022 15:00:19 +0100 Subject: [PATCH 06/55] Merge bitcoin/bitcoin#24219: Fix implicit-integer-sign-change in bloom fad84a25956ec081f22aebbda309d168a3dc0004 refactor: Fixup uint64_t-cast style in touched line (MarcoFalke) fa041878de786f5be74ec74a06ec407c99ca8656 Fix implicit-integer-sign-change in bloom (MarcoFalke) Pull request description: Signed values don't really make sense when using `std::vector::operator[]`. Fix that and remove the suppression. ACKs for top commit: PastaPastaPasta: utACK fad84a25956ec081f22aebbda309d168a3dc0004 theStack: Code-review ACK fad84a25956ec081f22aebbda309d168a3dc0004 Tree-SHA512: 7139dd9aa098c41e4af1b6e63dd80e71a92b0a98062d1676b01fe550ffa8e21a5f84a578afa7a536d70dad1b8a5017625e3a9e2dda6f864b452ec77b130ddf2a --- src/common/bloom.cpp | 6 +++--- test/sanitizer_suppressions/ubsan | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/common/bloom.cpp b/src/common/bloom.cpp index 6fc86cf72a..315258fb14 100644 --- a/src/common/bloom.cpp +++ b/src/common/bloom.cpp @@ -314,8 +314,8 @@ void CRollingBloomFilter::insert(Span vKey) /* FastMod works with the upper bits of h, so it is safe to ignore that the lower bits of h are already used for bit. */ uint32_t pos = FastRange32(h, data.size()); /* The lowest bit of pos is ignored, and set to zero for the first bit, and to one for the second. */ - data[pos & ~1] = (data[pos & ~1] & ~(((uint64_t)1) << bit)) | ((uint64_t)(nGeneration & 1)) << bit; - data[pos | 1] = (data[pos | 1] & ~(((uint64_t)1) << bit)) | ((uint64_t)(nGeneration >> 1)) << bit; + data[pos & ~1U] = (data[pos & ~1U] & ~(uint64_t{1} << bit)) | (uint64_t(nGeneration & 1)) << bit; + data[pos | 1] = (data[pos | 1] & ~(uint64_t{1} << bit)) | (uint64_t(nGeneration >> 1)) << bit; } } @@ -326,7 +326,7 @@ bool CRollingBloomFilter::contains(Span vKey) const int bit = h & 0x3F; uint32_t pos = FastRange32(h, data.size()); /* If the relevant bit is not set in either data[pos & ~1] or data[pos | 1], the filter does not contain vKey */ - if (!(((data[pos & ~1] | data[pos | 1]) >> bit) & 1)) { + if (!(((data[pos & ~1U] | data[pos | 1]) >> bit) & 1)) { return false; } } diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 42df86bbdd..ba08cef801 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -48,7 +48,6 @@ implicit-integer-sign-change:*/include/c++/ implicit-integer-sign-change:*/new_allocator.h implicit-integer-sign-change:addrman.h implicit-integer-sign-change:bech32.cpp -implicit-integer-sign-change:common/bloom.cpp implicit-integer-sign-change:chain.cpp implicit-integer-sign-change:chain.h implicit-integer-sign-change:compat/stdin.cpp From 2a2a2693d0fd429f6aa193809b7905e77aada25e Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 13 Oct 2021 13:48:36 +0200 Subject: [PATCH 07/55] Merge bitcoin/bitcoin#23253: bitcoin-tx: Reject non-integral and out of range int strings fa6f29de516c7af5206b91b59ada466032329250 bitcoin-tx: Reject non-integral and out of range multisig numbers (MarcoFalke) fafab8ea5e6ed6b87fac57a5cd16a8135236cdd6 bitcoin-tx: Reject non-integral and out of range sequence ids (MarcoFalke) fa53d3d8266ad0257315d07b71b4f8a711134622 test: Check that bitcoin-tx accepts whitespace around sequence id and multisig numbers (MarcoFalke) Pull request description: Seems odd to silently accept arbitrary strings that don't even represent integral values. Fix that. ACKs for top commit: practicalswift: cr ACK fa6f29de516c7af5206b91b59ada466032329250 laanwj: Code review ACK fa6f29de516c7af5206b91b59ada466032329250 Empact: Code review ACK https://github.com/bitcoin/bitcoin/pull/23253/commits/fa6f29de516c7af5206b91b59ada466032329250 promag: Code review ACK fa6f29de516c7af5206b91b59ada466032329250. Tree-SHA512: e31f7f21fe55ac069e755557bdbcae8d5d29e20ff82e441ebdfc65153e3a31a4edd46ad3e6dea5190ecbd1b8ea5a8f94daa5d59a3b7558e46e794e30db0e6c79 --- src/bitcoin-tx.cpp | 19 ++++++++--- test/util/data/bitcoin-util-test.json | 48 +++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 0eb122f086..6f98a71622 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -220,6 +220,16 @@ static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal) tx.nLockTime = (unsigned int) newLocktime; } +template +static T TrimAndParse(const std::string& int_str, const std::string& err) +{ + const auto parsed{ToIntegral(TrimString(int_str))}; + if (!parsed.has_value()) { + throw std::runtime_error(err + " '" + int_str + "'"); + } + return parsed.value(); +} + static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInput) { std::vector vStrInputParts = SplitString(strInput, ':'); @@ -245,8 +255,9 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu // extract the optional sequence number uint32_t nSequenceIn = CTxIn::SEQUENCE_FINAL; - if (vStrInputParts.size() > 2) - nSequenceIn = std::stoul(vStrInputParts[2]); + if (vStrInputParts.size() > 2) { + nSequenceIn = TrimAndParse(vStrInputParts.at(2), "invalid TX sequence id"); + } // append to transaction input list CTxIn txin(txid, vout, CScript(), nSequenceIn); @@ -324,10 +335,10 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s CAmount value = ExtractAndValidateValue(vStrInputParts[0]); // Extract REQUIRED - uint32_t required = stoul(vStrInputParts[1]); + const uint32_t required{TrimAndParse(vStrInputParts.at(1), "invalid multisig required number")}; // Extract NUMKEYS - uint32_t numkeys = stoul(vStrInputParts[2]); + const uint32_t numkeys{TrimAndParse(vStrInputParts.at(2), "invalid multisig total number")}; // Validate there are the correct number of pubkeys if (vStrInputParts.size() < numkeys + 3) diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 5d78d2edb1..4722f684f1 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -425,6 +425,30 @@ "output_cmp": "txcreatedata2.json", "description": "Creates a new transaction with one input, one address output and one data (zero value) output (output in json)" }, + { "exec": "./dash-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0:11aa"], + "return_code": 1, + "error_txt": "error: invalid TX sequence id '11aa'", + "description": "Try to parse a sequence number outside the allowed range" + }, + { "exec": "./dash-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0:-1"], + "return_code": 1, + "error_txt": "error: invalid TX sequence id '-1'", + "description": "Try to parse a sequence number outside the allowed range" + }, + { "exec": "./dash-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0:4294967296"], + "return_code": 1, + "error_txt": "error: invalid TX sequence id '4294967296'", + "description": "Try to parse a sequence number outside the allowed range" + }, { "exec": "./dash-tx", "args": ["-create", @@ -433,6 +457,14 @@ "output_cmp": "txcreatedata_seq0.hex", "description": "Creates a new transaction with one input with sequence number and one address output" }, + { "exec": "./dash-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0: 4294967293 ", + "outaddr=0.18:XijDvbYpPmznwgpWD3DkdYNfGmRP2KoVSk"], + "output_cmp": "txcreatedata_seq0.hex", + "description": "Creates a new transaction with one input with sequence number (+whitespace) and one address output" + }, { "exec": "./dash-tx", "args": ["-json", @@ -457,15 +489,27 @@ "output_cmp": "txcreatedata_seq1.json", "description": "Adds a new input with sequence number to a transaction (output in json)" }, + { "exec": "./dash-tx", + "args": ["-create", "outmultisig=1:-2:3:02a5:021:02df", "nversion=1"], + "return_code": 1, + "error_txt": "error: invalid multisig required number '-2'", + "description": "Try to parse a multisig number outside the allowed range" + }, + { "exec": "./dash-tx", + "args": ["-create", "outmultisig=1:2:3a:02a5:021:02df", "nversion=1"], + "return_code": 1, + "error_txt": "error: invalid multisig total number '3a'", + "description": "Try to parse a multisig number outside the allowed range" + }, { "exec": "./dash-tx", "args": ["-create", "outmultisig=1:2:3:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397:021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d:02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485", "nversion=1"], "output_cmp": "txcreatemultisig1.hex", "description": "Creates a new transaction with a single 2-of-3 multisig output" }, { "exec": "./dash-tx", - "args": ["-json", "-create", "outmultisig=1:2:3:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397:021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d:02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485", "nversion=1"], + "args": ["-json", "-create", "outmultisig=1: 2: 3:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397:021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d:02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485", "nversion=1"], "output_cmp": "txcreatemultisig1.json", - "description": "Creates a new transaction with a single 2-of-3 multisig output (output in json)" + "description": "Creates a new transaction with a single 2-of-3 multisig output (with whitespace, output in json)" }, { "exec": "./dash-tx", "args": ["-create", "outmultisig=1:2:3:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397:021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d:02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485:S", "nversion=1"], From f147373a324a22383f86684ef87c3ea0f3ea5da2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 28 Feb 2022 08:34:27 +0100 Subject: [PATCH 08/55] Merge bitcoin/bitcoin#24449: fuzz: FuzzedFileProvider::write should not return negative value fc471814dc34abb4d5479803ebb1033b572eda43 fuzz: FuzzedFileProvider::write should not return negative value (eugene) Pull request description: Doing so can lead to a glibc crash (from 2005 but I think it's relevant https://sourceware.org/bugzilla/show_bug.cgi?id=2074). Also the manpage for fopencookie warns against this: https://man7.org/linux/man-pages/man3/fopencookie.3.html. This would invalidate the autofile seeds (and maybe others?) in qa-assets. On another note, I noticed that FuzzedFileProvider::seek has some confusing behavior with SEEK_END. It seems to me that if these handlers are supposed to mimic the real functions, that SEEK_END would use the offset from the end of the stream, rather than changing the offset with a random value between 0 and 4096. I could also open a PR to fix SEEK_END, but it would invalidate the seeds. ACKs for top commit: MarcoFalke: cr ACK fc471814dc34abb4d5479803ebb1033b572eda43 Tree-SHA512: 9db41637f0df7f2b2407b82531cbc34f4ba9393063b63ec6786372e808fe991f7f24df45936c203fe0f9fc49686180c65ad57c2ce7d49e0c5402240616bcfede --- src/test/fuzz/util.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index bd55d99f97..ddfbc97dbf 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -456,7 +456,7 @@ ssize_t FuzzedFileProvider::write(void* cookie, const char* buf, size_t size) SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider); const ssize_t n = fuzzed_file->m_fuzzed_data_provider.ConsumeIntegralInRange(0, size); if (AdditionOverflow(fuzzed_file->m_offset, (int64_t)n)) { - return fuzzed_file->m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; + return 0; } fuzzed_file->m_offset += n; return n; From 39316088585984faf4ea422745e64f0adef916f8 Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 14 Sep 2021 11:10:08 +0200 Subject: [PATCH 09/55] Merge bitcoin/bitcoin#22543: test: Use MiniWallet in mempool_limit.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 08634e82c68ea1be79e1395f4f551082f497023f fix typos in logging messages (ShubhamPalriwala) d447ded6babebe7c7948e585c9e78bf34dbef226 replace: self.nodes[0] with node (ShubhamPalriwala) dddca3899c4738e512313a85aeb006310e34e31f test: use MiniWallet in mempool_limit.py (ShubhamPalriwala) Pull request description: This is a PR proposed in #20078 This PR enables running another non-wallet functional test even when the wallet is disabled thanks to the MiniWallet, i.e. it can be run even when bitcoin-core is compiled with --disable-wallet. It also includes changes in wallet.py in the form of a new method, `create_large_transactions()` for the MiniWallet to create large transactions. Efforts for this feature started in #20874 but were not continued and that PR was closed hence I picked this up. To test this PR locally, compile and build bitcoin-core without the wallet and run: ``` $ test/functional/mempool_limit.py ``` ACKs for top commit: amitiuttarwar: ACK 08634e8, only git changes since last push (and one new line). Zero-1729: ACK 08634e82c68ea1be79e1395f4f551082f497023f 🧉 Tree-SHA512: 0f744ad26bf7a5a784aac1ed5077b59c95a36d1ff3ad0087ffd10ac8d5979f7362c63c20c2ce2bfa650fda02dfbcd60b1fceee049a2465c8d221cce51c20369f --- test/functional/mempool_limit.py | 83 +++++++++++++++++--------------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index 3229537737..01e2add039 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -6,8 +6,11 @@ from decimal import Decimal +from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error, create_confirmed_utxos, create_lots_of_big_transactions, gen_return_txouts +from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error, gen_return_txouts +from test_framework.wallet import MiniWallet + class MempoolLimitTest(BitcoinTestFramework): def set_test_params(self): @@ -20,55 +23,59 @@ def set_test_params(self): ]] self.supports_cli = False - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() + def send_large_txs(self, node, miniwallet, txouts, fee_rate, tx_batch_size): + for _ in range(tx_batch_size): + tx = miniwallet.create_self_transfer(from_node=node, fee_rate=fee_rate)['tx'] + for txout in txouts: + tx.vout.append(txout) + miniwallet.sendrawtransaction(from_node=node, tx_hex=tx.serialize().hex()) def run_test(self): txouts = gen_return_txouts() - relayfee = self.nodes[0].getnetworkinfo()['relayfee'] + node=self.nodes[0] + miniwallet = MiniWallet(node) + relayfee = node.getnetworkinfo()['relayfee'] + + self.log.info('Check that mempoolminfee is minrelaytxfee') + assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) + assert_equal(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) - self.log.info('Check that mempoolminfee is minrelytxfee') - assert_equal(self.nodes[0].getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) - assert_equal(self.nodes[0].getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) + tx_batch_size = 25 + num_of_batches = 3 + # Generate UTXOs to flood the mempool + # 1 to create a tx initially that will be evicted from the mempool later + # 3 batches of multiple transactions with a fee rate much higher than the previous UTXO + # And 1 more to verify that this tx does not get added to the mempool with a fee rate less than the mempoolminfee + self.generate(miniwallet, 1 + (num_of_batches * tx_batch_size) + 1) - txids = [] - utxos = create_confirmed_utxos(self, relayfee, self.nodes[0], 491) + # Mine 99 blocks so that the UTXOs are allowed to be spent + self.generate(node, COINBASE_MATURITY - 1) self.log.info('Create a mempool tx that will be evicted') - us0 = utxos.pop() - inputs = [{ "txid" : us0["txid"], "vout" : us0["vout"]}] - outputs = {self.nodes[0].getnewaddress() : 0.0001} - tx = self.nodes[0].createrawtransaction(inputs, outputs) - self.nodes[0].settxfee(relayfee) # specifically fund this tx with low fee - txF = self.nodes[0].fundrawtransaction(tx) - self.nodes[0].settxfee(0) # return to automatic fee selection - txFS = self.nodes[0].signrawtransactionwithwallet(txF['hex']) - txid = self.nodes[0].sendrawtransaction(txFS['hex']) - - relayfee = self.nodes[0].getnetworkinfo()['relayfee'] - base_fee = relayfee*100 - for i in range (3): - txids.append([]) - txids[i] = create_lots_of_big_transactions(self.nodes[0], txouts, utxos[30*i:30*i+30], 30, (i+1)*base_fee) + tx_to_be_evicted_id = miniwallet.send_self_transfer(from_node=node, fee_rate=relayfee)["txid"] + + # Increase the tx fee rate massively to give the subsequent transactions a higher priority in the mempool + base_fee = relayfee * 1000 + + self.log.info("Fill up the mempool with txs with higher fee rate") + for batch_of_txid in range(num_of_batches): + fee_rate=(batch_of_txid + 1) * base_fee + self.send_large_txs(node, miniwallet, txouts, fee_rate, tx_batch_size) self.log.info('The tx should be evicted by now') - assert txid not in self.nodes[0].getrawmempool() - txdata = self.nodes[0].gettransaction(txid) - assert txdata['confirmations'] == 0 #confirmation should still be 0 + # The number of transactions created should be greater than the ones present in the mempool + assert_greater_than(tx_batch_size * num_of_batches, len(node.getrawmempool())) + # Initial tx created should not be present in the mempool anymore as it had a lower fee rate + assert tx_to_be_evicted_id not in node.getrawmempool() - self.log.info('Check that mempoolminfee is larger than minrelytxfee') - assert_equal(self.nodes[0].getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) - assert_greater_than(self.nodes[0].getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) + self.log.info('Check that mempoolminfee is larger than minrelaytxfee') + assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) + assert_greater_than(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) + # Deliberately try to create a tx with a fee less than the minimum mempool fee to assert that it does not get added to the mempool self.log.info('Create a mempool tx that will not pass mempoolminfee') - us0 = utxos.pop() - inputs = [{ "txid" : us0["txid"], "vout" : us0["vout"]}] - outputs = {self.nodes[0].getnewaddress() : 0.0001} - tx = self.nodes[0].createrawtransaction(inputs, outputs) - # specifically fund this tx with a fee < mempoolminfee, >= than minrelaytxfee - txF = self.nodes[0].fundrawtransaction(tx, {'feeRate': relayfee}) - txFS = self.nodes[0].signrawtransactionwithwallet(txF['hex']) - assert_raises_rpc_error(-26, "mempool min fee not met", self.nodes[0].sendrawtransaction, txFS['hex']) + assert_raises_rpc_error(-26, "mempool min fee not met", miniwallet.send_self_transfer, from_node=node, fee_rate=relayfee, mempool_valid=False) + if __name__ == '__main__': MempoolLimitTest().main() From 620146bcb8fe0935409ee2eeb33a9f1a3a91e6bd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Oct 2024 06:28:12 +0000 Subject: [PATCH 10/55] chore: sync chainstate loading logic with upstream Match formatting with what upcoming commits expect to limit conflicts --- src/init.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 147f845859..8c56f23abf 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1869,9 +1869,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) uiInterface.InitMessage(_("Loading block index…").translated); do { - bool failed_verification = false; const auto load_block_index_start_time{SteadyClock::now()}; - try { LOCK(cs_main); @@ -1997,6 +1995,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // block tree into BlockIndex()! bool failed_chainstate_init = false; + for (CChainState* chainstate : chainman.GetAll()) { chainstate->InitCoinsDB( /* cache_size_bytes */ nCoinDBCache, @@ -2065,6 +2064,16 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) strLoadError = _("Error upgrading evo database for EHF"); break; } + } catch (const std::exception& e) { + LogPrintf("%s\n", e.what()); + strLoadError = _("Error opening block database"); + break; + } + + bool failed_verification = false; + + try { + LOCK(cs_main); for (CChainState* chainstate : chainman.GetAll()) { if (!is_coinsview_empty(chainstate)) { From 2455c06464fbdcdd9306cba6bf1e013dd8af20f0 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Oct 2024 06:49:03 +0000 Subject: [PATCH 11/55] node: Extract chainstate loading sequence --- src/Makefile.am | 2 + src/init.cpp | 294 ++---------------------------------- src/node/chainstate.cpp | 322 ++++++++++++++++++++++++++++++++++++++++ src/node/chainstate.h | 49 ++++++ 4 files changed, 388 insertions(+), 279 deletions(-) create mode 100644 src/node/chainstate.cpp create mode 100644 src/node/chainstate.h diff --git a/src/Makefile.am b/src/Makefile.am index 55ffd7a6c6..1ce5f56fe1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -269,6 +269,7 @@ BITCOIN_CORE_H = \ netgroup.h \ netmessagemaker.h \ node/blockstorage.h \ + node/chainstate.h \ node/coin.h \ node/coinstats.h \ node/connection_types.h \ @@ -501,6 +502,7 @@ libbitcoin_server_a_SOURCES = \ netgroup.cpp \ net_processing.cpp \ node/blockstorage.cpp \ + node/chainstate.cpp \ node/coin.cpp \ node/coinstats.cpp \ node/connection_types.cpp \ diff --git a/src/init.cpp b/src/init.cpp index 8c56f23abf..b42dfb590c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -38,6 +38,7 @@ #include #include #include +#include // for LoadChainstate #include #include #include @@ -1823,7 +1824,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) nTotalCache -= nCoinDBCache; int64_t nCoinCacheUsage = nTotalCache; // the rest goes to in-memory cache int64_t nMempoolSizeMax = args.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; - int64_t nEvoDbCache = 1024 * 1024 * 64; // TODO LogPrintf("Cache configuration:\n"); LogPrintf("* Using %.1f MiB for block index database\n", nBlockTreeDBCache * (1.0 / 1024 / 1024)); if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { @@ -1861,288 +1861,24 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.govman = std::make_unique(*node.mn_metaman, *node.netfulfilledman, *node.chainman, node.dmnman, *node.mn_sync); const bool fReset = fReindex; - auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); - }; bilingual_str strLoadError; uiInterface.InitMessage(_("Loading block index…").translated); - do { - const auto load_block_index_start_time{SteadyClock::now()}; - try { - LOCK(cs_main); - - node.evodb.reset(); - node.evodb = std::make_unique(nEvoDbCache, false, fReset || fReindexChainState); - - node.mnhf_manager.reset(); - node.mnhf_manager = std::make_unique(*node.evodb); - - chainman.InitializeChainstate(Assert(node.mempool.get()), *node.evodb, node.chain_helper, llmq::chainLocksHandler, llmq::quorumInstantSendManager); - chainman.m_total_coinstip_cache = nCoinCacheUsage; - chainman.m_total_coinsdb_cache = nCoinDBCache; - - auto& pblocktree{chainman.m_blockman.m_block_tree_db}; - // new CBlockTreeDB tries to delete the existing file, which - // fails if it's still open from the previous loop. Close it first: - pblocktree.reset(); - pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); - - // Same logic as above with pblocktree - node.dmnman.reset(); - node.dmnman = std::make_unique(chainman.ActiveChainstate(), *node.evodb); - node.mempool->ConnectManagers(node.dmnman.get()); - - node.cpoolman.reset(); - node.cpoolman = std::make_unique(*node.evodb); - - llmq::quorumSnapshotManager.reset(); - llmq::quorumSnapshotManager.reset(new llmq::CQuorumSnapshotManager(*node.evodb)); - - if (node.llmq_ctx) { - node.llmq_ctx->Interrupt(); - node.llmq_ctx->Stop(); - } - node.llmq_ctx.reset(); - node.llmq_ctx = std::make_unique(chainman, *node.dmnman, *node.evodb, *node.mn_metaman, *node.mnhf_manager, *node.sporkman, - *node.mempool, node.mn_activeman.get(), *node.mn_sync, /*unit_tests=*/false, /*wipe=*/fReset || fReindexChainState); - // Enable CMNHFManager::{Process, Undo}Block - node.mnhf_manager->ConnectManagers(node.chainman.get(), node.llmq_ctx->qman.get()); - - node.chain_helper.reset(); - node.chain_helper = std::make_unique(*node.cpoolman, *node.dmnman, *node.mnhf_manager, *node.govman, *(node.llmq_ctx->quorum_block_processor), *node.chainman, - chainparams.GetConsensus(), *node.mn_sync, *node.sporkman, *(node.llmq_ctx->clhandler), *(node.llmq_ctx->qman)); - - if (fReset) { - pblocktree->WriteReindexing(true); - //If we're reindexing in prune mode, wipe away unusable block files and all undo data files - if (fPruneMode) - CleanupBlockRevFiles(); - } - - if (ShutdownRequested()) break; - - // LoadBlockIndex will load m_have_pruned if we've ever removed a - // block file from disk. - // Note that it also sets fReindex based on the disk flag! - // From here on out fReindex and fReset mean something different! - if (!chainman.LoadBlockIndex()) { - if (ShutdownRequested()) break; - strLoadError = _("Error loading block database"); - break; - } - - if (is_governance_enabled && !args.GetBoolArg("-txindex", DEFAULT_TXINDEX) && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { // TODO remove this when pruning is fixed. See https://github.com/dashpay/dash/pull/1817 and https://github.com/dashpay/dash/pull/1743 - return InitError(_("Transaction index can't be disabled with governance validation enabled. Either start with -disablegovernance command line switch or enable transaction index.")); - } - - // If the loaded chain has a wrong genesis, bail out immediately - // (we're likely using a testnet datadir, or the other way around). - if (!chainman.BlockIndex().empty() && - !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { - return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); - } - - if (!chainparams.GetConsensus().hashDevnetGenesisBlock.IsNull() && !chainman.BlockIndex().empty() && - !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashDevnetGenesisBlock)) { - return InitError(_("Incorrect or no devnet genesis block found. Wrong datadir for devnet specified?")); - } - - if (!fReset && !fReindexChainState) { - // Check for changed -addressindex state - if (!fAddressIndex && fAddressIndex != args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) { - strLoadError = _("You need to rebuild the database using -reindex to enable -addressindex"); - break; - } - - // Check for changed -timestampindex state - if (!fTimestampIndex && fTimestampIndex != args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX)) { - strLoadError = _("You need to rebuild the database using -reindex to enable -timestampindex"); - break; - } - - // Check for changed -spentindex state - if (!fSpentIndex && fSpentIndex != args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX)) { - strLoadError = _("You need to rebuild the database using -reindex to enable -spentindex"); - break; - } - } - - chainman.InitAdditionalIndexes(); - - LogPrintf("%s: address index %s\n", __func__, fAddressIndex ? "enabled" : "disabled"); - LogPrintf("%s: timestamp index %s\n", __func__, fTimestampIndex ? "enabled" : "disabled"); - LogPrintf("%s: spent index %s\n", __func__, fSpentIndex ? "enabled" : "disabled"); - - // Check for changed -prune state. What we are concerned about is a user who has pruned blocks - // in the past, but is now trying to run unpruned. - if (chainman.m_blockman.m_have_pruned && !fPruneMode) { - strLoadError = _("You need to rebuild the database using -reindex to go back to unpruned mode. This will redownload the entire blockchain"); - break; - } - - // At this point blocktree args are consistent with what's on disk. - // If we're not mid-reindex (based on disk + args), add a genesis block on disk - // (otherwise we use the one already on disk). - // This is called again in ThreadImport after the reindex completes. - if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) { - strLoadError = _("Error initializing block database"); - break; - } - - // At this point we're either in reindex or we've loaded a useful - // block tree into BlockIndex()! - - bool failed_chainstate_init = false; - - for (CChainState* chainstate : chainman.GetAll()) { - chainstate->InitCoinsDB( - /* cache_size_bytes */ nCoinDBCache, - /* in_memory */ false, - /* should_wipe */ fReset || fReindexChainState); - - chainstate->CoinsErrorCatcher().AddReadErrCallback([]() { - uiInterface.ThreadSafeMessageBox( - _("Error reading from database, shutting down."), - "", CClientUIInterface::MSG_ERROR); - }); - - // If necessary, upgrade from older database format. - // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!chainstate->CoinsDB().Upgrade()) { - strLoadError = _("Error upgrading chainstate database"); - failed_chainstate_init = true; - break; - } - - // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!chainstate->ReplayBlocks()) { - strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate."); - failed_chainstate_init = true; - break; - } - - // The on-disk coinsdb is now in a good state, create the cache - chainstate->InitCoinsCache(nCoinCacheUsage); - assert(chainstate->CanFlushToDisk()); - - // flush evodb - // TODO: CEvoDB instance should probably be a part of CChainState - // (for multiple chainstates to actually work in parallel) - // and not a global - if (&chainman.ActiveChainstate() == chainstate && !node.evodb->CommitRootTransaction()) { - strLoadError = _("Failed to commit EvoDB"); - failed_chainstate_init = true; - break; - } - - if (!is_coinsview_empty(chainstate)) { - // LoadChainTip initializes the chain based on CoinsTip()'s best block - if (!chainstate->LoadChainTip()) { - strLoadError = _("Error initializing block database"); - failed_chainstate_init = true; - break; // out of the per-chainstate loop - } - assert(chainstate->m_chain.Tip() != nullptr); - } - } - - if (failed_chainstate_init) { - break; // out of the chainstate activation do-while - } - - if (!node.dmnman->MigrateDBIfNeeded()) { - strLoadError = _("Error upgrading evo database"); - break; - } - if (!node.dmnman->MigrateDBIfNeeded2()) { - strLoadError = _("Error upgrading evo database"); - break; - } - if (!node.mnhf_manager->ForceSignalDBUpdate()) { - strLoadError = _("Error upgrading evo database for EHF"); - break; - } - } catch (const std::exception& e) { - LogPrintf("%s\n", e.what()); - strLoadError = _("Error opening block database"); - break; - } - - bool failed_verification = false; - - try { - LOCK(cs_main); - - for (CChainState* chainstate : chainman.GetAll()) { - if (!is_coinsview_empty(chainstate)) { - uiInterface.InitMessage(_("Verifying blocks…").translated); - if (chainman.m_blockman.m_have_pruned && args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS) > MIN_BLOCKS_TO_KEEP) { - LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", - MIN_BLOCKS_TO_KEEP); - } - - const CBlockIndex* tip = chainstate->m_chain.Tip(); - RPCNotifyBlockChange(tip); - if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { - strLoadError = _("The block database contains a block which appears to be from the future. " - "This may be due to your computer's date and time being set incorrectly. " - "Only rebuild the block database if you are sure that your computer's date and time are correct"); - failed_verification = true; - break; - } - const bool v19active{DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V19)}; - if (v19active) { - bls::bls_legacy_scheme.store(false); - LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); - } - - if (!CVerifyDB().VerifyDB( - *chainstate, chainparams, chainstate->CoinsDB(), - *node.evodb, - args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), - args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS))) { - strLoadError = _("Corrupted block database detected"); - failed_verification = true; - break; - } - - // VerifyDB() disconnects blocks which might result in us switching back to legacy. - // Make sure we use the right scheme. - if (v19active && bls::bls_legacy_scheme.load()) { - bls::bls_legacy_scheme.store(false); - LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); - } - - if (args.GetArg("-checklevel", DEFAULT_CHECKLEVEL) >= 3) { - chainstate->ResetBlockFailureFlags(nullptr); - } - - } else { - // TODO: CEvoDB instance should probably be a part of CChainState - // (for multiple chainstates to actually work in parallel) - // and not a global - if (&chainman.ActiveChainstate() == chainstate && !node.evodb->IsEmpty()) { - // EvoDB processed some blocks earlier but we have no blocks anymore, something is wrong - strLoadError = _("Error initializing block database"); - failed_verification = true; - break; - } - } - } - } catch (const std::exception& e) { - LogPrintf("%s\n", e.what()); - strLoadError = _("Error opening block database"); - failed_verification = true; - break; - } - - if (!failed_verification) { - fLoaded = true; - LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); - } - } while(false); + bool rv = LoadChainstate(fLoaded, + strLoadError, + fReset, + chainman, + node, + fPruneMode, + is_governance_enabled, + chainparams, + args, + fReindexChainState, + nBlockTreeDBCache, + nCoinDBCache, + nCoinCacheUsage); + if (!rv) return false; if (!fLoaded && !ShutdownRequested()) { // first suggest a reindex diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp new file mode 100644 index 0000000000..727a91a328 --- /dev/null +++ b/src/node/chainstate.cpp @@ -0,0 +1,322 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include // for CChainParams +#include // for DeploymentActiveAfter +#include // for RPCNotifyBlockChange +#include // for GetTime, GetTimeMillis +#include // for bilingual_str +#include // for CleanupBlockRevFiles, fHavePruned, fReindex +#include // for NodeContext +#include // for InitError, uiInterface, and CClientUIInterface member access +#include // for ShutdownRequested +#include // for a lot of things + +#include // for CChainstateHelper +#include // for CCreditPoolManager +#include // for CDeterministicMNManager +#include // for CEvoDB +#include // for CMNHFManager +#include // for llmq::chainLocksHandler +#include // for LLMQContext +#include // for llmq::quorumInstantSendManager +#include // for llmq::quorumSnapshotManager + +bool LoadChainstate(bool& fLoaded, + bilingual_str& strLoadError, + bool fReset, + ChainstateManager& chainman, + NodeContext& node, + bool fPruneMode, + bool is_governance_enabled, + const CChainParams& chainparams, + const ArgsManager& args, + bool fReindexChainState, + int64_t nBlockTreeDBCache, + int64_t nCoinDBCache, + int64_t nCoinCacheUsage) { + auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); + }; + + do { + const auto load_block_index_start_time{SteadyClock::now()}; + try { + LOCK(cs_main); + + int64_t nEvoDbCache{64 * 1024 * 1024}; // TODO + node.evodb.reset(); + node.evodb = std::make_unique(nEvoDbCache, false, fReset || fReindexChainState); + + node.mnhf_manager.reset(); + node.mnhf_manager = std::make_unique(*node.evodb); + + chainman.InitializeChainstate(Assert(node.mempool.get()), *node.evodb, node.chain_helper, llmq::chainLocksHandler, llmq::quorumInstantSendManager); + chainman.m_total_coinstip_cache = nCoinCacheUsage; + chainman.m_total_coinsdb_cache = nCoinDBCache; + + auto& pblocktree{chainman.m_blockman.m_block_tree_db}; + // new CBlockTreeDB tries to delete the existing file, which + // fails if it's still open from the previous loop. Close it first: + pblocktree.reset(); + pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); + + // Same logic as above with pblocktree + node.dmnman.reset(); + node.dmnman = std::make_unique(chainman.ActiveChainstate(), *node.evodb); + node.mempool->ConnectManagers(node.dmnman.get()); + + node.cpoolman.reset(); + node.cpoolman = std::make_unique(*node.evodb); + + llmq::quorumSnapshotManager.reset(); + llmq::quorumSnapshotManager.reset(new llmq::CQuorumSnapshotManager(*node.evodb)); + + if (node.llmq_ctx) { + node.llmq_ctx->Interrupt(); + node.llmq_ctx->Stop(); + } + node.llmq_ctx.reset(); + node.llmq_ctx = std::make_unique(chainman, *node.dmnman, *node.evodb, *node.mn_metaman, *node.mnhf_manager, *node.sporkman, + *node.mempool, node.mn_activeman.get(), *node.mn_sync, /*unit_tests=*/false, /*wipe=*/fReset || fReindexChainState); + // Enable CMNHFManager::{Process, Undo}Block + node.mnhf_manager->ConnectManagers(node.chainman.get(), node.llmq_ctx->qman.get()); + + node.chain_helper.reset(); + node.chain_helper = std::make_unique(*node.cpoolman, *node.dmnman, *node.mnhf_manager, *node.govman, *(node.llmq_ctx->quorum_block_processor), *node.chainman, + chainparams.GetConsensus(), *node.mn_sync, *node.sporkman, *(node.llmq_ctx->clhandler), *(node.llmq_ctx->qman)); + + if (fReset) { + pblocktree->WriteReindexing(true); + //If we're reindexing in prune mode, wipe away unusable block files and all undo data files + if (fPruneMode) + CleanupBlockRevFiles(); + } + + if (ShutdownRequested()) break; + + // LoadBlockIndex will load m_have_pruned if we've ever removed a + // block file from disk. + // Note that it also sets fReindex based on the disk flag! + // From here on out fReindex and fReset mean something different! + if (!chainman.LoadBlockIndex()) { + if (ShutdownRequested()) break; + strLoadError = _("Error loading block database"); + break; + } + + if (is_governance_enabled && !args.GetBoolArg("-txindex", DEFAULT_TXINDEX) && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { // TODO remove this when pruning is fixed. See https://github.com/dashpay/dash/pull/1817 and https://github.com/dashpay/dash/pull/1743 + return InitError(_("Transaction index can't be disabled with governance validation enabled. Either start with -disablegovernance command line switch or enable transaction index.")); + } + + // If the loaded chain has a wrong genesis, bail out immediately + // (we're likely using a testnet datadir, or the other way around). + if (!chainman.BlockIndex().empty() && + !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { + return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); + } + + if (!chainparams.GetConsensus().hashDevnetGenesisBlock.IsNull() && !chainman.BlockIndex().empty() && + !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashDevnetGenesisBlock)) { + return InitError(_("Incorrect or no devnet genesis block found. Wrong datadir for devnet specified?")); + } + + if (!fReset && !fReindexChainState) { + // Check for changed -addressindex state + if (!fAddressIndex && fAddressIndex != args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) { + strLoadError = _("You need to rebuild the database using -reindex to enable -addressindex"); + break; + } + + // Check for changed -timestampindex state + if (!fTimestampIndex && fTimestampIndex != args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX)) { + strLoadError = _("You need to rebuild the database using -reindex to enable -timestampindex"); + break; + } + + // Check for changed -spentindex state + if (!fSpentIndex && fSpentIndex != args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX)) { + strLoadError = _("You need to rebuild the database using -reindex to enable -spentindex"); + break; + } + } + + chainman.InitAdditionalIndexes(); + + LogPrintf("%s: address index %s\n", __func__, fAddressIndex ? "enabled" : "disabled"); + LogPrintf("%s: timestamp index %s\n", __func__, fTimestampIndex ? "enabled" : "disabled"); + LogPrintf("%s: spent index %s\n", __func__, fSpentIndex ? "enabled" : "disabled"); + + // Check for changed -prune state. What we are concerned about is a user who has pruned blocks + // in the past, but is now trying to run unpruned. + if (chainman.m_blockman.m_have_pruned && !fPruneMode) { + strLoadError = _("You need to rebuild the database using -reindex to go back to unpruned mode. This will redownload the entire blockchain"); + break; + } + + // At this point blocktree args are consistent with what's on disk. + // If we're not mid-reindex (based on disk + args), add a genesis block on disk + // (otherwise we use the one already on disk). + // This is called again in ThreadImport after the reindex completes. + if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) { + strLoadError = _("Error initializing block database"); + break; + } + + // At this point we're either in reindex or we've loaded a useful + // block tree into BlockIndex()! + + bool failed_chainstate_init = false; + + for (CChainState* chainstate : chainman.GetAll()) { + chainstate->InitCoinsDB( + /* cache_size_bytes */ nCoinDBCache, + /* in_memory */ false, + /* should_wipe */ fReset || fReindexChainState); + + chainstate->CoinsErrorCatcher().AddReadErrCallback([]() { + uiInterface.ThreadSafeMessageBox( + _("Error reading from database, shutting down."), + "", CClientUIInterface::MSG_ERROR); + }); + + // If necessary, upgrade from older database format. + // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate + if (!chainstate->CoinsDB().Upgrade()) { + strLoadError = _("Error upgrading chainstate database"); + failed_chainstate_init = true; + break; + } + + // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate + if (!chainstate->ReplayBlocks()) { + strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate."); + failed_chainstate_init = true; + break; + } + + // The on-disk coinsdb is now in a good state, create the cache + chainstate->InitCoinsCache(nCoinCacheUsage); + assert(chainstate->CanFlushToDisk()); + + // flush evodb + // TODO: CEvoDB instance should probably be a part of CChainState + // (for multiple chainstates to actually work in parallel) + // and not a global + if (&chainman.ActiveChainstate() == chainstate && !node.evodb->CommitRootTransaction()) { + strLoadError = _("Failed to commit EvoDB"); + failed_chainstate_init = true; + break; + } + + if (!is_coinsview_empty(chainstate)) { + // LoadChainTip initializes the chain based on CoinsTip()'s best block + if (!chainstate->LoadChainTip()) { + strLoadError = _("Error initializing block database"); + failed_chainstate_init = true; + break; // out of the per-chainstate loop + } + assert(chainstate->m_chain.Tip() != nullptr); + } + } + + if (failed_chainstate_init) { + break; // out of the chainstate activation do-while + } + + if (!node.dmnman->MigrateDBIfNeeded()) { + strLoadError = _("Error upgrading evo database"); + break; + } + if (!node.dmnman->MigrateDBIfNeeded2()) { + strLoadError = _("Error upgrading evo database"); + break; + } + if (!node.mnhf_manager->ForceSignalDBUpdate()) { + strLoadError = _("Error upgrading evo database for EHF"); + break; + } + } catch (const std::exception& e) { + LogPrintf("%s\n", e.what()); + strLoadError = _("Error opening block database"); + break; + } + + bool failed_verification = false; + + try { + LOCK(cs_main); + + for (CChainState* chainstate : chainman.GetAll()) { + if (!is_coinsview_empty(chainstate)) { + uiInterface.InitMessage(_("Verifying blocks…").translated); + if (chainman.m_blockman.m_have_pruned && args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS) > MIN_BLOCKS_TO_KEEP) { + LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", + MIN_BLOCKS_TO_KEEP); + } + + const CBlockIndex* tip = chainstate->m_chain.Tip(); + RPCNotifyBlockChange(tip); + if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { + strLoadError = _("The block database contains a block which appears to be from the future. " + "This may be due to your computer's date and time being set incorrectly. " + "Only rebuild the block database if you are sure that your computer's date and time are correct"); + failed_verification = true; + break; + } + const bool v19active{DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V19)}; + if (v19active) { + bls::bls_legacy_scheme.store(false); + LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); + } + + if (!CVerifyDB().VerifyDB( + *chainstate, chainparams, chainstate->CoinsDB(), + *node.evodb, + args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), + args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS))) { + strLoadError = _("Corrupted block database detected"); + failed_verification = true; + break; + } + + // VerifyDB() disconnects blocks which might result in us switching back to legacy. + // Make sure we use the right scheme. + if (v19active && bls::bls_legacy_scheme.load()) { + bls::bls_legacy_scheme.store(false); + LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); + } + + if (args.GetArg("-checklevel", DEFAULT_CHECKLEVEL) >= 3) { + chainstate->ResetBlockFailureFlags(nullptr); + } + + } else { + // TODO: CEvoDB instance should probably be a part of CChainState + // (for multiple chainstates to actually work in parallel) + // and not a global + if (&chainman.ActiveChainstate() == chainstate && !node.evodb->IsEmpty()) { + // EvoDB processed some blocks earlier but we have no blocks anymore, something is wrong + strLoadError = _("Error initializing block database"); + failed_verification = true; + break; + } + } + } + } catch (const std::exception& e) { + LogPrintf("%s\n", e.what()); + strLoadError = _("Error opening block database"); + failed_verification = true; + break; + } + + if (!failed_verification) { + fLoaded = true; + LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); + } + } while(false); + return true; +} diff --git a/src/node/chainstate.h b/src/node/chainstate.h new file mode 100644 index 0000000000..15b7b216fc --- /dev/null +++ b/src/node/chainstate.h @@ -0,0 +1,49 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_CHAINSTATE_H +#define BITCOIN_NODE_CHAINSTATE_H + +#include // for int64_t + +class ArgsManager; +struct bilingual_str; +class CChainParams; +class ChainstateManager; +struct NodeContext; + +/** This sequence can have 4 types of outcomes: + * + * 1. Success + * 2. Shutdown requested + * - nothing failed but a shutdown was triggered in the middle of the + * sequence + * 3. Soft failure + * - a failure that might be recovered from with a reindex + * 4. Hard failure + * - a failure that definitively cannot be recovered from with a reindex + * + * Currently, LoadChainstate returns a bool which: + * - if false + * - Definitely a "Hard failure" + * - if true + * - if fLoaded -> "Success" + * - if ShutdownRequested() -> "Shutdown requested" + * - else -> "Soft failure" + */ +bool LoadChainstate(bool& fLoaded, + bilingual_str& strLoadError, + bool fReset, + ChainstateManager& chainman, + NodeContext& node, + bool fPruneMode, + bool is_governance_enabled, + const CChainParams& chainparams, + const ArgsManager& args, + bool fReindexChainState, + int64_t nBlockTreeDBCache, + int64_t nCoinDBCache, + int64_t nCoinCacheUsage); + +#endif // BITCOIN_NODE_CHAINSTATE_H From 9ab08c42e4d8ed08d8853c525c680e7838f12c0b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 20 Sep 2021 17:08:18 -0400 Subject: [PATCH 12/55] node/chainstate: Decouple from GetTimeMillis --- src/init.cpp | 4 ++++ src/node/chainstate.cpp | 4 +--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index b42dfb590c..fefe7845d1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1865,6 +1865,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) uiInterface.InitMessage(_("Loading block index…").translated); + const auto load_block_index_start_time{SteadyClock::now()}; bool rv = LoadChainstate(fLoaded, strLoadError, fReset, @@ -1879,6 +1880,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) nCoinDBCache, nCoinCacheUsage); if (!rv) return false; + if (fLoaded) { + LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); + } if (!fLoaded && !ShutdownRequested()) { // first suggest a reindex diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 727a91a328..e8dd229d42 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -7,7 +7,7 @@ #include // for CChainParams #include // for DeploymentActiveAfter #include // for RPCNotifyBlockChange -#include // for GetTime, GetTimeMillis +#include // for GetTime #include // for bilingual_str #include // for CleanupBlockRevFiles, fHavePruned, fReindex #include // for NodeContext @@ -43,7 +43,6 @@ bool LoadChainstate(bool& fLoaded, }; do { - const auto load_block_index_start_time{SteadyClock::now()}; try { LOCK(cs_main); @@ -315,7 +314,6 @@ bool LoadChainstate(bool& fLoaded, if (!failed_verification) { fLoaded = true; - LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); } } while(false); return true; From d7419e42d6adb0e1b0c0de0b05147a98168edff5 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Oct 2024 08:58:02 +0000 Subject: [PATCH 13/55] node/chainstate: Decouple from stringy errors --- src/init.cpp | 70 +++++++++++++++++++++-- src/node/chainstate.cpp | 121 +++++++++++++--------------------------- src/node/chainstate.h | 70 ++++++++++++++++------- 3 files changed, 152 insertions(+), 109 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index fefe7845d1..60aad46ee8 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1864,11 +1864,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) bilingual_str strLoadError; uiInterface.InitMessage(_("Loading block index…").translated); - const auto load_block_index_start_time{SteadyClock::now()}; - bool rv = LoadChainstate(fLoaded, - strLoadError, - fReset, + auto rv = LoadChainstate(fReset, chainman, node, fPruneMode, @@ -1879,8 +1876,69 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) nBlockTreeDBCache, nCoinDBCache, nCoinCacheUsage); - if (!rv) return false; - if (fLoaded) { + if (rv.has_value()) { + switch (rv.value()) { + case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB: + strLoadError = _("Error loading block database"); + break; + case ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK: + return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); + case ChainstateLoadingError::ERROR_BAD_DEVNET_GENESIS_BLOCK: + return InitError(_("Incorrect or no devnet genesis block found. Wrong datadir for devnet specified?")); + case ChainstateLoadingError::ERROR_TXINDEX_DISABLED_WHEN_GOV_ENABLED: + return InitError(_("Transaction index can't be disabled with governance validation enabled. Either start with -disablegovernance command line switch or enable transaction index.")); + case ChainstateLoadingError::ERROR_ADDRIDX_NEEDS_REINDEX: + strLoadError = _("You need to rebuild the database using -reindex to enable -addressindex"); + break; + case ChainstateLoadingError::ERROR_SPENTIDX_NEEDS_REINDEX: + strLoadError = _("You need to rebuild the database using -reindex to enable -spentindex"); + break; + case ChainstateLoadingError::ERROR_TIMEIDX_NEEDS_REINDEX: + strLoadError = _("You need to rebuild the database using -reindex to enable -timestampindex"); + break; + case ChainstateLoadingError::ERROR_PRUNED_NEEDS_REINDEX: + strLoadError = _("You need to rebuild the database using -reindex to go back to unpruned mode. This will redownload the entire blockchain"); + break; + case ChainstateLoadingError::ERROR_LOAD_GENESIS_BLOCK_FAILED: + strLoadError = _("Error initializing block database"); + break; + case ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED: + strLoadError = _("Error upgrading chainstate database"); + break; + case ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED: + strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate."); + break; + case ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED: + strLoadError = _("Error initializing block database"); + break; + case ChainstateLoadingError::ERROR_EVO_DB_SANITY_FAILED: + strLoadError = _("Error initializing block database"); + break; + case ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED: + strLoadError = _("Error opening block database"); + break; + case ChainstateLoadingError::ERROR_BLOCK_FROM_FUTURE: + strLoadError = _("The block database contains a block which appears to be from the future. " + "This may be due to your computer's date and time being set incorrectly. " + "Only rebuild the block database if you are sure that your computer's date and time are correct"); + break; + case ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB: + strLoadError = _("Corrupted block database detected"); + break; + case ChainstateLoadingError::ERROR_COMMITING_EVO_DB: + strLoadError = _("Failed to commit Evo database"); + break; + case ChainstateLoadingError::ERROR_UPGRADING_EVO_DB: + strLoadError = _("Error upgrading Evo database"); + break; + case ChainstateLoadingError::ERROR_UPGRADING_SIGNALS_DB: + strLoadError = _("Error upgrading evo database for EHF"); + break; + case ChainstateLoadingError::SHUTDOWN_PROBED: + break; + } + } else { + fLoaded = true; LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); } diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index e8dd229d42..b8f7c3ef5a 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -8,7 +8,6 @@ #include // for DeploymentActiveAfter #include // for RPCNotifyBlockChange #include // for GetTime -#include // for bilingual_str #include // for CleanupBlockRevFiles, fHavePruned, fReindex #include // for NodeContext #include // for InitError, uiInterface, and CClientUIInterface member access @@ -25,19 +24,18 @@ #include // for llmq::quorumInstantSendManager #include // for llmq::quorumSnapshotManager -bool LoadChainstate(bool& fLoaded, - bilingual_str& strLoadError, - bool fReset, - ChainstateManager& chainman, - NodeContext& node, - bool fPruneMode, - bool is_governance_enabled, - const CChainParams& chainparams, - const ArgsManager& args, - bool fReindexChainState, - int64_t nBlockTreeDBCache, - int64_t nCoinDBCache, - int64_t nCoinCacheUsage) { +std::optional LoadChainstate(bool fReset, + ChainstateManager& chainman, + NodeContext& node, + bool fPruneMode, + bool is_governance_enabled, + const CChainParams& chainparams, + const ArgsManager& args, + bool fReindexChainState, + int64_t nBlockTreeDBCache, + int64_t nCoinDBCache, + int64_t nCoinCacheUsage) +{ auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); }; @@ -95,51 +93,49 @@ bool LoadChainstate(bool& fLoaded, CleanupBlockRevFiles(); } - if (ShutdownRequested()) break; + if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED; // LoadBlockIndex will load m_have_pruned if we've ever removed a // block file from disk. // Note that it also sets fReindex based on the disk flag! // From here on out fReindex and fReset mean something different! if (!chainman.LoadBlockIndex()) { - if (ShutdownRequested()) break; - strLoadError = _("Error loading block database"); - break; + if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED; + return ChainstateLoadingError::ERROR_LOADING_BLOCK_DB; } - if (is_governance_enabled && !args.GetBoolArg("-txindex", DEFAULT_TXINDEX) && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { // TODO remove this when pruning is fixed. See https://github.com/dashpay/dash/pull/1817 and https://github.com/dashpay/dash/pull/1743 - return InitError(_("Transaction index can't be disabled with governance validation enabled. Either start with -disablegovernance command line switch or enable transaction index.")); + // TODO: Remove this when pruning is fixed. + // See https://github.com/dashpay/dash/pull/1817 and https://github.com/dashpay/dash/pull/1743 + if (is_governance_enabled && !args.GetBoolArg("-txindex", DEFAULT_TXINDEX) && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { + return ChainstateLoadingError::ERROR_TXINDEX_DISABLED_WHEN_GOV_ENABLED; } // If the loaded chain has a wrong genesis, bail out immediately // (we're likely using a testnet datadir, or the other way around). if (!chainman.BlockIndex().empty() && !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { - return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); + return ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK; } if (!chainparams.GetConsensus().hashDevnetGenesisBlock.IsNull() && !chainman.BlockIndex().empty() && !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashDevnetGenesisBlock)) { - return InitError(_("Incorrect or no devnet genesis block found. Wrong datadir for devnet specified?")); + return ChainstateLoadingError::ERROR_BAD_DEVNET_GENESIS_BLOCK; } if (!fReset && !fReindexChainState) { // Check for changed -addressindex state if (!fAddressIndex && fAddressIndex != args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) { - strLoadError = _("You need to rebuild the database using -reindex to enable -addressindex"); - break; + return ChainstateLoadingError::ERROR_ADDRIDX_NEEDS_REINDEX; } // Check for changed -timestampindex state if (!fTimestampIndex && fTimestampIndex != args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX)) { - strLoadError = _("You need to rebuild the database using -reindex to enable -timestampindex"); - break; + return ChainstateLoadingError::ERROR_TIMEIDX_NEEDS_REINDEX; } // Check for changed -spentindex state if (!fSpentIndex && fSpentIndex != args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX)) { - strLoadError = _("You need to rebuild the database using -reindex to enable -spentindex"); - break; + return ChainstateLoadingError::ERROR_SPENTIDX_NEEDS_REINDEX; } } @@ -152,8 +148,7 @@ bool LoadChainstate(bool& fLoaded, // Check for changed -prune state. What we are concerned about is a user who has pruned blocks // in the past, but is now trying to run unpruned. if (chainman.m_blockman.m_have_pruned && !fPruneMode) { - strLoadError = _("You need to rebuild the database using -reindex to go back to unpruned mode. This will redownload the entire blockchain"); - break; + return ChainstateLoadingError::ERROR_PRUNED_NEEDS_REINDEX; } // At this point blocktree args are consistent with what's on disk. @@ -161,15 +156,12 @@ bool LoadChainstate(bool& fLoaded, // (otherwise we use the one already on disk). // This is called again in ThreadImport after the reindex completes. if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) { - strLoadError = _("Error initializing block database"); - break; + return ChainstateLoadingError::ERROR_LOAD_GENESIS_BLOCK_FAILED; } // At this point we're either in reindex or we've loaded a useful // block tree into BlockIndex()! - bool failed_chainstate_init = false; - for (CChainState* chainstate : chainman.GetAll()) { chainstate->InitCoinsDB( /* cache_size_bytes */ nCoinDBCache, @@ -185,16 +177,12 @@ bool LoadChainstate(bool& fLoaded, // If necessary, upgrade from older database format. // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate if (!chainstate->CoinsDB().Upgrade()) { - strLoadError = _("Error upgrading chainstate database"); - failed_chainstate_init = true; - break; + return ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED; } // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate if (!chainstate->ReplayBlocks()) { - strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate."); - failed_chainstate_init = true; - break; + return ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED; } // The on-disk coinsdb is now in a good state, create the cache @@ -206,46 +194,29 @@ bool LoadChainstate(bool& fLoaded, // (for multiple chainstates to actually work in parallel) // and not a global if (&chainman.ActiveChainstate() == chainstate && !node.evodb->CommitRootTransaction()) { - strLoadError = _("Failed to commit EvoDB"); - failed_chainstate_init = true; - break; + return ChainstateLoadingError::ERROR_COMMITING_EVO_DB; } if (!is_coinsview_empty(chainstate)) { // LoadChainTip initializes the chain based on CoinsTip()'s best block if (!chainstate->LoadChainTip()) { - strLoadError = _("Error initializing block database"); - failed_chainstate_init = true; - break; // out of the per-chainstate loop + return ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED; } assert(chainstate->m_chain.Tip() != nullptr); } } - if (failed_chainstate_init) { - break; // out of the chainstate activation do-while - } - - if (!node.dmnman->MigrateDBIfNeeded()) { - strLoadError = _("Error upgrading evo database"); - break; - } - if (!node.dmnman->MigrateDBIfNeeded2()) { - strLoadError = _("Error upgrading evo database"); - break; + if (!node.dmnman->MigrateDBIfNeeded() || !node.dmnman->MigrateDBIfNeeded2()) { + return ChainstateLoadingError::ERROR_UPGRADING_EVO_DB; } if (!node.mnhf_manager->ForceSignalDBUpdate()) { - strLoadError = _("Error upgrading evo database for EHF"); - break; + return ChainstateLoadingError::ERROR_UPGRADING_SIGNALS_DB; } } catch (const std::exception& e) { LogPrintf("%s\n", e.what()); - strLoadError = _("Error opening block database"); - break; + return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; } - bool failed_verification = false; - try { LOCK(cs_main); @@ -260,11 +231,7 @@ bool LoadChainstate(bool& fLoaded, const CBlockIndex* tip = chainstate->m_chain.Tip(); RPCNotifyBlockChange(tip); if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { - strLoadError = _("The block database contains a block which appears to be from the future. " - "This may be due to your computer's date and time being set incorrectly. " - "Only rebuild the block database if you are sure that your computer's date and time are correct"); - failed_verification = true; - break; + return ChainstateLoadingError::ERROR_BLOCK_FROM_FUTURE; } const bool v19active{DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V19)}; if (v19active) { @@ -277,9 +244,7 @@ bool LoadChainstate(bool& fLoaded, *node.evodb, args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS))) { - strLoadError = _("Corrupted block database detected"); - failed_verification = true; - break; + return ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB; } // VerifyDB() disconnects blocks which might result in us switching back to legacy. @@ -299,22 +264,14 @@ bool LoadChainstate(bool& fLoaded, // and not a global if (&chainman.ActiveChainstate() == chainstate && !node.evodb->IsEmpty()) { // EvoDB processed some blocks earlier but we have no blocks anymore, something is wrong - strLoadError = _("Error initializing block database"); - failed_verification = true; - break; + return ChainstateLoadingError::ERROR_EVO_DB_SANITY_FAILED; } } } } catch (const std::exception& e) { LogPrintf("%s\n", e.what()); - strLoadError = _("Error opening block database"); - failed_verification = true; - break; - } - - if (!failed_verification) { - fLoaded = true; + return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; } } while(false); - return true; + return std::nullopt; } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index 15b7b216fc..0c8e547cac 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -6,13 +6,36 @@ #define BITCOIN_NODE_CHAINSTATE_H #include // for int64_t +#include // for std::optional class ArgsManager; -struct bilingual_str; class CChainParams; class ChainstateManager; struct NodeContext; +enum class ChainstateLoadingError { + ERROR_LOADING_BLOCK_DB, + ERROR_BAD_GENESIS_BLOCK, + ERROR_BAD_DEVNET_GENESIS_BLOCK, + ERROR_TXINDEX_DISABLED_WHEN_GOV_ENABLED, + ERROR_ADDRIDX_NEEDS_REINDEX, + ERROR_SPENTIDX_NEEDS_REINDEX, + ERROR_TIMEIDX_NEEDS_REINDEX, + ERROR_PRUNED_NEEDS_REINDEX, + ERROR_LOAD_GENESIS_BLOCK_FAILED, + ERROR_CHAINSTATE_UPGRADE_FAILED, + ERROR_REPLAYBLOCKS_FAILED, + ERROR_LOADCHAINTIP_FAILED, + ERROR_EVO_DB_SANITY_FAILED, + ERROR_GENERIC_BLOCKDB_OPEN_FAILED, + ERROR_BLOCK_FROM_FUTURE, + ERROR_CORRUPTED_BLOCK_DB, + ERROR_COMMITING_EVO_DB, + ERROR_UPGRADING_EVO_DB, + ERROR_UPGRADING_SIGNALS_DB, + SHUTDOWN_PROBED, +}; + /** This sequence can have 4 types of outcomes: * * 1. Success @@ -24,26 +47,31 @@ struct NodeContext; * 4. Hard failure * - a failure that definitively cannot be recovered from with a reindex * - * Currently, LoadChainstate returns a bool which: - * - if false - * - Definitely a "Hard failure" - * - if true - * - if fLoaded -> "Success" - * - if ShutdownRequested() -> "Shutdown requested" - * - else -> "Soft failure" + * Currently, LoadChainstate returns a std::optional + * which: + * + * - if has_value() + * - Either "Soft failure", "Hard failure", or "Shutdown requested", + * differentiable by the specific enumerator. + * + * Note that a return value of SHUTDOWN_PROBED means ONLY that "during + * this sequence, when we explicitly checked ShutdownRequested() at + * arbitrary points, one of those calls returned true". Therefore, a + * return value other than SHUTDOWN_PROBED does not guarantee that + * ShutdownRequested() hasn't been called indirectly. + * - else + * - Success! */ -bool LoadChainstate(bool& fLoaded, - bilingual_str& strLoadError, - bool fReset, - ChainstateManager& chainman, - NodeContext& node, - bool fPruneMode, - bool is_governance_enabled, - const CChainParams& chainparams, - const ArgsManager& args, - bool fReindexChainState, - int64_t nBlockTreeDBCache, - int64_t nCoinDBCache, - int64_t nCoinCacheUsage); +std::optional LoadChainstate(bool fReset, + ChainstateManager& chainman, + NodeContext& node, + bool fPruneMode, + bool is_governance_enabled, + const CChainParams& chainparams, + const ArgsManager& args, + bool fReindexChainState, + int64_t nBlockTreeDBCache, + int64_t nCoinDBCache, + int64_t nCoinCacheUsage); #endif // BITCOIN_NODE_CHAINSTATE_H From ee9d3dd5fce686e66a774ee18a1f82e9750cdb75 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 14 Dec 2024 15:02:39 +0000 Subject: [PATCH 14/55] node/chainstate: Decouple from ArgsManager --- src/init.cpp | 9 +++++++-- src/node/chainstate.cpp | 25 +++++++++++++++---------- src/node/chainstate.h | 10 +++++++--- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 60aad46ee8..823c07513a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1869,13 +1869,18 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) chainman, node, fPruneMode, + args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX), is_governance_enabled, + args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX), + args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX), + args.GetBoolArg("-txindex", DEFAULT_TXINDEX), chainparams, - args, fReindexChainState, nBlockTreeDBCache, nCoinDBCache, - nCoinCacheUsage); + nCoinCacheUsage, + args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS), + args.GetArg("-checklevel", DEFAULT_CHECKLEVEL)); if (rv.has_value()) { switch (rv.value()) { case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB: diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index b8f7c3ef5a..edb246f5db 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -28,13 +28,18 @@ std::optional LoadChainstate(bool fReset, ChainstateManager& chainman, NodeContext& node, bool fPruneMode, + bool is_addrindex_enabled, bool is_governance_enabled, + bool is_spentindex_enabled, + bool is_timeindex_enabled, + bool is_txindex_enabled, const CChainParams& chainparams, - const ArgsManager& args, bool fReindexChainState, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, - int64_t nCoinCacheUsage) + int64_t nCoinCacheUsage, + unsigned int check_blocks, + unsigned int check_level) { auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); @@ -106,7 +111,7 @@ std::optional LoadChainstate(bool fReset, // TODO: Remove this when pruning is fixed. // See https://github.com/dashpay/dash/pull/1817 and https://github.com/dashpay/dash/pull/1743 - if (is_governance_enabled && !args.GetBoolArg("-txindex", DEFAULT_TXINDEX) && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { + if (is_governance_enabled && !is_txindex_enabled && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { return ChainstateLoadingError::ERROR_TXINDEX_DISABLED_WHEN_GOV_ENABLED; } @@ -124,17 +129,17 @@ std::optional LoadChainstate(bool fReset, if (!fReset && !fReindexChainState) { // Check for changed -addressindex state - if (!fAddressIndex && fAddressIndex != args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) { + if (!fAddressIndex && fAddressIndex != is_addrindex_enabled) { return ChainstateLoadingError::ERROR_ADDRIDX_NEEDS_REINDEX; } // Check for changed -timestampindex state - if (!fTimestampIndex && fTimestampIndex != args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX)) { + if (!fTimestampIndex && fTimestampIndex != is_timeindex_enabled) { return ChainstateLoadingError::ERROR_TIMEIDX_NEEDS_REINDEX; } // Check for changed -spentindex state - if (!fSpentIndex && fSpentIndex != args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX)) { + if (!fSpentIndex && fSpentIndex != is_spentindex_enabled) { return ChainstateLoadingError::ERROR_SPENTIDX_NEEDS_REINDEX; } } @@ -223,7 +228,7 @@ std::optional LoadChainstate(bool fReset, for (CChainState* chainstate : chainman.GetAll()) { if (!is_coinsview_empty(chainstate)) { uiInterface.InitMessage(_("Verifying blocks…").translated); - if (chainman.m_blockman.m_have_pruned && args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS) > MIN_BLOCKS_TO_KEEP) { + if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) { LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", MIN_BLOCKS_TO_KEEP); } @@ -242,8 +247,8 @@ std::optional LoadChainstate(bool fReset, if (!CVerifyDB().VerifyDB( *chainstate, chainparams, chainstate->CoinsDB(), *node.evodb, - args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), - args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS))) { + check_level, + check_blocks)) { return ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB; } @@ -254,7 +259,7 @@ std::optional LoadChainstate(bool fReset, LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); } - if (args.GetArg("-checklevel", DEFAULT_CHECKLEVEL) >= 3) { + if (check_level >= 3) { chainstate->ResetBlockFailureFlags(nullptr); } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index 0c8e547cac..d2bd675c95 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -8,7 +8,6 @@ #include // for int64_t #include // for std::optional -class ArgsManager; class CChainParams; class ChainstateManager; struct NodeContext; @@ -66,12 +65,17 @@ std::optional LoadChainstate(bool fReset, ChainstateManager& chainman, NodeContext& node, bool fPruneMode, + bool is_addrindex_enabled, bool is_governance_enabled, + bool is_spentindex_enabled, + bool is_timeindex_enabled, + bool is_txindex_enabled, const CChainParams& chainparams, - const ArgsManager& args, bool fReindexChainState, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, - int64_t nCoinCacheUsage); + int64_t nCoinCacheUsage, + unsigned int check_blocks, + unsigned int check_level); #endif // BITCOIN_NODE_CHAINSTATE_H From 440fd3fe21ac12822807985b98e918e437b88326 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 15 Dec 2024 10:59:46 +0000 Subject: [PATCH 15/55] ci: drop distro LLVM packages, move Clang install up, set defaults Also simplify the download and execution of `llvm.sh` --- contrib/containers/ci/Dockerfile | 46 ++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/contrib/containers/ci/Dockerfile b/contrib/containers/ci/Dockerfile index 29bb92fff2..cc58a2cf60 100644 --- a/contrib/containers/ci/Dockerfile +++ b/contrib/containers/ci/Dockerfile @@ -7,7 +7,6 @@ ENV DEBIAN_FRONTEND="noninteractive" TZ="Europe/London" # (zlib1g-dev is needed for the Qt host binary builds, but should not be used by target binaries) ENV APT_ARGS="-y --no-install-recommends --no-upgrade" - # Install packages for i386; disabled on aarch64 and arm64 hosts RUN (dpkg --print-architecture | grep -Eq 'aarch64|arm64' || dpkg --add-architecture i386) RUN (dpkg --print-architecture | grep -Eq 'aarch64|arm64' || (apt-get update && apt-get install $APT_ARGS \ @@ -23,13 +22,11 @@ RUN apt-get update && apt-get install $APT_ARGS \ bsdmainutils \ curl \ ccache \ - clang \ cmake \ g++ \ gettext \ git \ - libc++-dev \ - libc++abi-dev \ + gnupg \ libtool \ libxcb-icccm4 \ libxcb-image0 \ @@ -42,12 +39,36 @@ RUN apt-get update && apt-get install $APT_ARGS \ libxcb-xinerama0 \ libxcb-xkb1 \ libxkbcommon-x11-0 \ - wget \ + lsb-release \ + software-properties-common \ unzip \ + wget \ m4 \ pkg-config \ zlib1g-dev +# Install Clang+LLVM and set it as default +# We don't need all packages but the default set doesn't include some +# packages we want so we will need to install some of them manually. +ARG LLVM_VERSION=16 +RUN set -ex; \ + echo "Installing LLVM and Clang ${LLVM_VERSION}..."; \ + curl -sL https://apt.llvm.org/llvm.sh | bash -s "${LLVM_VERSION}"; \ + echo "Installing additional packages..."; \ + apt-get update && apt-get install $APT_ARGS \ + "clang-format-${LLVM_VERSION}" \ + "clang-tidy-${LLVM_VERSION}" \ + "libc++-${LLVM_VERSION}-dev" \ + "libc++abi-${LLVM_VERSION}-dev" \ + "libclang-rt-${LLVM_VERSION}-dev"; \ + rm -rf /var/lib/apt/lists/*; \ + echo "Setting defaults..."; \ + lldbUpdAltArgs="update-alternatives --install /usr/bin/llvm-config llvm-config /usr/bin/llvm-config-${LLVM_VERSION} 100"; \ + for binName in clang clang++ clang-format clang-tidy clangd ld.lld lldb lldb-server; do \ + lldbUpdAltArgs="${lldbUpdAltArgs} --slave /usr/bin/${binName} ${binName} /usr/bin/${binName}-${LLVM_VERSION}"; \ + done; \ + sh -c "${lldbUpdAltArgs}"; + # Python setup # PYTHON_VERSION should match the value in .python-version ARG PYTHON_VERSION=3.9.18 @@ -61,7 +82,6 @@ RUN apt-get update && apt-get install $APT_ARGS \ libreadline-dev \ libsqlite3-dev \ libssl-dev \ - llvm \ make \ tk-dev \ xz-utils @@ -135,20 +155,6 @@ RUN \ update-alternatives --set x86_64-w64-mingw32-g++ /usr/bin/x86_64-w64-mingw32-g++-posix; \ exit 0 -ARG LLVM_VERSION=16 -# Setup Clang+LLVM support -RUN apt-get update && apt-get install $APT_ARGS \ - lsb-release \ - software-properties-common \ - gnupg \ - && rm -rf /var/lib/apt/lists/* - -RUN cd /tmp && \ - wget https://apt.llvm.org/llvm.sh && \ - chmod +x llvm.sh && \ - /tmp/llvm.sh ${LLVM_VERSION} && \ - rm -rf /tmp/llvm.sh - RUN \ mkdir -p /src/dash && \ mkdir -p /cache/ccache && \ From 64cdc421302686bf07bad76bbadb9e8ac734258a Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 15 Dec 2024 10:57:08 +0000 Subject: [PATCH 16/55] ci: add LLVM library path to `LD_LIBRARY_PATH` and GDB allowlist --- contrib/containers/ci/Dockerfile | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/contrib/containers/ci/Dockerfile b/contrib/containers/ci/Dockerfile index cc58a2cf60..658b5dbffd 100644 --- a/contrib/containers/ci/Dockerfile +++ b/contrib/containers/ci/Dockerfile @@ -68,6 +68,8 @@ RUN set -ex; \ lldbUpdAltArgs="${lldbUpdAltArgs} --slave /usr/bin/${binName} ${binName} /usr/bin/${binName}-${LLVM_VERSION}"; \ done; \ sh -c "${lldbUpdAltArgs}"; +# LD_LIBRARY_PATH is empty by default, this is the first entry +ENV LD_LIBRARY_PATH="/usr/lib/llvm-${LLVM_VERSION}/lib" # Python setup # PYTHON_VERSION should match the value in .python-version @@ -107,14 +109,15 @@ ARG DASH_HASH_VERSION=1.4.0 RUN git clone --depth 1 --no-tags --branch=${DASH_HASH_VERSION} https://github.com/dashpay/dash_hash RUN cd dash_hash && pip3 install -r requirements.txt . +# Add user with specified (or default) user/group ids and setup configuration files ARG USER_ID=1000 ARG GROUP_ID=1000 - -# add user with specified (or default) user/group ids -ENV USER_ID="${USER_ID}" -ENV GROUP_ID="${GROUP_ID}" -RUN groupadd -g ${GROUP_ID} dash -RUN useradd -u ${USER_ID} -g dash -s /bin/bash -m -d /home/dash dash +RUN set -ex; \ + groupadd -g ${GROUP_ID} dash; \ + useradd -u ${USER_ID} -g dash -s /bin/bash -m -d /home/dash dash; \ + mkdir -p /home/dash/.config/gdb; \ + echo "add-auto-load-safe-path /usr/lib/llvm-${LLVM_VERSION}/lib" | tee /home/dash/.config/gdb/gdbinit; \ + chown ${USER_ID}:${GROUP_ID} -R /home/dash # Packages needed for all target builds RUN apt-get update && apt-get install $APT_ARGS \ From b7099eed4758f97c0b7ed9f426947c327f366e2c Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 15 Dec 2024 11:02:54 +0000 Subject: [PATCH 17/55] ci: remove redundant `version` attribute, avoid `lldb` personality error --- contrib/containers/develop/docker-compose.yml | 7 ++++--- contrib/containers/guix/docker-compose.yml | 1 - 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/containers/develop/docker-compose.yml b/contrib/containers/develop/docker-compose.yml index 95241c0a56..9238c97183 100644 --- a/contrib/containers/develop/docker-compose.yml +++ b/contrib/containers/develop/docker-compose.yml @@ -1,17 +1,18 @@ -version: "3.9" services: container: entrypoint: /bin/bash build: context: '..' dockerfile: './develop/Dockerfile' - tty: true # Equivalent to -t - stdin_open: true # Equivalent to -i ports: - "9998:9998" # Mainnet Ports - "9999:9999" - "19998:19998" # Testnet Ports - "19999:19999" + security_opt: + - seccomp:unconfined + stdin_open: true # Equivalent to -i + tty: true # Equivalent to -t # A note about volumes: # diff --git a/contrib/containers/guix/docker-compose.yml b/contrib/containers/guix/docker-compose.yml index dc90916531..b4f6861a08 100644 --- a/contrib/containers/guix/docker-compose.yml +++ b/contrib/containers/guix/docker-compose.yml @@ -1,4 +1,3 @@ -version: "3.9" services: guix_ubuntu: build: From e7702292d16cb029307e5447eb80e7b9a94007db Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 16 Nov 2024 07:21:54 +0000 Subject: [PATCH 18/55] ci: purge package manager cache after each interaction --- contrib/containers/ci/Dockerfile | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/contrib/containers/ci/Dockerfile b/contrib/containers/ci/Dockerfile index 658b5dbffd..c5f47397a1 100644 --- a/contrib/containers/ci/Dockerfile +++ b/contrib/containers/ci/Dockerfile @@ -45,7 +45,8 @@ RUN apt-get update && apt-get install $APT_ARGS \ wget \ m4 \ pkg-config \ - zlib1g-dev + zlib1g-dev \ + && rm -rf /var/lib/apt/lists/* # Install Clang+LLVM and set it as default # We don't need all packages but the default set doesn't include some @@ -86,7 +87,9 @@ RUN apt-get update && apt-get install $APT_ARGS \ libssl-dev \ make \ tk-dev \ - xz-utils + xz-utils \ + && rm -rf /var/lib/apt/lists/* + ENV PYENV_ROOT="/usr/local/pyenv" ENV PATH="${PYENV_ROOT}/shims:${PYENV_ROOT}/bin:${PATH}" RUN curl https://pyenv.run | bash \ @@ -134,7 +137,8 @@ RUN apt-get update && apt-get install $APT_ARGS \ valgrind \ wine-stable \ wine64 \ - xorriso + xorriso \ + && rm -rf /var/lib/apt/lists/* ARG CPPCHECK_VERSION=2.13.0 RUN curl -sL "https://github.com/danmar/cppcheck/archive/${CPPCHECK_VERSION}.tar.gz" | tar -xvzf - --directory /tmp/ From eef863554a1e9011dafdf21a4b5c78ac7c879e6c Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 16 Nov 2024 07:44:52 +0000 Subject: [PATCH 19/55] ci: install `i386` packages only if host is `amd64`, merge layers --- contrib/containers/ci/Dockerfile | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/contrib/containers/ci/Dockerfile b/contrib/containers/ci/Dockerfile index c5f47397a1..69da54c93e 100644 --- a/contrib/containers/ci/Dockerfile +++ b/contrib/containers/ci/Dockerfile @@ -7,13 +7,17 @@ ENV DEBIAN_FRONTEND="noninteractive" TZ="Europe/London" # (zlib1g-dev is needed for the Qt host binary builds, but should not be used by target binaries) ENV APT_ARGS="-y --no-install-recommends --no-upgrade" -# Install packages for i386; disabled on aarch64 and arm64 hosts -RUN (dpkg --print-architecture | grep -Eq 'aarch64|arm64' || dpkg --add-architecture i386) -RUN (dpkg --print-architecture | grep -Eq 'aarch64|arm64' || (apt-get update && apt-get install $APT_ARGS \ - g++-multilib \ - wine32) && rm -rf /var/lib/apt/lists/*) - -RUN apt-get update && apt-get install $APT_ARGS \ +# Install packages for i386 on amd64 hosts, then install common packages +RUN set -ex; \ + apt-get update && \ + if [ "$(dpkg --print-architecture)" = "amd64" ]; then \ + dpkg --add-architecture i386 && \ + apt-get update && \ + apt-get install $APT_ARGS \ + g++-multilib \ + wine32; \ + fi; \ + apt-get install $APT_ARGS \ autotools-dev \ automake \ autoconf \ From 187fe17650bcea97042d39208fdfad4766615635 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 8 Nov 2024 17:01:49 +0000 Subject: [PATCH 20/55] ci: don't stage packages in `/tmp`, reduce layers for `cppcheck` build --- contrib/containers/ci/Dockerfile | 34 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/contrib/containers/ci/Dockerfile b/contrib/containers/ci/Dockerfile index 69da54c93e..85df2c3105 100644 --- a/contrib/containers/ci/Dockerfile +++ b/contrib/containers/ci/Dockerfile @@ -111,10 +111,28 @@ RUN pip3 install \ pyzmq==22.3.0 \ vulture==2.3 -# dash_hash ARG DASH_HASH_VERSION=1.4.0 -RUN git clone --depth 1 --no-tags --branch=${DASH_HASH_VERSION} https://github.com/dashpay/dash_hash -RUN cd dash_hash && pip3 install -r requirements.txt . +RUN set -ex; \ + cd /tmp; \ + git clone --depth 1 --no-tags --branch=${DASH_HASH_VERSION} https://github.com/dashpay/dash_hash; \ + cd dash_hash && pip3 install -r requirements.txt .; \ + cd .. && rm -rf dash_hash + +ARG CPPCHECK_VERSION=2.13.0 +RUN set -ex; \ + curl -fL "https://github.com/danmar/cppcheck/archive/${CPPCHECK_VERSION}.tar.gz" -o /tmp/cppcheck.tar.gz; \ + mkdir -p /opt/cppcheck && tar -xzf /tmp/cppcheck.tar.gz -C /opt/cppcheck --strip-components=1 && rm /tmp/cppcheck.tar.gz; \ + cd /opt/cppcheck; \ + mkdir build && cd build && cmake .. && cmake --build . -j "$(( $(nproc) - 1 ))"; \ + mkdir /usr/local/share/Cppcheck && ln -s /opt/cppcheck/cfg/ /usr/local/share/Cppcheck/cfg; \ + rm -rf /tmp/cppcheck.tar.gz +ENV PATH="/opt/cppcheck/build/bin:${PATH}" + +ARG SHELLCHECK_VERSION=v0.7.1 +RUN set -ex; \ + curl -fL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" -o /tmp/shellcheck.tar.xz; \ + mkdir -p /opt/shellcheck && tar -xf /tmp/shellcheck.tar.xz -C /opt/shellcheck --strip-components=1 && rm /tmp/shellcheck.tar.xz +ENV PATH="/opt/shellcheck:${PATH}" # Add user with specified (or default) user/group ids and setup configuration files ARG USER_ID=1000 @@ -144,16 +162,6 @@ RUN apt-get update && apt-get install $APT_ARGS \ xorriso \ && rm -rf /var/lib/apt/lists/* -ARG CPPCHECK_VERSION=2.13.0 -RUN curl -sL "https://github.com/danmar/cppcheck/archive/${CPPCHECK_VERSION}.tar.gz" | tar -xvzf - --directory /tmp/ -RUN cd /tmp/cppcheck-${CPPCHECK_VERSION} && mkdir build && cd build && cmake .. && cmake --build . -j 8 -ENV PATH="/tmp/cppcheck-${CPPCHECK_VERSION}/build/bin:${PATH}" -RUN mkdir /usr/local/share/Cppcheck && ln -s /tmp/cppcheck-${CPPCHECK_VERSION}/cfg/ /usr/local/share/Cppcheck/cfg - -ARG SHELLCHECK_VERSION=v0.7.1 -RUN curl -sL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/ -ENV PATH="/tmp/shellcheck-${SHELLCHECK_VERSION}:${PATH}" - # This is a hack. It is needed because gcc-multilib and g++-multilib are conflicting with g++-arm-linux-gnueabihf. This is # due to gcc-multilib installing the following symbolic link, which is needed for -m32 support. However, this causes # arm builds to also have the asm folder implicitly in the include search path. This is kind of ok, because the asm folder From 8dd0db7de98d8e65260d08cdc900455294834f6d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 22 Nov 2024 06:35:50 +0000 Subject: [PATCH 21/55] ci: fix "LC_ALL: cannot change locale (en_US.UTF-8)" in Guix container --- contrib/containers/guix/Dockerfile | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/contrib/containers/guix/Dockerfile b/contrib/containers/guix/Dockerfile index 4b09a24c12..9039147011 100644 --- a/contrib/containers/guix/Dockerfile +++ b/contrib/containers/guix/Dockerfile @@ -18,7 +18,11 @@ RUN apt-get update && \ sudo \ wget \ xz-utils && \ - rm -rf /var/lib/apt/lists/* + rm -rf /var/lib/apt/lists/*; \ + targetLocale="en_US.UTF-8"; \ + locale-gen ${targetLocale} && \ + update-locale LC_ALL=${targetLocale} && \ + update-locale LANG=${targetLocale}; ARG guix_download_path=ftp://ftp.gnu.org/gnu/guix ARG guix_version=1.4.0 @@ -30,8 +34,7 @@ ENV PATH="/usr/local/bin:/usr/local/guix/current/bin:$PATH" # Application Setup # https://guix.gnu.org/manual/en/html_node/Application-Setup.html -ENV GUIX_LOCPATH="/usr/local/guix/profile" \ - LC_ALL="en_US.UTF-8" +ENV GUIX_LOCPATH="/usr/local/guix/profile" RUN guix_file_name=guix-binary-${guix_version}.$(uname -m)-linux.tar.xz && \ eval "guix_checksum=\${guix_checksum_$(uname -m)}" && \ From a49162ffae5302b036c1e91dfd808c2ea8a9830a Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 18 Nov 2024 09:52:43 +0000 Subject: [PATCH 22/55] merge bitcoin#27314: Fix handling of `CXX=clang++` when building `qt` package --- depends/packages/qt.mk | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 2333cc8831..6cb6e53ed7 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -159,9 +159,15 @@ $(package)_config_opts_linux += -dbus-runtime ifneq ($(LTO),) $(package)_config_opts_linux += -ltcg endif -$(package)_config_opts_linux += -platform linux-g++ -xplatform bitcoin-linux-g++ -ifneq (,$(findstring -stdlib=libc++,$($(1)_cxx))) -$(package)_config_opts_x86_64_linux = -xplatform linux-clang-libc++ + +ifneq (,$(findstring clang,$($(package)_cxx))) + ifneq (,$(findstring -stdlib=libc++,$($(package)_cxx))) + $(package)_config_opts_linux += -platform linux-clang-libc++ -xplatform linux-clang-libc++ + else + $(package)_config_opts_linux += -platform linux-clang -xplatform linux-clang + endif +else + $(package)_config_opts_linux += -platform linux-g++ -xplatform bitcoin-linux-g++ endif $(package)_config_opts_mingw32 = -no-opengl From 4f1b5c165ba8a632e069540b2a330ccf09a497f6 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 19 Nov 2024 08:14:08 +0000 Subject: [PATCH 23/55] merge bitcoin#28954: Reduce use of bash -c --- ci/dash/test_integrationtests.sh | 2 +- ci/dash/test_unittests.sh | 4 ++-- ci/test/00_setup_env.sh | 1 - ci/test/00_setup_env_native_multiprocess.sh | 2 +- ci/test/00_setup_env_s390x.sh | 1 - 5 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ci/dash/test_integrationtests.sh b/ci/dash/test_integrationtests.sh index 8a0013b1a9..86a00c8f1b 100755 --- a/ci/dash/test_integrationtests.sh +++ b/ci/dash/test_integrationtests.sh @@ -42,7 +42,7 @@ echo "Using socketevents mode: $SOCKETEVENTS" EXTRA_ARGS="--dashd-arg=-socketevents=$SOCKETEVENTS" set +e -LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib ${TEST_RUNNER_ENV} ./test/functional/test_runner.py --ci --attempts=3 --ansi --combinedlogslen=4000 --timeout-factor=${TEST_RUNNER_TIMEOUT_FACTOR} ${TEST_RUNNER_EXTRA} --failfast --nocleanup --tmpdir=$(pwd)/testdatadirs $PASS_ARGS $EXTRA_ARGS +LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib ./test/functional/test_runner.py --ci --attempts=3 --ansi --combinedlogslen=4000 --timeout-factor=${TEST_RUNNER_TIMEOUT_FACTOR} ${TEST_RUNNER_EXTRA} --failfast --nocleanup --tmpdir=$(pwd)/testdatadirs $PASS_ARGS $EXTRA_ARGS RESULT=$? set -e diff --git a/ci/dash/test_unittests.sh b/ci/dash/test_unittests.sh index b321232710..37a0a4575e 100755 --- a/ci/dash/test_unittests.sh +++ b/ci/dash/test_unittests.sh @@ -29,8 +29,8 @@ if [ "$DIRECT_WINE_EXEC_TESTS" = "true" ]; then wine ./src/test/test_dash.exe else if [ "$RUN_UNIT_TESTS_SEQUENTIAL" = "true" ]; then - ${TEST_RUNNER_ENV} ./src/test/test_dash --catch_system_errors=no -l test_suite + ./src/test/test_dash --catch_system_errors=no -l test_suite else - ${TEST_RUNNER_ENV} make $MAKEJOBS check VERBOSE=1 + make $MAKEJOBS check VERBOSE=1 fi fi diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh index 587dfc2bfe..2023705cf8 100755 --- a/ci/test/00_setup_env.sh +++ b/ci/test/00_setup_env.sh @@ -44,7 +44,6 @@ export RUN_SECURITY_TESTS=${RUN_SECURITY_TESTS:-false} # This is needed because some ci machines have slow CPU or disk, so sanitizers # might be slow or a reindex might be waiting on disk IO. export TEST_RUNNER_TIMEOUT_FACTOR=${TEST_RUNNER_TIMEOUT_FACTOR:-4} -export TEST_RUNNER_ENV=${TEST_RUNNER_ENV:-} export RUN_FUZZ_TESTS=${RUN_FUZZ_TESTS:-false} export EXPECTED_TESTS_DURATION_IN_SECONDS=${EXPECTED_TESTS_DURATION_IN_SECONDS:-1000} diff --git a/ci/test/00_setup_env_native_multiprocess.sh b/ci/test/00_setup_env_native_multiprocess.sh index 743e30e6c0..7029ea5395 100755 --- a/ci/test/00_setup_env_native_multiprocess.sh +++ b/ci/test/00_setup_env_native_multiprocess.sh @@ -12,4 +12,4 @@ export DEP_OPTS="DEBUG=1 MULTIPROCESS=1" export GOAL="install" export TEST_RUNNER_EXTRA="--v2transport" export BITCOIN_CONFIG="--with-boost-process --enable-debug CC=clang CXX=clang++" # Use clang to avoid OOM -export TEST_RUNNER_ENV="BITCOIND=dash-node" +export BITCOIND=dash-node # Used in functional tests diff --git a/ci/test/00_setup_env_s390x.sh b/ci/test/00_setup_env_s390x.sh index c71a4891b9..a46bde95b5 100755 --- a/ci/test/00_setup_env_s390x.sh +++ b/ci/test/00_setup_env_s390x.sh @@ -19,7 +19,6 @@ fi # Use debian to avoid 404 apt errors export CONTAINER_NAME=ci_s390x export RUN_UNIT_TESTS=true -export TEST_RUNNER_ENV="LC_ALL=C" export RUN_FUNCTIONAL_TESTS=true export GOAL="install" export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests --with-boost-process" # GUI tests disabled for now, see https://github.com/bitcoin/bitcoin/issues/23730 From d0131a52598f465c3443dc863695f4362da86912 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 15 Dec 2024 13:12:04 +0000 Subject: [PATCH 24/55] trivial: sort `BUILD_TARGET` on GitHub and in `matrix.sh` alphabetically --- .github/workflows/build.yml | 12 ++++++------ ci/dash/matrix.sh | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 75d319b09f..514032ec92 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -114,22 +114,22 @@ jobs: - build_target: linux64 host: x86_64-pc-linux-gnu depends_on: linux64 - - build_target: linux64_tsan - host: x86_64-pc-linux-gnu - depends_on: linux64 - - build_target: linux64_ubsan + - build_target: linux64_cxx20 host: x86_64-pc-linux-gnu depends_on: linux64 - build_target: linux64_fuzz host: x86_64-pc-linux-gnu depends_on: linux64 - - build_target: linux64_cxx20 + - build_target: linux64_nowallet host: x86_64-pc-linux-gnu depends_on: linux64 - build_target: linux64_sqlite host: x86_64-pc-linux-gnu depends_on: linux64 - - build_target: linux64_nowallet + - build_target: linux64_tsan + host: x86_64-pc-linux-gnu + depends_on: linux64 + - build_target: linux64_ubsan host: x86_64-pc-linux-gnu depends_on: linux64 container: diff --git a/ci/dash/matrix.sh b/ci/dash/matrix.sh index 82a9233df1..6eeb2a7565 100755 --- a/ci/dash/matrix.sh +++ b/ci/dash/matrix.sh @@ -18,28 +18,28 @@ export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ if [ "$BUILD_TARGET" = "arm-linux" ]; then source ./ci/test/00_setup_env_arm.sh -elif [ "$BUILD_TARGET" = "win64" ]; then - source ./ci/test/00_setup_env_win64.sh elif [ "$BUILD_TARGET" = "linux64" ]; then source ./ci/test/00_setup_env_native_qt5.sh elif [ "$BUILD_TARGET" = "linux64_asan" ]; then source ./ci/test/00_setup_env_native_asan.sh -elif [ "$BUILD_TARGET" = "linux64_tsan" ]; then - source ./ci/test/00_setup_env_native_tsan.sh -elif [ "$BUILD_TARGET" = "linux64_ubsan" ]; then - source ./ci/test/00_setup_env_native_ubsan.sh -elif [ "$BUILD_TARGET" = "linux64_fuzz" ]; then - source ./ci/test/00_setup_env_native_fuzz.sh elif [ "$BUILD_TARGET" = "linux64_cxx20" ]; then source ./ci/test/00_setup_env_native_cxx20.sh -elif [ "$BUILD_TARGET" = "linux64_sqlite" ]; then - source ./ci/test/00_setup_env_native_sqlite.sh +elif [ "$BUILD_TARGET" = "linux64_fuzz" ]; then + source ./ci/test/00_setup_env_native_fuzz.sh elif [ "$BUILD_TARGET" = "linux64_nowallet" ]; then source ./ci/test/00_setup_env_native_nowallet.sh +elif [ "$BUILD_TARGET" = "linux64_sqlite" ]; then + source ./ci/test/00_setup_env_native_sqlite.sh +elif [ "$BUILD_TARGET" = "linux64_tsan" ]; then + source ./ci/test/00_setup_env_native_tsan.sh +elif [ "$BUILD_TARGET" = "linux64_ubsan" ]; then + source ./ci/test/00_setup_env_native_ubsan.sh elif [ "$BUILD_TARGET" = "linux64_valgrind" ]; then source ./ci/test/00_setup_env_native_valgrind.sh elif [ "$BUILD_TARGET" = "mac" ]; then source ./ci/test/00_setup_env_mac.sh elif [ "$BUILD_TARGET" = "s390x" ]; then source ./ci/test/00_setup_env_s390x.sh +elif [ "$BUILD_TARGET" = "win64" ]; then + source ./ci/test/00_setup_env_win64.sh fi From 26cc5a1c90e1bfc9fc2a6ac19a9646efce305cdf Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:30:38 +0000 Subject: [PATCH 25/55] ci: use underscore to separate variant name from target triple --- .gitlab-ci.yml | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ae521f655b..c612899f73 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -193,13 +193,13 @@ x86_64-w64-mingw32: variables: HOST: x86_64-w64-mingw32 -x86_64-pc-linux-gnu-debug: +x86_64-pc-linux-gnu_debug: extends: .build-depends-template variables: HOST: x86_64-pc-linux-gnu DEP_OPTS: "DEBUG=1" -x86_64-pc-linux-gnu-nowallet: +x86_64-pc-linux-gnu_nowallet: extends: - .build-depends-template - .skip-in-fast-mode-template @@ -207,7 +207,7 @@ x86_64-pc-linux-gnu-nowallet: HOST: x86_64-pc-linux-gnu DEP_OPTS: "NO_WALLET=1" -x86_64-pc-linux-gnu-multiprocess: +x86_64-pc-linux-gnu_multiprocess: extends: - .build-depends-template - .skip-in-fast-mode-template @@ -243,7 +243,7 @@ win64-build: linux64-build: extends: .build-template needs: - - x86_64-pc-linux-gnu-debug + - x86_64-pc-linux-gnu_debug variables: BUILD_TARGET: linux64 @@ -252,7 +252,7 @@ linux64_cxx20-build: - .build-template - .skip-in-fast-mode-template needs: - - x86_64-pc-linux-gnu-debug + - x86_64-pc-linux-gnu_debug variables: BUILD_TARGET: linux64_cxx20 @@ -261,7 +261,7 @@ linux64_sqlite-build: - .build-template - .skip-in-fast-mode-template needs: - - x86_64-pc-linux-gnu-debug + - x86_64-pc-linux-gnu_debug variables: BUILD_TARGET: linux64_sqlite @@ -270,7 +270,7 @@ linux64_fuzz-build: - .build-template - .skip-in-fast-mode-template needs: - - x86_64-pc-linux-gnu-debug + - x86_64-pc-linux-gnu_debug variables: BUILD_TARGET: linux64_fuzz @@ -279,7 +279,7 @@ linux64_fuzz-build: # - .build-template # - .skip-in-fast-mode-template # needs: -# - x86_64-pc-linux-gnu-debug +# - x86_64-pc-linux-gnu_debug # variables: # BUILD_TARGET: linux64_asan @@ -288,7 +288,7 @@ linux64_tsan-build: - .build-template - .skip-in-fast-mode-template needs: - - x86_64-pc-linux-gnu-debug + - x86_64-pc-linux-gnu_debug variables: BUILD_TARGET: linux64_tsan @@ -297,7 +297,7 @@ linux64_ubsan-build: - .build-template - .skip-in-fast-mode-template needs: - - x86_64-pc-linux-gnu-debug + - x86_64-pc-linux-gnu_debug variables: BUILD_TARGET: linux64_ubsan @@ -306,7 +306,7 @@ linux64_nowallet-build: - .build-template - .skip-in-fast-mode-template needs: - - x86_64-pc-linux-gnu-nowallet + - x86_64-pc-linux-gnu_nowallet variables: BUILD_TARGET: linux64_nowallet @@ -315,7 +315,7 @@ linux64_multiprocess-build: - .build-template - .skip-in-fast-mode-template needs: - - x86_64-pc-linux-gnu-multiprocess + - x86_64-pc-linux-gnu_multiprocess variables: BUILD_TARGET: linux64_multiprocess @@ -324,7 +324,7 @@ linux64_multiprocess-build: # - .build-template # - .skip-in-fast-mode-template # needs: -# - x86_64-pc-linux-gnu-debug +# - x86_64-pc-linux-gnu_debug # variables: # BUILD_TARGET: linux64_valgrind From 27d9763b1b76d96d936628df72f9e5aef5b1718a Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 19 Nov 2024 08:20:56 +0000 Subject: [PATCH 26/55] fix: add `linux64_multiprocess` `BUILD_TARGET` to matrix, mend C(XX) --- .gitlab-ci.yml | 2 +- ci/dash/matrix.sh | 2 ++ ci/test/00_setup_env_native_multiprocess.sh | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c612899f73..27fc871dd1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -213,7 +213,7 @@ x86_64-pc-linux-gnu_multiprocess: - .skip-in-fast-mode-template variables: HOST: x86_64-pc-linux-gnu - DEP_OPTS: "MULTIPROCESS=1" + DEP_OPTS: "DEBUG=1 MULTIPROCESS=1" x86_64-apple-darwin: extends: diff --git a/ci/dash/matrix.sh b/ci/dash/matrix.sh index 6eeb2a7565..9822a72789 100755 --- a/ci/dash/matrix.sh +++ b/ci/dash/matrix.sh @@ -26,6 +26,8 @@ elif [ "$BUILD_TARGET" = "linux64_cxx20" ]; then source ./ci/test/00_setup_env_native_cxx20.sh elif [ "$BUILD_TARGET" = "linux64_fuzz" ]; then source ./ci/test/00_setup_env_native_fuzz.sh +elif [ "$BUILD_TARGET" = "linux64_multiprocess" ]; then + source ./ci/test/00_setup_env_native_multiprocess.sh elif [ "$BUILD_TARGET" = "linux64_nowallet" ]; then source ./ci/test/00_setup_env_native_nowallet.sh elif [ "$BUILD_TARGET" = "linux64_sqlite" ]; then diff --git a/ci/test/00_setup_env_native_multiprocess.sh b/ci/test/00_setup_env_native_multiprocess.sh index 7029ea5395..d23fae22b5 100755 --- a/ci/test/00_setup_env_native_multiprocess.sh +++ b/ci/test/00_setup_env_native_multiprocess.sh @@ -11,5 +11,5 @@ export PACKAGES="cmake python3 llvm clang" export DEP_OPTS="DEBUG=1 MULTIPROCESS=1" export GOAL="install" export TEST_RUNNER_EXTRA="--v2transport" -export BITCOIN_CONFIG="--with-boost-process --enable-debug CC=clang CXX=clang++" # Use clang to avoid OOM +export BITCOIN_CONFIG="--with-boost-process --enable-debug CC=clang-16 CXX=clang++-16" # Use clang to avoid OOM export BITCOIND=dash-node # Used in functional tests From ff29c621034158fbdcabf89408410734b8f90f76 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:29:31 +0000 Subject: [PATCH 27/55] test: run CoinJoin RPC tests using blank wallet --- test/functional/rpc_coinjoin.py | 40 ++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 76d2e73794..6bd2068dd9 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -19,43 +19,51 @@ def set_test_params(self): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def setup_nodes(self): + self.add_nodes(self.num_nodes) + self.start_nodes() + def run_test(self): - self.test_coinjoin_start_stop() - self.test_coinjoin_setamount() - self.test_coinjoin_setrounds() + node = self.nodes[0] + + node.createwallet(wallet_name='w1', blank=True, disable_private_keys=False) + w1 = node.get_wallet_rpc('w1') + self.test_coinjoin_start_stop(w1) + self.test_coinjoin_setamount(w1) + self.test_coinjoin_setrounds(w1) - def test_coinjoin_start_stop(self): + def test_coinjoin_start_stop(self, node): # Start Mixing - self.nodes[0].coinjoin("start") + node.coinjoin('start') # Get CoinJoin info - cj_info = self.nodes[0].getcoinjoininfo() + cj_info = node.getcoinjoininfo() # Ensure that it started properly assert_equal(cj_info['enabled'], True) assert_equal(cj_info['running'], True) # Stop mixing - self.nodes[0].coinjoin("stop") + node.coinjoin('stop') # Get CoinJoin info - cj_info = self.nodes[0].getcoinjoininfo() + cj_info = node.getcoinjoininfo() # Ensure that it stopped properly assert_equal(cj_info['enabled'], True) assert_equal(cj_info['running'], False) - def test_coinjoin_setamount(self): + def test_coinjoin_setamount(self, node): # Try normal values - self.nodes[0].setcoinjoinamount(50) - cj_info = self.nodes[0].getcoinjoininfo() + node.setcoinjoinamount(50) + cj_info = node.getcoinjoininfo() assert_equal(cj_info['max_amount'], 50) # Try large values - self.nodes[0].setcoinjoinamount(1200000) - cj_info = self.nodes[0].getcoinjoininfo() + node.setcoinjoinamount(1200000) + cj_info = node.getcoinjoininfo() assert_equal(cj_info['max_amount'], 1200000) - def test_coinjoin_setrounds(self): + def test_coinjoin_setrounds(self, node): # Try normal values - self.nodes[0].setcoinjoinrounds(5) - cj_info = self.nodes[0].getcoinjoininfo() + node.setcoinjoinrounds(5) + cj_info = node.getcoinjoininfo() assert_equal(cj_info['max_rounds'], 5) From c6dd3dd567a8f9e0d2c9780f40c1f496df013b6f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 07:38:30 +0000 Subject: [PATCH 28/55] test: rename test functions to reflect RPC used, simplify them --- test/functional/rpc_coinjoin.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 6bd2068dd9..935d2817f6 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -29,8 +29,8 @@ def run_test(self): node.createwallet(wallet_name='w1', blank=True, disable_private_keys=False) w1 = node.get_wallet_rpc('w1') self.test_coinjoin_start_stop(w1) - self.test_coinjoin_setamount(w1) - self.test_coinjoin_setrounds(w1) + self.test_setcoinjoinamount(w1) + self.test_setcoinjoinrounds(w1) def test_coinjoin_start_stop(self, node): # Start Mixing @@ -49,22 +49,16 @@ def test_coinjoin_start_stop(self, node): assert_equal(cj_info['enabled'], True) assert_equal(cj_info['running'], False) - def test_coinjoin_setamount(self, node): - # Try normal values - node.setcoinjoinamount(50) - cj_info = node.getcoinjoininfo() - assert_equal(cj_info['max_amount'], 50) - - # Try large values - node.setcoinjoinamount(1200000) - cj_info = node.getcoinjoininfo() - assert_equal(cj_info['max_amount'], 1200000) + def test_setcoinjoinamount(self, node): + # Test normal and large values + for value in [50, 1200000]: + node.setcoinjoinamount(value) + assert_equal(node.getcoinjoininfo()['max_amount'], value) - def test_coinjoin_setrounds(self, node): - # Try normal values + def test_setcoinjoinrounds(self, node): + # Test normal values node.setcoinjoinrounds(5) - cj_info = node.getcoinjoininfo() - assert_equal(cj_info['max_rounds'], 5) + assert_equal(node.getcoinjoininfo()['max_rounds'], 5) if __name__ == '__main__': From a1b256b06f2bc30c6a3686f11a1849e0dbdb92be Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:33:22 +0000 Subject: [PATCH 29/55] test: extend CoinJoin RPC tests to include more cases, add logging --- test/functional/rpc_coinjoin.py | 54 ++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 935d2817f6..8ca9c865fa 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -4,13 +4,21 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.messages import ( + COIN, + MAX_MONEY, +) +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) -''' -rpc_coinjoin.py - -Tests CoinJoin basic RPC -''' +# See coinjoin/options.h +COINJOIN_ROUNDS_DEFAULT = 4 +COINJOIN_ROUNDS_MAX = 16 +COINJOIN_ROUNDS_MIN = 2 +COINJOIN_TARGET_MAX = int(MAX_MONEY / COIN) +COINJOIN_TARGET_MIN = 2 class CoinJoinTest(BitcoinTestFramework): def set_test_params(self): @@ -33,33 +41,45 @@ def run_test(self): self.test_setcoinjoinrounds(w1) def test_coinjoin_start_stop(self, node): - # Start Mixing + self.log.info('"coinjoin" subcommands should update mixing status') + # Start mix session and ensure it's reported node.coinjoin('start') - # Get CoinJoin info cj_info = node.getcoinjoininfo() - # Ensure that it started properly assert_equal(cj_info['enabled'], True) assert_equal(cj_info['running'], True) + # Repeated start should yield error + assert_raises_rpc_error(-32603, 'Mixing has been started already.', node.coinjoin, 'start') - # Stop mixing + # Stop mix session and ensure it's reported node.coinjoin('stop') - # Get CoinJoin info cj_info = node.getcoinjoininfo() - # Ensure that it stopped properly assert_equal(cj_info['enabled'], True) assert_equal(cj_info['running'], False) + # Repeated stop should yield error + assert_raises_rpc_error(-32603, 'No mix session to stop', node.coinjoin, 'stop') + + # Reset mix session + assert_equal(node.coinjoin('reset'), "Mixing was reset") def test_setcoinjoinamount(self, node): + self.log.info('"setcoinjoinamount" should update mixing target') # Test normal and large values - for value in [50, 1200000]: + for value in [COINJOIN_TARGET_MIN, 50, 1200000, COINJOIN_TARGET_MAX]: node.setcoinjoinamount(value) assert_equal(node.getcoinjoininfo()['max_amount'], value) + # Test values below minimum and above maximum + for value in [COINJOIN_TARGET_MIN - 1, COINJOIN_TARGET_MAX + 1]: + assert_raises_rpc_error(-8, "Invalid amount of DASH as mixing goal amount", node.setcoinjoinamount, value) def test_setcoinjoinrounds(self, node): - # Test normal values - node.setcoinjoinrounds(5) - assert_equal(node.getcoinjoininfo()['max_rounds'], 5) - + self.log.info('"setcoinjoinrounds" should update mixing rounds') + # Test acceptable values + for value in [COINJOIN_ROUNDS_MIN, COINJOIN_ROUNDS_DEFAULT, COINJOIN_ROUNDS_MAX]: + node.setcoinjoinrounds(value) + assert_equal(node.getcoinjoininfo()['max_rounds'], value) + # Test values below minimum and above maximum + for value in [COINJOIN_ROUNDS_MIN - 1, COINJOIN_ROUNDS_MAX + 1]: + assert_raises_rpc_error(-8, "Invalid number of rounds", node.setcoinjoinrounds, value) if __name__ == '__main__': CoinJoinTest().main() From 16c2e13fb48c3e7cdab945310280766e12e2f260 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:31:46 +0000 Subject: [PATCH 30/55] test: add functional tests for `coinjoinsalt` RPC --- test/functional/rpc_coinjoin.py | 73 +++++++++++++++++++++++++++++++++ test/functional/test_runner.py | 2 - 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 8ca9c865fa..70f9955377 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -3,13 +3,17 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. +import random + from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import ( COIN, MAX_MONEY, + uint256_to_string, ) from test_framework.util import ( assert_equal, + assert_is_hex_string, assert_raises_rpc_error, ) @@ -36,9 +40,22 @@ def run_test(self): node.createwallet(wallet_name='w1', blank=True, disable_private_keys=False) w1 = node.get_wallet_rpc('w1') + self.test_salt_presence(w1) self.test_coinjoin_start_stop(w1) self.test_setcoinjoinamount(w1) self.test_setcoinjoinrounds(w1) + self.test_coinjoinsalt(w1) + w1.unloadwallet() + + node.createwallet(wallet_name='w2', blank=True, disable_private_keys=True) + w2 = node.get_wallet_rpc('w2') + self.test_coinjoinsalt_disabled(w2) + w2.unloadwallet() + + def test_salt_presence(self, node): + self.log.info('Salt should be automatically generated in new wallet') + # Will raise exception if no salt generated + assert_is_hex_string(node.coinjoinsalt('get')) def test_coinjoin_start_stop(self, node): self.log.info('"coinjoin" subcommands should update mixing status') @@ -81,5 +98,61 @@ def test_setcoinjoinrounds(self, node): for value in [COINJOIN_ROUNDS_MIN - 1, COINJOIN_ROUNDS_MAX + 1]: assert_raises_rpc_error(-8, "Invalid number of rounds", node.setcoinjoinrounds, value) + def test_coinjoinsalt(self, node): + self.log.info('"coinjoinsalt generate" should fail if salt already present') + assert_raises_rpc_error(-32600, 'Wallet "w1" already has set CoinJoin salt!', node.coinjoinsalt, 'generate') + + self.log.info('"coinjoinsalt" subcommands should succeed if no balance and not mixing') + # 'coinjoinsalt generate' should return a new salt if overwrite enabled + s1 = node.coinjoinsalt('get') + assert_equal(node.coinjoinsalt('generate', True), True) + s2 = node.coinjoinsalt('get') + assert s1 != s2 + + # 'coinjoinsalt get' should fetch newly generated value (i.e. new salt should persist) + node.unloadwallet('w1') + node.loadwallet('w1') + node = self.nodes[0].get_wallet_rpc('w1') + assert_equal(s2, node.coinjoinsalt('get')) + + # 'coinjoinsalt set' should work with random hashes + s1 = uint256_to_string(random.getrandbits(256)) + node.coinjoinsalt('set', s1) + assert_equal(s1, node.coinjoinsalt('get')) + assert s1 != s2 + + # 'coinjoinsalt set' shouldn't work with nonsense values + s2 = format(0, '064x') + assert_raises_rpc_error(-8, "Invalid CoinJoin salt value", node.coinjoinsalt, 'set', s2, True) + s2 = s2[0:63] + 'h' + assert_raises_rpc_error(-8, "salt must be hexadecimal string (not '%s')" % s2, node.coinjoinsalt, 'set', s2, True) + + self.log.info('"coinjoinsalt generate" and "coinjoinsalt set" should fail if mixing') + # Start mix session + node.coinjoin('start') + assert_equal(node.getcoinjoininfo()['running'], True) + + # 'coinjoinsalt generate' and 'coinjoinsalt set' should fail when mixing + assert_raises_rpc_error(-4, 'Wallet "w1" is currently mixing, cannot change salt!', node.coinjoinsalt, 'generate', True) + assert_raises_rpc_error(-4, 'Wallet "w1" is currently mixing, cannot change salt!', node.coinjoinsalt, 'set', s1, True) + + # 'coinjoinsalt get' should still work + assert_equal(node.coinjoinsalt('get'), s1) + + # Stop mix session + node.coinjoin('stop') + assert_equal(node.getcoinjoininfo()['running'], False) + + # 'coinjoinsalt generate' and 'coinjoinsalt set' should start working again + assert_equal(node.coinjoinsalt('generate', True), True) + assert_equal(node.coinjoinsalt('set', s1, True), True) + + def test_coinjoinsalt_disabled(self, node): + self.log.info('"coinjoinsalt" subcommands should fail if private keys disabled') + for subcommand in ['generate', 'get']: + assert_raises_rpc_error(-32600, 'Wallet "w2" has private keys disabled, cannot perform CoinJoin!', node.coinjoinsalt, subcommand) + s1 = uint256_to_string(random.getrandbits(256)) + assert_raises_rpc_error(-32600, 'Wallet "w2" has private keys disabled, cannot perform CoinJoin!', node.coinjoinsalt, 'set', s1) + if __name__ == '__main__': CoinJoinTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2b311fca34..e1c2b7680a 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -892,8 +892,6 @@ def _get_uncovered_rpc_commands(self): # Consider RPC generate covered, because it is overloaded in # test_framework/test_node.py and not seen by the coverage check. covered_cmds = set({'generate'}) - # TODO: implement functional tests for coinjoinsalt - covered_cmds.add('coinjoinsalt') # TODO: implement functional tests for voteraw covered_cmds.add('voteraw') # TODO: implement functional tests for getmerkleblocks From 04ce1fea52b392d61fa4cbcd6c9a934aa225879e Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 12:13:27 +0000 Subject: [PATCH 31/55] ci: deduplicate macOS SDK setup logic Co-authored-by: UdjinM6 --- .gitlab-ci.yml | 21 +++-------------- ci/dash/build_depends.sh | 13 ++--------- contrib/containers/guix/Dockerfile | 13 ++++++----- contrib/containers/guix/scripts/guix-start | 13 ++--------- contrib/containers/guix/scripts/setup-sdk | 27 ++++++++++++++++++++++ 5 files changed, 41 insertions(+), 46 deletions(-) create mode 100755 contrib/containers/guix/scripts/setup-sdk diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ae521f655b..ad27984761 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -40,26 +40,11 @@ builder-image: needs: - builder-image image: $CI_REGISTRY_IMAGE:builder-$CI_COMMIT_REF_SLUG - variables: - SDK_URL: https://bitcoincore.org/depends-sources/sdks - XCODE_VERSION: "15.0" - XCODE_BUILD_ID: 15A240d before_script: - - echo HOST=$HOST - | - if [ "$HOST" = "x86_64-apple-darwin" ]; then - mkdir -p depends/SDKs - mkdir -p depends/sdk-sources - OSX_SDK_BASENAME="Xcode-${XCODE_VERSION}-${XCODE_BUILD_ID}-extracted-SDK-with-libcxx-headers.tar.gz" - OSX_SDK_PATH="depends/sdk-sources/${OSX_SDK_BASENAME}" - if [ ! -f "$OSX_SDK_PATH" ]; then - echo "Downloading MacOS SDK" - curl --location --fail "${SDK_URL}/${OSX_SDK_BASENAME}" -o "$OSX_SDK_PATH" - fi - if [ -f "$OSX_SDK_PATH" ]; then - echo "Extracting MacOS SDK" - tar -C depends/SDKs -xf "$OSX_SDK_PATH" - fi + echo HOST=${HOST} + if [[ "${HOST}" == "x86_64-apple-darwin" ]]; then + ./contrib/containers/guix/scripts/setup-sdk fi script: - make -j$(nproc) -C depends HOST=$HOST $DEP_OPTS diff --git a/ci/dash/build_depends.sh b/ci/dash/build_depends.sh index 6ad5803ae5..3f1ff978eb 100755 --- a/ci/dash/build_depends.sh +++ b/ci/dash/build_depends.sh @@ -20,17 +20,8 @@ mkdir -p $CACHE_DIR/sdk-sources ln -s $CACHE_DIR/depends ${DEPENDS_DIR}/built ln -s $CACHE_DIR/sdk-sources ${DEPENDS_DIR}/sdk-sources -mkdir -p ${DEPENDS_DIR}/SDKs - -if [ -n "$XCODE_VERSION" ]; then - OSX_SDK_BASENAME="Xcode-${XCODE_VERSION}-${XCODE_BUILD_ID}-extracted-SDK-with-libcxx-headers.tar.gz" - OSX_SDK_PATH="${DEPENDS_DIR}/sdk-sources/${OSX_SDK_BASENAME}" - if [ ! -f "$OSX_SDK_PATH" ]; then - curl --location --fail "${SDK_URL}/${OSX_SDK_BASENAME}" -o "$OSX_SDK_PATH" - fi - if [ -f "$OSX_SDK_PATH" ]; then - tar -C ${DEPENDS_DIR}/SDKs -xf "$OSX_SDK_PATH" - fi +if [[ "${HOST}" == "x86_64-apple-darwin" ]]; then + ./contrib/containers/guix/scripts/setup-sdk fi make $MAKEJOBS -C depends HOST=$HOST $DEP_OPTS diff --git a/contrib/containers/guix/Dockerfile b/contrib/containers/guix/Dockerfile index 9039147011..861678502c 100644 --- a/contrib/containers/guix/Dockerfile +++ b/contrib/containers/guix/Dockerfile @@ -77,18 +77,19 @@ RUN usermod -aG sudo ${USERNAME} && \ echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers # Copy required files to container -COPY --from=docker_root ./motd.txt /etc/motd -COPY --from=docker_root ./scripts/entrypoint /usr/local/bin/entrypoint -COPY --from=docker_root ./scripts/guix-check /usr/local/bin/guix-check -COPY --from=docker_root ./scripts/guix-start /usr/local/bin/guix-start +COPY --from=docker_root ./motd.txt /etc/motd +COPY --from=docker_root ./scripts/entrypoint /usr/local/bin/entrypoint +COPY --from=docker_root ./scripts/guix-check /usr/local/bin/guix-check +COPY --from=docker_root ./scripts/guix-start /usr/local/bin/guix-start +COPY --from=docker_root ./scripts/setup-sdk /usr/local/bin/setup-sdk # Create directories for mounting to save/restore cache and grant necessary permissions RUN mkdir -p \ /home/${USERNAME}/.cache \ - /src/dash/depends/{built,sources,work} && \ + /src/dash/depends/{built,sources,work}; \ chown -R ${USER_ID}:${GROUP_ID} \ /home/${USERNAME}/.cache \ - /src + /src; WORKDIR "/src/dash" diff --git a/contrib/containers/guix/scripts/guix-start b/contrib/containers/guix/scripts/guix-start index 4d0c6f6dba..50264c42d0 100755 --- a/contrib/containers/guix/scripts/guix-start +++ b/contrib/containers/guix/scripts/guix-start @@ -9,19 +9,10 @@ if [[ ! -d "${WORKSPACE_PATH}" || ! "${WORKSPACE_PATH}" = /* || ! -f "${WORKSPAC exit 1 fi -XCODE_VERSION="15.0" -XCODE_RELEASE="15A240d" -XCODE_ARCHIVE="Xcode-${XCODE_VERSION}-${XCODE_RELEASE}-extracted-SDK-with-libcxx-headers" -XCODE_SOURCE="${XCODE_SOURCE:-https://bitcoincore.org/depends-sources/sdks}" - export SDK_PATH="${SDK_PATH:-${WORKSPACE_PATH}/depends/SDKs}" +export SDK_SRCS="${SDK_PATH:-${WORKSPACE_PATH}/depends/sdk-sources}" -# Check if macOS SDK is present, if not, download it -if [[ ! -d "${SDK_PATH}/${XCODE_ARCHIVE}" ]]; then - echo "Preparing macOS SDK..." - mkdir -p "${SDK_PATH}" - curl -L "${XCODE_SOURCE}/${XCODE_ARCHIVE}.tar.gz" | tar -xz -C "${SDK_PATH}" -fi +./contrib/containers/guix/scripts/setup-sdk # Add safe.directory option only when WORKSPACE_PATH was specified via cmd-line arguments (happens in CI) if [[ -n "${1}" ]]; then diff --git a/contrib/containers/guix/scripts/setup-sdk b/contrib/containers/guix/scripts/setup-sdk new file mode 100755 index 0000000000..4550aeed64 --- /dev/null +++ b/contrib/containers/guix/scripts/setup-sdk @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# Copyright (c) 2024 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +export LC_ALL=C.UTF-8 + +set -eo pipefail + +SDK_URL="${SDK_URL:-https://bitcoincore.org/depends-sources/sdks}" +SDK_PATH="${SDK_PATH:-depends/SDKs}" +SDK_SRCS="${SDK_SOURCES:-depends/sdk-sources}" +XCODE_VERSION="${XCODE_VERSION:-15.0}" +XCODE_RELEASE="${XCODE_RELEASE:-15A240d}" +XCODE_ARCHIVE="Xcode-${XCODE_VERSION}-${XCODE_RELEASE}-extracted-SDK-with-libcxx-headers" +XCODE_AR_PATH="${SDK_SRCS}/${XCODE_ARCHIVE}.tar.gz" + +if [ ! -d "${SDK_PATH}/${XCODE_ARCHIVE}" ]; then + if [ ! -f "${XCODE_AR_PATH}" ]; then + echo "Downloading macOS SDK..." + mkdir -p "${SDK_SRCS}" + curl --location --fail "${SDK_URL}/${XCODE_ARCHIVE}.tar.gz" -o "${XCODE_AR_PATH}" + fi + echo "Extracting macOS SDK..." + mkdir -p "${SDK_PATH}" + tar -C "${SDK_PATH}" -xf "${XCODE_AR_PATH}" +fi From 7071282a2d37c55ff0f1478e5c9bf2bc166a5ca2 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 12:19:12 +0000 Subject: [PATCH 32/55] node/chainstate: Decouple from concept of NodeContext --- src/init.cpp | 16 +++++++++- src/node/chainstate.cpp | 71 ++++++++++++++++++++++++----------------- src/node/chainstate.h | 36 +++++++++++++++++++-- 3 files changed, 91 insertions(+), 32 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 823c07513a..88d9221451 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1867,7 +1867,21 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const auto load_block_index_start_time{SteadyClock::now()}; auto rv = LoadChainstate(fReset, chainman, - node, + *node.govman, + *node.mn_metaman, + *node.mn_sync, + *node.sporkman, + node.mn_activeman, + node.chain_helper, + node.cpoolman, + node.dmnman, + node.evodb, + node.mnhf_manager, + llmq::chainLocksHandler, + llmq::quorumInstantSendManager, + llmq::quorumSnapshotManager, + node.llmq_ctx, + node.mempool.get(), fPruneMode, args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX), is_governance_enabled, diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index edb246f5db..49894e42eb 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -9,7 +9,6 @@ #include // for RPCNotifyBlockChange #include // for GetTime #include // for CleanupBlockRevFiles, fHavePruned, fReindex -#include // for NodeContext #include // for InitError, uiInterface, and CClientUIInterface member access #include // for ShutdownRequested #include // for a lot of things @@ -26,7 +25,21 @@ std::optional LoadChainstate(bool fReset, ChainstateManager& chainman, - NodeContext& node, + CGovernanceManager& govman, + CMasternodeMetaMan& mn_metaman, + CMasternodeSync& mn_sync, + CSporkManager& sporkman, + std::unique_ptr& mn_activeman, + std::unique_ptr& chain_helper, + std::unique_ptr& cpoolman, + std::unique_ptr& dmnman, + std::unique_ptr& evodb, + std::unique_ptr& mnhf_manager, + std::unique_ptr& clhandler, + std::unique_ptr& isman, + std::unique_ptr& qsnapman, + std::unique_ptr& llmq_ctx, + CTxMemPool* mempool, bool fPruneMode, bool is_addrindex_enabled, bool is_governance_enabled, @@ -50,13 +63,13 @@ std::optional LoadChainstate(bool fReset, LOCK(cs_main); int64_t nEvoDbCache{64 * 1024 * 1024}; // TODO - node.evodb.reset(); - node.evodb = std::make_unique(nEvoDbCache, false, fReset || fReindexChainState); + evodb.reset(); + evodb = std::make_unique(nEvoDbCache, false, fReset || fReindexChainState); - node.mnhf_manager.reset(); - node.mnhf_manager = std::make_unique(*node.evodb); + mnhf_manager.reset(); + mnhf_manager = std::make_unique(*evodb); - chainman.InitializeChainstate(Assert(node.mempool.get()), *node.evodb, node.chain_helper, llmq::chainLocksHandler, llmq::quorumInstantSendManager); + chainman.InitializeChainstate(Assert(mempool), *evodb, chain_helper, clhandler, isman); chainman.m_total_coinstip_cache = nCoinCacheUsage; chainman.m_total_coinsdb_cache = nCoinDBCache; @@ -67,29 +80,29 @@ std::optional LoadChainstate(bool fReset, pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); // Same logic as above with pblocktree - node.dmnman.reset(); - node.dmnman = std::make_unique(chainman.ActiveChainstate(), *node.evodb); - node.mempool->ConnectManagers(node.dmnman.get()); + dmnman.reset(); + dmnman = std::make_unique(chainman.ActiveChainstate(), *evodb); + mempool->ConnectManagers(dmnman.get()); - node.cpoolman.reset(); - node.cpoolman = std::make_unique(*node.evodb); + cpoolman.reset(); + cpoolman = std::make_unique(*evodb); - llmq::quorumSnapshotManager.reset(); - llmq::quorumSnapshotManager.reset(new llmq::CQuorumSnapshotManager(*node.evodb)); + qsnapman.reset(); + qsnapman.reset(new llmq::CQuorumSnapshotManager(*evodb)); - if (node.llmq_ctx) { - node.llmq_ctx->Interrupt(); - node.llmq_ctx->Stop(); + if (llmq_ctx) { + llmq_ctx->Interrupt(); + llmq_ctx->Stop(); } - node.llmq_ctx.reset(); - node.llmq_ctx = std::make_unique(chainman, *node.dmnman, *node.evodb, *node.mn_metaman, *node.mnhf_manager, *node.sporkman, - *node.mempool, node.mn_activeman.get(), *node.mn_sync, /*unit_tests=*/false, /*wipe=*/fReset || fReindexChainState); + llmq_ctx.reset(); + llmq_ctx = std::make_unique(chainman, *dmnman, *evodb, mn_metaman, *mnhf_manager, sporkman, + *mempool, mn_activeman.get(), mn_sync, /*unit_tests=*/false, /*wipe=*/fReset || fReindexChainState); // Enable CMNHFManager::{Process, Undo}Block - node.mnhf_manager->ConnectManagers(node.chainman.get(), node.llmq_ctx->qman.get()); + mnhf_manager->ConnectManagers(&chainman, llmq_ctx->qman.get()); - node.chain_helper.reset(); - node.chain_helper = std::make_unique(*node.cpoolman, *node.dmnman, *node.mnhf_manager, *node.govman, *(node.llmq_ctx->quorum_block_processor), *node.chainman, - chainparams.GetConsensus(), *node.mn_sync, *node.sporkman, *(node.llmq_ctx->clhandler), *(node.llmq_ctx->qman)); + chain_helper.reset(); + chain_helper = std::make_unique(*cpoolman, *dmnman, *mnhf_manager, govman, *(llmq_ctx->quorum_block_processor), chainman, + chainparams.GetConsensus(), mn_sync, sporkman, *(llmq_ctx->clhandler), *(llmq_ctx->qman)); if (fReset) { pblocktree->WriteReindexing(true); @@ -198,7 +211,7 @@ std::optional LoadChainstate(bool fReset, // TODO: CEvoDB instance should probably be a part of CChainState // (for multiple chainstates to actually work in parallel) // and not a global - if (&chainman.ActiveChainstate() == chainstate && !node.evodb->CommitRootTransaction()) { + if (&chainman.ActiveChainstate() == chainstate && !evodb->CommitRootTransaction()) { return ChainstateLoadingError::ERROR_COMMITING_EVO_DB; } @@ -211,10 +224,10 @@ std::optional LoadChainstate(bool fReset, } } - if (!node.dmnman->MigrateDBIfNeeded() || !node.dmnman->MigrateDBIfNeeded2()) { + if (!dmnman->MigrateDBIfNeeded() || !dmnman->MigrateDBIfNeeded2()) { return ChainstateLoadingError::ERROR_UPGRADING_EVO_DB; } - if (!node.mnhf_manager->ForceSignalDBUpdate()) { + if (!mnhf_manager->ForceSignalDBUpdate()) { return ChainstateLoadingError::ERROR_UPGRADING_SIGNALS_DB; } } catch (const std::exception& e) { @@ -246,7 +259,7 @@ std::optional LoadChainstate(bool fReset, if (!CVerifyDB().VerifyDB( *chainstate, chainparams, chainstate->CoinsDB(), - *node.evodb, + *evodb, check_level, check_blocks)) { return ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB; @@ -267,7 +280,7 @@ std::optional LoadChainstate(bool fReset, // TODO: CEvoDB instance should probably be a part of CChainState // (for multiple chainstates to actually work in parallel) // and not a global - if (&chainman.ActiveChainstate() == chainstate && !node.evodb->IsEmpty()) { + if (&chainman.ActiveChainstate() == chainstate && !evodb->IsEmpty()) { // EvoDB processed some blocks earlier but we have no blocks anymore, something is wrong return ChainstateLoadingError::ERROR_EVO_DB_SANITY_FAILED; } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index d2bd675c95..0dc31b01c4 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -6,11 +6,29 @@ #define BITCOIN_NODE_CHAINSTATE_H #include // for int64_t +#include // for std::unique_ptr #include // for std::optional +class CActiveMasternodeManager; class CChainParams; +class CChainstateHelper; +class CCreditPoolManager; +class CDeterministicMNManager; +class CEvoDB; +class CGovernanceManager; class ChainstateManager; -struct NodeContext; +class CMasternodeMetaMan; +class CMasternodeSync; +class CMNHFManager; +class CSporkManager; +class CTxMemPool; +struct LLMQContext; + +namespace llmq { +class CChainLocksHandler; +class CInstantSendManager; +class CQuorumSnapshotManager; +} enum class ChainstateLoadingError { ERROR_LOADING_BLOCK_DB, @@ -63,7 +81,21 @@ enum class ChainstateLoadingError { */ std::optional LoadChainstate(bool fReset, ChainstateManager& chainman, - NodeContext& node, + CGovernanceManager& govman, + CMasternodeMetaMan& mn_metaman, + CMasternodeSync& mn_sync, + CSporkManager& sporkman, + std::unique_ptr& mn_activeman, + std::unique_ptr& chain_helper, + std::unique_ptr& cpoolman, + std::unique_ptr& dmnman, + std::unique_ptr& evodb, + std::unique_ptr& mnhf_manager, + std::unique_ptr& clhandler, + std::unique_ptr& isman, + std::unique_ptr& qsnapman, + std::unique_ptr& llmq_ctx, + CTxMemPool* mempool, bool fPruneMode, bool is_addrindex_enabled, bool is_governance_enabled, From 29c736280dd0034b093187fa8fe796d743efceb0 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 14 Dec 2024 17:10:01 +0000 Subject: [PATCH 33/55] Move mempool nullptr Assert out of LoadChainstate --- src/init.cpp | 2 +- src/node/chainstate.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 88d9221451..bd9fa4e499 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1881,7 +1881,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) llmq::quorumInstantSendManager, llmq::quorumSnapshotManager, node.llmq_ctx, - node.mempool.get(), + Assert(node.mempool.get()), fPruneMode, args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX), is_governance_enabled, diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 49894e42eb..96652513a9 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -69,7 +69,7 @@ std::optional LoadChainstate(bool fReset, mnhf_manager.reset(); mnhf_manager = std::make_unique(*evodb); - chainman.InitializeChainstate(Assert(mempool), *evodb, chain_helper, clhandler, isman); + chainman.InitializeChainstate(mempool, *evodb, chain_helper, clhandler, isman); chainman.m_total_coinstip_cache = nCoinCacheUsage; chainman.m_total_coinsdb_cache = nCoinDBCache; From 2ea1bbc7aa7ae8a7c97efe9f004a26e8bbb53b8b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 11 Nov 2021 16:20:44 -0500 Subject: [PATCH 34/55] Move init logistics message for BAD_GENESIS_BLOCK to init.cpp --- src/init.cpp | 2 ++ src/node/chainstate.cpp | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index bd9fa4e499..0ae823a5fd 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1901,6 +1901,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) strLoadError = _("Error loading block database"); break; case ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK: + // If the loaded chain has a wrong genesis, bail out immediately + // (we're likely using a testnet datadir, or the other way around). return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); case ChainstateLoadingError::ERROR_BAD_DEVNET_GENESIS_BLOCK: return InitError(_("Incorrect or no devnet genesis block found. Wrong datadir for devnet specified?")); diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 96652513a9..0b81986ee4 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -128,8 +128,6 @@ std::optional LoadChainstate(bool fReset, return ChainstateLoadingError::ERROR_TXINDEX_DISABLED_WHEN_GOV_ENABLED; } - // If the loaded chain has a wrong genesis, bail out immediately - // (we're likely using a testnet datadir, or the other way around). if (!chainman.BlockIndex().empty() && !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { return ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK; From 53231ca29d0b033c9dbe152cc15db210c0f1a284 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Oct 2024 08:33:33 +0000 Subject: [PATCH 35/55] node/chainstate: Remove do/while loop --- src/node/chainstate.cpp | 381 ++++++++++++++++++++-------------------- 1 file changed, 190 insertions(+), 191 deletions(-) diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 0b81986ee4..1e3ad84208 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -58,236 +58,235 @@ std::optional LoadChainstate(bool fReset, return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); }; - do { - try { - LOCK(cs_main); + try { + LOCK(cs_main); - int64_t nEvoDbCache{64 * 1024 * 1024}; // TODO - evodb.reset(); - evodb = std::make_unique(nEvoDbCache, false, fReset || fReindexChainState); + int64_t nEvoDbCache{64 * 1024 * 1024}; // TODO + evodb.reset(); + evodb = std::make_unique(nEvoDbCache, false, fReset || fReindexChainState); - mnhf_manager.reset(); - mnhf_manager = std::make_unique(*evodb); + mnhf_manager.reset(); + mnhf_manager = std::make_unique(*evodb); - chainman.InitializeChainstate(mempool, *evodb, chain_helper, clhandler, isman); - chainman.m_total_coinstip_cache = nCoinCacheUsage; - chainman.m_total_coinsdb_cache = nCoinDBCache; + chainman.InitializeChainstate(mempool, *evodb, chain_helper, clhandler, isman); + chainman.m_total_coinstip_cache = nCoinCacheUsage; + chainman.m_total_coinsdb_cache = nCoinDBCache; - auto& pblocktree{chainman.m_blockman.m_block_tree_db}; - // new CBlockTreeDB tries to delete the existing file, which - // fails if it's still open from the previous loop. Close it first: - pblocktree.reset(); - pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); + auto& pblocktree{chainman.m_blockman.m_block_tree_db}; + // new CBlockTreeDB tries to delete the existing file, which + // fails if it's still open from the previous loop. Close it first: + pblocktree.reset(); + pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); - // Same logic as above with pblocktree - dmnman.reset(); - dmnman = std::make_unique(chainman.ActiveChainstate(), *evodb); - mempool->ConnectManagers(dmnman.get()); + // Same logic as above with pblocktree + dmnman.reset(); + dmnman = std::make_unique(chainman.ActiveChainstate(), *evodb); + mempool->ConnectManagers(dmnman.get()); - cpoolman.reset(); - cpoolman = std::make_unique(*evodb); + cpoolman.reset(); + cpoolman = std::make_unique(*evodb); - qsnapman.reset(); - qsnapman.reset(new llmq::CQuorumSnapshotManager(*evodb)); + qsnapman.reset(); + qsnapman.reset(new llmq::CQuorumSnapshotManager(*evodb)); - if (llmq_ctx) { - llmq_ctx->Interrupt(); - llmq_ctx->Stop(); - } - llmq_ctx.reset(); - llmq_ctx = std::make_unique(chainman, *dmnman, *evodb, mn_metaman, *mnhf_manager, sporkman, - *mempool, mn_activeman.get(), mn_sync, /*unit_tests=*/false, /*wipe=*/fReset || fReindexChainState); - // Enable CMNHFManager::{Process, Undo}Block - mnhf_manager->ConnectManagers(&chainman, llmq_ctx->qman.get()); - - chain_helper.reset(); - chain_helper = std::make_unique(*cpoolman, *dmnman, *mnhf_manager, govman, *(llmq_ctx->quorum_block_processor), chainman, - chainparams.GetConsensus(), mn_sync, sporkman, *(llmq_ctx->clhandler), *(llmq_ctx->qman)); - - if (fReset) { - pblocktree->WriteReindexing(true); - //If we're reindexing in prune mode, wipe away unusable block files and all undo data files - if (fPruneMode) - CleanupBlockRevFiles(); - } + if (llmq_ctx) { + llmq_ctx->Interrupt(); + llmq_ctx->Stop(); + } + llmq_ctx.reset(); + llmq_ctx = std::make_unique(chainman, *dmnman, *evodb, mn_metaman, *mnhf_manager, sporkman, + *mempool, mn_activeman.get(), mn_sync, /*unit_tests=*/false, /*wipe=*/fReset || fReindexChainState); + // Enable CMNHFManager::{Process, Undo}Block + mnhf_manager->ConnectManagers(&chainman, llmq_ctx->qman.get()); + + chain_helper.reset(); + chain_helper = std::make_unique(*cpoolman, *dmnman, *mnhf_manager, govman, *(llmq_ctx->quorum_block_processor), chainman, + chainparams.GetConsensus(), mn_sync, sporkman, *(llmq_ctx->clhandler), *(llmq_ctx->qman)); + + if (fReset) { + pblocktree->WriteReindexing(true); + //If we're reindexing in prune mode, wipe away unusable block files and all undo data files + if (fPruneMode) + CleanupBlockRevFiles(); + } + + if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED; + // LoadBlockIndex will load m_have_pruned if we've ever removed a + // block file from disk. + // Note that it also sets fReindex based on the disk flag! + // From here on out fReindex and fReset mean something different! + if (!chainman.LoadBlockIndex()) { if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED; + return ChainstateLoadingError::ERROR_LOADING_BLOCK_DB; + } - // LoadBlockIndex will load m_have_pruned if we've ever removed a - // block file from disk. - // Note that it also sets fReindex based on the disk flag! - // From here on out fReindex and fReset mean something different! - if (!chainman.LoadBlockIndex()) { - if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED; - return ChainstateLoadingError::ERROR_LOADING_BLOCK_DB; - } + // TODO: Remove this when pruning is fixed. + // See https://github.com/dashpay/dash/pull/1817 and https://github.com/dashpay/dash/pull/1743 + if (is_governance_enabled && !is_txindex_enabled && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { + return ChainstateLoadingError::ERROR_TXINDEX_DISABLED_WHEN_GOV_ENABLED; + } + + if (!chainman.BlockIndex().empty() && + !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { + return ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK; + } - // TODO: Remove this when pruning is fixed. - // See https://github.com/dashpay/dash/pull/1817 and https://github.com/dashpay/dash/pull/1743 - if (is_governance_enabled && !is_txindex_enabled && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { - return ChainstateLoadingError::ERROR_TXINDEX_DISABLED_WHEN_GOV_ENABLED; + if (!chainparams.GetConsensus().hashDevnetGenesisBlock.IsNull() && !chainman.BlockIndex().empty() && + !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashDevnetGenesisBlock)) { + return ChainstateLoadingError::ERROR_BAD_DEVNET_GENESIS_BLOCK; + } + + if (!fReset && !fReindexChainState) { + // Check for changed -addressindex state + if (!fAddressIndex && fAddressIndex != is_addrindex_enabled) { + return ChainstateLoadingError::ERROR_ADDRIDX_NEEDS_REINDEX; } - if (!chainman.BlockIndex().empty() && - !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { - return ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK; + // Check for changed -timestampindex state + if (!fTimestampIndex && fTimestampIndex != is_timeindex_enabled) { + return ChainstateLoadingError::ERROR_TIMEIDX_NEEDS_REINDEX; } - if (!chainparams.GetConsensus().hashDevnetGenesisBlock.IsNull() && !chainman.BlockIndex().empty() && - !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashDevnetGenesisBlock)) { - return ChainstateLoadingError::ERROR_BAD_DEVNET_GENESIS_BLOCK; + // Check for changed -spentindex state + if (!fSpentIndex && fSpentIndex != is_spentindex_enabled) { + return ChainstateLoadingError::ERROR_SPENTIDX_NEEDS_REINDEX; } + } - if (!fReset && !fReindexChainState) { - // Check for changed -addressindex state - if (!fAddressIndex && fAddressIndex != is_addrindex_enabled) { - return ChainstateLoadingError::ERROR_ADDRIDX_NEEDS_REINDEX; - } + chainman.InitAdditionalIndexes(); - // Check for changed -timestampindex state - if (!fTimestampIndex && fTimestampIndex != is_timeindex_enabled) { - return ChainstateLoadingError::ERROR_TIMEIDX_NEEDS_REINDEX; - } + LogPrintf("%s: address index %s\n", __func__, fAddressIndex ? "enabled" : "disabled"); + LogPrintf("%s: timestamp index %s\n", __func__, fTimestampIndex ? "enabled" : "disabled"); + LogPrintf("%s: spent index %s\n", __func__, fSpentIndex ? "enabled" : "disabled"); - // Check for changed -spentindex state - if (!fSpentIndex && fSpentIndex != is_spentindex_enabled) { - return ChainstateLoadingError::ERROR_SPENTIDX_NEEDS_REINDEX; - } - } + // Check for changed -prune state. What we are concerned about is a user who has pruned blocks + // in the past, but is now trying to run unpruned. + if (chainman.m_blockman.m_have_pruned && !fPruneMode) { + return ChainstateLoadingError::ERROR_PRUNED_NEEDS_REINDEX; + } - chainman.InitAdditionalIndexes(); + // At this point blocktree args are consistent with what's on disk. + // If we're not mid-reindex (based on disk + args), add a genesis block on disk + // (otherwise we use the one already on disk). + // This is called again in ThreadImport after the reindex completes. + if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) { + return ChainstateLoadingError::ERROR_LOAD_GENESIS_BLOCK_FAILED; + } - LogPrintf("%s: address index %s\n", __func__, fAddressIndex ? "enabled" : "disabled"); - LogPrintf("%s: timestamp index %s\n", __func__, fTimestampIndex ? "enabled" : "disabled"); - LogPrintf("%s: spent index %s\n", __func__, fSpentIndex ? "enabled" : "disabled"); + // At this point we're either in reindex or we've loaded a useful + // block tree into BlockIndex()! + + for (CChainState* chainstate : chainman.GetAll()) { + chainstate->InitCoinsDB( + /* cache_size_bytes */ nCoinDBCache, + /* in_memory */ false, + /* should_wipe */ fReset || fReindexChainState); + + chainstate->CoinsErrorCatcher().AddReadErrCallback([]() { + uiInterface.ThreadSafeMessageBox( + _("Error reading from database, shutting down."), + "", CClientUIInterface::MSG_ERROR); + }); + + // If necessary, upgrade from older database format. + // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate + if (!chainstate->CoinsDB().Upgrade()) { + return ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED; + } - // Check for changed -prune state. What we are concerned about is a user who has pruned blocks - // in the past, but is now trying to run unpruned. - if (chainman.m_blockman.m_have_pruned && !fPruneMode) { - return ChainstateLoadingError::ERROR_PRUNED_NEEDS_REINDEX; + // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate + if (!chainstate->ReplayBlocks()) { + return ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED; } - // At this point blocktree args are consistent with what's on disk. - // If we're not mid-reindex (based on disk + args), add a genesis block on disk - // (otherwise we use the one already on disk). - // This is called again in ThreadImport after the reindex completes. - if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) { - return ChainstateLoadingError::ERROR_LOAD_GENESIS_BLOCK_FAILED; + // The on-disk coinsdb is now in a good state, create the cache + chainstate->InitCoinsCache(nCoinCacheUsage); + assert(chainstate->CanFlushToDisk()); + + // flush evodb + // TODO: CEvoDB instance should probably be a part of CChainState + // (for multiple chainstates to actually work in parallel) + // and not a global + if (&chainman.ActiveChainstate() == chainstate && !evodb->CommitRootTransaction()) { + return ChainstateLoadingError::ERROR_COMMITING_EVO_DB; } - // At this point we're either in reindex or we've loaded a useful - // block tree into BlockIndex()! - - for (CChainState* chainstate : chainman.GetAll()) { - chainstate->InitCoinsDB( - /* cache_size_bytes */ nCoinDBCache, - /* in_memory */ false, - /* should_wipe */ fReset || fReindexChainState); - - chainstate->CoinsErrorCatcher().AddReadErrCallback([]() { - uiInterface.ThreadSafeMessageBox( - _("Error reading from database, shutting down."), - "", CClientUIInterface::MSG_ERROR); - }); - - // If necessary, upgrade from older database format. - // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!chainstate->CoinsDB().Upgrade()) { - return ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED; + if (!is_coinsview_empty(chainstate)) { + // LoadChainTip initializes the chain based on CoinsTip()'s best block + if (!chainstate->LoadChainTip()) { + return ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED; } + assert(chainstate->m_chain.Tip() != nullptr); + } + } - // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!chainstate->ReplayBlocks()) { - return ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED; + if (!dmnman->MigrateDBIfNeeded() || !dmnman->MigrateDBIfNeeded2()) { + return ChainstateLoadingError::ERROR_UPGRADING_EVO_DB; + } + if (!mnhf_manager->ForceSignalDBUpdate()) { + return ChainstateLoadingError::ERROR_UPGRADING_SIGNALS_DB; + } + } catch (const std::exception& e) { + LogPrintf("%s\n", e.what()); + return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; + } + + try { + LOCK(cs_main); + + for (CChainState* chainstate : chainman.GetAll()) { + if (!is_coinsview_empty(chainstate)) { + uiInterface.InitMessage(_("Verifying blocks…").translated); + if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) { + LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", + MIN_BLOCKS_TO_KEEP); } - // The on-disk coinsdb is now in a good state, create the cache - chainstate->InitCoinsCache(nCoinCacheUsage); - assert(chainstate->CanFlushToDisk()); + const CBlockIndex* tip = chainstate->m_chain.Tip(); + RPCNotifyBlockChange(tip); + if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { + return ChainstateLoadingError::ERROR_BLOCK_FROM_FUTURE; + } + const bool v19active{DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V19)}; + if (v19active) { + bls::bls_legacy_scheme.store(false); + LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); + } - // flush evodb - // TODO: CEvoDB instance should probably be a part of CChainState - // (for multiple chainstates to actually work in parallel) - // and not a global - if (&chainman.ActiveChainstate() == chainstate && !evodb->CommitRootTransaction()) { - return ChainstateLoadingError::ERROR_COMMITING_EVO_DB; + if (!CVerifyDB().VerifyDB( + *chainstate, chainparams, chainstate->CoinsDB(), + *evodb, + check_level, + check_blocks)) { + return ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB; } - if (!is_coinsview_empty(chainstate)) { - // LoadChainTip initializes the chain based on CoinsTip()'s best block - if (!chainstate->LoadChainTip()) { - return ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED; - } - assert(chainstate->m_chain.Tip() != nullptr); + // VerifyDB() disconnects blocks which might result in us switching back to legacy. + // Make sure we use the right scheme. + if (v19active && bls::bls_legacy_scheme.load()) { + bls::bls_legacy_scheme.store(false); + LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); } - } - if (!dmnman->MigrateDBIfNeeded() || !dmnman->MigrateDBIfNeeded2()) { - return ChainstateLoadingError::ERROR_UPGRADING_EVO_DB; - } - if (!mnhf_manager->ForceSignalDBUpdate()) { - return ChainstateLoadingError::ERROR_UPGRADING_SIGNALS_DB; - } - } catch (const std::exception& e) { - LogPrintf("%s\n", e.what()); - return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; - } + if (check_level >= 3) { + chainstate->ResetBlockFailureFlags(nullptr); + } - try { - LOCK(cs_main); - - for (CChainState* chainstate : chainman.GetAll()) { - if (!is_coinsview_empty(chainstate)) { - uiInterface.InitMessage(_("Verifying blocks…").translated); - if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) { - LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", - MIN_BLOCKS_TO_KEEP); - } - - const CBlockIndex* tip = chainstate->m_chain.Tip(); - RPCNotifyBlockChange(tip); - if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { - return ChainstateLoadingError::ERROR_BLOCK_FROM_FUTURE; - } - const bool v19active{DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V19)}; - if (v19active) { - bls::bls_legacy_scheme.store(false); - LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); - } - - if (!CVerifyDB().VerifyDB( - *chainstate, chainparams, chainstate->CoinsDB(), - *evodb, - check_level, - check_blocks)) { - return ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB; - } - - // VerifyDB() disconnects blocks which might result in us switching back to legacy. - // Make sure we use the right scheme. - if (v19active && bls::bls_legacy_scheme.load()) { - bls::bls_legacy_scheme.store(false); - LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); - } - - if (check_level >= 3) { - chainstate->ResetBlockFailureFlags(nullptr); - } - - } else { - // TODO: CEvoDB instance should probably be a part of CChainState - // (for multiple chainstates to actually work in parallel) - // and not a global - if (&chainman.ActiveChainstate() == chainstate && !evodb->IsEmpty()) { - // EvoDB processed some blocks earlier but we have no blocks anymore, something is wrong - return ChainstateLoadingError::ERROR_EVO_DB_SANITY_FAILED; - } + } else { + // TODO: CEvoDB instance should probably be a part of CChainState + // (for multiple chainstates to actually work in parallel) + // and not a global + if (&chainman.ActiveChainstate() == chainstate && !evodb->IsEmpty()) { + // EvoDB processed some blocks earlier but we have no blocks anymore, something is wrong + return ChainstateLoadingError::ERROR_EVO_DB_SANITY_FAILED; } } - } catch (const std::exception& e) { - LogPrintf("%s\n", e.what()); - return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; } - } while(false); + } catch (const std::exception& e) { + LogPrintf("%s\n", e.what()); + return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; + } + return std::nullopt; } From 913411ed7311779f4f8907829e34c72584c002f7 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 10 Nov 2021 15:57:14 -0500 Subject: [PATCH 36/55] Split off VerifyLoadedChainstate --- src/init.cpp | 45 ++++++++++++++++++++++++++--------------- src/node/chainstate.cpp | 31 +++++++++++++++++++--------- src/node/chainstate.h | 22 ++++++++++++++------ 3 files changed, 67 insertions(+), 31 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 0ae823a5fd..81a03cb100 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1892,9 +1892,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) fReindexChainState, nBlockTreeDBCache, nCoinDBCache, - nCoinCacheUsage, - args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS), - args.GetArg("-checklevel", DEFAULT_CHECKLEVEL)); + nCoinCacheUsage); if (rv.has_value()) { switch (rv.value()) { case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB: @@ -1932,20 +1930,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) case ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED: strLoadError = _("Error initializing block database"); break; - case ChainstateLoadingError::ERROR_EVO_DB_SANITY_FAILED: - strLoadError = _("Error initializing block database"); - break; case ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED: strLoadError = _("Error opening block database"); break; - case ChainstateLoadingError::ERROR_BLOCK_FROM_FUTURE: - strLoadError = _("The block database contains a block which appears to be from the future. " - "This may be due to your computer's date and time being set incorrectly. " - "Only rebuild the block database if you are sure that your computer's date and time are correct"); - break; - case ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB: - strLoadError = _("Corrupted block database detected"); - break; case ChainstateLoadingError::ERROR_COMMITING_EVO_DB: strLoadError = _("Failed to commit Evo database"); break; @@ -1959,8 +1946,34 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) break; } } else { - fLoaded = true; - LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); + auto rv2 = VerifyLoadedChainstate(chainman, + *Assert(node.evodb.get()), + fReset, + fReindexChainState, + chainparams, + args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS), + args.GetArg("-checklevel", DEFAULT_CHECKLEVEL)); + if (rv2.has_value()) { + switch (rv2.value()) { + case ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE: + strLoadError = _("The block database contains a block which appears to be from the future. " + "This may be due to your computer's date and time being set incorrectly. " + "Only rebuild the block database if you are sure that your computer's date and time are correct"); + break; + case ChainstateLoadVerifyError::ERROR_CORRUPTED_BLOCK_DB: + strLoadError = _("Corrupted block database detected"); + break; + case ChainstateLoadVerifyError::ERROR_EVO_DB_SANITY_FAILED: + strLoadError = _("Error initializing block database"); + break; + case ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE: + strLoadError = _("Error opening block database"); + break; + } + } else { + fLoaded = true; + LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); + } } if (!fLoaded && !ShutdownRequested()) { diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 1e3ad84208..350daec44c 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -50,9 +50,7 @@ std::optional LoadChainstate(bool fReset, bool fReindexChainState, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, - int64_t nCoinCacheUsage, - unsigned int check_blocks, - unsigned int check_level) + int64_t nCoinCacheUsage) { auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); @@ -232,6 +230,21 @@ std::optional LoadChainstate(bool fReset, return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; } + return std::nullopt; +} + +std::optional VerifyLoadedChainstate(ChainstateManager& chainman, + CEvoDB& evodb, + bool fReset, + bool fReindexChainState, + const CChainParams& chainparams, + unsigned int check_blocks, + unsigned int check_level) +{ + auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); + }; + try { LOCK(cs_main); @@ -246,7 +259,7 @@ std::optional LoadChainstate(bool fReset, const CBlockIndex* tip = chainstate->m_chain.Tip(); RPCNotifyBlockChange(tip); if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { - return ChainstateLoadingError::ERROR_BLOCK_FROM_FUTURE; + return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE; } const bool v19active{DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V19)}; if (v19active) { @@ -256,10 +269,10 @@ std::optional LoadChainstate(bool fReset, if (!CVerifyDB().VerifyDB( *chainstate, chainparams, chainstate->CoinsDB(), - *evodb, + evodb, check_level, check_blocks)) { - return ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB; + return ChainstateLoadVerifyError::ERROR_CORRUPTED_BLOCK_DB; } // VerifyDB() disconnects blocks which might result in us switching back to legacy. @@ -277,15 +290,15 @@ std::optional LoadChainstate(bool fReset, // TODO: CEvoDB instance should probably be a part of CChainState // (for multiple chainstates to actually work in parallel) // and not a global - if (&chainman.ActiveChainstate() == chainstate && !evodb->IsEmpty()) { + if (&chainman.ActiveChainstate() == chainstate && !evodb.IsEmpty()) { // EvoDB processed some blocks earlier but we have no blocks anymore, something is wrong - return ChainstateLoadingError::ERROR_EVO_DB_SANITY_FAILED; + return ChainstateLoadVerifyError::ERROR_EVO_DB_SANITY_FAILED; } } } } catch (const std::exception& e) { LogPrintf("%s\n", e.what()); - return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; + return ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE; } return std::nullopt; diff --git a/src/node/chainstate.h b/src/node/chainstate.h index 0dc31b01c4..ad35f9ecc8 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -43,10 +43,7 @@ enum class ChainstateLoadingError { ERROR_CHAINSTATE_UPGRADE_FAILED, ERROR_REPLAYBLOCKS_FAILED, ERROR_LOADCHAINTIP_FAILED, - ERROR_EVO_DB_SANITY_FAILED, ERROR_GENERIC_BLOCKDB_OPEN_FAILED, - ERROR_BLOCK_FROM_FUTURE, - ERROR_CORRUPTED_BLOCK_DB, ERROR_COMMITING_EVO_DB, ERROR_UPGRADING_EVO_DB, ERROR_UPGRADING_SIGNALS_DB, @@ -106,8 +103,21 @@ std::optional LoadChainstate(bool fReset, bool fReindexChainState, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, - int64_t nCoinCacheUsage, - unsigned int check_blocks, - unsigned int check_level); + int64_t nCoinCacheUsage); + +enum class ChainstateLoadVerifyError { + ERROR_BLOCK_FROM_FUTURE, + ERROR_CORRUPTED_BLOCK_DB, + ERROR_EVO_DB_SANITY_FAILED, + ERROR_GENERIC_FAILURE, +}; + +std::optional VerifyLoadedChainstate(ChainstateManager& chainman, + CEvoDB& evodb, + bool fReset, + bool fReindexChainState, + const CChainParams& chainparams, + unsigned int check_blocks, + unsigned int check_level); #endif // BITCOIN_NODE_CHAINSTATE_H From a141f5d9a7babb1bf9f35acaef87244f5fa3972a Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 18 Aug 2021 13:39:34 -0400 Subject: [PATCH 37/55] node/chainstate: Decouple from concept of uiInterface --- src/init.cpp | 8 +++++++- src/node/chainstate.cpp | 13 +++++-------- src/node/chainstate.h | 4 +++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 81a03cb100..d58ef908ca 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1892,7 +1892,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) fReindexChainState, nBlockTreeDBCache, nCoinDBCache, - nCoinCacheUsage); + nCoinCacheUsage, + []() { + uiInterface.ThreadSafeMessageBox( + _("Error reading from database, shutting down."), + "", CClientUIInterface::MSG_ERROR); + }); if (rv.has_value()) { switch (rv.value()) { case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB: @@ -1946,6 +1951,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) break; } } else { + uiInterface.InitMessage(_("Verifying blocks…").translated); auto rv2 = VerifyLoadedChainstate(chainman, *Assert(node.evodb.get()), fReset, diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 350daec44c..4562d750b5 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -9,7 +9,6 @@ #include // for RPCNotifyBlockChange #include // for GetTime #include // for CleanupBlockRevFiles, fHavePruned, fReindex -#include // for InitError, uiInterface, and CClientUIInterface member access #include // for ShutdownRequested #include // for a lot of things @@ -50,7 +49,8 @@ std::optional LoadChainstate(bool fReset, bool fReindexChainState, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, - int64_t nCoinCacheUsage) + int64_t nCoinCacheUsage, + std::function coins_error_cb) { auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); @@ -181,11 +181,9 @@ std::optional LoadChainstate(bool fReset, /* in_memory */ false, /* should_wipe */ fReset || fReindexChainState); - chainstate->CoinsErrorCatcher().AddReadErrCallback([]() { - uiInterface.ThreadSafeMessageBox( - _("Error reading from database, shutting down."), - "", CClientUIInterface::MSG_ERROR); - }); + if (coins_error_cb) { + chainstate->CoinsErrorCatcher().AddReadErrCallback(coins_error_cb); + } // If necessary, upgrade from older database format. // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate @@ -250,7 +248,6 @@ std::optional VerifyLoadedChainstate(ChainstateManage for (CChainState* chainstate : chainman.GetAll()) { if (!is_coinsview_empty(chainstate)) { - uiInterface.InitMessage(_("Verifying blocks…").translated); if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) { LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", MIN_BLOCKS_TO_KEEP); diff --git a/src/node/chainstate.h b/src/node/chainstate.h index ad35f9ecc8..442d8b5f92 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -6,6 +6,7 @@ #define BITCOIN_NODE_CHAINSTATE_H #include // for int64_t +#include // for std::function #include // for std::unique_ptr #include // for std::optional @@ -103,7 +104,8 @@ std::optional LoadChainstate(bool fReset, bool fReindexChainState, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, - int64_t nCoinCacheUsage); + int64_t nCoinCacheUsage, + std::function coins_error_cb = nullptr); enum class ChainstateLoadVerifyError { ERROR_BLOCK_FROM_FUTURE, From d3345eeccc5fd972635eef6051bbf81cc7e0406e Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Oct 2024 08:57:52 +0000 Subject: [PATCH 38/55] node/chainstate: Reduce coupling of LogPrintf --- src/init.cpp | 94 +++++++++++++++++++++++------------------ src/node/chainstate.cpp | 10 +---- 2 files changed, 55 insertions(+), 49 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index d58ef908ca..007b64113b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1865,39 +1865,45 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) uiInterface.InitMessage(_("Loading block index…").translated); const auto load_block_index_start_time{SteadyClock::now()}; - auto rv = LoadChainstate(fReset, - chainman, - *node.govman, - *node.mn_metaman, - *node.mn_sync, - *node.sporkman, - node.mn_activeman, - node.chain_helper, - node.cpoolman, - node.dmnman, - node.evodb, - node.mnhf_manager, - llmq::chainLocksHandler, - llmq::quorumInstantSendManager, - llmq::quorumSnapshotManager, - node.llmq_ctx, - Assert(node.mempool.get()), - fPruneMode, - args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX), - is_governance_enabled, - args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX), - args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX), - args.GetBoolArg("-txindex", DEFAULT_TXINDEX), - chainparams, - fReindexChainState, - nBlockTreeDBCache, - nCoinDBCache, - nCoinCacheUsage, - []() { - uiInterface.ThreadSafeMessageBox( - _("Error reading from database, shutting down."), - "", CClientUIInterface::MSG_ERROR); - }); + std::optional rv; + try { + rv = LoadChainstate(fReset, + chainman, + *node.govman, + *node.mn_metaman, + *node.mn_sync, + *node.sporkman, + node.mn_activeman, + node.chain_helper, + node.cpoolman, + node.dmnman, + node.evodb, + node.mnhf_manager, + llmq::chainLocksHandler, + llmq::quorumInstantSendManager, + llmq::quorumSnapshotManager, + node.llmq_ctx, + Assert(node.mempool.get()), + fPruneMode, + args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX), + is_governance_enabled, + args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX), + args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX), + args.GetBoolArg("-txindex", DEFAULT_TXINDEX), + chainparams, + fReindexChainState, + nBlockTreeDBCache, + nCoinDBCache, + nCoinCacheUsage, + []() { + uiInterface.ThreadSafeMessageBox( + _("Error reading from database, shutting down."), + "", CClientUIInterface::MSG_ERROR); + }); + } catch (const std::exception& e) { + LogPrintf("%s\n", e.what()); + rv = ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; + } if (rv.has_value()) { switch (rv.value()) { case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB: @@ -1951,14 +1957,20 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) break; } } else { - uiInterface.InitMessage(_("Verifying blocks…").translated); - auto rv2 = VerifyLoadedChainstate(chainman, - *Assert(node.evodb.get()), - fReset, - fReindexChainState, - chainparams, - args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS), - args.GetArg("-checklevel", DEFAULT_CHECKLEVEL)); + std::optional rv2; + try { + uiInterface.InitMessage(_("Verifying blocks…").translated); + rv2 = VerifyLoadedChainstate(chainman, + *Assert(node.evodb.get()), + fReset, + fReindexChainState, + chainparams, + args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS), + args.GetArg("-checklevel", DEFAULT_CHECKLEVEL)); + } catch (const std::exception& e) { + LogPrintf("%s\n", e.what()); + rv2 = ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE; + } if (rv2.has_value()) { switch (rv2.value()) { case ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE: diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 4562d750b5..35165200fa 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -56,7 +56,7 @@ std::optional LoadChainstate(bool fReset, return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); }; - try { + { LOCK(cs_main); int64_t nEvoDbCache{64 * 1024 * 1024}; // TODO @@ -223,9 +223,6 @@ std::optional LoadChainstate(bool fReset, if (!mnhf_manager->ForceSignalDBUpdate()) { return ChainstateLoadingError::ERROR_UPGRADING_SIGNALS_DB; } - } catch (const std::exception& e) { - LogPrintf("%s\n", e.what()); - return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; } return std::nullopt; @@ -243,7 +240,7 @@ std::optional VerifyLoadedChainstate(ChainstateManage return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); }; - try { + { LOCK(cs_main); for (CChainState* chainstate : chainman.GetAll()) { @@ -293,9 +290,6 @@ std::optional VerifyLoadedChainstate(ChainstateManage } } } - } catch (const std::exception& e) { - LogPrintf("%s\n", e.what()); - return ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE; } return std::nullopt; From 94c0ceb29c1328b4b29363997f63cbf714255f68 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Oct 2024 12:32:20 +0000 Subject: [PATCH 39/55] Move -checkblocks LogPrintf to AppInitMain --- src/init.cpp | 7 ++++++- src/node/chainstate.cpp | 5 ----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 007b64113b..f6e4de9e55 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1960,12 +1960,17 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) std::optional rv2; try { uiInterface.InitMessage(_("Verifying blocks…").translated); + auto check_blocks = args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS); + if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) { + LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", + MIN_BLOCKS_TO_KEEP); + } rv2 = VerifyLoadedChainstate(chainman, *Assert(node.evodb.get()), fReset, fReindexChainState, chainparams, - args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS), + check_blocks, args.GetArg("-checklevel", DEFAULT_CHECKLEVEL)); } catch (const std::exception& e) { LogPrintf("%s\n", e.what()); diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 35165200fa..88cb8add95 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -245,11 +245,6 @@ std::optional VerifyLoadedChainstate(ChainstateManage for (CChainState* chainstate : chainman.GetAll()) { if (!is_coinsview_empty(chainstate)) { - if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) { - LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", - MIN_BLOCKS_TO_KEEP); - } - const CBlockIndex* tip = chainstate->m_chain.Tip(); RPCNotifyBlockChange(tip); if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { From f7aef8d331decc871108e50890567c5bc7967b1e Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 18 Aug 2021 14:36:28 -0400 Subject: [PATCH 40/55] init: Delay RPC block notif until warmup finished --- src/init.cpp | 10 ++++++++++ src/node/chainstate.cpp | 2 -- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index f6e4de9e55..eb4fd0d685 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2400,7 +2400,17 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 13: finished + // At this point, the RPC is "started", but still in warmup, which means it + // cannot yet be called. Before we make it callable, we need to make sure + // that the RPC's view of the best block is valid and consistent with + // ChainstateManager's ActiveTip. + // + // If we do not do this, RPC's view of the best block will be height=0 and + // hash=0x0. This will lead to erroroneous responses for things like + // waitforblockheight. + RPCNotifyBlockChange(chainman.ActiveTip()); SetRPCWarmupFinished(); + uiInterface.InitMessage(_("Done loading").translated); for (const auto& client : node.chain_clients) { diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 88cb8add95..59a91679ef 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -6,7 +6,6 @@ #include // for CChainParams #include // for DeploymentActiveAfter -#include // for RPCNotifyBlockChange #include // for GetTime #include // for CleanupBlockRevFiles, fHavePruned, fReindex #include // for ShutdownRequested @@ -246,7 +245,6 @@ std::optional VerifyLoadedChainstate(ChainstateManage for (CChainState* chainstate : chainman.GetAll()) { if (!is_coinsview_empty(chainstate)) { const CBlockIndex* tip = chainstate->m_chain.Tip(); - RPCNotifyBlockChange(tip); if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE; } From fdf803d013bb33b8c1e4114ce664de09cc9ab36e Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 22 Sep 2021 15:36:10 -0400 Subject: [PATCH 41/55] node/chainstate: Decouple from GetTime --- src/init.cpp | 3 ++- src/node/chainstate.cpp | 6 +++--- src/node/chainstate.h | 3 ++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index eb4fd0d685..88610adfcb 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1971,7 +1971,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) fReindexChainState, chainparams, check_blocks, - args.GetArg("-checklevel", DEFAULT_CHECKLEVEL)); + args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), + static_cast(GetTime)); } catch (const std::exception& e) { LogPrintf("%s\n", e.what()); rv2 = ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE; diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 59a91679ef..6ea69fcbb5 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -6,7 +6,6 @@ #include // for CChainParams #include // for DeploymentActiveAfter -#include // for GetTime #include // for CleanupBlockRevFiles, fHavePruned, fReindex #include // for ShutdownRequested #include // for a lot of things @@ -233,7 +232,8 @@ std::optional VerifyLoadedChainstate(ChainstateManage bool fReindexChainState, const CChainParams& chainparams, unsigned int check_blocks, - unsigned int check_level) + unsigned int check_level, + std::function get_unix_time_seconds) { auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull(); @@ -245,7 +245,7 @@ std::optional VerifyLoadedChainstate(ChainstateManage for (CChainState* chainstate : chainman.GetAll()) { if (!is_coinsview_empty(chainstate)) { const CBlockIndex* tip = chainstate->m_chain.Tip(); - if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) { + if (tip && tip->nTime > get_unix_time_seconds() + 2 * 60 * 60) { return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE; } const bool v19active{DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V19)}; diff --git a/src/node/chainstate.h b/src/node/chainstate.h index 442d8b5f92..c838fd7851 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -120,6 +120,7 @@ std::optional VerifyLoadedChainstate(ChainstateManage bool fReindexChainState, const CChainParams& chainparams, unsigned int check_blocks, - unsigned int check_level); + unsigned int check_level, + std::function get_unix_time_seconds); #endif // BITCOIN_NODE_CHAINSTATE_H From c405492874b47ec15b9543473758f592a1cdcddd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 22 Sep 2021 15:36:24 -0400 Subject: [PATCH 42/55] node/chainstate: Decouple from ShutdownRequested --- src/init.cpp | 1 + src/node/chainstate.cpp | 6 +++--- src/node/chainstate.h | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 88610adfcb..023f07f678 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1895,6 +1895,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) nBlockTreeDBCache, nCoinDBCache, nCoinCacheUsage, + ShutdownRequested, []() { uiInterface.ThreadSafeMessageBox( _("Error reading from database, shutting down."), diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 6ea69fcbb5..1fbf0b3d00 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -7,7 +7,6 @@ #include // for CChainParams #include // for DeploymentActiveAfter #include // for CleanupBlockRevFiles, fHavePruned, fReindex -#include // for ShutdownRequested #include // for a lot of things #include // for CChainstateHelper @@ -48,6 +47,7 @@ std::optional LoadChainstate(bool fReset, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, int64_t nCoinCacheUsage, + std::function shutdown_requested, std::function coins_error_cb) { auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { @@ -106,14 +106,14 @@ std::optional LoadChainstate(bool fReset, CleanupBlockRevFiles(); } - if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED; + if (shutdown_requested && shutdown_requested()) return ChainstateLoadingError::SHUTDOWN_PROBED; // LoadBlockIndex will load m_have_pruned if we've ever removed a // block file from disk. // Note that it also sets fReindex based on the disk flag! // From here on out fReindex and fReset mean something different! if (!chainman.LoadBlockIndex()) { - if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED; + if (shutdown_requested && shutdown_requested()) return ChainstateLoadingError::SHUTDOWN_PROBED; return ChainstateLoadingError::ERROR_LOADING_BLOCK_DB; } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index c838fd7851..2c023a538b 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -70,10 +70,10 @@ enum class ChainstateLoadingError { * differentiable by the specific enumerator. * * Note that a return value of SHUTDOWN_PROBED means ONLY that "during - * this sequence, when we explicitly checked ShutdownRequested() at + * this sequence, when we explicitly checked shutdown_requested() at * arbitrary points, one of those calls returned true". Therefore, a * return value other than SHUTDOWN_PROBED does not guarantee that - * ShutdownRequested() hasn't been called indirectly. + * shutdown_requested() hasn't been called indirectly. * - else * - Success! */ @@ -105,6 +105,7 @@ std::optional LoadChainstate(bool fReset, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, int64_t nCoinCacheUsage, + std::function shutdown_requested = nullptr, std::function coins_error_cb = nullptr); enum class ChainstateLoadVerifyError { From d7f1e234c5c7240f66ff9649b175cc699919cae6 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 14 Dec 2024 19:46:44 +0000 Subject: [PATCH 43/55] validation: VerifyDB only needs Consensus::Params --- src/init.cpp | 5 +++-- src/node/chainstate.cpp | 21 +++++++++++---------- src/node/chainstate.h | 11 ++++++++--- src/rpc/blockchain.cpp | 2 +- src/test/evo_deterministicmns_tests.cpp | 4 ++-- src/validation.cpp | 8 ++++---- src/validation.h | 2 +- 7 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 023f07f678..feb69482fe 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1890,7 +1890,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX), args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX), args.GetBoolArg("-txindex", DEFAULT_TXINDEX), - chainparams, + chainparams.GetConsensus(), + chainparams.NetworkIDString(), fReindexChainState, nBlockTreeDBCache, nCoinDBCache, @@ -1970,7 +1971,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) *Assert(node.evodb.get()), fReset, fReindexChainState, - chainparams, + chainparams.GetConsensus(), check_blocks, args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), static_cast(GetTime)); diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 1fbf0b3d00..8923f6a81f 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -4,7 +4,7 @@ #include -#include // for CChainParams +#include // for Consensus::Params #include // for DeploymentActiveAfter #include // for CleanupBlockRevFiles, fHavePruned, fReindex #include // for a lot of things @@ -42,7 +42,8 @@ std::optional LoadChainstate(bool fReset, bool is_spentindex_enabled, bool is_timeindex_enabled, bool is_txindex_enabled, - const CChainParams& chainparams, + const Consensus::Params& consensus_params, + const std::string& network_id, bool fReindexChainState, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, @@ -97,7 +98,7 @@ std::optional LoadChainstate(bool fReset, chain_helper.reset(); chain_helper = std::make_unique(*cpoolman, *dmnman, *mnhf_manager, govman, *(llmq_ctx->quorum_block_processor), chainman, - chainparams.GetConsensus(), mn_sync, sporkman, *(llmq_ctx->clhandler), *(llmq_ctx->qman)); + consensus_params, mn_sync, sporkman, *(llmq_ctx->clhandler), *(llmq_ctx->qman)); if (fReset) { pblocktree->WriteReindexing(true); @@ -119,17 +120,17 @@ std::optional LoadChainstate(bool fReset, // TODO: Remove this when pruning is fixed. // See https://github.com/dashpay/dash/pull/1817 and https://github.com/dashpay/dash/pull/1743 - if (is_governance_enabled && !is_txindex_enabled && chainparams.NetworkIDString() != CBaseChainParams::REGTEST) { + if (is_governance_enabled && !is_txindex_enabled && network_id != CBaseChainParams::REGTEST) { return ChainstateLoadingError::ERROR_TXINDEX_DISABLED_WHEN_GOV_ENABLED; } if (!chainman.BlockIndex().empty() && - !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { + !chainman.m_blockman.LookupBlockIndex(consensus_params.hashGenesisBlock)) { return ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK; } - if (!chainparams.GetConsensus().hashDevnetGenesisBlock.IsNull() && !chainman.BlockIndex().empty() && - !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashDevnetGenesisBlock)) { + if (!consensus_params.hashDevnetGenesisBlock.IsNull() && !chainman.BlockIndex().empty() && + !chainman.m_blockman.LookupBlockIndex(consensus_params.hashDevnetGenesisBlock)) { return ChainstateLoadingError::ERROR_BAD_DEVNET_GENESIS_BLOCK; } @@ -230,7 +231,7 @@ std::optional VerifyLoadedChainstate(ChainstateManage CEvoDB& evodb, bool fReset, bool fReindexChainState, - const CChainParams& chainparams, + const Consensus::Params& consensus_params, unsigned int check_blocks, unsigned int check_level, std::function get_unix_time_seconds) @@ -248,14 +249,14 @@ std::optional VerifyLoadedChainstate(ChainstateManage if (tip && tip->nTime > get_unix_time_seconds() + 2 * 60 * 60) { return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE; } - const bool v19active{DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V19)}; + const bool v19active{DeploymentActiveAfter(tip, consensus_params, Consensus::DEPLOYMENT_V19)}; if (v19active) { bls::bls_legacy_scheme.store(false); LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); } if (!CVerifyDB().VerifyDB( - *chainstate, chainparams, chainstate->CoinsDB(), + *chainstate, consensus_params, chainstate->CoinsDB(), evodb, check_level, check_blocks)) { diff --git a/src/node/chainstate.h b/src/node/chainstate.h index 2c023a538b..c2aa6d827f 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -9,9 +9,9 @@ #include // for std::function #include // for std::unique_ptr #include // for std::optional +#include // for std::string class CActiveMasternodeManager; -class CChainParams; class CChainstateHelper; class CCreditPoolManager; class CDeterministicMNManager; @@ -31,6 +31,10 @@ class CInstantSendManager; class CQuorumSnapshotManager; } +namespace Consensus { +struct Params; +} + enum class ChainstateLoadingError { ERROR_LOADING_BLOCK_DB, ERROR_BAD_GENESIS_BLOCK, @@ -100,7 +104,8 @@ std::optional LoadChainstate(bool fReset, bool is_spentindex_enabled, bool is_timeindex_enabled, bool is_txindex_enabled, - const CChainParams& chainparams, + const Consensus::Params& consensus_params, + const std::string& network_id, bool fReindexChainState, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, @@ -119,7 +124,7 @@ std::optional VerifyLoadedChainstate(ChainstateManage CEvoDB& evodb, bool fReset, bool fReindexChainState, - const CChainParams& chainparams, + const Consensus::Params& consensus_params, unsigned int check_blocks, unsigned int check_level, std::function get_unix_time_seconds); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 3f44150ce5..2eb41cee62 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1642,7 +1642,7 @@ static RPCHelpMan verifychain() CChainState& active_chainstate = chainman.ActiveChainstate(); return CVerifyDB().VerifyDB( - active_chainstate, Params(), active_chainstate.CoinsTip(), *CHECK_NONFATAL(node.evodb), check_level, check_depth); + active_chainstate, Params().GetConsensus(), active_chainstate.CoinsTip(), *CHECK_NONFATAL(node.evodb), check_level, check_depth); }, }; } diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index ae82700ad2..8783081f9c 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -816,8 +816,8 @@ void FuncVerifyDB(TestChainSetup& setup) // Verify db consistency LOCK(cs_main); - BOOST_REQUIRE(CVerifyDB().VerifyDB(chainman.ActiveChainstate(), Params(), chainman.ActiveChainstate().CoinsTip(), - *(setup.m_node.evodb), 4, 2)); + BOOST_REQUIRE(CVerifyDB().VerifyDB(chainman.ActiveChainstate(), Params().GetConsensus(), + chainman.ActiveChainstate().CoinsTip(), *(setup.m_node.evodb), 4, 2)); } BOOST_AUTO_TEST_SUITE(evo_dip3_activation_tests) diff --git a/src/validation.cpp b/src/validation.cpp index 5f093aa1b2..7f4cc03148 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4139,7 +4139,7 @@ CVerifyDB::~CVerifyDB() bool CVerifyDB::VerifyDB( CChainState& chainstate, - const CChainParams& chainparams, + const Consensus::Params& consensus_params, CCoinsView& coinsview, CEvoDB& evoDb, int nCheckLevel, int nCheckDepth) @@ -4185,10 +4185,10 @@ bool CVerifyDB::VerifyDB( } CBlock block; // check level 0: read from disk - if (!ReadBlockFromDisk(block, pindex, chainparams.GetConsensus())) + if (!ReadBlockFromDisk(block, pindex, consensus_params)) return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); // check level 1: verify block validity - if (nCheckLevel >= 1 && !CheckBlock(block, state, chainparams.GetConsensus())) + if (nCheckLevel >= 1 && !CheckBlock(block, state, consensus_params)) return error("%s: *** found bad block at %d, hash=%s (%s)\n", __func__, pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); // check level 2: verify undo validity @@ -4236,7 +4236,7 @@ bool CVerifyDB::VerifyDB( uiInterface.ShowProgress(_("Verifying blocks…").translated, percentageDone, false); pindex = chainstate.m_chain.Next(pindex); CBlock block; - if (!ReadBlockFromDisk(block, pindex, chainparams.GetConsensus())) + if (!ReadBlockFromDisk(block, pindex, consensus_params)) return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); if (!chainstate.ConnectBlock(block, state, pindex, coins)) return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); diff --git a/src/validation.h b/src/validation.h index 1644549a3c..ab12d9378d 100644 --- a/src/validation.h +++ b/src/validation.h @@ -367,7 +367,7 @@ class CVerifyDB { ~CVerifyDB(); bool VerifyDB( CChainState& chainstate, - const CChainParams& chainparams, + const Consensus::Params& consensus_params, CCoinsView& coinsview, CEvoDB& evoDb, int nCheckLevel, From 4ab182751e6c755d10964475d5d87046ff51069b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 21 Sep 2021 11:37:03 -0400 Subject: [PATCH 44/55] node/caches: Extract cache calculation logic --- src/Makefile.am | 2 ++ src/init.cpp | 41 +++++++++++++---------------------------- src/node/caches.cpp | 38 ++++++++++++++++++++++++++++++++++++++ src/node/caches.h | 22 ++++++++++++++++++++++ 4 files changed, 75 insertions(+), 28 deletions(-) create mode 100644 src/node/caches.cpp create mode 100644 src/node/caches.h diff --git a/src/Makefile.am b/src/Makefile.am index 1ce5f56fe1..2fd158fb21 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -269,6 +269,7 @@ BITCOIN_CORE_H = \ netgroup.h \ netmessagemaker.h \ node/blockstorage.h \ + node/caches.h \ node/chainstate.h \ node/coin.h \ node/coinstats.h \ @@ -502,6 +503,7 @@ libbitcoin_server_a_SOURCES = \ netgroup.cpp \ net_processing.cpp \ node/blockstorage.cpp \ + node/caches.cpp \ node/chainstate.cpp \ node/coin.cpp \ node/coinstats.cpp \ diff --git a/src/init.cpp b/src/init.cpp index feb69482fe..803db7244b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -38,6 +38,7 @@ #include #include #include +#include // for CalculateCacheSizes #include // for LoadChainstate #include #include @@ -1805,36 +1806,20 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) bool fReindexChainState = args.GetBoolArg("-reindex-chainstate", false); // cache size calculations - int64_t nTotalCache = (args.GetArg("-dbcache", nDefaultDbCache) << 20); - nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache - nTotalCache = std::min(nTotalCache, nMaxDbCache << 20); // total cache cannot be greater than nMaxDbcache - int64_t nBlockTreeDBCache = std::min(nTotalCache / 8, nMaxBlockDBCache << 20); - nTotalCache -= nBlockTreeDBCache; - int64_t nTxIndexCache = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0); - nTotalCache -= nTxIndexCache; - int64_t filter_index_cache = 0; - if (!g_enabled_filter_types.empty()) { - size_t n_indexes = g_enabled_filter_types.size(); - int64_t max_cache = std::min(nTotalCache / 8, max_filter_index_cache << 20); - filter_index_cache = max_cache / n_indexes; - nTotalCache -= filter_index_cache * n_indexes; - } - int64_t nCoinDBCache = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache - nCoinDBCache = std::min(nCoinDBCache, nMaxCoinsDBCache << 20); // cap total coins db cache - nTotalCache -= nCoinDBCache; - int64_t nCoinCacheUsage = nTotalCache; // the rest goes to in-memory cache + CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size()); + int64_t nMempoolSizeMax = args.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; LogPrintf("Cache configuration:\n"); - LogPrintf("* Using %.1f MiB for block index database\n", nBlockTreeDBCache * (1.0 / 1024 / 1024)); + LogPrintf("* Using %.1f MiB for block index database\n", cache_sizes.block_tree_db * (1.0 / 1024 / 1024)); if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { - LogPrintf("* Using %.1f MiB for transaction index database\n", nTxIndexCache * (1.0 / 1024 / 1024)); + LogPrintf("* Using %.1f MiB for transaction index database\n", cache_sizes.tx_index * (1.0 / 1024 / 1024)); } for (BlockFilterType filter_type : g_enabled_filter_types) { LogPrintf("* Using %.1f MiB for %s block filter index database\n", - filter_index_cache * (1.0 / 1024 / 1024), BlockFilterTypeName(filter_type)); + cache_sizes.filter_index * (1.0 / 1024 / 1024), BlockFilterTypeName(filter_type)); } - LogPrintf("* Using %.1f MiB for chain state database\n", nCoinDBCache * (1.0 / 1024 / 1024)); - LogPrintf("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)\n", nCoinCacheUsage * (1.0 / 1024 / 1024), nMempoolSizeMax * (1.0 / 1024 / 1024)); + LogPrintf("* Using %.1f MiB for chain state database\n", cache_sizes.coins_db * (1.0 / 1024 / 1024)); + LogPrintf("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)\n", cache_sizes.coins * (1.0 / 1024 / 1024), nMempoolSizeMax * (1.0 / 1024 / 1024)); assert(!node.mempool); assert(!node.chainman); @@ -1893,9 +1878,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) chainparams.GetConsensus(), chainparams.NetworkIDString(), fReindexChainState, - nBlockTreeDBCache, - nCoinDBCache, - nCoinCacheUsage, + cache_sizes.block_tree_db, + cache_sizes.coins_db, + cache_sizes.coins, ShutdownRequested, []() { uiInterface.ThreadSafeMessageBox( @@ -2091,14 +2076,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return InitError(*error); } - g_txindex = std::make_unique(nTxIndexCache, false, fReindex); + g_txindex = std::make_unique(cache_sizes.tx_index, false, fReindex); if (!g_txindex->Start(chainman.ActiveChainstate())) { return false; } } for (const auto& filter_type : g_enabled_filter_types) { - InitBlockFilterIndex(filter_type, filter_index_cache, false, fReindex); + InitBlockFilterIndex(filter_type, cache_sizes.filter_index, false, fReindex); if (!GetBlockFilterIndex(filter_type)->Start(chainman.ActiveChainstate())) { return false; } diff --git a/src/node/caches.cpp b/src/node/caches.cpp new file mode 100644 index 0000000000..952dce642e --- /dev/null +++ b/src/node/caches.cpp @@ -0,0 +1,38 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include + +CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) +{ + int64_t nTotalCache = (args.GetArg("-dbcache", nDefaultDbCache) << 20); + nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache + nTotalCache = std::min(nTotalCache, nMaxDbCache << 20); // total cache cannot be greater than nMaxDbcache + int64_t nBlockTreeDBCache = std::min(nTotalCache / 8, nMaxBlockDBCache << 20); + nTotalCache -= nBlockTreeDBCache; + int64_t nTxIndexCache = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0); + nTotalCache -= nTxIndexCache; + int64_t filter_index_cache = 0; + if (n_indexes > 0) { + int64_t max_cache = std::min(nTotalCache / 8, max_filter_index_cache << 20); + filter_index_cache = max_cache / n_indexes; + nTotalCache -= filter_index_cache * n_indexes; + } + int64_t nCoinDBCache = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache + nCoinDBCache = std::min(nCoinDBCache, nMaxCoinsDBCache << 20); // cap total coins db cache + nTotalCache -= nCoinDBCache; + int64_t nCoinCacheUsage = nTotalCache; // the rest goes to in-memory cache + + return { + nBlockTreeDBCache, + nCoinDBCache, + nCoinCacheUsage, + nTxIndexCache, + filter_index_cache, + }; +} diff --git a/src/node/caches.h b/src/node/caches.h new file mode 100644 index 0000000000..437e7d10e5 --- /dev/null +++ b/src/node/caches.h @@ -0,0 +1,22 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_CACHES_H +#define BITCOIN_NODE_CACHES_H + +#include // for size_t +#include // for int64_t + +class ArgsManager; + +struct CacheSizes { + int64_t block_tree_db; + int64_t coins_db; + int64_t coins; + int64_t tx_index; + int64_t filter_index; +}; +CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes = 0); + +#endif // BITCOIN_NODE_CACHES_H From 52bb35d9c8f61d1be19552dd981375208b4e6e6f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 6 Dec 2021 16:52:18 -0500 Subject: [PATCH 45/55] node/caches: Remove intermediate variables --- src/node/caches.cpp | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/node/caches.cpp b/src/node/caches.cpp index 952dce642e..a257c2f24a 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -13,26 +13,20 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) int64_t nTotalCache = (args.GetArg("-dbcache", nDefaultDbCache) << 20); nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache nTotalCache = std::min(nTotalCache, nMaxDbCache << 20); // total cache cannot be greater than nMaxDbcache - int64_t nBlockTreeDBCache = std::min(nTotalCache / 8, nMaxBlockDBCache << 20); - nTotalCache -= nBlockTreeDBCache; - int64_t nTxIndexCache = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0); - nTotalCache -= nTxIndexCache; - int64_t filter_index_cache = 0; + CacheSizes sizes; + sizes.block_tree_db = std::min(nTotalCache / 8, nMaxBlockDBCache << 20); + nTotalCache -= sizes.block_tree_db; + sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0); + nTotalCache -= sizes.tx_index; + sizes.filter_index = 0; if (n_indexes > 0) { int64_t max_cache = std::min(nTotalCache / 8, max_filter_index_cache << 20); - filter_index_cache = max_cache / n_indexes; - nTotalCache -= filter_index_cache * n_indexes; + sizes.filter_index = max_cache / n_indexes; + nTotalCache -= sizes.filter_index * n_indexes; } - int64_t nCoinDBCache = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache - nCoinDBCache = std::min(nCoinDBCache, nMaxCoinsDBCache << 20); // cap total coins db cache - nTotalCache -= nCoinDBCache; - int64_t nCoinCacheUsage = nTotalCache; // the rest goes to in-memory cache - - return { - nBlockTreeDBCache, - nCoinDBCache, - nCoinCacheUsage, - nTxIndexCache, - filter_index_cache, - }; + sizes.coins_db = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache + sizes.coins_db = std::min(sizes.coins_db, nMaxCoinsDBCache << 20); // cap total coins db cache + nTotalCache -= sizes.coins_db; + sizes.coins = nTotalCache; // the rest goes to in-memory cache + return sizes; } From c06e07461eb96e3dfa57e1ddd851a9e0b88007fc Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Oct 2024 14:16:24 +0000 Subject: [PATCH 46/55] node/chainstate: Add options for in-memory DBs --- src/init.cpp | 2 ++ src/node/chainstate.cpp | 6 ++++-- src/node/chainstate.h | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 803db7244b..8d8186200b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1881,6 +1881,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) cache_sizes.block_tree_db, cache_sizes.coins_db, cache_sizes.coins, + /*block_tree_db_in_memory=*/false, + /*coins_db_in_memory=*/false, ShutdownRequested, []() { uiInterface.ThreadSafeMessageBox( diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 8923f6a81f..9835c8960d 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -48,6 +48,8 @@ std::optional LoadChainstate(bool fReset, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, int64_t nCoinCacheUsage, + bool block_tree_db_in_memory, + bool coins_db_in_memory, std::function shutdown_requested, std::function coins_error_cb) { @@ -73,7 +75,7 @@ std::optional LoadChainstate(bool fReset, // new CBlockTreeDB tries to delete the existing file, which // fails if it's still open from the previous loop. Close it first: pblocktree.reset(); - pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); + pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, block_tree_db_in_memory, fReset)); // Same logic as above with pblocktree dmnman.reset(); @@ -177,7 +179,7 @@ std::optional LoadChainstate(bool fReset, for (CChainState* chainstate : chainman.GetAll()) { chainstate->InitCoinsDB( /* cache_size_bytes */ nCoinDBCache, - /* in_memory */ false, + /* in_memory */ coins_db_in_memory, /* should_wipe */ fReset || fReindexChainState); if (coins_error_cb) { diff --git a/src/node/chainstate.h b/src/node/chainstate.h index c2aa6d827f..b7f2942869 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -110,6 +110,8 @@ std::optional LoadChainstate(bool fReset, int64_t nBlockTreeDBCache, int64_t nCoinDBCache, int64_t nCoinCacheUsage, + bool block_tree_db_in_memory, + bool coins_db_in_memory, std::function shutdown_requested = nullptr, std::function coins_error_cb = nullptr); From 459f33983bd12055c8797d6c4006c246fe1214b0 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 14 Dec 2024 18:57:39 +0000 Subject: [PATCH 47/55] node/chainstate: extract Dash post-`InitializeChainstate` logic --- src/init.cpp | 11 +---- src/node/chainstate.cpp | 91 ++++++++++++++++++++++++++++++----------- src/node/chainstate.h | 27 ++++++++++++ 3 files changed, 96 insertions(+), 33 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 8d8186200b..23632d288d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -335,15 +335,8 @@ void PrepareShutdown(NodeContext& node) chainstate->ResetCoinsViews(); } } - node.chain_helper.reset(); - if (node.mnhf_manager) { - node.mnhf_manager->DisconnectManagers(); - } - node.llmq_ctx.reset(); - llmq::quorumSnapshotManager.reset(); - node.mempool->DisconnectManagers(); - node.dmnman.reset(); - node.cpoolman.reset(); + DashChainstateSetupClose(node.chain_helper, node.cpoolman, node.dmnman, node.mnhf_manager, + llmq::quorumSnapshotManager, node.llmq_ctx, Assert(node.mempool.get())); node.mnhf_manager.reset(); node.evodb.reset(); } diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 9835c8960d..f831d11eb5 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -77,30 +77,9 @@ std::optional LoadChainstate(bool fReset, pblocktree.reset(); pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, block_tree_db_in_memory, fReset)); - // Same logic as above with pblocktree - dmnman.reset(); - dmnman = std::make_unique(chainman.ActiveChainstate(), *evodb); - mempool->ConnectManagers(dmnman.get()); - - cpoolman.reset(); - cpoolman = std::make_unique(*evodb); - - qsnapman.reset(); - qsnapman.reset(new llmq::CQuorumSnapshotManager(*evodb)); - - if (llmq_ctx) { - llmq_ctx->Interrupt(); - llmq_ctx->Stop(); - } - llmq_ctx.reset(); - llmq_ctx = std::make_unique(chainman, *dmnman, *evodb, mn_metaman, *mnhf_manager, sporkman, - *mempool, mn_activeman.get(), mn_sync, /*unit_tests=*/false, /*wipe=*/fReset || fReindexChainState); - // Enable CMNHFManager::{Process, Undo}Block - mnhf_manager->ConnectManagers(&chainman, llmq_ctx->qman.get()); - - chain_helper.reset(); - chain_helper = std::make_unique(*cpoolman, *dmnman, *mnhf_manager, govman, *(llmq_ctx->quorum_block_processor), chainman, - consensus_params, mn_sync, sporkman, *(llmq_ctx->clhandler), *(llmq_ctx->qman)); + DashChainstateSetup(chainman, govman, mn_metaman, mn_sync, sporkman, mn_activeman, chain_helper, cpoolman, + dmnman, evodb, mnhf_manager, qsnapman, llmq_ctx, mempool, fReset, fReindexChainState, + consensus_params); if (fReset) { pblocktree->WriteReindexing(true); @@ -229,6 +208,70 @@ std::optional LoadChainstate(bool fReset, return std::nullopt; } +void DashChainstateSetup(ChainstateManager& chainman, + CGovernanceManager& govman, + CMasternodeMetaMan& mn_metaman, + CMasternodeSync& mn_sync, + CSporkManager& sporkman, + std::unique_ptr& mn_activeman, + std::unique_ptr& chain_helper, + std::unique_ptr& cpoolman, + std::unique_ptr& dmnman, + std::unique_ptr& evodb, + std::unique_ptr& mnhf_manager, + std::unique_ptr& qsnapman, + std::unique_ptr& llmq_ctx, + CTxMemPool* mempool, + bool fReset, + bool fReindexChainState, + const Consensus::Params& consensus_params) +{ + // Same logic as pblocktree + dmnman.reset(); + dmnman = std::make_unique(chainman.ActiveChainstate(), *evodb); + mempool->ConnectManagers(dmnman.get()); + + cpoolman.reset(); + cpoolman = std::make_unique(*evodb); + + qsnapman.reset(); + qsnapman.reset(new llmq::CQuorumSnapshotManager(*evodb)); + + if (llmq_ctx) { + llmq_ctx->Interrupt(); + llmq_ctx->Stop(); + } + llmq_ctx.reset(); + llmq_ctx = std::make_unique(chainman, *dmnman, *evodb, mn_metaman, *mnhf_manager, sporkman, + *mempool, mn_activeman.get(), mn_sync, /*unit_tests=*/false, /*wipe=*/fReset || fReindexChainState); + // Enable CMNHFManager::{Process, Undo}Block + mnhf_manager->ConnectManagers(&chainman, llmq_ctx->qman.get()); + + chain_helper.reset(); + chain_helper = std::make_unique(*cpoolman, *dmnman, *mnhf_manager, govman, *(llmq_ctx->quorum_block_processor), chainman, + consensus_params, mn_sync, sporkman, *(llmq_ctx->clhandler), *(llmq_ctx->qman)); +} + +void DashChainstateSetupClose(std::unique_ptr& chain_helper, + std::unique_ptr& cpoolman, + std::unique_ptr& dmnman, + std::unique_ptr& mnhf_manager, + std::unique_ptr& qsnapman, + std::unique_ptr& llmq_ctx, + CTxMemPool* mempool) + +{ + chain_helper.reset(); + if (mnhf_manager) { + mnhf_manager->DisconnectManagers(); + } + llmq_ctx.reset(); + qsnapman.reset(); + cpoolman.reset(); + mempool->DisconnectManagers(); + dmnman.reset(); +} + std::optional VerifyLoadedChainstate(ChainstateManager& chainman, CEvoDB& evodb, bool fReset, diff --git a/src/node/chainstate.h b/src/node/chainstate.h index b7f2942869..e85c4a7a76 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -115,6 +115,33 @@ std::optional LoadChainstate(bool fReset, std::function shutdown_requested = nullptr, std::function coins_error_cb = nullptr); +/** Initialize Dash-specific components during chainstate initialization */ +void DashChainstateSetup(ChainstateManager& chainman, + CGovernanceManager& govman, + CMasternodeMetaMan& mn_metaman, + CMasternodeSync& mn_sync, + CSporkManager& sporkman, + std::unique_ptr& mn_activeman, + std::unique_ptr& chain_helper, + std::unique_ptr& cpoolman, + std::unique_ptr& dmnman, + std::unique_ptr& evodb, + std::unique_ptr& mnhf_manager, + std::unique_ptr& qsnapman, + std::unique_ptr& llmq_ctx, + CTxMemPool* mempool, + bool fReset, + bool fReindexChainState, + const Consensus::Params& consensus_params); + +void DashChainstateSetupClose(std::unique_ptr& chain_helper, + std::unique_ptr& cpoolman, + std::unique_ptr& dmnman, + std::unique_ptr& mnhf_manager, + std::unique_ptr& qsnapman, + std::unique_ptr& llmq_ctx, + CTxMemPool* mempool); + enum class ChainstateLoadVerifyError { ERROR_BLOCK_FROM_FUTURE, ERROR_CORRUPTED_BLOCK_DB, From 09ab62948ffd726e11bc7c113dd9ba6bdb60346d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 14 Dec 2024 19:38:07 +0000 Subject: [PATCH 48/55] test/setup: Use LoadChainstate --- src/test/util/setup_common.cpp | 106 ++++++++++++------ src/test/util/setup_common.h | 20 +++- .../validation_chainstatemanager_tests.cpp | 24 +++- 3 files changed, 108 insertions(+), 42 deletions(-) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 06404731d8..21d0e5c724 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -30,6 +30,8 @@ #include #include #include +#include // for fReindex, fPruneMode +#include // for LoadChainstate #include #include #include @@ -38,6 +40,7 @@ #include #include #include