diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index ee4d891a57..ad4e9704ef 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -14,8 +14,10 @@ #include "csvmodelwriter.h" #include "editaddressdialog.h" #include "createsparknamepage.h" +#include "guiconstants.h" #include "guiutil.h" #include "platformstyle.h" +#include "validation.h" #include "bip47/paymentcode.h" #include "bip47/paymentchannel.h" @@ -23,6 +25,7 @@ #include #include #include +#include AddressBookPage::AddressBookPage(const PlatformStyle *_platformStyle, Mode _mode, Tabs _tab, QWidget *parent, bool isReused) : QDialog(parent), @@ -219,7 +222,16 @@ void AddressBookPage::setModel(AddressTableModel *_model) bool AddressBookPage::updateSpark() { - const bool sparkAllowed = model && model->IsSparkAllowed(); + bool sparkAllowed; + { + TRY_LOCK(cs_main, lockMain); + if (!lockMain) { + // Retry this page even if another page completes the shared refresh. + QTimer::singleShot(MODEL_UPDATE_DELAY, this, &AddressBookPage::updateSpark); + return false; + } + sparkAllowed = model && model->IsSparkAllowed(); + } populateAddressTypes(sparkAllowed); chooseAddressType(0); diff --git a/src/qt/automintmodel.cpp b/src/qt/automintmodel.cpp index 670d4f86ba..380faa61f4 100644 --- a/src/qt/automintmodel.cpp +++ b/src/qt/automintmodel.cpp @@ -111,8 +111,14 @@ void IncomingFundNotifier::check() void IncomingFundNotifier::importTransactions() { - LOCK2(cs_main, cs); - LOCK(wallet->cs_wallet); + TRY_LOCK(cs_main, lockMain); + TRY_LOCK(cs, lock); + TRY_LOCK(wallet->cs_wallet, lockWallet); + if (!lockMain || !lock || !lockWallet) { + // This queued startup scan must retry after long-running validation. + QTimer::singleShot(MODEL_UPDATE_DELAY, this, &IncomingFundNotifier::importTransactions); + return; + } for (auto const &tx : wallet->mapWallet) { if (tx.second.GetAvailableCredit() > 0 || tx.second.GetImmatureCredit() > 0) { @@ -247,6 +253,10 @@ void AutoMintSparkModel::checkAutoMintSpark(bool force) return; } + // The periodic check can try again on its next timer tick. + TRY_LOCK(cs_main, lockMain); + if (!lockMain) + return; bool allowed = spark::IsSparkAllowed(); if (!allowed) { return; diff --git a/src/qt/test/CMakeLists.txt b/src/qt/test/CMakeLists.txt index 486dc4de33..96aa322409 100644 --- a/src/qt/test/CMakeLists.txt +++ b/src/qt/test/CMakeLists.txt @@ -14,6 +14,7 @@ add_executable(test_firo-qt ${CMAKE_CURRENT_SOURCE_DIR}/test_main.cpp ${CMAKE_CURRENT_SOURCE_DIR}/uritests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_sendcoinsentry.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/sparkmodeltests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../test/test_bitcoin.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../test/testutil.cpp ) diff --git a/src/qt/test/sparkmodeltests.cpp b/src/qt/test/sparkmodeltests.cpp new file mode 100644 index 0000000000..32e87668d5 --- /dev/null +++ b/src/qt/test/sparkmodeltests.cpp @@ -0,0 +1,118 @@ +#include "sparkmodeltests.h" + +#include "addressbookpage.h" +#include "addresstablemodel.h" +#include "automintmodel.h" +#include "optionsmodel.h" +#include "platformstyle.h" +#include "sparkmodel.h" + +#include "masternode-sync.h" +#include "validation.h" +#include "wallet/wallet.h" + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace { + +// Exercise the GUI callback while another thread owns the real core lock. +// The timeout lets a blocking regression fail instead of hanging the suite. +bool WithContendedLock(CCriticalSection& mutex, const std::function& callback) +{ + std::promise locked, release; + auto released = release.get_future(); + bool timedOut = false; + std::jthread worker([&] { + LOCK(mutex); + locked.set_value(); + timedOut = released.wait_for(std::chrono::seconds(5)) != std::future_status::ready; + }); + locked.get_future().wait(); + callback(); + release.set_value(); + worker.join(); + return !timedOut; +} + +} // namespace + +void SparkModelTests::importRetries_data() +{ + QTest::addColumn("walletLock"); + QTest::newRow("chain-lock") << false; + QTest::newRow("wallet-lock") << true; +} + +void SparkModelTests::importRetries() +{ + QFETCH(bool, walletLock); + CWallet wallet; + IncomingFundNotifier notifier(&wallet); + QTimer* timer = notifier.findChild(); + QVERIFY(timer); + + bool deferred = false; + const bool responsive = WithContendedLock(walletLock ? wallet.cs_wallet : cs_main, [&] { + // Deliver the constructor's queued startup scan on the GUI thread. + QCoreApplication::sendPostedEvents(¬ifier, QEvent::MetaCall); + deferred = !timer->isActive(); + }); + QVERIFY(responsive); + QVERIFY(deferred); + // No new block or transaction notification is needed to retry the scan. + QTRY_VERIFY(timer->isActive()); +} + +void SparkModelTests::addressBookDefers() +{ + CWallet wallet; + AddressTableModel model(&wallet); + const std::unique_ptr style(PlatformStyle::instantiate("other")); + QVERIFY(style); + AddressBookPage page(style.get(), AddressBookPage::ForEditing, AddressBookPage::SendingTab, nullptr); + page.setModel(&model); + QComboBox* types = page.findChild("addressType"); + QVERIFY(types); + const int originalCount = types->count(); + types->addItem("Retain this selection while busy"); + types->setCurrentIndex(originalCount); + + bool updated = true; + const bool responsive = WithContendedLock(cs_main, [&] { updated = page.updateSpark(); }); + QVERIFY(responsive); + QVERIFY(!updated); + QCOMPARE(types->count(), originalCount + 1); + QCOMPARE(types->currentIndex(), originalCount); + + // Retry independently of another page or block-tip notification. + QTRY_COMPARE(types->count(), originalCount); +} + +void SparkModelTests::autoMintDefers() +{ + CWallet wallet; + OptionsModel options; + SparkModel model(nullptr, &wallet, &options); + // Reach the activation check even though this test has no network peers. + const CMasternodeSync savedSync = masternodeSync; + CConnman connman(0, 0); + masternodeSync.Reset(); + masternodeSync.SwitchToNextAsset(connman); + masternodeSync.SwitchToNextAsset(connman); + const bool responsive = WithContendedLock(cs_main, [&] { + model.getAutoMintSparkModel()->checkAutoMintSpark(); + }); + masternodeSync = savedSync; + QVERIFY(responsive); + QVERIFY(!model.getAutoMintSparkModel()->isSparkAnonymizing()); +} diff --git a/src/qt/test/sparkmodeltests.h b/src/qt/test/sparkmodeltests.h new file mode 100644 index 0000000000..39965a2835 --- /dev/null +++ b/src/qt/test/sparkmodeltests.h @@ -0,0 +1,17 @@ +#ifndef FIRO_QT_TEST_SPARKMODELTESTS_H +#define FIRO_QT_TEST_SPARKMODELTESTS_H + +#include + +class SparkModelTests : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void importRetries_data(); + void importRetries(); + void addressBookDefers(); + void autoMintDefers(); +}; + +#endif // FIRO_QT_TEST_SPARKMODELTESTS_H diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index 5d0d720375..6827edfbe0 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -13,6 +13,7 @@ #include "uritests.h" #include "compattests.h" #include "test_sendcoinsentry.h" +#include "sparkmodeltests.h" #include #include #include @@ -59,6 +60,10 @@ int main(int argc, char *argv[]) if (QTest::qExec(&test4) != 0) fInvalid = true; + SparkModelTests sparkModelTests; + if (QTest::qExec(&sparkModelTests) != 0) + fInvalid = true; + ECC_Stop(); return fInvalid; }