diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 272499d1c3b6..b569974da1ff 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -12,12 +12,9 @@ bool IsFinalTx(const CTransactionRef& tx, int nBlockHeight, int64_t nBlockTime) { - AssertLockHeld(cs_main); // Time based nLockTime implemented in 0.1.6 if (tx->nLockTime == 0) return true; - if (nBlockHeight == 0) - nBlockHeight = chainActive.Height(); if (nBlockTime == 0) nBlockTime = GetAdjustedTime(); if ((int64_t)tx->nLockTime < ((int64_t)tx->nLockTime < LOCKTIME_THRESHOLD ? (int64_t)nBlockHeight : nBlockTime)) diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index 876f8d83fc95..bc54e1c9021b 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -39,6 +39,6 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma * Check if transaction is final and can be included in a block with the * specified height and time. Consensus critical. */ -bool IsFinalTx(const CTransactionRef& tx, int nBlockHeight = 0, int64_t nBlockTime = 0); +bool IsFinalTx(const CTransactionRef& tx, int nBlockHeight, int64_t nBlockTime = 0); #endif // BITCOIN_CONSENSUS_TX_VERIFY_H diff --git a/src/init.cpp b/src/init.cpp index 10e6fea7187d..b15b926e95dd 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1971,11 +1971,9 @@ bool AppInitMain() // ********************************************************* Step 12: finished - SetRPCWarmupFinished(); - uiInterface.InitMessage(_("Done loading")); - #ifdef ENABLE_WALLET if (pwalletMain) { + uiInterface.InitMessage(_("Reaccepting wallet transactions...")); pwalletMain->postInitProcess(scheduler); // StakeMiner thread disabled by default on regtest @@ -1985,5 +1983,8 @@ bool AppInitMain() } #endif + SetRPCWarmupFinished(); + uiInterface.InitMessage(_("Done loading")); + return true; } diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 9f67ba079ba7..17e5365ebb95 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -10,19 +10,22 @@ namespace interfaces { WalletBalances Wallet::getBalances() { WalletBalances result; - result.balance = m_wallet.GetAvailableBalance(); - result.unconfirmed_balance = m_wallet.GetUnconfirmedBalance(ISMINE_SPENDABLE_TRANSPARENT); - result.immature_balance = m_wallet.GetImmatureBalance(); + CWallet::Balance balance = m_wallet.GetBalance(); + result.balance = balance.m_mine_trusted + balance.m_mine_trusted_shield; + result.unconfirmed_balance = balance.m_mine_untrusted_pending; + result.immature_balance = balance.m_mine_immature; result.have_watch_only = m_wallet.HaveWatchOnly(); if (result.have_watch_only) { result.watch_only_balance = m_wallet.GetWatchOnlyBalance(); result.unconfirmed_watch_only_balance = m_wallet.GetUnconfirmedWatchOnlyBalance(); result.immature_watch_only_balance = m_wallet.GetImmatureWatchOnlyBalance(); } - result.delegate_balance = m_wallet.GetDelegatedBalance(); - result.coldstaked_balance = m_wallet.GetColdStakingBalance(); - result.shielded_balance = m_wallet.GetAvailableShieldedBalance(); - result.unconfirmed_shielded_balance = m_wallet.GetUnconfirmedShieldedBalance(); + result.delegate_balance = balance.m_mine_cs_delegated_trusted; + if (result.have_coldstaking) { // At the moment, the GUI is not using the cold staked balance. + result.coldstaked_balance = m_wallet.GetColdStakingBalance(); + } + result.shielded_balance = balance.m_mine_trusted_shield; + result.unconfirmed_shielded_balance = balance.m_mine_untrusted_shielded_balance; return result; } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index b1d8aa6f363f..b5a08633ec06 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -21,6 +21,7 @@ struct WalletBalances CAmount watch_only_balance{0}; CAmount unconfirmed_watch_only_balance{0}; CAmount immature_watch_only_balance{0}; + bool have_coldstaking{false}; CAmount delegate_balance{0}; CAmount coldstaked_balance{0}; CAmount shielded_balance{0}; diff --git a/src/masternode-sync.cpp b/src/masternode-sync.cpp index 243c1e2cc012..a7352ebaf803 100644 --- a/src/masternode-sync.cpp +++ b/src/masternode-sync.cpp @@ -78,6 +78,11 @@ bool CMasternodeSync::IsBlockchainSynced() return true; } +bool CMasternodeSync::IsBlockchainSyncedReadOnly() const +{ + return fBlockchainSynced; +} + void CMasternodeSync::Reset() { fBlockchainSynced = false; diff --git a/src/masternode-sync.h b/src/masternode-sync.h index 69f930f5efc9..41b5dd0c083d 100644 --- a/src/masternode-sync.h +++ b/src/masternode-sync.h @@ -98,6 +98,8 @@ class CMasternodeSync bool IsBlockchainSynced(); void ClearFulfilledRequest(); + bool IsBlockchainSyncedReadOnly() const; + // Sync message dispatcher bool MessageDispatcher(CNode* pfrom, std::string& strCommand, CDataStream& vRecv); diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index 18559865bac1..136faffd829e 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -135,6 +135,26 @@ QString ClientModel::getLastBlockHash() const return QString::fromStdString(nHash.GetHex()); } +uint256 ClientModel::getLastBlockProcessed() const +{ + return cacheTip == nullptr ? Params().GenesisBlock().GetHash() : cacheTip->GetBlockHash(); +} + +int ClientModel::getLastBlockProcessedHeight() const +{ + return cacheTip == nullptr ? 0 : cacheTip->nHeight; +} + +int64_t ClientModel::getLastBlockProcessedTime() const +{ + return cacheTip == nullptr ? Params().GenesisBlock().GetBlockTime() : cacheTip->GetBlockTime(); +} + +bool ClientModel::isTipCached() const +{ + return cacheTip; +} + double ClientModel::getVerificationProgress() const { return Checkpoints::GuessVerificationProgress(cacheTip); diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index e2284aaf04bd..f783aaecd931 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -64,7 +64,11 @@ class ClientModel : public QObject int getNumBlocks(); QDateTime getLastBlockDate() const; QString getLastBlockHash() const; + uint256 getLastBlockProcessed() const; + int getLastBlockProcessedHeight() const; + int64_t getLastBlockProcessedTime() const; double getVerificationProgress() const; + bool isTipCached() const; quint64 getTotalBytesRecv() const; quint64 getTotalBytesSent() const; @@ -114,7 +118,7 @@ class ClientModel : public QObject QString cachedMasternodeCountString; bool cachedReindexing; bool cachedImporting; - bool cachedInitialSync; + std::atomic cachedInitialSync{false}; int numBlocksAtStartup; diff --git a/src/qt/coincontroldialog.h b/src/qt/coincontroldialog.h index 0d60c1288ae1..79faa027e328 100644 --- a/src/qt/coincontroldialog.h +++ b/src/qt/coincontroldialog.h @@ -65,6 +65,7 @@ class CoinControlDialog : public QDialog void clearPayAmounts(); void addPayAmount(const CAmount& amount, bool isShieldedRecipient); void setSelectionType(bool isTransparent) { fSelectTransparent = isTransparent; } + bool hasModel() { return model; } CCoinControl* coinControl{nullptr}; diff --git a/src/qt/macnotificationhandler.mm b/src/qt/macnotificationhandler.mm index 3a3a6a339c29..080405e9792f 100644 --- a/src/qt/macnotificationhandler.mm +++ b/src/qt/macnotificationhandler.mm @@ -25,25 +25,10 @@ - (NSString *)__bundleIdentifier { // check if users OS has support for NSUserNotification if(this->hasUserNotificationCenterSupport()) { - // okay, seems like 10.8+ - QByteArray utf8 = title.toUtf8(); - char* cString = (char *)utf8.constData(); - NSString *titleMac = [[NSString alloc] initWithUTF8String:cString]; - - utf8 = text.toUtf8(); - cString = (char *)utf8.constData(); - NSString *textMac = [[NSString alloc] initWithUTF8String:cString]; - - // do everything weak linked (because we will keep <10.8 compatibility) - id userNotification = [[NSClassFromString(@"NSUserNotification") alloc] init]; - [userNotification performSelector:@selector(setTitle:) withObject:titleMac]; - [userNotification performSelector:@selector(setInformativeText:) withObject:textMac]; - - id notificationCenterInstance = [NSClassFromString(@"NSUserNotificationCenter") performSelector:@selector(defaultUserNotificationCenter)]; - [notificationCenterInstance performSelector:@selector(deliverNotification:) withObject:userNotification]; - - [titleMac release]; - [textMac release]; + NSUserNotification* userNotification = [[NSUserNotification alloc] init]; + userNotification.title = title.toNSString(); + userNotification.informativeText = text.toNSString(); + [[NSUserNotificationCenter defaultUserNotificationCenter] deliverNotification: userNotification]; [userNotification release]; } } diff --git a/src/qt/pivx.cpp b/src/qt/pivx.cpp index c2b5d50732a0..63fe0e7574e9 100644 --- a/src/qt/pivx.cpp +++ b/src/qt/pivx.cpp @@ -25,6 +25,7 @@ #ifdef ENABLE_WALLET #include "paymentserver.h" #include "walletmodel.h" +#include "interfaces/wallet.h" #endif #include "masternodeconfig.h" @@ -69,6 +70,8 @@ Q_IMPORT_PLUGIN(QGifPlugin); // Declare meta types used for QMetaObject::invokeMethod Q_DECLARE_METATYPE(bool*) Q_DECLARE_METATYPE(CAmount) +Q_DECLARE_METATYPE(interfaces::WalletBalances); +Q_DECLARE_METATYPE(uint256) static void InitMessage(const std::string& message) { @@ -449,16 +452,17 @@ void BitcoinApplication::requestShutdown() qDebug() << __func__ << ": Requesting shutdown"; startThread(); window->hide(); - window->setClientModel(0); + if (walletModel) walletModel->stop(); + window->setClientModel(nullptr); pollShutdownTimer->stop(); #ifdef ENABLE_WALLET window->removeAllWallets(); delete walletModel; - walletModel = 0; + walletModel = nullptr; #endif delete clientModel; - clientModel = 0; + clientModel = nullptr; // Show a simple window indicating shutdown status ShutdownWindow::showShutdownWindow(window); @@ -486,6 +490,7 @@ void BitcoinApplication::initializeResult(int retval) #ifdef ENABLE_WALLET if (pwalletMain) { walletModel = new WalletModel(pwalletMain, optionsModel); + walletModel->setClientModel(clientModel); window->addWallet(PIVXGUI::DEFAULT_WALLET, walletModel); window->setCurrentWallet(PIVXGUI::DEFAULT_WALLET); @@ -569,6 +574,8 @@ int main(int argc, char* argv[]) // Need to pass name here as CAmount is a typedef (see http://qt-project.org/doc/qt-5/qmetatype.html#qRegisterMetaType) // IMPORTANT if it is no longer a typedef use the normal variant above qRegisterMetaType("CAmount"); + qRegisterMetaType("interfaces::WalletBalances"); + qRegisterMetaType("size_t"); /// 3. Application identification // must be set before OptionsModel is initialized or translations are loaded, diff --git a/src/qt/pivx/coldstakingwidget.cpp b/src/qt/pivx/coldstakingwidget.cpp index 10af040dbfb9..d069b5e4963a 100644 --- a/src/qt/pivx/coldstakingwidget.cpp +++ b/src/qt/pivx/coldstakingwidget.cpp @@ -204,7 +204,6 @@ ColdStakingWidget::ColdStakingWidget(PIVXGUI* parent) : void ColdStakingWidget::loadWalletModel() { if (walletModel) { - coinControlDialog->setModel(walletModel); sendMultiRow->setWalletModel(walletModel); txModel = walletModel->getTransactionTableModel(); csModel = new ColdStakingModel(walletModel, txModel, walletModel->getAddressTableModel(), this); @@ -213,10 +212,10 @@ void ColdStakingWidget::loadWalletModel() addressTableModel = walletModel->getAddressTableModel(); addressesFilter = new AddressFilterProxyModel(AddressTableModel::ColdStaking, this); - addressesFilter->setSourceModel(addressTableModel); addressesFilter->sort(sortType, sortOrder); - ui->listViewStakingAddress->setModel(addressesFilter); + addressesFilter->setSourceModel(addressTableModel); ui->listViewStakingAddress->setModelColumn(AddressTableModel::Address); + ui->listViewStakingAddress->setModel(addressesFilter); connect(txModel, &TransactionTableModel::txArrived, this, &ColdStakingWidget::onTxArrived); @@ -225,8 +224,6 @@ void ColdStakingWidget::loadWalletModel() ui->containerHistoryLabel->setVisible(false); ui->emptyContainer->setVisible(false); ui->listView->setVisible(false); - - tryRefreshDelegations(); } } @@ -246,8 +243,14 @@ void ColdStakingWidget::walletSynced(bool sync) } } +void ColdStakingWidget::showEvent(QShowEvent *event) +{ + tryRefreshDelegations(); +} + void ColdStakingWidget::tryRefreshDelegations() { + if (!isVisible()) return; // Check for min update time to not reload the UI so often if the node is syncing. int64_t now = GetTime(); if (lastRefreshTime + LOAD_MIN_TIME_INTERVAL < now) { @@ -533,6 +536,7 @@ void ColdStakingWidget::onCoinControlClicked() { if (isInDelegation) { if (walletModel->getBalance() > 0) { + if (!coinControlDialog->hasModel()) coinControlDialog->setModel(walletModel); coinControlDialog->refreshDialog(); setCoinControlPayAmounts(); coinControlDialog->exec(); diff --git a/src/qt/pivx/coldstakingwidget.h b/src/qt/pivx/coldstakingwidget.h index 385dacf1f9f8..1fc256756189 100644 --- a/src/qt/pivx/coldstakingwidget.h +++ b/src/qt/pivx/coldstakingwidget.h @@ -48,6 +48,8 @@ class ColdStakingWidget : public PWidget void run(int type) override; void onError(QString error, int type) override; + void showEvent(QShowEvent *event) override; + public Q_SLOTS: void walletSynced(bool sync); diff --git a/src/qt/pivx/dashboardwidget.cpp b/src/qt/pivx/dashboardwidget.cpp index 16adb55ae516..7f29578ccc3c 100644 --- a/src/qt/pivx/dashboardwidget.cpp +++ b/src/qt/pivx/dashboardwidget.cpp @@ -181,7 +181,6 @@ void DashboardWidget::loadWalletModel() filter->setSortCaseSensitivity(Qt::CaseInsensitive); filter->setFilterCaseSensitivity(Qt::CaseInsensitive); filter->setSortRole(Qt::EditRole); - filter->setSourceModel(txModel); // Read filter settings QSettings settings; @@ -190,10 +189,10 @@ void DashboardWidget::loadWalletModel() filterByType = (filterIndex == -1) ? TransactionFilterProxy::ALL_TYPES : filterByType; filter->setTypeFilter(filterByType); // Set filter ui->comboBoxSortType->setCurrentIndex(filterIndex); // Set item in ComboBox - // Read sort settings changeSort(settings.value("transactionSort", SortTx::DATE_DESC).toInt()); + filter->setSourceModel(txModel); txHolder->setFilter(filter); ui->listTransactions->setModel(filter); ui->listTransactions->setModelColumn(TransactionTableModel::ToAddress); @@ -212,15 +211,9 @@ void DashboardWidget::loadWalletModel() // Notification pop-up for new transaction connect(txModel, &TransactionTableModel::rowsInserted, this, &DashboardWidget::processNewTransaction); #ifdef USE_QTCHARTS - // chart filter - stakesFilter = new TransactionFilterProxy(); - stakesFilter->setDynamicSortFilter(true); - stakesFilter->setOnlyStakes(true); - stakesFilter->setSourceModel(txModel); - hasStakes = stakesFilter->rowCount() > 0; - onHideChartsChanged(walletModel->getOptionsModel()->isHideCharts()); - connect(walletModel->getOptionsModel(), &OptionsModel::hideChartsChanged, this, &DashboardWidget::onHideChartsChanged); + connect(walletModel->getOptionsModel(), &OptionsModel::hideChartsChanged, this, + &DashboardWidget::onHideChartsChanged); #endif } // update the display unit, to not use the default ("PIV") @@ -230,10 +223,11 @@ void DashboardWidget::loadWalletModel() void DashboardWidget::onTxArrived(const QString& hash, const bool& isCoinStake, const bool& isCSAnyType) { showList(); + if (!isVisible()) return; #ifdef USE_QTCHARTS if (isCoinStake) { // Update value if this is our first stake - if (!hasStakes) + if (!hasStakes && stakesFilter) hasStakes = stakesFilter->rowCount() > 0; tryChartRefresh(); } @@ -307,7 +301,6 @@ void DashboardWidget::changeSort(int nSortIndex) ui->comboBoxSort->setCurrentIndex(nSortIndex); filter->sort(nColumnIndex, order); - ui->listTransactions->update(); // Store settings QSettings settings; @@ -341,6 +334,7 @@ void DashboardWidget::walletSynced(bool sync) this->isSync = sync; ui->layoutWarning->setVisible(!this->isSync); #ifdef USE_QTCHARTS + if (!isVisible()) return; tryChartRefresh(); #endif } @@ -367,7 +361,9 @@ void DashboardWidget::tryChartRefresh() } else { // Check for min update time to not reload the UI so often if the node is syncing. int64_t now = GetTime(); - if (lastRefreshTime + CHART_LOAD_MIN_TIME_INTERVAL < now) { + int chartLoadIntervalTime = CHART_LOAD_MIN_TIME_INTERVAL; + if (clientModel->inInitialBlockDownload()) chartLoadIntervalTime *= 6; // 90 seconds update + if (lastRefreshTime + chartLoadIntervalTime < now) { lastRefreshTime = now; refreshChart(); } @@ -415,7 +411,7 @@ void DashboardWidget::loadChart() void DashboardWidget::showHideEmptyChart(bool showEmpty, bool loading, bool forceView) { - if (stakesFilter->rowCount() > SHOW_EMPTY_CHART_VIEW_THRESHOLD || forceView) { + if ((stakesFilter && stakesFilter->rowCount() > SHOW_EMPTY_CHART_VIEW_THRESHOLD) || forceView) { ui->layoutChart->setVisible(!showEmpty); ui->emptyContainerChart->setVisible(showEmpty); } @@ -487,6 +483,7 @@ void DashboardWidget::changeChartColors() void DashboardWidget::updateStakeFilter() { + if (!stakesFilter) return; if (chartShow != ALL) { bool filterByMonth = false; if (monthFilter != 0 && chartShow == MONTH) { @@ -524,7 +521,10 @@ void DashboardWidget::updateStakeFilter() // pair PIV, zPIV const QMap> DashboardWidget::getAmountBy() { - updateStakeFilter(); + if (filterUpdateNeeded) { + filterUpdateNeeded = false; + updateStakeFilter(); + } const int size = stakesFilter->rowCount(); QMap> amountBy; // Get all of the stakes @@ -620,6 +620,7 @@ void DashboardWidget::onChartYearChanged(const QString& yearStr) int newYear = yearStr.toInt(); if (newYear != yearFilter) { yearFilter = newYear; + filterUpdateNeeded = true; refreshChart(); } } @@ -631,6 +632,7 @@ void DashboardWidget::onChartMonthChanged(const QString& monthStr) int newMonth = ui->comboBoxMonths->currentData().toInt(); if (newMonth != monthFilter) { monthFilter = newMonth; + filterUpdateNeeded = true; refreshChart(); #ifndef Q_OS_MAC // quick hack to re paint the chart view. @@ -823,7 +825,7 @@ void DashboardWidget::onChartArrowClicked(bool goLeft) } } } - + filterUpdateNeeded = true; refreshChart(); //Check if data end day is current date and monthfilter is current month bool fEndDayisCurrent = dataenddate == currentDate.day() && monthFilter == currentDate.month(); @@ -874,6 +876,23 @@ void DashboardWidget::windowResizeEvent(QResizeEvent* event) void DashboardWidget::onHideChartsChanged(bool fHide) { fShowCharts = !fHide; + + if (fShowCharts) { + if (!stakesFilter) { + stakesFilter = new TransactionFilterProxy(this); + stakesFilter->setDynamicSortFilter(false); + stakesFilter->setSortCaseSensitivity(Qt::CaseInsensitive); + stakesFilter->setFilterCaseSensitivity(Qt::CaseInsensitive); + stakesFilter->setOnlyStakes(true); + } + stakesFilter->setSourceModel(txModel); + hasStakes = stakesFilter->rowCount() > 0; + } else { + if (stakesFilter) { + stakesFilter->setSourceModel(nullptr); + } + } + // Hide charts if requested ui->right->setVisible(fShowCharts); if (fShowCharts) tryChartRefresh(); diff --git a/src/qt/pivx/dashboardwidget.h b/src/qt/pivx/dashboardwidget.h index 40596db98891..f2092c92d6fb 100644 --- a/src/qt/pivx/dashboardwidget.h +++ b/src/qt/pivx/dashboardwidget.h @@ -170,6 +170,7 @@ private Q_SLOTS: ChartData* chartData{nullptr}; bool hasStakes{false}; bool fShowCharts{true}; + std::atomic filterUpdateNeeded{false}; void initChart(); void showHideEmptyChart(bool show, bool loading, bool forceView = false); diff --git a/src/qt/pivx/loadingdialog.h b/src/qt/pivx/loadingdialog.h index 82f912729066..707945c9c0ee 100644 --- a/src/qt/pivx/loadingdialog.h +++ b/src/qt/pivx/loadingdialog.h @@ -25,6 +25,7 @@ class Worker : public QObject { runnable = nullptr; } virtual void clean() {}; + void setType(int _type) { type = _type; } public Q_SLOTS: void process(); Q_SIGNALS: diff --git a/src/qt/pivx/mnmodel.cpp b/src/qt/pivx/mnmodel.cpp index 31bb1ba8690a..04ae3c5430cb 100644 --- a/src/qt/pivx/mnmodel.cpp +++ b/src/qt/pivx/mnmodel.cpp @@ -22,7 +22,7 @@ void MNModel::updateMNList() int end = nodes.size(); nodes.clear(); collateralTxAccepted.clear(); - for (CMasternodeConfig::CMasternodeEntry mne : masternodeConfig.getEntries()) { + for (const CMasternodeConfig::CMasternodeEntry& mne : masternodeConfig.getEntries()) { int nIndex; if (!mne.castOutputIndex(nIndex)) continue; @@ -35,14 +35,9 @@ void MNModel::updateMNList() } nodes.insert(QString::fromStdString(mne.getAlias()), std::make_pair(QString::fromStdString(mne.getIp()), pmn)); if (pwalletMain) { - bool txAccepted = false; - { - LOCK2(cs_main, pwalletMain->cs_wallet); - const CWalletTx *walletTx = pwalletMain->GetWalletTx(txHash); - if (walletTx && walletTx->GetDepthInMainChain() >= MasternodeCollateralMinConf()) { - txAccepted = true; - } - } + const CWalletTx *walletTx = pwalletMain->GetWalletTx(txHash); + bool txAccepted = walletTx && + WITH_LOCK(pwalletMain->cs_wallet, return walletTx->GetDepthInMainChain()) >= MasternodeCollateralMinConf(); collateralTxAccepted.insert(mne.getTxHash(), txAccepted); } } @@ -115,13 +110,8 @@ QVariant MNModel::data(const QModelIndex &index, int role) const if (!isAvailable) return false; std::string txHash = rec->vin.prevout.hash.GetHex(); if (!collateralTxAccepted.value(txHash)) { - bool txAccepted = false; - { - LOCK2(cs_main, pwalletMain->cs_wallet); - const CWalletTx *walletTx = pwalletMain->GetWalletTx(rec->vin.prevout.hash); - txAccepted = walletTx && walletTx->GetDepthInMainChain() > 0; - } - return txAccepted; + const CWalletTx *walletTx = pwalletMain->GetWalletTx(rec->vin.prevout.hash); + return walletTx && WITH_LOCK(pwalletMain->cs_wallet, return walletTx->GetDepthInMainChain()) > 0; } return true; } diff --git a/src/qt/pivx/pwidget.cpp b/src/qt/pivx/pwidget.cpp index 02c826c6cbd6..d9dd52e01b8c 100644 --- a/src/qt/pivx/pwidget.cpp +++ b/src/qt/pivx/pwidget.cpp @@ -108,6 +108,8 @@ bool PWidget::execute(int type, std::unique_ptr pctx WalletWorker* _worker = static_cast(task->worker.data()); _worker->setContext(std::move(pctx)); } + // Update type + task->worker->setType(type); } QThreadPool::globalInstance()->start(task.data()); return true; diff --git a/src/qt/pivx/send.cpp b/src/qt/pivx/send.cpp index f02c5b8c530d..b3e21f817e98 100644 --- a/src/qt/pivx/send.cpp +++ b/src/qt/pivx/send.cpp @@ -20,6 +20,7 @@ #include "openuridialog.h" #define REQUEST_PREPARE_TX 1 +#define REQUEST_REFRESH_BALANCE 2 SendWidget::SendWidget(PIVXGUI* parent) : PWidget(parent), @@ -142,9 +143,10 @@ void SendWidget::refreshAmounts() } nDisplayUnit = walletModel->getOptionsModel()->getDisplayUnit(); - ui->labelAmountSend->setText(GUIUtil::formatBalance(total, nDisplayUnit, false)); CAmount totalAmount = 0; + CAmount delegatedBalance = 0; + QString titleTotalRemaining; if (coinControlDialog->coinControl->HasSelected()) { // Set remaining balance to the sum of the coinControl selected inputs std::vector coins; @@ -154,22 +156,41 @@ void SendWidget::refreshAmounts() selectedBalance += coin.value; } totalAmount = selectedBalance - total; - ui->labelTitleTotalRemaining->setText(tr("Total remaining from the selected UTXO")); + titleTotalRemaining = tr("Total remaining from the selected UTXO"); } else { - // Wallet's unlocked balance. - totalAmount = isTransparent ? (walletModel->getUnlockedBalance(nullptr, fDelegationsChecked, false) - total) - : (walletModel->GetWalletBalances().shielded_balance - total); - ui->labelTitleTotalRemaining->setText(tr("Unlocked remaining")); + interfaces::WalletBalances balances = walletModel->GetWalletBalances(); + if (isTransparent) { + totalAmount = balances.balance - balances.shielded_balance - walletModel->getLockedBalance() - total; + if (!fDelegationsChecked) { + totalAmount -= balances.delegate_balance; + } + // show delegated balance if exist + delegatedBalance = balances.delegate_balance; + } else { + totalAmount = balances.shielded_balance - total; + } + titleTotalRemaining = tr("Unlocked remaining"); } + QString type = isTransparent ? "transparent" : "shielded"; - ui->labelAmountRemaining->setText( - GUIUtil::formatBalance( - totalAmount, - nDisplayUnit, - false) + " " + type - ); + QString labelAmountRemaining = GUIUtil::formatBalance( totalAmount, nDisplayUnit, false) + " " + type; + QMetaObject::invokeMethod(this, "updateAmounts", Qt::QueuedConnection, + Q_ARG(QString, titleTotalRemaining), + Q_ARG(QString, GUIUtil::formatBalance(total, nDisplayUnit, false)), + Q_ARG(QString, labelAmountRemaining), + Q_ARG(CAmount, delegatedBalance)); +} + +void SendWidget::updateAmounts(const QString& _titleTotalRemaining, + const QString& _labelAmountSend, + const QString& _labelAmountRemaining, + CAmount _delegationBalance) +{ + ui->labelTitleTotalRemaining->setText(_titleTotalRemaining); + ui->labelAmountSend->setText(_labelAmountSend); + ui->labelAmountRemaining->setText(_labelAmountRemaining); // show or hide delegations checkbox if need be - showHideCheckBoxDelegations(); + showHideCheckBoxDelegations(_delegationBalance); } void SendWidget::loadClientModel() @@ -184,7 +205,6 @@ void SendWidget::loadClientModel() void SendWidget::loadWalletModel() { if (walletModel) { - coinControlDialog->setModel(walletModel); if (walletModel->getOptionsModel()) { // display unit nDisplayUnit = walletModel->getOptionsModel()->getDisplayUnit(); @@ -203,9 +223,6 @@ void SendWidget::loadWalletModel() setCustomFeeSelected(true, nCustomFee); } - // Refresh - refreshAmounts(); - // TODO: This only happen when the coin control features are modified in other screen, check before do this if the wallet has another screen modifying it. // Coin Control //connect(walletModel->getOptionsModel(), &OptionsModel::coinControlFeaturesChanged, [this](){}); @@ -227,7 +244,7 @@ void SendWidget::clearAll(bool fClearSettings) if (fClearSettings) onResetSettings(); hideContactsMenu(); clearEntries(); - refreshAmounts(); + tryRefreshAmounts(); } void SendWidget::onResetSettings() @@ -243,19 +260,19 @@ void SendWidget::onResetCustomOptions(bool fRefreshAmounts) if (ui->checkBoxDelegations->isChecked()) ui->checkBoxDelegations->setChecked(false); resetCoinControl(); if (fRefreshAmounts) { - refreshAmounts(); + tryRefreshAmounts(); } } void SendWidget::resetCoinControl() { - coinControlDialog->coinControl->SetNull(); + if (coinControlDialog) coinControlDialog->coinControl->SetNull(); ui->btnCoinControl->setActive(false); } void SendWidget::resetChangeAddress() { - coinControlDialog->coinControl->destChange = CNoDestination(); + if (coinControlDialog) coinControlDialog->coinControl->destChange = CNoDestination(); ui->btnChangeAddress->setActive(false); ui->btnChangeAddress->setVisible(isTransparent); } @@ -326,13 +343,7 @@ void SendWidget::showEvent(QShowEvent *event) { // Set focus on last recipient address when Send-window is displayed setFocusOnLastEntry(); - - // Update cached delegated balance - CAmount cachedDelegatedBalance_new = walletModel->getDelegatedBalance(); - if (cachedDelegatedBalance != cachedDelegatedBalance_new) { - cachedDelegatedBalance = cachedDelegatedBalance_new; - refreshAmounts(); - } + tryRefreshAmounts(); } void SendWidget::setFocusOnLastEntry() @@ -340,19 +351,19 @@ void SendWidget::setFocusOnLastEntry() if (!entries.isEmpty()) entries.last()->setFocus(); } -void SendWidget::showHideCheckBoxDelegations() +void SendWidget::showHideCheckBoxDelegations(CAmount delegationBalance) { // Show checkbox only when there is any available owned delegation and // coincontrol is not selected, and we are trying to spend transparent PIVs. - const bool isCControl = coinControlDialog->coinControl->HasSelected(); - const bool hasDel = cachedDelegatedBalance > 0; + const bool isCControl = coinControlDialog ? coinControlDialog->coinControl->HasSelected() : false; + const bool hasDel = delegationBalance > 0; const bool showCheckBox = isTransparent && !isCControl && hasDel; ui->checkBoxDelegations->setVisible(showCheckBox); if (showCheckBox) ui->checkBoxDelegations->setToolTip( tr("Possibly spend coins delegated for cold-staking (currently available: %1").arg( - GUIUtil::formatBalance(cachedDelegatedBalance, nDisplayUnit, false)) + GUIUtil::formatBalance(delegationBalance, nDisplayUnit, false)) ); } @@ -472,7 +483,9 @@ OperationResult SendWidget::prepareTransparent(WalletModelTransaction* currentTr if (!walletModel) return errorOut("Error, no wallet model loaded"); // prepare transaction for getting txFee earlier WalletModel::SendCoinsReturn prepareStatus; - prepareStatus = walletModel->prepareTransaction(currentTransaction, coinControlDialog->coinControl, fDelegationsChecked); + prepareStatus = walletModel->prepareTransaction(currentTransaction, + coinControlDialog ? coinControlDialog->coinControl : nullptr, + fDelegationsChecked); // process prepareStatus and on error generate message shown to user CClientUIInterface::MessageBoxFlags informType; @@ -521,9 +534,6 @@ bool SendWidget::sendFinalStep() ); if (sendStatus.status == WalletModel::OK) { - // if delegations were spent, update cachedBalance - if (fStakeDelegationVoided) - cachedDelegatedBalance = walletModel->getDelegatedBalance(); clearAll(false); inform(tr("Transaction sent")); dialog->deleteLater(); @@ -537,8 +547,8 @@ bool SendWidget::sendFinalStep() void SendWidget::run(int type) { - assert(!processingResult); if (type == REQUEST_PREPARE_TX) { + assert(!processingResult); if (!isProcessing) { isProcessing = true; OperationResult result(false); @@ -553,6 +563,12 @@ void SendWidget::run(int type) } isProcessing = false; } + } else if (type == REQUEST_REFRESH_BALANCE) { + if (!isUpdatingBalance) { + isUpdatingBalance = true; + refreshAmounts(); + isUpdatingBalance = false; + } } } @@ -562,9 +578,16 @@ void SendWidget::onError(QString error, int type) processingResultError = error; } -void SendWidget::updateEntryLabels(QList recipients) +void SendWidget::tryRefreshAmounts() +{ + if (!execute(REQUEST_REFRESH_BALANCE)) { + inform(tr("Processing full, refreshing amounts later")); + } +} + +void SendWidget::updateEntryLabels(const QList& recipients) { - for (SendCoinsRecipient rec : recipients) { + for (const SendCoinsRecipient& rec : recipients) { QString label = rec.label; if (!label.isNull()) { QString labelOld = walletModel->getAddressTableModel()->labelForAddress(rec.address); @@ -664,12 +687,14 @@ void SendWidget::onChangeCustomFeeClicked() void SendWidget::onCoinControlClicked() { if (walletModel->getBalance() > 0) { + // future: move coin control initialization and refresh to a worker thread. + if (!coinControlDialog->hasModel()) coinControlDialog->setModel(walletModel); coinControlDialog->setSelectionType(isTransparent); coinControlDialog->refreshDialog(); setCoinControlPayAmounts(); coinControlDialog->exec(); ui->btnCoinControl->setActive(coinControlDialog->coinControl->HasSelected()); - refreshAmounts(); + tryRefreshAmounts(); } else { inform(tr("You don't have any %1 to select.").arg(CURRENCY_UNIT.c_str())); } @@ -752,7 +777,7 @@ void SendWidget::setCoinControlPayAmounts() void SendWidget::onValueChanged() { - refreshAmounts(); + tryRefreshAmounts(); } void SendWidget::onCheckBoxChanged() @@ -760,7 +785,7 @@ void SendWidget::onCheckBoxChanged() const bool checked = ui->checkBoxDelegations->isChecked(); if (checked != fDelegationsChecked) { fDelegationsChecked = checked; - refreshAmounts(); + tryRefreshAmounts(); } } @@ -776,7 +801,7 @@ void SendWidget::onPIVSelected(bool _isTransparent) resetChangeAddress(); resetCoinControl(); - refreshAmounts(); + tryRefreshAmounts(); updateStyle(coinIcon); } @@ -946,7 +971,7 @@ void SendWidget::onDeleteClicked() focusedEntry = nullptr; // Update total amounts - refreshAmounts(); + tryRefreshAmounts(); setFocusOnLastEntry(); } } diff --git a/src/qt/pivx/send.h b/src/qt/pivx/send.h index 6feac119ad20..fcc4f9038408 100644 --- a/src/qt/pivx/send.h +++ b/src/qt/pivx/send.h @@ -57,6 +57,10 @@ public Q_SLOTS: void onValueChanged(); void refreshAmounts(); void changeTheme(bool isLightTheme, QString &theme) override; + void updateAmounts(const QString& titleTotalRemaining, + const QString& labelAmountSend, + const QString& labelAmountRemaining, + CAmount _delegationBalance); protected: void resizeEvent(QResizeEvent *event) override; @@ -87,7 +91,6 @@ private Q_SLOTS: SendCustomFeeDialog* customFeeDialog = nullptr; bool isCustomFeeSelected = false; bool fDelegationsChecked = false; - CAmount cachedDelegatedBalance{0}; int nDisplayUnit; QList entries; @@ -99,6 +102,9 @@ private Q_SLOTS: Optional processingResultError{nullopt}; std::atomic processingResult{false}; + // Balance update + std::atomic isUpdatingBalance{false}; + ContactsDropdown *menuContacts = nullptr; TooltipMenu *menu = nullptr; // Current focus entry @@ -113,13 +119,14 @@ private Q_SLOTS: OperationResult prepareTransparent(WalletModelTransaction* tx); bool sendFinalStep(); void setFocusOnLastEntry(); - void showHideCheckBoxDelegations(); - void updateEntryLabels(QList recipients); + void showHideCheckBoxDelegations(CAmount delegationBalance); + void updateEntryLabels(const QList& recipients); void setCustomFeeSelected(bool isSelected, const CAmount& customFee = DEFAULT_TRANSACTION_FEE); void setCoinControlPayAmounts(); void resetCoinControl(); void resetChangeAddress(); void hideContactsMenu(); + void tryRefreshAmounts(); }; #endif // SEND_H diff --git a/src/qt/pivx/settings/settingsinformationwidget.cpp b/src/qt/pivx/settings/settingsinformationwidget.cpp index af536782402e..2294a3ddbf0d 100644 --- a/src/qt/pivx/settings/settingsinformationwidget.cpp +++ b/src/qt/pivx/settings/settingsinformationwidget.cpp @@ -14,7 +14,7 @@ #include -#define REQUEST_UPDATE_MN_COUNT 0 +#define REQUEST_UPDATE_COUNTS 0 SettingsInformationWidget::SettingsInformationWidget(PIVXGUI* _window,QWidget *parent) : PWidget(_window,parent), @@ -141,6 +141,7 @@ void SettingsInformationWidget::setNumConnections(int count) void SettingsInformationWidget::setNumBlocks(int count) { + if (!isVisible()) return; ui->labelInfoBlockNumber->setText(QString::number(count)); if (clientModel) { ui->labelInfoBlockTime->setText(clientModel->getLastBlockDate().toString()); @@ -168,7 +169,7 @@ void SettingsInformationWidget::showEvent(QShowEvent *event) if (clientModel) { clientModel->startMasternodesTimer(); // Initial masternodes count value, running in a worker thread to not lock mnmanager mutex in the main thread. - execute(REQUEST_UPDATE_MN_COUNT); + execute(REQUEST_UPDATE_COUNTS); } } @@ -181,15 +182,17 @@ void SettingsInformationWidget::hideEvent(QHideEvent *event) { void SettingsInformationWidget::run(int type) { - if (type == REQUEST_UPDATE_MN_COUNT) { + if (type == REQUEST_UPDATE_COUNTS) { QMetaObject::invokeMethod(this, "setMasternodeCount", Qt::QueuedConnection, Q_ARG(QString, clientModel->getMasternodesCount())); + QMetaObject::invokeMethod(this, "setNumBlocks", + Qt::QueuedConnection, Q_ARG(int, clientModel->getLastBlockProcessedHeight())); } } void SettingsInformationWidget::onError(QString error, int type) { - if (type == REQUEST_UPDATE_MN_COUNT) { + if (type == REQUEST_UPDATE_COUNTS) { setMasternodeCount(tr("No available data")); } } diff --git a/src/qt/pivx/topbar.cpp b/src/qt/pivx/topbar.cpp index 52b3d0e34631..29b9ac02355d 100644 --- a/src/qt/pivx/topbar.cpp +++ b/src/qt/pivx/topbar.cpp @@ -13,13 +13,11 @@ #include "bitcoinunits.h" #include "qt/pivx/balancebubble.h" #include "clientmodel.h" -#include "qt/guiconstants.h" #include "qt/guiutil.h" #include "optionsmodel.h" #include "qt/platformstyle.h" #include "walletmodel.h" #include "addresstablemodel.h" -#include "guiinterface.h" #include "masternode-sync.h" #include "wallet/wallet.h" @@ -474,28 +472,9 @@ void TopBar::setNumBlocks(int count) if (!clientModel) return; - // Acquire current block source - enum BlockSource blockSource = clientModel->getBlockSource(); - std::string text = ""; - switch (blockSource) { - case BLOCK_SOURCE_NETWORK: - text = "Synchronizing.."; - break; - case BLOCK_SOURCE_DISK: - text = "Importing blocks from disk.."; - break; - case BLOCK_SOURCE_REINDEX: - text = "Reindexing blocks on disk.."; - break; - case BLOCK_SOURCE_NONE: - // Case: not Importing, not Reindexing and no network connection - text = "No block source available.."; - ui->pushButtonSync->setChecked(false); - break; - } - + std::string text; bool needState = true; - if (masternodeSync.IsBlockchainSynced()) { + if (masternodeSync.IsBlockchainSyncedReadOnly()) { // chain synced Q_EMIT walletSynced(true); if (masternodeSync.IsSynced()) { @@ -523,7 +502,7 @@ void TopBar::setNumBlocks(int count) Q_EMIT walletSynced(false); } - if (needState) { + if (needState && clientModel->isTipCached()) { // Represent time from last generated block in human readable text QDateTime lastBlockDate = clientModel->getLastBlockDate(); QDateTime currentDate = QDateTime::currentDateTime(); diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 2d3639c40705..6ef501c243fc 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -562,17 +562,8 @@ void TransactionRecord::loadHotOrColdStakeOrContract( ExtractAddress(p2csUtxo.scriptPubKey, false, record.address); } -void TransactionRecord::updateStatus(const CWalletTx& wtx) +void TransactionRecord::updateStatus(const CWalletTx& wtx, int chainHeight) { - AssertLockHeld(cs_main); - int chainHeight = chainActive.Height(); - - CBlockIndex *pindex = nullptr; - // Find the block the tx is in - BlockMap::iterator mi = mapBlockIndex.find(wtx.hashBlock); - if (mi != mapBlockIndex.end()) - pindex = (*mi).second; - // Determine transaction status // Update time if needed @@ -581,8 +572,8 @@ void TransactionRecord::updateStatus(const CWalletTx& wtx) // Sort order, unrecorded transactions sort to the top status.sortKey = strprintf("%010d-%01d-%010u-%03d", - (pindex ? pindex->nHeight : std::numeric_limits::max()), - (wtx.IsCoinBase() ? 1 : 0), + wtx.m_confirm.block_height, + ((wtx.IsCoinBase() || wtx.IsCoinStake()) ? 1 : 0), time, idx); @@ -634,12 +625,12 @@ void TransactionRecord::updateStatus(const CWalletTx& wtx) status.status = TransactionStatus::Confirmed; } } + status.needsUpdate = false; } -bool TransactionRecord::statusUpdateNeeded() +bool TransactionRecord::statusUpdateNeeded(int blockHeight) const { - AssertLockHeld(cs_main); - return status.cur_num_blocks != chainActive.Height(); + return status.cur_num_blocks != blockHeight || status.needsUpdate; } int TransactionRecord::getOutputIndex() const diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index fdbfc235167d..6bf51cd2cc12 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -62,6 +62,8 @@ class TransactionStatus /** Current number of blocks (to know whether cached status is still valid) */ int cur_num_blocks; + + bool needsUpdate; }; /** UI model for a transaction. A core transaction can be represented by multiple UI transactions if it has @@ -181,11 +183,11 @@ class TransactionRecord /** Update status from core wallet tx. */ - void updateStatus(const CWalletTx& wtx); + void updateStatus(const CWalletTx& wtx, int chainHeight); /** Return whether a status update is needed. */ - bool statusUpdateNeeded(); + bool statusUpdateNeeded(int blockHeight) const; /** Return transaction status */ diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index f9260e850885..8472467c2d7a 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -239,24 +239,22 @@ class TransactionTablePriv break; } if (showTransaction) { - LOCK2(cs_main, wallet->cs_wallet); // Find transaction in wallet - auto mi = wallet->mapWallet.find(hash); - if (mi == wallet->mapWallet.end()) { + const CWalletTx* wtx = wallet->GetWalletTx(hash); + if (!wtx) { qWarning() << "TransactionTablePriv::updateWallet : Warning: Got CT_NEW, but transaction is not in wallet"; break; } - const CWalletTx& wtx = mi->second; // As old transactions are still getting updated (+20k range), // do not add them if we deliberately didn't load them at startup. - if (cachedWallet.size() >= MAX_AMOUNT_LOADED_RECORDS && wtx.GetTxTime() < nFirstLoadedTxTime) { + if (cachedWallet.size() >= MAX_AMOUNT_LOADED_RECORDS && wtx->GetTxTime() < nFirstLoadedTxTime) { return; } // Added -- insert at the right position QList toInsert = - TransactionRecord::decomposeTransaction(wallet, wtx); + TransactionRecord::decomposeTransaction(wallet, *wtx); if (!toInsert.isEmpty()) { /* only if something to insert */ parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex + toInsert.size() - 1); int insert_idx = lowerIndex; @@ -283,6 +281,10 @@ class TransactionTablePriv case CT_UPDATED: // Miscellaneous updates -- nothing to do, status update will take care of this, and is only computed for // visible transactions. + for (int i = lowerIndex; i < upperIndex; i++) { + TransactionRecord *rec = &cachedWallet[i]; + rec->status.needsUpdate = true; + } break; } } @@ -297,32 +299,25 @@ class TransactionTablePriv return hasZcTxes; } - TransactionRecord* index(int idx) + TransactionRecord* index(int cur_block_num, const uint256& cur_block_hash, int idx) { if (idx >= 0 && idx < cachedWallet.size()) { TransactionRecord* rec = &cachedWallet[idx]; - - // Get required locks upfront. This avoids the GUI from getting - // stuck if the core is holding the locks for a longer time - for - // example, during a wallet rescan. - // - // If a status update is needed (blocks came in since last check), - // update the status of this transaction from the wallet. Otherwise, - // simply re-use the cached status. - TRY_LOCK(cs_main, lockMain); - if (lockMain) { + if (!cur_block_hash.IsNull() && rec->statusUpdateNeeded(cur_block_num)) { + // If a status update is needed (blocks came in since last check), + // update the status of this transaction from the wallet. Otherwise, + // simply re-use the cached status. TRY_LOCK(wallet->cs_wallet, lockWallet); - if (lockWallet && rec->statusUpdateNeeded()) { - std::map::iterator mi = wallet->mapWallet.find(rec->hash); - + if (lockWallet) { + auto mi = wallet->mapWallet.find(rec->hash); if (mi != wallet->mapWallet.end()) { - rec->updateStatus(mi->second); + rec->updateStatus(mi->second, cur_block_num); } } } return rec; } - return 0; + return nullptr; } }; @@ -809,7 +804,9 @@ QVariant TransactionTableModel::headerData(int section, Qt::Orientation orientat QModelIndex TransactionTableModel::index(int row, int column, const QModelIndex& parent) const { Q_UNUSED(parent); - TransactionRecord* data = priv->index(row); + TransactionRecord* data = priv->index(walletModel->getLastBlockProcessedNum(), + walletModel->getLastBlockProcessed(), + row); if (data) { return createIndex(row, column, data); } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index bfb3d7673550..b6aca41ea06f 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -7,6 +7,7 @@ #include "walletmodel.h" #include "addresstablemodel.h" +#include "qt/clientmodel.h" #include "guiconstants.h" #include "optionsmodel.h" #include "recentrequeststablemodel.h" @@ -28,7 +29,7 @@ #include #include -#include +#include #include #include @@ -48,7 +49,7 @@ WalletModel::WalletModel(CWallet* wallet, OptionsModel* optionsModel, QObject* p // This timer will be fired repeatedly to update the balance pollTimer = new QTimer(this); connect(pollTimer, &QTimer::timeout, this, &WalletModel::pollBalanceChanged); - pollTimer->start(MODEL_UPDATE_DELAY); + pollTimer->start(MODEL_UPDATE_DELAY * 5); subscribeToCoreSignals(); } @@ -192,53 +193,78 @@ bool WalletModel::isWalletLocked(bool fFullUnlocked) const return (status == Locked || (!fFullUnlocked && status == UnlockedForStaking)); } -bool IsImportingOrReindexing() +static bool IsImportingOrReindexing() { return fImporting || fReindex; } +std::atomic processingBalance{false}; + +static bool processBalanceChangeInternal(WalletModel* walletModel) +{ + int chainHeight = walletModel->getLastBlockProcessedNum(); + const uint256& blockHash = walletModel->getLastBlockProcessed(); + + if (walletModel->hasForceCheckBalance() || chainHeight != walletModel->getCacheNumBLocks()) { + // Try to get lock only if needed + TRY_LOCK(pwalletMain->cs_wallet, lockWallet); + if (!lockWallet) + return false; + + walletModel->setfForceCheckBalanceChanged(false); + + // Balance and number of transactions might have changed + walletModel->setCacheNumBlocks(chainHeight); + walletModel->setCacheBlockHash(blockHash); + walletModel->checkBalanceChanged(walletModel->getBalances()); + QMetaObject::invokeMethod(walletModel, "updateTxModelData", Qt::QueuedConnection); + QMetaObject::invokeMethod(walletModel, "pollFinished", Qt::QueuedConnection); + + // Address in receive tab may have been used + Q_EMIT walletModel->notifyReceiveAddressChanged(); + } + return true; +} + +static void processBalanceChange(WalletModel* walletModel) +{ + if (!processBalanceChangeInternal(walletModel)) { + processingBalance = false; + } +} + void WalletModel::pollBalanceChanged() { + if (processingBalance || !m_client_model) return; + // Wait a little bit more when the wallet is reindexing and/or importing, no need to lock cs_main so often. - if (IsImportingOrReindexing()) { + if (IsImportingOrReindexing() || m_client_model->inInitialBlockDownload()) { static uint8_t waitLonger = 0; waitLonger++; - if (waitLonger < 10) // 10 seconds + if (waitLonger < 6) // 30 seconds return; waitLonger = 0; } - // Get required locks upfront. This avoids the GUI from getting stuck on - // periodical polls if the core is holding the locks for a longer time - - // for example, during a wallet rescan. - TRY_LOCK(cs_main, lockMain); - if (!lockMain) - return; - TRY_LOCK(wallet->cs_wallet, lockWallet); - if (!lockWallet) - return; - // Don't continue processing if the chain tip time is less than the first // key creation time as there is no need to iterate over the transaction // table model in this case. - auto tip = chainActive.Tip(); - if (tip->GetBlockTime() < getCreationTime()) + int64_t blockTime = clientModel().getLastBlockProcessedTime(); + if (blockTime < getCreationTime()) return; - int chainHeight = tip->nHeight; - if (fForceCheckBalanceChanged || chainHeight != cachedNumBlocks) { - fForceCheckBalanceChanged = false; + // Avoid recomputing wallet balances unless a tx changed or + // BlockTip notification was received. + if (!fForceCheckBalanceChanged && m_cached_best_block_hash == getLastBlockProcessed()) return; - // Balance and number of transactions might have changed - cachedNumBlocks = chainHeight; - - checkBalanceChanged(walletWrapper.getBalances()); - if (transactionTableModel) { - transactionTableModel->updateConfirmations(); - } + processingBalance = true; + pollFuture = QtConcurrent::run(processBalanceChange, this); +} - // Address in receive tab may have been used - Q_EMIT notifyReceiveAddressChanged(); +void WalletModel::updateTxModelData() +{ + if (transactionTableModel) { + transactionTableModel->updateConfirmations(); } } @@ -252,7 +278,25 @@ void WalletModel::checkBalanceChanged(const interfaces::WalletBalances& newBalan { if (newBalance.balanceChanged(m_cached_balances)) { m_cached_balances = newBalance; - Q_EMIT balanceChanged(m_cached_balances); + QMetaObject::invokeMethod(this, "balanceNotify", Qt::QueuedConnection); + } +} + +void WalletModel::balanceNotify() +{ + Q_EMIT balanceChanged(m_cached_balances); +} + +void WalletModel::pollFinished() +{ + processingBalance = false; +} + +void WalletModel::stop() +{ + if(pollFuture.isRunning()) { + pollFuture.cancel(); + pollFuture.setPaused(true); } } @@ -944,7 +988,7 @@ bool WalletModel::getMNCollateralCandidate(COutPoint& outPoint) bool WalletModel::isSpent(const COutPoint& outpoint) const { - LOCK2(cs_main, wallet->cs_wallet); + LOCK(wallet->cs_wallet); return wallet->IsSpent(outpoint.hash, outpoint.n); } @@ -1021,19 +1065,19 @@ void WalletModel::listCoins(std::map>& bool WalletModel::isLockedCoin(uint256 hash, unsigned int n) const { - LOCK2(cs_main, wallet->cs_wallet); + LOCK(wallet->cs_wallet); return wallet->IsLockedCoin(hash, n); } void WalletModel::lockCoin(COutPoint& output) { - LOCK2(cs_main, wallet->cs_wallet); + LOCK(wallet->cs_wallet); wallet->LockCoin(output); } void WalletModel::unlockCoin(COutPoint& output) { - LOCK2(cs_main, wallet->cs_wallet); + LOCK(wallet->cs_wallet); wallet->UnlockCoin(output); } @@ -1093,3 +1137,19 @@ Optional WalletModel::getShieldedAddressFromSpendDesc(const uint256& tx Optional opAddr = wallet->GetSaplingScriptPubKeyMan()->GetAddressFromInputIfPossible(txHash, index); return opAddr ? Optional(QString::fromStdString(KeyIO::EncodePaymentAddress(*opAddr))) : nullopt; } + +void WalletModel::setClientModel(ClientModel* client_model) +{ + m_client_model = client_model; +} + +uint256 WalletModel::getLastBlockProcessed() const +{ + return m_client_model ? m_client_model->getLastBlockProcessed() : UINT256_ZERO; +} + +int WalletModel::getLastBlockProcessedNum() const +{ + return m_client_model ? m_client_model->getLastBlockProcessedHeight() : 0; +} + diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 3ad51df80780..ce0ffb91c61a 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -22,8 +22,10 @@ #include #include +#include class AddressTableModel; +class ClientModel; class OptionsModel; class RecentRequestsTableModel; class TransactionTableModel; @@ -327,6 +329,22 @@ class WalletModel : public QObject void loadReceiveRequests(std::vector& vReceiveRequests); bool saveReceiveRequest(const std::string& sAddress, const int64_t nId, const std::string& sRequest); + ClientModel& clientModel() const { return *m_client_model; } + void setClientModel(ClientModel* client_model); + + uint256 getLastBlockProcessed() const; + int getLastBlockProcessedNum() const; + + interfaces::WalletBalances getBalances() { return walletWrapper.getBalances(); }; + bool hasForceCheckBalance() { return fForceCheckBalanceChanged; } + void setCacheNumBlocks(int _cachedNumBlocks) { cachedNumBlocks = _cachedNumBlocks; } + int getCacheNumBLocks() { return cachedNumBlocks; } + void setCacheBlockHash(const uint256& _blockHash) { m_cached_best_block_hash = _blockHash; } + void setfForceCheckBalanceChanged(bool _fForceCheckBalanceChanged) { fForceCheckBalanceChanged = _fForceCheckBalanceChanged; } + Q_INVOKABLE void checkBalanceChanged(const interfaces::WalletBalances& new_balances); + + void stop(); + private: CWallet* wallet; // Simple Wallet interface. @@ -341,6 +359,7 @@ class WalletModel : public QObject std::unique_ptr m_handler_show_progress; std::unique_ptr m_handler_notify_watch_only_changed; std::unique_ptr m_handler_notify_walletbacked; + ClientModel* m_client_model; bool fHaveWatchOnly; bool fForceCheckBalanceChanged; @@ -358,12 +377,13 @@ class WalletModel : public QObject EncryptionStatus cachedEncryptionStatus; int cachedNumBlocks; + uint256 m_cached_best_block_hash; QTimer* pollTimer; + QFuture pollFuture; void subscribeToCoreSignals(); void unsubscribeFromCoreSignals(); - Q_INVOKABLE void checkBalanceChanged(const interfaces::WalletBalances& new_balances); Q_SIGNALS: // Signal that balance in wallet changed @@ -393,6 +413,12 @@ class WalletModel : public QObject void notifyReceiveAddressChanged(); public Q_SLOTS: + /* Wallet balances changes */ + void balanceNotify(); + /* Update transaction model after wallet changes */ + void updateTxModelData(); + /* Balance polling process finished */ + void pollFinished(); /* Wallet status might have changed */ void updateStatus(); /* New transaction, or transaction changed status */ diff --git a/src/sapling/saplingscriptpubkeyman.cpp b/src/sapling/saplingscriptpubkeyman.cpp index decc1753e4d5..3366baee001b 100644 --- a/src/sapling/saplingscriptpubkeyman.cpp +++ b/src/sapling/saplingscriptpubkeyman.cpp @@ -9,6 +9,7 @@ void SaplingScriptPubKeyMan::AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid) { + AssertLockHeld(wallet->cs_wallet); mapTxSaplingNullifiers.emplace(nullifier, wtxid); std::pair range; @@ -17,7 +18,7 @@ void SaplingScriptPubKeyMan::AddToSaplingSpends(const uint256& nullifier, const } bool SaplingScriptPubKeyMan::IsSaplingSpent(const uint256& nullifier) const { - LOCK(cs_main); + LOCK(wallet->cs_wallet); // future: move to AssertLockHeld() std::pair range; range = mapTxSaplingNullifiers.equal_range(nullifier); @@ -202,7 +203,7 @@ void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int64_t nWi } void SaplingScriptPubKeyMan::IncrementNoteWitnesses(const CBlockIndex* pindex, - const CBlock* pblockIn, + const CBlock* pblock, SaplingMerkleTree& saplingTree) { LOCK(wallet->cs_wallet); @@ -216,17 +217,6 @@ void SaplingScriptPubKeyMan::IncrementNoteWitnesses(const CBlockIndex* pindex, nWitnessCacheNeedsUpdate = true; } - const CBlock* pblock {pblockIn}; - CBlock block; - if (!pblock) { - if (!ReadBlockFromDisk(block, pindex)) { - std::string read_failed_str = strprintf("Unable to read block %d (%s) from disk.", - pindex->nHeight, pindex->GetBlockHash().ToString()); - throw std::runtime_error(read_failed_str); - } - pblock = █ - } - for (const auto& tx : pblock->vtx) { if (!tx->IsShieldedTx()) continue; @@ -470,7 +460,7 @@ void SaplingScriptPubKeyMan::GetFilteredNotes( bool requireSpendingKey, bool ignoreLocked) const { - LOCK2(cs_main, wallet->cs_wallet); + LOCK(wallet->cs_wallet); for (auto& p : wallet->mapWallet) { const CWalletTx& wtx = p.second; @@ -481,7 +471,7 @@ void SaplingScriptPubKeyMan::GetFilteredNotes( } // Filter the transactions before checking for notes - if (!CheckFinalTx(wtx.tx) || + if (!IsFinalTx(wtx.tx, wallet->GetLastBlockHeight() + 1, GetAdjustedTime()) || wtx.GetDepthInMainChain() < minDepth || wtx.GetDepthInMainChain() > maxDepth) { continue; diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp index b08cc8b8735c..c3a86997c2e2 100644 --- a/src/test/librust/sapling_rpc_wallet_tests.cpp +++ b/src/test/librust/sapling_rpc_wallet_tests.cpp @@ -416,7 +416,8 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) CMutableTransaction mtx; mtx.vout.emplace_back(5 * COIN, GetScriptForDestination(taddr)); // Add to wallet and get the updated wtx - pwalletMain->LoadToWallet({pwalletMain, MakeTransactionRef(mtx)}); + CWalletTx wtxIn(pwalletMain, MakeTransactionRef(mtx)); + pwalletMain->LoadToWallet(wtxIn); CWalletTx& wtx = pwalletMain->mapWallet.at(mtx.GetHash()); // Fake-mine the transaction @@ -428,12 +429,13 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) auto blockHash = block.GetHash(); CBlockIndex fakeIndex {block}; fakeIndex.nHeight = 1; - mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + BlockMap::iterator mi = mapBlockIndex.emplace(blockHash, &fakeIndex).first; + fakeIndex.phashBlock = &((*mi).first); chainActive.SetTip(&fakeIndex); BOOST_CHECK(chainActive.Contains(&fakeIndex)); BOOST_CHECK_EQUAL(1, chainActive.Height()); - wtx.SetMerkleBranch(blockHash, 0); - pwalletMain->LoadToWallet(wtx); + std::vector vtxConflicted; + pwalletMain->BlockConnected(std::make_shared(block), mi->second, vtxConflicted); BOOST_CHECK_MESSAGE(pwalletMain->GetAvailableBalance() > 0, "tx not confirmed"); // Context that shieldsendmany requires diff --git a/src/test/librust/sapling_wallet_tests.cpp b/src/test/librust/sapling_wallet_tests.cpp index b675b89219d7..af0cc84688ae 100644 --- a/src/test/librust/sapling_wallet_tests.cpp +++ b/src/test/librust/sapling_wallet_tests.cpp @@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(GetConflictedSaplingNotes) { auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first; BOOST_CHECK(saplingNoteData.size() > 0); wtx.SetSaplingNoteData(saplingNoteData); - wtx.SetMerkleBranch(block.GetHash(), 0); + wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeIndex.nHeight, block.GetHash(), 0); BOOST_CHECK(wallet.LoadToWallet(wtx)); // Simulate receiving new block and ChainTip signal @@ -351,8 +351,8 @@ BOOST_AUTO_TEST_CASE(SaplingNullifierIsSpent) { BOOST_CHECK(chainActive.Contains(&fakeIndex)); BOOST_CHECK_EQUAL(0, chainActive.Height()); - wtx.SetMerkleBranch(block.GetHash(), 0); - wallet.LoadToWallet(wtx); + std::vector vtxConflicted; + wallet.BlockConnected(std::make_shared(block), mi->second, vtxConflicted); // Verify note has been spent BOOST_CHECK(wallet.GetSaplingScriptPubKeyMan()->IsSaplingSpent(nullifier)); @@ -414,13 +414,14 @@ BOOST_AUTO_TEST_CASE(NavigateFromSaplingNullifierToNote) { BOOST_CHECK_EQUAL(0, chainActive.Height()); // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe - wtx.SetMerkleBranch(block.GetHash(), 0); + wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeIndex.nHeight, block.GetHash(), 0); auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first; BOOST_CHECK(saplingNoteData.size() > 0); wtx.SetSaplingNoteData(saplingNoteData); wallet.LoadToWallet(wtx); // Verify dummy note is now spent, as AddToWallet invokes AddToSpends() + wallet.SetLastBlockProcessed(mi->second); BOOST_CHECK(wallet.GetSaplingScriptPubKeyMan()->IsSaplingSpent(nullifier)); // Test invariant: no witnesses means no nullifier. @@ -512,7 +513,7 @@ BOOST_AUTO_TEST_CASE(SpentSaplingNoteIsFromMe) { auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first; BOOST_CHECK(saplingNoteData.size() > 0); wtx.SetSaplingNoteData(saplingNoteData); - wtx.SetMerkleBranch(block.GetHash(), 0); + wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeIndex.nHeight, block.GetHash(), 0); wallet.LoadToWallet(wtx); // Simulate receiving new block and ChainTip signal. @@ -520,6 +521,7 @@ BOOST_AUTO_TEST_CASE(SpentSaplingNoteIsFromMe) { // in the output descriptions of wtx. wallet.IncrementNoteWitnesses(&fakeIndex, &block, saplingTree); wallet.GetSaplingScriptPubKeyMan()->UpdateSaplingNullifierNoteMapForBlock(&block); + wallet.SetLastBlockProcessed(mi->second); // Retrieve the updated wtx from wallet wtx = wallet.mapWallet.at(wtx.GetHash()); @@ -588,8 +590,9 @@ BOOST_AUTO_TEST_CASE(SpentSaplingNoteIsFromMe) { auto saplingNoteData2 = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx2.tx).first; BOOST_CHECK(saplingNoteData2.size() > 0); wtx2.SetSaplingNoteData(saplingNoteData2); - wtx2.SetMerkleBranch(block2.GetHash(), 0); + wtx2.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeIndex2.nHeight, block2.GetHash(), 0); wallet.LoadToWallet(wtx2); + wallet.SetLastBlockProcessed(mi2->second); // Verify note B is spent. AddToWallet invokes AddToSpends which updates mapTxSaplingNullifiers BOOST_CHECK(wallet.GetSaplingScriptPubKeyMan()->IsSaplingSpent(nullifier2)); @@ -611,8 +614,10 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesEmptyChain) { auto consensusParams = RegtestActivateSapling(); CWallet& wallet = *pwalletMain; - LOCK(wallet.cs_wallet); - setupWallet(wallet); + { + LOCK(wallet.cs_wallet); + setupWallet(wallet); + } auto sk = GetTestMasterSaplingSpendingKey(); CWalletTx wtx = GetValidSaplingReceive(Params().GetConsensus(), wallet, sk, 10, true); @@ -650,18 +655,19 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesEmptyChain) { BOOST_AUTO_TEST_CASE(CachedWitnessesChainTip) { auto consensusParams = RegtestActivateSapling(); + libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); CWallet& wallet = *pwalletMain; - LOCK(wallet.cs_wallet); - setupWallet(wallet); + { + LOCK(wallet.cs_wallet); + setupWallet(wallet); + BOOST_CHECK(wallet.AddSaplingZKey(sk)); + BOOST_CHECK(wallet.HaveSaplingSpendingKey(sk.ToXFVK())); + } uint256 anchors1; CBlock block1; SaplingMerkleTree saplingTree; - libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); - BOOST_CHECK(wallet.AddSaplingZKey(sk)); - BOOST_CHECK(wallet.HaveSaplingSpendingKey(sk.ToXFVK())); - { // First block (case tested in _empty_chain) CBlockIndex index1(block1); @@ -728,15 +734,15 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesChainTip) { BOOST_AUTO_TEST_CASE(CachedWitnessesDecrementFirst) { auto consensusParams = RegtestActivateSapling(); + libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); CWallet& wallet = *pwalletMain; - LOCK(wallet.cs_wallet); - setupWallet(wallet); + { + LOCK(wallet.cs_wallet); + setupWallet(wallet); + BOOST_CHECK(wallet.AddSaplingZKey(sk)); + } SaplingMerkleTree saplingTree; - - libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); - BOOST_CHECK(wallet.AddSaplingZKey(sk)); - { // First block (case tested in _empty_chain) CBlock block1; @@ -796,9 +802,13 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesDecrementFirst) { BOOST_AUTO_TEST_CASE(CachedWitnessesCleanIndex) { auto consensusParams = RegtestActivateSapling(); + libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); CWallet& wallet = *pwalletMain; - LOCK(wallet.cs_wallet); - setupWallet(wallet); + { + LOCK(wallet.cs_wallet); + setupWallet(wallet); + BOOST_CHECK(wallet.AddSaplingZKey(sk)); + } std::vector blocks; std::vector indices; @@ -808,9 +818,6 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesCleanIndex) { SaplingMerkleTree saplingRiTree = saplingTree; std::vector> saplingWitnesses; - libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); - BOOST_CHECK(wallet.AddSaplingZKey(sk)); - // Generate a chain size_t numBlocks = WITNESS_CACHE_SIZE + 10; blocks.resize(numBlocks); @@ -871,12 +878,13 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesCleanIndex) { BOOST_AUTO_TEST_CASE(ClearNoteWitnessCache) { auto consensusParams = RegtestActivateSapling(); - CWallet& wallet = *pwalletMain; - LOCK(wallet.cs_wallet); - setupWallet(wallet); - libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); - BOOST_CHECK(wallet.AddSaplingZKey(sk)); + CWallet& wallet = *pwalletMain; + { + LOCK(wallet.cs_wallet); + setupWallet(wallet); + BOOST_CHECK(wallet.AddSaplingZKey(sk)); + } CWalletTx wtx = GetValidSaplingReceive(Params().GetConsensus(), wallet, sk, 10, true); @@ -919,7 +927,8 @@ BOOST_AUTO_TEST_CASE(UpdatedSaplingNoteData) { auto consensusParams = RegtestActivateSapling(); CWallet& wallet = *pwalletMain; - LOCK(wallet.cs_wallet); + // Need to lock cs_main for now due the lock ordering. future: revamp all of this function to only lock where is needed. + LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); auto m = GetTestMasterSaplingSpendingKey(); @@ -967,7 +976,7 @@ BOOST_AUTO_TEST_CASE(UpdatedSaplingNoteData) { auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first; BOOST_CHECK(saplingNoteData.size() == 1); // wallet only has key for change output wtx.SetSaplingNoteData(saplingNoteData); - wtx.SetMerkleBranch(block.GetHash(), 0); + wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeIndex.nHeight, block.GetHash(), 0); wallet.LoadToWallet(wtx); // Simulate receiving new block and ChainTip signal @@ -1083,7 +1092,7 @@ BOOST_AUTO_TEST_CASE(MarkAffectedSaplingTransactionsDirty) { auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first; BOOST_CHECK(saplingNoteData.size() > 0); wtx.SetSaplingNoteData(saplingNoteData); - wtx.SetMerkleBranch(block.GetHash(), 0); + wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeIndex.nHeight, block.GetHash(), 0); wallet.LoadToWallet(wtx); // Simulate receiving new block and ChainTip signal diff --git a/src/test/librust/utiltest.cpp b/src/test/librust/utiltest.cpp index be1c63e33ae3..caf2511daf1b 100644 --- a/src/test/librust/utiltest.cpp +++ b/src/test/librust/utiltest.cpp @@ -43,6 +43,11 @@ CKey AddTestCKeyToKeyStore(CBasicKeyStore& keyStore, bool genNewKey) { return tsk; } +CKey AddTestCKeyToWallet(CWallet& wallet, bool genNewKey) { + LOCK(wallet.cs_wallet); + return AddTestCKeyToKeyStore(wallet, genNewKey); +} + TestSaplingNote GetTestSaplingNote(const libzcash::SaplingPaymentAddress& pa, CAmount value) { // Generate dummy Sapling note libzcash::SaplingNote note(pa, value); @@ -80,13 +85,13 @@ CWalletTx GetValidSaplingReceive(const Consensus::Params& consensusParams, // Two dummy input (to trick coinbase check), one or many shielded outputs CWalletTx GetValidSaplingReceive(const Consensus::Params& consensusParams, - CBasicKeyStore& keyStoreFrom, + CWallet& keyStoreFrom, CAmount inputAmount, std::vector vDest, bool genNewKey, const CWallet* pwalletIn) { // From taddr - CKey tsk = AddTestCKeyToKeyStore(keyStoreFrom, genNewKey); + CKey tsk = AddTestCKeyToWallet(keyStoreFrom, genNewKey); auto scriptPubKey = GetScriptForDestination(tsk.GetPubKey().GetID()); // Two equal dummy inputs to by-pass the coinbase check. @@ -97,7 +102,7 @@ CWalletTx GetValidSaplingReceive(const Consensus::Params& consensusParams, // Single input, single shielded output CWalletTx GetValidSaplingReceive(const Consensus::Params& consensusParams, - CBasicKeyStore& keyStore, + CWallet& keyStore, const libzcash::SaplingExtendedSpendingKey &sk, CAmount value, bool genNewKey, diff --git a/src/test/librust/utiltest.h b/src/test/librust/utiltest.h index df2e69d17e5d..4f6e8412015f 100644 --- a/src/test/librust/utiltest.h +++ b/src/test/librust/utiltest.h @@ -35,6 +35,7 @@ void RegtestDeactivateSapling(); libzcash::SaplingExtendedSpendingKey GetTestMasterSaplingSpendingKey(); CKey AddTestCKeyToKeyStore(CBasicKeyStore& keyStore, bool genNewKey = false); +CKey AddTestCKeyToWallet(CWallet& wallet, bool genNewKey = false); /** * Generates a dummy destination script @@ -60,7 +61,7 @@ CWalletTx GetValidSaplingReceive(const Consensus::Params& consensusParams, * Single dummy input, one or many shielded outputs. */ CWalletTx GetValidSaplingReceive(const Consensus::Params& consensusParams, - CBasicKeyStore& keyStoreFrom, + CWallet& keyStoreFrom, CAmount inputAmount, std::vector vDest, bool genNewKey = false, @@ -70,7 +71,7 @@ CWalletTx GetValidSaplingReceive(const Consensus::Params& consensusParams, * Single dummy input, single shielded output to sk default address. */ CWalletTx GetValidSaplingReceive(const Consensus::Params& consensusParams, - CBasicKeyStore& keyStore, + CWallet& keyStore, const libzcash::SaplingExtendedSpendingKey &sk, CAmount value, bool genNewKey = false, diff --git a/src/validation.cpp b/src/validation.cpp index dcee17ea8a0e..e4af3d8a27ee 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -177,6 +177,7 @@ std::set setDirtyFileInfo; CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) { + AssertLockHeld(cs_main); // Find the first block the caller has in the main chain for (const uint256& hash : locator.vHave) { BlockMap::iterator mi = mapBlockIndex.find(hash); @@ -1912,7 +1913,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara UpdateTip(pindexDelete->pprev); // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: - GetMainSignals().BlockDisconnected(pblock, pindexDelete->nHeight); + GetMainSignals().BlockDisconnected(pblock, pindexDelete->GetBlockHash(), pindexDelete->nHeight, pindexDelete->GetBlockTime()); return true; } @@ -3296,18 +3297,6 @@ bool ProcessNewBlock(CValidationState& state, CNode* pfrom, const std::shared_pt } } - if (pwalletMain) { - /* disable multisend - // If turned on MultiSend will send a transaction (or more) on the after maturity of a stake - if (pwalletMain->isMultiSendEnabled()) - pwalletMain->MultiSend(); - */ - - // If turned on Auto Combine will scan wallet for dust to combine - if (pwalletMain->fCombineDust) - pwalletMain->AutoCombineDust(g_connman.get()); - } - LogPrintf("%s : ACCEPTED Block %ld in %ld milliseconds with size=%d\n", __func__, newHeight, GetTimeMillis() - nStartTime, GetSerializeSize(*pblock, SER_DISK, CLIENT_VERSION)); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index ad4af45dd7b1..ab2b92527642 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -37,7 +37,7 @@ struct MainSignalsInstance { */ boost::signals2::signal &, const CBlockIndex *pindex, const std::vector &)> BlockConnected; /** Notifies listeners of a block being disconnected */ - boost::signals2::signal &, int nBlockHeight)> BlockDisconnected; + boost::signals2::signal &, const uint256& blockHash, int nBlockHeight, int64_t blockTime)> BlockDisconnected; /** Notifies listeners of a transaction removal from the mempool */ boost::signals2::signal TransactionRemovedFromMempool; /** Notifies listeners of a new active block chain. */ @@ -98,7 +98,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) conns.UpdatedBlockTip = g_signals.m_internals->UpdatedBlockTip.connect(std::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); conns.TransactionAddedToMempool = g_signals.m_internals->TransactionAddedToMempool.connect(std::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, std::placeholders::_1)); conns.BlockConnected = g_signals.m_internals->BlockConnected.connect(std::bind(&CValidationInterface::BlockConnected, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); - conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1, std::placeholders::_2)); + conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4)); conns.TransactionRemovedFromMempool = g_signals.m_internals->TransactionRemovedFromMempool.connect(std::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, std::placeholders::_1)); conns.SetBestChain = g_signals.m_internals->SetBestChain.connect(std::bind(&CValidationInterface::SetBestChain, pwalletIn, std::placeholders::_1)); conns.Broadcast = g_signals.m_internals->Broadcast.connect(std::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, std::placeholders::_1)); @@ -163,9 +163,9 @@ void CMainSignals::BlockConnected(const std::shared_ptr &pblock, c }); } -void CMainSignals::BlockDisconnected(const std::shared_ptr &pblock, int nBlockHeight) { - m_internals->m_schedulerClient.AddToProcessQueue([pblock, nBlockHeight, this] { - m_internals->BlockDisconnected(pblock, nBlockHeight); +void CMainSignals::BlockDisconnected(const std::shared_ptr &pblock, const uint256& blockHash, int nBlockHeight, int64_t blockTime) { + m_internals->m_schedulerClient.AddToProcessQueue([pblock, blockHash, nBlockHeight, blockTime, this] { + m_internals->BlockDisconnected(pblock, blockHash, nBlockHeight, blockTime); }); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 7377e13ae286..27bfd9d4ce17 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -93,7 +93,7 @@ class CValidationInterface { * * Called on a background thread. */ - virtual void BlockDisconnected(const std::shared_ptr &block, int nBlockHeight) {} + virtual void BlockDisconnected(const std::shared_ptr &block, const uint256& blockHash, int nBlockHeight, int64_t blockTime) {} /** * Notifies listeners of the new active block chain on-disk. * @@ -138,7 +138,7 @@ class CMainSignals { void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload); void TransactionAddedToMempool(const CTransactionRef &ptxn); void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::shared_ptr> &); - void BlockDisconnected(const std::shared_ptr &block, int nBlockHeight); + void BlockDisconnected(const std::shared_ptr &block, const uint256& blockHash, int nBlockHeight, int64_t blockTime); void SetBestChain(const CBlockLocator &); void Broadcast(CConnman* connman); void BlockChecked(const CBlock&, const CValidationState&); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 03a153f5ebcd..6a150e2c32d9 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -62,9 +62,9 @@ void WalletTxToJSON(const CWalletTx& wtx, UniValue& entry) if (wtx.IsCoinBase() || wtx.IsCoinStake()) entry.pushKV("generated", true); if (confirms > 0) { - entry.pushKV("blockhash", wtx.hashBlock.GetHex()); - entry.pushKV("blockindex", wtx.nIndex); - entry.pushKV("blocktime", mapBlockIndex[wtx.hashBlock]->GetBlockTime()); + entry.pushKV("blockhash", wtx.m_confirm.hashBlock.GetHex()); + entry.pushKV("blockindex", wtx.m_confirm.nIndex); + entry.pushKV("blocktime", mapBlockIndex[wtx.m_confirm.hashBlock]->GetBlockTime()); } else { entry.pushKV("trusted", wtx.IsTrusted()); } @@ -1895,6 +1895,7 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request) pwalletMain->BlockUntilSyncedToCurrentChain(); LOCK2(cs_main, pwalletMain->cs_wallet); + int nBlockHeight = chainActive.Height(); // pivx address CTxDestination address = DecodeDestination(request.params[0].get_str()); @@ -1913,7 +1914,7 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request) CAmount nAmount = 0; for (std::map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !IsFinalTx(wtx.tx)) + if (wtx.IsCoinBase() || !IsFinalTx(wtx.tx, nBlockHeight)) continue; for (const CTxOut& txout : wtx.tx->vout) @@ -1955,6 +1956,7 @@ UniValue getreceivedbylabel(const JSONRPCRequest& request) pwalletMain->BlockUntilSyncedToCurrentChain(); LOCK2(cs_main, pwalletMain->cs_wallet); + int nBlockHeight = chainActive.Height(); // Minimum confirmations int nMinDepth = 1; @@ -1969,7 +1971,7 @@ UniValue getreceivedbylabel(const JSONRPCRequest& request) CAmount nAmount = 0; for (std::map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !IsFinalTx(wtx.tx)) + if (wtx.IsCoinBase() || !IsFinalTx(wtx.tx, nBlockHeight)) continue; for (const CTxOut& txout : wtx.tx->vout) { @@ -2302,7 +2304,7 @@ struct tallyitem { } }; -UniValue ListReceived(const UniValue& params, bool by_label) +UniValue ListReceived(const UniValue& params, bool by_label, int nBlockHeight) { // Minimum confirmations int nMinDepth = 1; @@ -2335,7 +2337,7 @@ UniValue ListReceived(const UniValue& params, bool by_label) for (std::map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !IsFinalTx(wtx.tx)) + if (wtx.IsCoinBase() || !IsFinalTx(wtx.tx, nBlockHeight)) continue; int nDepth = wtx.GetDepthInMainChain(); @@ -2477,8 +2479,8 @@ UniValue listreceivedbyaddress(const JSONRPCRequest& request) pwalletMain->BlockUntilSyncedToCurrentChain(); LOCK2(cs_main, pwalletMain->cs_wallet); - - return ListReceived(request.params, false); + int nBlockHeight = chainActive.Height(); + return ListReceived(request.params, false, nBlockHeight); } UniValue listreceivedbyshieldaddress(const JSONRPCRequest& request) @@ -2563,9 +2565,9 @@ UniValue listreceivedbyshieldaddress(const JSONRPCRequest& request) if (pwalletMain->mapWallet.count(entry.op.hash)) { const CWalletTx& wtx = pwalletMain->mapWallet.at(entry.op.hash); - if (!wtx.hashBlock.IsNull()) - height = mapBlockIndex[wtx.hashBlock]->nHeight; - index = wtx.nIndex; + if (!wtx.m_confirm.hashBlock.IsNull()) + height = mapBlockIndex[wtx.m_confirm.hashBlock]->nHeight; + index = wtx.m_confirm.nIndex; time = wtx.GetTxTime(); } @@ -2613,8 +2615,8 @@ UniValue listreceivedbylabel(const JSONRPCRequest& request) pwalletMain->BlockUntilSyncedToCurrentChain(); LOCK2(cs_main, pwalletMain->cs_wallet); - - return ListReceived(request.params, true); + int nBlockHeight = chainActive.Height(); + return ListReceived(request.params, true, nBlockHeight); } UniValue listcoldutxos(const JSONRPCRequest& request) @@ -3054,20 +3056,23 @@ UniValue abandontransaction(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 1) throw std::runtime_error( - "abandontransaction \"txid\"\n" - "\nMark in-wallet transaction \"txid\" as abandoned\n" - "This will mark this transaction and all its in-wallet descendants as abandoned which will allow\n" - "for their inputs to be respent. It can be used to replace \"stuck\" or evicted transactions.\n" - "It only works on transactions which are not included in a block and are not currently in the mempool.\n" - "It has no effect on transactions which are already conflicted or abandoned.\n" - "\nArguments:\n" - "1. \"txid\" (string, required) The transaction id\n" - "\nResult:\n" - "\nExamples:\n" - + HelpExampleCli("abandontransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"") - + HelpExampleRpc("abandontransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"") + "abandontransaction \"txid\"\n" + "\nMark in-wallet transaction \"txid\" as abandoned\n" + "This will mark this transaction and all its in-wallet descendants as abandoned which will allow\n" + "for their inputs to be respent. It can be used to replace \"stuck\" or evicted transactions.\n" + "It only works on transactions which are not included in a block and are not currently in the mempool.\n" + "It has no effect on transactions which are already abandoned.\n" + "\nArguments:\n" + "1. \"txid\" (string, required) The transaction id\n" + "\nResult:\n" + "\nExamples:\n" + + HelpExampleCli("abandontransaction", + "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"") + + HelpExampleRpc("abandontransaction", + "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"") ); + EnsureWallet(); EnsureWalletIsUnlocked(); // Make sure the results are valid at least up to the most recent block diff --git a/src/wallet/test/wallet_shielded_balances_tests.cpp b/src/wallet/test/wallet_shielded_balances_tests.cpp index 7db6cc02e1ce..caaa1c61b0ef 100644 --- a/src/wallet/test/wallet_shielded_balances_tests.cpp +++ b/src/wallet/test/wallet_shielded_balances_tests.cpp @@ -291,7 +291,7 @@ struct FakeBlock CBlockIndex* pindex; }; -FakeBlock SimpleFakeMine(CWalletTx& wtx, SaplingMerkleTree& currentTree) +FakeBlock SimpleFakeMine(CWalletTx& wtx, SaplingMerkleTree& currentTree, CWallet& wallet) { FakeBlock fakeBlock; fakeBlock.block.nVersion = 8; @@ -306,7 +306,8 @@ FakeBlock SimpleFakeMine(CWalletTx& wtx, SaplingMerkleTree& currentTree) fakeBlock.pindex->phashBlock = &mapBlockIndex.find(fakeBlock.block.GetHash())->first; chainActive.SetTip(fakeBlock.pindex); BOOST_CHECK(chainActive.Contains(fakeBlock.pindex)); - wtx.SetMerkleBranch(fakeBlock.pindex->GetBlockHash(), 0); + WITH_LOCK(wallet.cs_wallet, wallet.SetLastBlockProcessed(fakeBlock.pindex)); + wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeBlock.pindex->nHeight, fakeBlock.pindex->GetBlockHash(), 0); return fakeBlock; } @@ -347,7 +348,7 @@ BOOST_AUTO_TEST_CASE(GetShieldedAvailableCredit) // 2) Confirm the tx SaplingMerkleTree tree; - FakeBlock fakeBlock = SimpleFakeMine(wtxUpdated, tree); + FakeBlock fakeBlock = SimpleFakeMine(wtxUpdated, tree, wallet); // Simulate receiving a new block and updating the witnesses/nullifiers wallet.IncrementNoteWitnesses(fakeBlock.pindex, &fakeBlock.block, tree); wallet.GetSaplingScriptPubKeyMan()->UpdateSaplingNullifierNoteMapForBlock(&fakeBlock.block); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index d4016d4008ac..276cef991098 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -320,7 +320,7 @@ void removeTxFromMempool(CWalletTx& wtx) /** * Mimic block creation. */ -CBlockIndex* SimpleFakeMine(CWalletTx& wtx, CBlockIndex* pprev = nullptr) +CBlockIndex* SimpleFakeMine(CWalletTx& wtx, CWallet &wallet, CBlockIndex* pprev = nullptr) { CBlock block; block.vtx.emplace_back(wtx.tx); @@ -332,7 +332,8 @@ CBlockIndex* SimpleFakeMine(CWalletTx& wtx, CBlockIndex* pprev = nullptr) fakeIndex->phashBlock = &mapBlockIndex.find(block.GetHash())->first; chainActive.SetTip(fakeIndex); BOOST_CHECK(chainActive.Contains(fakeIndex)); - wtx.SetMerkleBranch(fakeIndex->GetBlockHash(), 0); + WITH_LOCK(wallet.cs_wallet, wallet.SetLastBlockProcessed(fakeIndex)); + wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeIndex->nHeight, fakeIndex->GetBlockHash(), 0); removeTxFromMempool(wtx); wtx.fInMempool = false; return fakeIndex; @@ -353,7 +354,8 @@ CWalletTx& BuildAndLoadTxToWallet(const std::vector& vin, mTx.vin = vin; mTx.vout = vout; CTransaction tx(mTx); - wallet.LoadToWallet({&wallet, MakeTransactionRef(tx)}); + CWalletTx wtx(&wallet, MakeTransactionRef(tx)); + wallet.LoadToWallet(wtx); return wallet.mapWallet.at(tx.GetHash()); } @@ -418,6 +420,7 @@ BOOST_AUTO_TEST_CASE(cached_balances_tests) LOCK2(cs_main, wallet.cs_wallet); wallet.SetMinVersion(FEATURE_PRE_SPLIT_KEYPOOL); wallet.SetupSPKM(false); + wallet.SetLastBlockProcessed(chainActive.Tip()); // Receive balance from an external source CTxDestination receivingAddr; @@ -441,7 +444,7 @@ BOOST_AUTO_TEST_CASE(cached_balances_tests) BOOST_CHECK_EQUAL(wallet.GetUnconfirmedBalance(), nCredit); // 2) Confirm tx and verify - SimpleFakeMine(wtxCredit); + SimpleFakeMine(wtxCredit, wallet); BOOST_CHECK_EQUAL(wallet.GetUnconfirmedBalance(), 0); BOOST_CHECK_EQUAL(wtxCredit.GetAvailableCredit(), nCredit); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f54d4077f7e3..862a7cd6600e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -703,6 +703,7 @@ bool CWallet::HasSaplingSPKM() const */ bool CWallet::IsSpent(const COutPoint& outpoint) const { + AssertLockHeld(cs_wallet); std::pair range; range = mapTxSpends.equal_range(outpoint); for (TxSpends::const_iterator it = range.first; it != range.second; ++it) { @@ -926,21 +927,17 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) bool fUpdated = false; if (!fInsertedNew) { - // Merge - if (!wtxIn.hashUnset() && wtxIn.hashBlock != wtx.hashBlock) { - wtx.hashBlock = wtxIn.hashBlock; + if (wtxIn.m_confirm.status != wtx.m_confirm.status) { + wtx.m_confirm.status = wtxIn.m_confirm.status; + wtx.m_confirm.nIndex = wtxIn.m_confirm.nIndex; + wtx.m_confirm.hashBlock = wtxIn.m_confirm.hashBlock; + wtx.m_confirm.block_height = wtxIn.m_confirm.block_height; wtx.UpdateTimeSmart(); fUpdated = true; - } - // If no longer abandoned, update - if (wtxIn.hashBlock.IsNull() && wtx.isAbandoned()) { - wtx.hashBlock = wtxIn.hashBlock; - if (!fUpdated) wtx.UpdateTimeSmart(); - fUpdated = true; - } - if (wtxIn.nIndex != -1 && wtxIn.nIndex != wtx.nIndex) { - wtx.nIndex = wtxIn.nIndex; - fUpdated = true; + } else { + assert(wtx.m_confirm.nIndex == wtxIn.m_confirm.nIndex); + assert(wtx.m_confirm.hashBlock == wtxIn.m_confirm.hashBlock); + assert(wtx.m_confirm.block_height == wtxIn.m_confirm.block_height); } if (HasSaplingSPKM() && m_sspk_man->UpdatedNoteData(wtxIn, wtx)) { fUpdated = true; @@ -977,8 +974,41 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) return true; } -bool CWallet::LoadToWallet(const CWalletTx& wtxIn) +// Internal function for now, this will be part of a chain interface class in the future. +Optional getTipBlockHeight(const uint256& hash) +{ + auto it = mapBlockIndex.find(hash); + CBlockIndex* pindex = it == mapBlockIndex.end() ? nullptr : it->second; + if (pindex && chainActive.Contains(pindex)) { + return Optional(pindex->nHeight); + } + return nullopt; +} + +bool CWallet::LoadToWallet(CWalletTx& wtxIn) { + LOCK2(cs_main, cs_wallet); + // If tx hasn't been reorged out of chain while wallet being shutdown + // change tx status to UNCONFIRMED and reset hashBlock/nIndex. + if (!wtxIn.m_confirm.hashBlock.IsNull()) { + Optional block_height = getTipBlockHeight(wtxIn.m_confirm.hashBlock); + if (block_height) { + // Update cached block height variable since it not stored in the + // serialized transaction. + wtxIn.m_confirm.block_height = *block_height; + } else if (wtxIn.isConflicted() || wtxIn.isConfirmed()) { + // If tx block (or conflicting block) was reorged out of chain + // while the wallet was shutdown, change tx status to UNCONFIRMED + // and reset block height, hash, and index. ABANDONED tx don't have + // associated blocks and don't need to be updated. The case where a + // transaction was reorged out while online and then reconfirmed + // while offline is covered by the rescan logic. + wtxIn.setUnconfirmed(); + wtxIn.m_confirm.hashBlock = UINT256_ZERO; + wtxIn.m_confirm.block_height = 0; + wtxIn.m_confirm.nIndex = 0; + } + } const uint256& hash = wtxIn.GetHash(); CWalletTx& wtx = mapWallet.emplace(hash, wtxIn).first->second; wtx.BindWallet(this); @@ -990,8 +1020,8 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn) auto it = mapWallet.find(txin.prevout.hash); if (it != mapWallet.end()) { CWalletTx& prevtx = it->second; - if (prevtx.nIndex == -1 && !prevtx.hashUnset()) { - MarkConflicted(prevtx.hashBlock, wtx.GetHash()); + if (prevtx.isConflicted()) { + MarkConflicted(prevtx.m_confirm.hashBlock, prevtx.m_confirm.block_height, wtx.GetHash()); } } } @@ -1049,19 +1079,19 @@ void CWallet::AddExternalNotesDataToTx(CWalletTx& wtx) const * Abandoned state should probably be more carefully tracked via different * posInBlock signals or by checking mempool presence when necessary. */ -bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const uint256& blockHash, int posInBlock, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const CWalletTx::Confirmation& confirm, bool fUpdate) { const CTransaction& tx = *ptx; { AssertLockHeld(cs_wallet); - if (!blockHash.IsNull() && !tx.HasZerocoinSpendInputs() && !tx.IsCoinBase()) { + if (!confirm.hashBlock.IsNull() && !tx.HasZerocoinSpendInputs() && !tx.IsCoinBase()) { for (const CTxIn& txin : tx.vin) { std::pair range = mapTxSpends.equal_range(txin.prevout); while (range.first != range.second) { if (range.first->second != tx.GetHash()) { - LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), blockHash.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); - MarkConflicted(blockHash, range.first->second); + LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), confirm.hashBlock.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); + MarkConflicted(confirm.hashBlock, confirm.block_height, range.first->second); } range.first++; } @@ -1103,10 +1133,9 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const uint256 if (isFromMe) AddExternalNotesDataToTx(wtx); } - // Get merkle branch if transaction was found in a block - if (!blockHash.IsNull()) { - wtx.SetMerkleBranch(blockHash, posInBlock); - } + // Block disconnection override an abandoned tx as unconfirmed + // which means user may have to call abandontransaction again + wtx.m_confirm = confirm; return AddToWallet(wtx, false); } @@ -1116,7 +1145,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const uint256 bool CWallet::AbandonTransaction(const uint256& hashTx) { - LOCK2(cs_main, cs_wallet); + LOCK(cs_wallet); CWalletDB walletdb(*dbw, "r+"); @@ -1127,7 +1156,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) auto it = mapWallet.find(hashTx); assert(it != mapWallet.end()); CWalletTx& origtx = it->second; - if (origtx.GetDepthInMainChain() > 0 || origtx.InMempool()) { + if (origtx.GetDepthInMainChain() != 0 || origtx.InMempool()) { return false; } @@ -1147,7 +1176,6 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) if (currentconfirm == 0 && !wtx.isAbandoned()) { // If the orig tx was not in block/mempool, none of its spends can be in mempool assert(!wtx.InMempool()); - wtx.nIndex = -1; wtx.setAbandoned(); wtx.MarkDirty(); walletdb.WriteTx(wtx); @@ -1174,18 +1202,11 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) return true; } -void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) +void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx) { - LOCK2(cs_main, cs_wallet); - - int conflictconfirms = 0; - if (mapBlockIndex.count(hashBlock)) { - CBlockIndex* pindex = mapBlockIndex[hashBlock]; - if (chainActive.Contains(pindex)) { - conflictconfirms = -(chainActive.Height() - pindex->nHeight + 1); - } - } + LOCK(cs_wallet); + int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1; // If number of conflict confirms cannot be determined, this means // that the block is still unknown or not yet part of the main chain, // for example when loading the wallet during a reindex. Do nothing in that @@ -1212,8 +1233,10 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) if (conflictconfirms < currentconfirm) { // Block is 'more conflicted' than current confirm; update. // Mark transaction as conflicted with this block. - wtx.nIndex = -1; - wtx.hashBlock = hashBlock; + wtx.m_confirm.nIndex = 0; + wtx.m_confirm.hashBlock = hashBlock; + wtx.m_confirm.block_height = conflicting_height; + wtx.setConflicted(); wtx.MarkDirty(); walletdb.WriteTx(wtx); // Iterate over all its outputs, and mark transactions in the wallet that spend them conflicted too @@ -1236,12 +1259,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) } } -void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pindexBlockConnected, int posInBlock) +void CWallet::SyncTransaction(const CTransactionRef& ptx, const CWalletTx::Confirmation& confirm) { - if (!AddToWalletIfInvolvingMe(ptx, - (pindexBlockConnected) ? pindexBlockConnected->GetBlockHash() : uint256(), - posInBlock, - true)) { + if (!AddToWalletIfInvolvingMe(ptx, confirm, true)) { return; // Not one of ours } @@ -1250,8 +1270,9 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pin void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { - LOCK2(cs_main, cs_wallet); - SyncTransaction(ptx, NULL, -1); + LOCK(cs_wallet); + CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); + SyncTransaction(ptx, confirm); auto it = mapWallet.find(ptx->GetHash()); if (it != mapWallet.end()) { @@ -1269,48 +1290,62 @@ void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) { void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) { - LOCK2(cs_main, cs_wallet); - // TODO: Tempoarily ensure that mempool removals are notified before - // connected transactions. This shouldn't matter, but the abandoned - // state of transactions in our wallet is currently cleared when we - // receive another notification and there is a race condition where - // notification of a connected conflict might cause an outside process - // to abandon a transaction and then have it inadvertently cleared by - // the notification that the conflicted transaction was evicted. - - for (const CTransactionRef& ptx : vtxConflicted) { - SyncTransaction(ptx, nullptr, -1); - TransactionRemovedFromMempool(ptx); - } - for (size_t i = 0; i < pblock->vtx.size(); i++) { - SyncTransaction(pblock->vtx[i], pindex, i); - TransactionRemovedFromMempool(pblock->vtx[i]); - } - - // Sapling: notify about the connected block - // Get prev block tree anchor - CBlockIndex* pprev = pindex->pprev; - SaplingMerkleTree oldSaplingTree; - bool isSaplingActive = (pprev) != nullptr && - Params().GetConsensus().NetworkUpgradeActive(pprev->nHeight, - Consensus::UPGRADE_V5_0); - if (isSaplingActive) { - assert(pcoinsTip->GetSaplingAnchorAt(pprev->hashFinalSaplingRoot, oldSaplingTree)); - } else { - assert(pcoinsTip->GetSaplingAnchorAt(SaplingMerkleTree::empty_root(), oldSaplingTree)); - } + { + LOCK(cs_wallet); - // Sapling: Update cached incremental witnesses - ChainTipAdded(pindex, pblock.get(), oldSaplingTree); + m_last_block_processed = pindex->GetBlockHash(); + m_last_block_processed_time = pindex->GetBlockTime(); + m_last_block_processed_height = pindex->nHeight; + for (size_t index = 0; index < pblock->vtx.size(); index++) { + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, m_last_block_processed_height, + m_last_block_processed, index); + SyncTransaction(pblock->vtx[index], confirm); + TransactionRemovedFromMempool(pblock->vtx[index]); + } + for (const CTransactionRef& ptx : vtxConflicted) { + TransactionRemovedFromMempool(ptx); + } - m_last_block_processed = pindex; + // Sapling: notify about the connected block + // Get prev block tree anchor + CBlockIndex* pprev = pindex->pprev; + SaplingMerkleTree oldSaplingTree; + bool isSaplingActive = (pprev) != nullptr && + Params().GetConsensus().NetworkUpgradeActive(pprev->nHeight, + Consensus::UPGRADE_V5_0); + if (isSaplingActive) { + assert(pcoinsTip->GetSaplingAnchorAt(pprev->hashFinalSaplingRoot, oldSaplingTree)); + } else { + assert(pcoinsTip->GetSaplingAnchorAt(SaplingMerkleTree::empty_root(), oldSaplingTree)); + } + + // Sapling: Update cached incremental witnesses + ChainTipAdded(pindex, pblock.get(), oldSaplingTree); + } // cs_wallet lock end + + // Auto-combine functionality + // If turned on Auto Combine will scan wallet for dust to combine + // Outside of the cs_wallet lock because requires cs_main for now + // due CreateTransaction/CommitTransaction dependency. + if (fCombineDust) { + AutoCombineDust(g_connman.get()); + } } -void CWallet::BlockDisconnected(const std::shared_ptr& pblock, int nBlockHeight) +void CWallet::BlockDisconnected(const std::shared_ptr& pblock, const uint256& blockHash, int nBlockHeight, int64_t blockTime) { - LOCK2(cs_main, cs_wallet); + LOCK(cs_wallet); + + // At block disconnection, this will change an abandoned transaction to + // be unconfirmed, whether or not the transaction is added back to the mempool. + // User may have to call abandontransaction again. It may be addressed in the + // future with a stickier abandoned state or even removing abandontransaction call. + m_last_block_processed_height = nBlockHeight - 1; + m_last_block_processed_time = blockTime; + m_last_block_processed = blockHash; for (const CTransactionRef& ptx : pblock->vtx) { - SyncTransaction(ptx, NULL, -1); + CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); + SyncTransaction(ptx, confirm); } if (Params().GetConsensus().NetworkUpgradeActive(nBlockHeight, Consensus::UPGRADE_V5_0)) { @@ -1324,17 +1359,18 @@ void CWallet::BlockUntilSyncedToCurrentChain() { AssertLockNotHeld(cs_main); AssertLockNotHeld(cs_wallet); - if (m_last_block_processed) { + { // Skip the queue-draining stuff if we know we're caught up with // chainActive.Tip()... // We could also take cs_wallet here, and call m_last_block_processed // protected by cs_wallet instead of cs_main, but as long as we need // cs_main here anyway, its easier to just call it cs_main-protected. + uint256 last_block_hash = WITH_LOCK(cs_wallet, return m_last_block_processed); LOCK(cs_main); const CBlockIndex* initialChainTip = chainActive.Tip(); - - if (m_last_block_processed->GetAncestor(initialChainTip->nHeight) == initialChainTip) { - return; + if (!last_block_hash.IsNull() && initialChainTip && + last_block_hash == initialChainTip->GetBlockHash()) { + return; } } @@ -1476,11 +1512,11 @@ int64_t CWalletTx::GetTxTime() const void CWalletTx::UpdateTimeSmart() { nTimeSmart = nTimeReceived; - if (!hashBlock.IsNull()) { - if (mapBlockIndex.count(hashBlock)) { - nTimeSmart = mapBlockIndex.at(hashBlock)->GetBlockTime(); + if (!m_confirm.hashBlock.IsNull()) { + if (mapBlockIndex.count(m_confirm.hashBlock)) { + nTimeSmart = mapBlockIndex.at(m_confirm.hashBlock)->GetBlockTime(); } else - LogPrintf("%s : found %s in block %s not in index\n", __func__, GetHash().ToString(), hashBlock.ToString()); + LogPrintf("%s : found %s in block %s not in index\n", __func__, GetHash().ToString(), m_confirm.hashBlock.ToString()); } } @@ -1559,7 +1595,6 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter, bool recalculate) const CAmount CWalletTx::GetImmatureCredit(bool fUseCache, const isminefilter& filter) const { - LOCK(cs_main); if (IsInMainChainImmature()) { return GetCachableAmount(IMMATURE_CREDIT, filter, !fUseCache); } @@ -1661,7 +1696,6 @@ CAmount CWalletTx::GetLockedCredit() const CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool& fUseCache) const { - LOCK(cs_main); if (IsInMainChainImmature()) { return GetCachableAmount(IMMATURE_CREDIT, ISMINE_WATCH_ONLY, !fUseCache); } @@ -1837,7 +1871,8 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b } for (int posInBlock = 0; posInBlock < (int) block.vtx.size(); posInBlock++) { const auto& tx = block.vtx[posInBlock]; - if (AddToWalletIfInvolvingMe(tx, pindex->GetBlockHash(), posInBlock, fUpdate)) { + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, pindex->nHeight, pindex->GetBlockHash(), posInBlock); + if (AddToWalletIfInvolvingMe(tx, confirm, fUpdate)) { myTxHashes.push_back(tx->GetHash()); ret++; } @@ -1895,11 +1930,9 @@ void CWallet::ReacceptWalletTransactions(bool fFirstLoad) } // Try to add wallet transactions to memory pool - for (std::pair& item: mapSorted) - { + for (std::pair& item: mapSorted) { CWalletTx& wtx = *(item.second); - LOCK(mempool.cs); CValidationState state; bool fSuccess = wtx.AcceptToMemoryPool(state, false); if (!fSuccess && fFirstLoad && GetTime() - wtx.GetTxTime() > 12*60*60) { @@ -1921,7 +1954,6 @@ void CWalletTx::RelayWalletTransaction(CConnman* connman) // Nothing to do. Return early return; } - LOCK(cs_main); if (GetDepthInMainChain() == 0 && !isAbandoned()) { const uint256& hash = GetHash(); LogPrintf("Relaying wtx %s\n", hash.ToString()); @@ -1956,6 +1988,11 @@ bool CWallet::Verify() if (gArgs.GetBoolArg("-salvagewallet", false)) { // Recover readable keypairs: CWallet dummyWallet; + // Even if we don't use this lock in this function, we want to preserve + // lock order in LoadToWallet if query of chain state is needed to know + // tx status. If lock can't be taken, tx confirmation status may be not + // reliable. + LOCK(cs_main); if (!CWalletDB::Recover(walletFile, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter)) return false; } @@ -2018,11 +2055,40 @@ void CWallet::ResendWalletTransactions(CConnman* connman) * @{ */ +CWallet::Balance CWallet::GetBalance(const int min_depth) const +{ + Balance ret; + { + LOCK(cs_wallet); + std::set trusted_parents; + for (const auto& entry : mapWallet) { + const CWalletTx& wtx = entry.second; + const bool is_trusted{wtx.IsTrusted()}; + const int tx_depth{wtx.GetDepthInMainChain()}; + const CAmount tx_credit_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE_TRANSPARENT)}; + const CAmount tx_credit_shield_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE_SHIELDED)}; + if (is_trusted && tx_depth >= min_depth) { + ret.m_mine_trusted += tx_credit_mine; + ret.m_mine_trusted_shield += tx_credit_shield_mine; + if (wtx.tx->HasP2CSOutputs()) { + ret.m_mine_cs_delegated_trusted += wtx.GetStakeDelegationCredit(); + } + } + if (!is_trusted && tx_depth == 0 && wtx.InMempool()) { + ret.m_mine_untrusted_pending += tx_credit_mine; + ret.m_mine_untrusted_shielded_balance += tx_credit_shield_mine; + } + ret.m_mine_immature += wtx.GetImmatureCredit(); + } + } + return ret; +} + CAmount CWallet::loopTxsBalance(std::function method) const { CAmount nTotal = 0; { - LOCK2(cs_main, cs_wallet); + LOCK(cs_wallet); for (const auto& it : mapWallet) { method(it.first, it.second, nTotal); } @@ -2090,10 +2156,20 @@ CAmount CWallet::GetLockedCoins() const { if (fLiteMode) return 0; - return loopTxsBalance([](const uint256& id, const CWalletTx& pcoin, CAmount& nTotal) { - if (pcoin.IsTrusted() && pcoin.GetDepthInMainChain() > 0) - nTotal += pcoin.GetLockedCredit(); - }); + LOCK(cs_wallet); + if (setLockedCoins.empty()) return 0; + + CAmount ret = 0; + for (const auto& coin : setLockedCoins) { + auto it = mapWallet.find(coin.hash); + if (it != mapWallet.end()) { + const CWalletTx& pcoin = it->second; + if (pcoin.IsTrusted() && pcoin.GetDepthInMainChain() > 0) { + ret += it->second.tx->vout.at(coin.n).nValue; + } + } + } + return ret; } CAmount CWallet::GetUnconfirmedBalance(isminetype filter) const @@ -2156,14 +2232,14 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const // trusted. CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth) const { - LOCK2(cs_main, cs_wallet); + LOCK(cs_wallet); CAmount balance = 0; for (const auto& entry : mapWallet) { const CWalletTx& wtx = entry.second; bool fConflicted; const int depth = wtx.GetDepthAndMempool(fConflicted); - if (!IsFinalTx(wtx.tx) || wtx.GetBlocksToMaturity() > 0 || depth < 0 || fConflicted) { + if (!IsFinalTx(wtx.tx, m_last_block_processed_height) || wtx.GetBlocksToMaturity() > 0 || depth < 0 || fConflicted) { continue; } @@ -2203,7 +2279,7 @@ CAmount CWallet::GetUnconfirmedShieldedBalance() const void CWallet::GetAvailableP2CSCoins(std::vector& vCoins) const { vCoins.clear(); { - LOCK2(cs_main, cs_wallet); + LOCK(cs_wallet); for (const auto& it : mapWallet) { const uint256& wtxid = it.first; const CWalletTx* pcoin = &it.second; @@ -2238,14 +2314,12 @@ void CWallet::GetAvailableP2CSCoins(std::vector& vCoins) const { /** * Test if the transaction is spendable. */ -bool CheckTXAvailability(const CWalletTx* pcoin, bool fOnlyConfirmed, int& nDepth, const CBlockIndex*& pindexRet) +static bool CheckTXAvailabilityInternal(const CWalletTx* pcoin, bool fOnlyConfirmed, int& nDepth) { - AssertLockHeld(cs_main); - if (!CheckFinalTx(pcoin->tx)) return false; if (fOnlyConfirmed && !pcoin->IsTrusted()) return false; if (pcoin->GetBlocksToMaturity() > 0) return false; - nDepth = pcoin->GetDepthInMainChain(pindexRet); + nDepth = pcoin->GetDepthInMainChain(); // We should not consider coins which aren't at least in our mempool // It's possible for these to be conflicted via ancestors which we may never be able to detect @@ -2254,10 +2328,23 @@ bool CheckTXAvailability(const CWalletTx* pcoin, bool fOnlyConfirmed, int& nDept return true; } -bool CheckTXAvailability(const CWalletTx* pcoin, bool fOnlyConfirmed, int& nDepth) +// cs_main lock required +static bool CheckTXAvailability(const CWalletTx* pcoin, bool fOnlyConfirmed, int& nDepth) +{ + AssertLockHeld(cs_main); + if (!CheckFinalTx(pcoin->tx)) return false; + return CheckTXAvailabilityInternal(pcoin, fOnlyConfirmed, nDepth); +} + +// cs_main lock NOT required +static bool CheckTXAvailability(const CWalletTx* pcoin, + bool fOnlyConfirmed, + int& nDepth, + int nBlockHeight) { - const CBlockIndex* pindexRet = nullptr; - return CheckTXAvailability(pcoin, fOnlyConfirmed, nDepth, pindexRet); + // Mimic CheckFinalTx without cs_main lock + if (!IsFinalTx(pcoin->tx, nBlockHeight + 1, GetAdjustedTime())) return false; + return CheckTXAvailabilityInternal(pcoin, fOnlyConfirmed, nDepth); } bool CWallet::GetMasternodeVinAndKeys(CTxIn& txinRet, CPubKey& pubKeyRet, CKey& keyRet, std::string strTxHash, std::string strOutputIndex, std::string& strError) @@ -2300,19 +2387,20 @@ bool CWallet::GetMasternodeVinAndKeys(CTxIn& txinRet, CPubKey& pubKeyRet, CKey& return error("%s: tx %s, index %d not a masternode collateral", __func__, strTxHash, nOutputIndex); } - // Check availability int nDepth = 0; { - LOCK(cs_main); - if (!CheckTXAvailability(wtx, true, nDepth)) { + LOCK(cs_wallet); + // Check availability + if (!CheckTXAvailability(wtx, true, nDepth, m_last_block_processed_height)) { strError = "Not available collateral transaction"; return error("%s: tx %s not available", __func__, strTxHash); } - } - // Skip spent coins - if (IsSpent(txHash, nOutputIndex)) { - strError = "Error: collateral already spent"; - return error("%s: tx %s already spent", __func__, strTxHash); + + // Skip spent coins + if (IsSpent(txHash, nOutputIndex)) { + strError = "Error: collateral already spent"; + return error("%s: tx %s already spent", __func__, strTxHash); + } } // Depth must be at least MASTERNODE_MIN_CONFIRMATIONS @@ -2404,14 +2492,14 @@ bool CWallet::AvailableCoins(std::vector* pCoins, // --> populates coinsFilter.fIncludeDelegated = true; { - LOCK2(cs_main, cs_wallet); + LOCK(cs_wallet); for (std::map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { const uint256& wtxid = it->first; const CWalletTx* pcoin = &(*it).second; // Check if the tx is selectable int nDepth; - if (!CheckTXAvailability(pcoin, coinsFilter.fOnlyConfirmed, nDepth)) + if (!CheckTXAvailability(pcoin, coinsFilter.fOnlyConfirmed, nDepth, m_last_block_processed_height)) continue; // Check min depth requirement for stake inputs @@ -2423,6 +2511,11 @@ bool CWallet::AvailableCoins(std::vector* pCoins, // --> populates for (unsigned int i = 0; i < pcoin->tx->vout.size(); i++) { const auto& output = pcoin->tx->vout[i]; + // Filter by value if needed + if (coinsFilter.nMaxOutValue > 0 && output.nValue > coinsFilter.nMaxOutValue) { + continue; + } + // Filter by specific destinations if needed if (coinsFilter.onlyFilteredDest && !coinsFilter.onlyFilteredDest->empty()) { CTxDestination address; @@ -2455,31 +2548,33 @@ bool CWallet::AvailableCoins(std::vector* pCoins, // --> populates } } -std::map > CWallet::AvailableCoinsByAddress(bool fConfirmed, CAmount maxCoinValue) +std::map > CWallet::AvailableCoinsByAddress(bool fConfirmed, CAmount maxCoinValue, bool fIncludeColdStaking) { CWallet::AvailableCoinsFilter coinFilter; coinFilter.fIncludeColdStaking = true; coinFilter.fOnlyConfirmed = fConfirmed; + coinFilter.fIncludeColdStaking = fIncludeColdStaking; + coinFilter.nMaxOutValue = maxCoinValue; std::vector vCoins; - // include cold AvailableCoins(&vCoins, nullptr, coinFilter); std::map > mapCoins; - for (COutput& out : vCoins) { - if (maxCoinValue > 0 && out.tx->tx->vout[out.i].nValue > maxCoinValue) - continue; - + for (const COutput& out : vCoins) { CTxDestination address; bool fColdStakeAddr = false; if (!ExtractDestination(out.tx->tx->vout[out.i].scriptPubKey, address, fColdStakeAddr)) { + bool isP2CS = out.tx->tx->vout[out.i].scriptPubKey.IsPayToColdStaking(); + if (isP2CS && !fIncludeColdStaking) { + // It must never happen as the coin filtering process shouldn't had added the P2CS in the first place + assert(false); + } // if this is a P2CS we don't have the owner key - check if we have the staking key fColdStakeAddr = true; - if ( !out.tx->tx->vout[out.i].scriptPubKey.IsPayToColdStaking() || - !ExtractDestination(out.tx->tx->vout[out.i].scriptPubKey, address, fColdStakeAddr) ) + if (!isP2CS || !ExtractDestination(out.tx->tx->vout[out.i].scriptPubKey, address, fColdStakeAddr) ) continue; } - mapCoins[address].push_back(out); + mapCoins[address].emplace_back(out); } return mapCoins; @@ -2538,13 +2633,13 @@ bool CWallet::StakeableCoins(std::vector* pCoins) // Check if the tx is selectable int nDepth; - const CBlockIndex* pindex = nullptr; - if (!CheckTXAvailability(pcoin, true, nDepth, pindex)) + if (!CheckTXAvailability(pcoin, true, nDepth)) continue; // Check min depth requirement for stake inputs if (nDepth < Params().GetConsensus().nStakeMinDepth) continue; + const CBlockIndex* pindex = nullptr; for (unsigned int index = 0; index < pcoin->tx->vout.size(); index++) { auto res = CheckOutputAvailability( @@ -2562,6 +2657,7 @@ bool CWallet::StakeableCoins(std::vector* pCoins) // found valid coin if (!pCoins) return true; + if (!pindex) pindex = mapBlockIndex.at(pcoin->m_confirm.hashBlock); pCoins->emplace_back(CStakeableOutput(pcoin, (int) index, nDepth, res.spendable, res.solvable, pindex)); } } @@ -2768,7 +2864,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov tx.vin.push_back(txin); if (lockUnspents) { - LOCK2(cs_main, cs_wallet); + LOCK(cs_wallet); LockCoin(txin.prevout); } } @@ -3052,16 +3148,14 @@ bool CWallet::CreateCoinStake( outPoint, it->pindex); - //new block came in, move on - if (WITH_LOCK(cs_main, return chainActive.Height()) != pindexPrev->nHeight) return false; + // New block came in, move on + if (WITH_LOCK(cs_wallet, return m_last_block_processed_height) != pindexPrev->nHeight) return false; // Make sure the wallet is unlocked and shutdown hasn't been requested if (IsLocked() || ShutdownRequested()) return false; // Make sure the stake input hasn't been spent since last check - // for now, IsSpent() requires cs_main lock due its internal call to GetDepthInMainChain. - // This dependency will be completely removed moving forward, in #2209. - if (WITH_LOCK(cs_main, return IsSpent(outPoint))) { + if (WITH_LOCK(cs_wallet, return IsSpent(outPoint))) { // remove it from the available coins it = availableCoins->erase(it); continue; @@ -3180,7 +3274,7 @@ std::string CWallet::CommitResult::ToString() const CWallet::CommitResult CWallet::CommitTransaction(CTransactionRef tx, CReserveKey& opReservekey, CConnman* connman) { - return CommitTransaction(tx, &opReservekey, connman); + return CommitTransaction(std::move(tx), &opReservekey, connman); } /** @@ -3460,7 +3554,7 @@ std::map CWallet::GetAddressBalances() for (std::pair walletEntry : mapWallet) { CWalletTx* pcoin = &walletEntry.second; - if (!IsFinalTx(pcoin->tx) || !pcoin->IsTrusted()) + if (!IsFinalTx(pcoin->tx, m_last_block_processed_height) || !pcoin->IsTrusted()) continue; if (pcoin->IsCoinBase() && pcoin->GetBlocksToMaturity() > 0) @@ -3749,7 +3843,7 @@ void CWallet::GetKeyBirthTimes(std::map& mapKeyBirth) const for (std::map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); it++) { // iterate over all wallet transactions... const CWalletTx& wtx = (*it).second; - BlockMap::const_iterator blit = mapBlockIndex.find(wtx.hashBlock); + BlockMap::const_iterator blit = mapBlockIndex.find(wtx.m_confirm.hashBlock); if (blit != mapBlockIndex.end() && chainActive.Contains(blit->second)) { // ... which are already in a block int nHeight = blit->second->nHeight; @@ -3796,13 +3890,17 @@ bool CWallet::LoadDestData(const CTxDestination& dest, const std::string& key, c void CWallet::AutoCombineDust(CConnman* connman) { - LOCK2(cs_main, cs_wallet); - const CBlockIndex* tip = chainActive.Tip(); - if (tip->nTime < (GetAdjustedTime() - 300) || IsLocked()) { - return; + { + LOCK(cs_wallet); + if (m_last_block_processed.IsNull() || + m_last_block_processed_time < (GetAdjustedTime() - 300) || + IsLocked()) { + return; + } } - std::map > mapCoinsByAddress = AvailableCoinsByAddress(true, nAutoCombineThreshold * COIN); + std::map > mapCoinsByAddress = + AvailableCoinsByAddress(true, nAutoCombineThreshold * COIN, false); //coins are sectioned by address. This combination code only wants to combine inputs that belong to the same address for (std::map >::iterator it = mapCoinsByAddress.begin(); it != mapCoinsByAddress.end(); it++) { @@ -3820,13 +3918,6 @@ void CWallet::AutoCombineDust(CConnman* connman) for (const COutput& out : vCoins) { if (!out.fSpendable) continue; - //no coins should get this far if they dont have proper maturity, this is double checking - if (out.tx->IsCoinStake() && out.tx->GetDepthInMainChain() < Params().GetConsensus().nCoinbaseMaturity + 1) - continue; - - // no p2cs accepted, those coins are "locked" - if (out.tx->tx->vout[out.i].scriptPubKey.IsPayToColdStaking()) - continue; COutPoint outpt(out.tx->GetHash(), out.i); coinControl->Select(outpt); @@ -3875,9 +3966,14 @@ void CWallet::AutoCombineDust(CConnman* connman) // 10% safety margin to avoid "Insufficient funds" errors vecSend[0].nAmount = nTotalRewardsValue - (nTotalRewardsValue / 10); - if (!CreateTransaction(vecSend, wtx, keyChange, nFeeRet, nChangePosInOut, strErr, coinControl, ALL_COINS, true, false, CAmount(0))) { - LogPrintf("AutoCombineDust createtransaction failed, reason: %s\n", strErr); - continue; + { + // For now, CreateTransaction requires cs_main lock. + LOCK2(cs_main, cs_wallet); + if (!CreateTransaction(vecSend, wtx, keyChange, nFeeRet, nChangePosInOut, strErr, coinControl, ALL_COINS, + true, false, CAmount(0))) { + LogPrintf("AutoCombineDust createtransaction failed, reason: %s\n", strErr); + continue; + } } //we don't combine below the threshold unless the fees are 0 to avoid paying fees over fees over fees @@ -4174,24 +4270,29 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) return nullptr; } - walletInstance->SetBestChain(chainActive.GetLocator()); + walletInstance->SetBestChain(WITH_LOCK(cs_main, return chainActive.GetLocator())); } LogPrintf("Wallet completed loading in %15dms\n", GetTimeMillis() - nStart); - CBlockIndex* pindexRescan = chainActive.Tip(); - if (gArgs.GetBoolArg("-rescan", false)) - pindexRescan = chainActive.Genesis(); - else { + LOCK(cs_main); + CBlockIndex* pindexRescan = chainActive.Genesis(); + if (!gArgs.GetBoolArg("-rescan", false)) { CWalletDB walletdb(*walletInstance->dbw); CBlockLocator locator; if (walletdb.ReadBestBlock(locator)) pindexRescan = FindForkInGlobalIndex(chainActive, locator); - else - pindexRescan = chainActive.Genesis(); } - walletInstance->m_last_block_processed = chainActive.Tip(); + { + LOCK(walletInstance->cs_wallet); + const CBlockIndex* tip = chainActive.Tip(); + if (tip) { + walletInstance->m_last_block_processed = tip->GetBlockHash(); + walletInstance->m_last_block_processed_height = tip->nHeight; + walletInstance->m_last_block_processed_time = tip->GetBlockTime(); + } + } RegisterValidationInterface(walletInstance); if (chainActive.Tip() && chainActive.Tip() != pindexRescan) { @@ -4283,58 +4384,22 @@ CWalletKey::CWalletKey(int64_t nExpires) nTimeExpires = nExpires; } -void CWalletTx::SetMerkleBranch(const uint256& blockHash, int posInBlock) -{ - // Update the tx's hashBlock - hashBlock = blockHash; - - // set the position of the transaction in the block - nIndex = posInBlock; -} - int CWalletTx::GetDepthInMainChain() const { - const CBlockIndex* pindexRet = nullptr; - return GetDepthInMainChain(pindexRet); -} + assert(pwallet != nullptr); + AssertLockHeld(pwallet->cs_wallet); + if (isUnconfirmed() || isAbandoned()) return 0; -int CWalletTx::GetDepthInMainChain(const CBlockIndex*& pindexRet) const -{ - if (hashUnset()) - return 0; - AssertLockHeld(cs_main); - int nResult; - - // Find the block it claims to be in - BlockMap::iterator mi = mapBlockIndex.find(hashBlock); - if (mi == mapBlockIndex.end()) { - nResult = 0; - } else { - CBlockIndex* pindex = (*mi).second; - if (!pindex || !chainActive.Contains(pindex)) { - nResult = 0; - } else { - pindexRet = pindex; - nResult = ((nIndex == -1) ? (-1) : 1) * (chainActive.Height() - pindex->nHeight + 1); - } - } - - return nResult; + return (pwallet->GetLastBlockHeight() - m_confirm.block_height + 1) * (isConflicted() ? -1 : 1); } int CWalletTx::GetBlocksToMaturity() const { - LOCK(cs_main); if (!(IsCoinBase() || IsCoinStake())) return 0; return std::max(0, (Params().GetConsensus().nCoinbaseMaturity + 1) - GetDepthInMainChain()); } -bool CWalletTx::IsInMainChain() const -{ - return GetDepthInMainChain() > 0; -} - bool CWalletTx::IsInMainChainImmature() const { if (!IsCoinBase() && !IsCoinStake()) return false; @@ -4604,9 +4669,7 @@ bool CWallet::LoadSaplingPaymentAddress( //////////////////////////////////////////////////////////// CWalletTx::CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) - : tx(std::move(arg)), - hashBlock(uint256()), - nIndex(-1) + : tx(std::move(arg)) { Init(pwalletIn); } @@ -4628,6 +4691,7 @@ void CWalletTx::Init(const CWallet* pwalletIn) fShieldedChangeCached = false; nShieldedChangeCached = 0; nOrderPos = -1; + m_confirm = Confirmation{}; } bool CWalletTx::IsTrusted() const @@ -4639,9 +4703,12 @@ bool CWalletTx::IsTrusted() const bool CWalletTx::IsTrusted(int& nDepth, bool& fConflicted) const { - // Quick answer in most cases - if (!IsFinalTx(tx)) - return false; + { + LOCK(pwallet->cs_wallet); // future: receive block height instead of locking here. + // Quick answer in most cases + if (!IsFinalTx(tx, pwallet->GetLastBlockHeight())) + return false; + } nDepth = GetDepthAndMempool(fConflicted); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c01e445e5ff0..100221e9b3d8 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -312,7 +312,9 @@ class CWalletTx private: const CWallet* pwallet; - /** Constant used in hashBlock to indicate tx has been abandoned */ + /** Constant used in hashBlock to indicate tx has been abandoned, only used at + * serialization/deserialization to avoid ambiguity with conflicted. + */ static const uint256 ABANDON_HASH; public: @@ -346,13 +348,36 @@ class CWalletTx void Init(const CWallet* pwalletIn); CTransactionRef tx; - uint256 hashBlock; - /* An nIndex == -1 means that hashBlock (in nonzero) refers to the earliest - * block in the chain we know this or any in-wallet dependency conflicts - * with. Older clients interpret nIndex == -1 as unconfirmed for backward - * compatibility. + + /* New transactions start as UNCONFIRMED. At BlockConnected, + * they will transition to CONFIRMED. In case of reorg, at BlockDisconnected, + * they roll back to UNCONFIRMED. If we detect a conflicting transaction at + * block connection, we update conflicted tx and its dependencies as CONFLICTED. + * If tx isn't confirmed and outside of mempool, the user may switch it to ABANDONED + * by using the abandontransaction call. This last status may be override by a CONFLICTED + * or CONFIRMED transition. + */ + enum Status { + UNCONFIRMED, + CONFIRMED, + CONFLICTED, + ABANDONED + }; + + /* Confirmation includes tx status and a triplet of {block height/block hash/tx index in block} + * at which tx has been confirmed. All three are set to 0 if tx is unconfirmed or abandoned. + * Meaning of these fields changes with CONFLICTED state where they instead point to block hash + * and block height of the deepest conflicting tx. */ - int nIndex; + struct Confirmation { + Status status; + int block_height; + uint256 hashBlock; + int nIndex; + Confirmation(Status s = UNCONFIRMED, int b = 0, const uint256& h = UINT256_ZERO, int i = 0) : status(s), block_height(b), hashBlock(h), nIndex(i) {} + }; + + Confirmation m_confirm; template void Serialize(Stream& s) const @@ -368,7 +393,9 @@ class CWalletTx std::vector dummy_vector1; //!< Used to be vMerkleBranch std::vector dummy_vector2; //!< Used to be vtxPrev char dummy_char = false; //!< Used to be fSpent - s << tx << hashBlock << dummy_vector1 << nIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_char; + uint256 serializedHash = isAbandoned() ? ABANDON_HASH : m_confirm.hashBlock; + int serializedIndex = isAbandoned() || isConflicted() ? -1 : m_confirm.nIndex; + s << tx << serializedHash << dummy_vector1 << serializedIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_char; if (this->tx->isSaplingVersion()) { s << mapSaplingNoteData; @@ -383,12 +410,29 @@ class CWalletTx std::vector dummy_vector1; //!< Used to be vMerkleBranch std::vector dummy_vector2; //!< Used to be vtxPrev char dummy_char; //! Used to be fSpent - s >> tx >> hashBlock >> dummy_vector1 >> nIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_char; + int serializedIndex; + s >> tx >> m_confirm.hashBlock >> dummy_vector1 >> serializedIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_char; if (this->tx->isSaplingVersion()) { s >> mapSaplingNoteData; } + /* At serialization/deserialization, an nIndex == -1 means that hashBlock refers to + * the earliest block in the chain we know this or any in-wallet ancestor conflicts + * with. If nIndex == -1 and hashBlock is ABANDON_HASH, it means transaction is abandoned. + * In same context, an nIndex >= 0 refers to a confirmed transaction (if hashBlock set) or + * unconfirmed one. Older clients interpret nIndex == -1 as unconfirmed for backward + * compatibility (pre-commit 9ac63d6). + */ + if (serializedIndex == -1 && m_confirm.hashBlock == ABANDON_HASH) { + setAbandoned(); + } else if (serializedIndex == -1) { + setConflicted(); + } else if (!m_confirm.hashBlock.IsNull()) { + m_confirm.nIndex = serializedIndex; + setConfirmed(); + } + ReadOrderPos(nOrderPos, mapValue); nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; @@ -470,22 +514,36 @@ class CWalletTx void RelayWalletTransaction(CConnman* connman); std::set GetConflicts() const; - void SetMerkleBranch(const uint256& blockHash, int posInBlock); - /** * Return depth of transaction in blockchain: * <0 : conflicts with a transaction this deep in the blockchain * 0 : in memory pool, waiting to be included in a block * >=1 : this many blocks deep in the main chain */ - int GetDepthInMainChain(const CBlockIndex*& pindexRet) const; - int GetDepthInMainChain() const; - bool IsInMainChain() const; + // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct + // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation + // "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to + // resolve the issue of member access into incomplete type CWallet. Note + // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" + // in place. + int GetDepthInMainChain() const NO_THREAD_SAFETY_ANALYSIS; bool IsInMainChainImmature() const; int GetBlocksToMaturity() const; - bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } - bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } - void setAbandoned() { hashBlock = ABANDON_HASH; } + + bool isAbandoned() const { return m_confirm.status == CWalletTx::ABANDONED; } + void setAbandoned() + { + m_confirm.status = CWalletTx::ABANDONED; + m_confirm.hashBlock = UINT256_ZERO; + m_confirm.block_height = 0; + m_confirm.nIndex = 0; + } + bool isConflicted() const { return m_confirm.status == CWalletTx::CONFLICTED; } + void setConflicted() { m_confirm.status = CWalletTx::CONFLICTED; } + bool isUnconfirmed() const { return m_confirm.status == CWalletTx::UNCONFIRMED; } + void setUnconfirmed() { m_confirm.status = CWalletTx::UNCONFIRMED; } + bool isConfirmed() const { return m_confirm.status == CWalletTx::CONFIRMED; } + void setConfirmed() { m_confirm.status = CWalletTx::CONFIRMED; } const uint256& GetHash() const { return tx->GetHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } @@ -529,7 +587,15 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * * Protected by cs_main (see BlockUntilSyncedToCurrentChain) */ - const CBlockIndex* m_last_block_processed{nullptr}; + uint256 m_last_block_processed GUARDED_BY(cs_wallet) = UINT256_ZERO; + + /* Height of last block processed is used by wallet to know depth of transactions + * without relying on Chain interface beyond asynchronous updates. For safety, we + * initialize it to -1. Height is a pointer on node's tip and doesn't imply + * that the wallet has scanned sequentially all blocks up to this one. + */ + int m_last_block_processed_height GUARDED_BY(cs_wallet) = -1; + int64_t m_last_block_processed_time GUARDED_BY(cs_wallet) = 0; int64_t nNextResend; int64_t nLastResend; @@ -545,14 +611,14 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void AddToSpends(const uint256& wtxid); /* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ - void MarkConflicted(const uint256& hashBlock, const uint256& hashTx); + void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx); template void SyncMetaData(std::pair::iterator, typename TxSpendMap::iterator> range); void ChainTipAdded(const CBlockIndex *pindex, const CBlock *pblock, SaplingMerkleTree saplingTree); /* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected */ - void SyncTransaction(const CTransactionRef& tx, const CBlockIndex *pindexBlockConnected, int posInBlock); + void SyncTransaction(const CTransactionRef& tx, const CWalletTx::Confirmation& confirm); bool IsKeyUsed(const CPubKey& vchPubKey); @@ -587,6 +653,22 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface //! Whether the wallet supports Sapling or not // bool IsSaplingUpgradeEnabled() const; + /** Get last block processed height */ + int GetLastBlockHeight() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) + { + AssertLockHeld(cs_wallet); + assert(m_last_block_processed_height >= 0); + return m_last_block_processed_height; + }; + /** Set last block processed height, currently only use in unit test */ + void SetLastBlockProcessed(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) + { + AssertLockHeld(cs_wallet); + m_last_block_processed_height = pindex->nHeight; + m_last_block_processed = pindex->GetBlockHash(); + m_last_block_processed_time = pindex->GetBlockTime(); + }; + /* SPKM Helpers */ const CKeyingMaterial& GetEncryptionKey() const; bool HasEncryptionKeys() const; @@ -697,7 +779,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool _fOnlySpendable, std::set* _onlyFilteredDest, int _minDepth, - bool _fIncludeLocked = false) : + bool _fIncludeLocked = false, + CAmount _nMaxOutValue = 0) : fIncludeDelegated(_fIncludeDelegated), fIncludeColdStaking(_fIncludeColdStaking), nCoinType(_nCoinType), @@ -705,7 +788,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface fOnlySpendable(_fOnlySpendable), onlyFilteredDest(_onlyFilteredDest), minDepth(_minDepth), - fIncludeLocked(_fIncludeLocked) {} + fIncludeLocked(_fIncludeLocked), + nMaxOutValue(_nMaxOutValue) {} bool fIncludeDelegated{true}; bool fIncludeColdStaking{false}; @@ -715,6 +799,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface std::set* onlyFilteredDest{nullptr}; int minDepth{0}; bool fIncludeLocked{false}; + // Select outputs with value <= nMaxOutValue + CAmount nMaxOutValue{0}; }; //! >> Available coins (generic) @@ -730,7 +816,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface //! >> Available coins (P2CS) void GetAvailableP2CSCoins(std::vector& vCoins) const; - std::map > AvailableCoinsByAddress(bool fConfirmed = true, CAmount maxCoinValue = 0); + std::map > AvailableCoinsByAddress(bool fConfirmed, CAmount maxCoinValue, bool fIncludeColdStaking); /// Get 10000 PIV output and keys which can be used for the Masternode bool GetMasternodeVinAndKeys(CTxIn& txinRet, CPubKey& pubKeyRet, @@ -850,11 +936,11 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose = true); - bool LoadToWallet(const CWalletTx& wtxIn); + bool LoadToWallet(CWalletTx& wtxIn); void TransactionAddedToMempool(const CTransactionRef& tx) override; void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) override; - void BlockDisconnected(const std::shared_ptr& pblock, int nBlockHeight) override; - bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const uint256& blockHash, int posInBlock, bool fUpdate); + void BlockDisconnected(const std::shared_ptr& pblock, const uint256& blockHash, int nBlockHeight, int64_t blockTime) override; + bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const CWalletTx::Confirmation& confirm, bool fUpdate); void EraseFromWallet(const uint256& hash); /** @@ -868,6 +954,16 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void ReacceptWalletTransactions(bool fFirstLoad = false); void ResendWalletTransactions(CConnman* connman) override; + struct Balance { + CAmount m_mine_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more + CAmount m_mine_untrusted_pending{0}; //!< Untrusted, but in mempool (pending) + CAmount m_mine_immature{0}; //!< Immature coinbases/coinstakes in the main chain + CAmount m_mine_trusted_shield{0}; //!< Trusted shield, at depth=GetBalance.min_depth or more + CAmount m_mine_untrusted_shielded_balance{0}; //!< Untrusted shield, but in mempool (pending) + CAmount m_mine_cs_delegated_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more. Part of m_mine_trusted as well + }; + Balance GetBalance(int min_depth = 0) const; + CAmount loopTxsBalance(std::functionmethod) const; CAmount GetAvailableBalance(bool fIncludeDelegated = true, bool fIncludeShielded = true) const; CAmount GetAvailableBalance(isminefilter& filter, bool useCache = false, int minDepth = 1) const; diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 19ad4a0eadd7..30d522c3d92e 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -172,7 +172,7 @@ void CZMQNotificationInterface::BlockConnected(const std::shared_ptr& pblock, int nBlockHeight) +void CZMQNotificationInterface::BlockDisconnected(const std::shared_ptr& pblock, const uint256& blockHash, int nBlockHeight, int64_t blockTime) { for (const CTransactionRef& ptx : pblock->vtx) { // Do a normal notify for each transaction removed in block disconnection diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index b4df973d9781..5bb17cd6c504 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -27,7 +27,7 @@ class CZMQNotificationInterface : public CValidationInterface // CValidationInterface void TransactionAddedToMempool(const CTransactionRef& tx) override; void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) override; - void BlockDisconnected(const std::shared_ptr& pblock, int nBlockHeight) override; + void BlockDisconnected(const std::shared_ptr& pblock, const uint256& blockHash, int nBlockHeight, int64_t blockTime) override; void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; private: diff --git a/test/functional/sapling_wallet_listreceived.py b/test/functional/sapling_wallet_listreceived.py index 5fbfd4c6963b..e05e803509c5 100755 --- a/test/functional/sapling_wallet_listreceived.py +++ b/test/functional/sapling_wallet_listreceived.py @@ -95,7 +95,7 @@ def run_test(self): assert_false(r[0]['change'], "Note should not be change") assert_equal(my_memo_hex, r[0]['memo']) assert_equal(0, r[0]['confirmations']) - assert_equal(-1, r[0]['blockindex']) + assert_equal(0, r[0]['blockindex']) assert_equal(0, r[0]['blockheight']) c = self.nodes[1].getsaplingnotescount(0) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index ff54527838f1..af87ac550cf6 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -58,6 +58,7 @@ # Longest test should go first, to favor running tests in parallel 'wallet_basic.py', # ~ 498 sec 'wallet_backup.py', # ~ 477 sec + 'wallet_reorgsrestore.py', # ~ 391 sec 'mempool_persist.py', # ~ 417 sec # vv Tests less than 5m vv diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index f9da3764f554..81b4474a21be 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -7,6 +7,7 @@ from test_framework.test_framework import PivxTestFramework from test_framework.util import ( assert_equal, + assert_raises_rpc_error, connect_nodes, Decimal, disconnect_nodes, @@ -32,6 +33,11 @@ def run_test(self): sync_mempools(self.nodes) self.nodes[1].generate(1) + # Can not abandon non-wallet transaction + assert_raises_rpc_error(-5, 'Invalid or non-wallet transaction id', lambda: self.nodes[0].abandontransaction('ff' * 32)) + # Can not abandon confirmed transaction + assert_raises_rpc_error(-5, 'Transaction not eligible for abandonment', lambda: self.nodes[0].abandontransaction(txA)) + sync_blocks(self.nodes) newbalance = self.nodes[0].getbalance() assert(balance - newbalance < Decimal("0.001")) #no more than fees lost diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py new file mode 100755 index 000000000000..6d254c784bb2 --- /dev/null +++ b/test/functional/wallet_reorgsrestore.py @@ -0,0 +1,107 @@ +#!/usr/bin/env python3 +# Copyright (c) 2019 The Bitcoin Core developers +# Copyright (c) 2021 The PIVX Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://www.opensource.org/licenses/mit-license.php. + +"""Test tx status in case of reorgs while wallet being shutdown. + +Wallet txn status rely on block connection/disconnection for its +accuracy. In case of reorgs happening while wallet being shutdown +block updates are not going to be received. At wallet loading, we +check against chain if confirmed txn are still in chain and change +their status if block in which they have been included has been +disconnected. +""" + +from decimal import Decimal +import os +import shutil + +from test_framework.test_framework import PivxTestFramework +from test_framework.util import ( + assert_equal, + connect_nodes, + disconnect_nodes, + sync_blocks, +) + +class ReorgsRestoreTest(PivxTestFramework): + def set_test_params(self): + self.num_nodes = 3 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + # Send a tx from which to conflict outputs later + txid_conflict_from = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) + self.nodes[0].generate(1) + sync_blocks(self.nodes) + + # Disconnect node1 from others to reorg its chain later + disconnect_nodes(self.nodes[0], 1) + disconnect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) + + # Send a tx to be unconfirmed later + txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) + tx = self.nodes[0].gettransaction(txid) + self.nodes[0].generate(4) + tx_before_reorg = self.nodes[0].gettransaction(txid) + assert_equal(tx_before_reorg["confirmations"], 4) + + # Disconnect node0 from node2 to broadcast a conflict on their respective chains + disconnect_nodes(self.nodes[0], 2) + nA = next(tx_out["vout"] for tx_out in self.nodes[0].gettransaction(txid_conflict_from)["details"] if tx_out["amount"] == Decimal("10")) + inputs = [] + inputs.append({"txid": txid_conflict_from, "vout": nA}) + outputs_1 = {} + outputs_2 = {} + + # Create a conflicted tx broadcast on node0 chain and conflicting tx broadcast on node2 chain. Both spend from txid_conflict_from + outputs_1[self.nodes[0].getnewaddress()] = Decimal("9.99998") + outputs_2[self.nodes[0].getnewaddress()] = Decimal("9.99998") + conflicted = self.nodes[0].signrawtransaction(self.nodes[0].createrawtransaction(inputs, outputs_1)) + conflicting = self.nodes[0].signrawtransaction(self.nodes[0].createrawtransaction(inputs, outputs_2)) + + conflicted_txid = self.nodes[0].sendrawtransaction(conflicted["hex"]) + self.nodes[0].generate(1) + conflicting_txid = self.nodes[2].sendrawtransaction(conflicting["hex"]) + self.nodes[2].generate(9) + + # Reconnect node0 and node2 and check that conflicted_txid is effectively conflicted + connect_nodes(self.nodes[0], 2) + sync_blocks([self.nodes[0], self.nodes[2]]) + conflicted = self.nodes[0].gettransaction(conflicted_txid) + conflicting = self.nodes[0].gettransaction(conflicting_txid) + assert_equal(conflicted["confirmations"], -9) + assert_equal(conflicted["walletconflicts"][0], conflicting["txid"]) + + # Node0 wallet is shutdown + self.stop_node(0) + self.start_node(0) + + # The block chain re-orgs and the tx is included in a different block + self.nodes[1].generate(9) + self.nodes[1].sendrawtransaction(tx["hex"]) + self.nodes[1].generate(1) + self.nodes[1].sendrawtransaction(conflicted["hex"]) + self.nodes[1].generate(1) + + # Node0 wallet file is loaded on longest sync'ed node1 + self.stop_node(1) + self.nodes[0].backupwallet(os.path.join(self.nodes[0].datadir, 'wallet.bak')) + shutil.copyfile(os.path.join(self.nodes[0].datadir, 'wallet.bak'), os.path.join(self.nodes[1].datadir, 'regtest', 'wallet.dat')) + self.start_node(1) + tx_after_reorg = self.nodes[1].gettransaction(txid) + # Check that normal confirmed tx is confirmed again but with different blockhash + assert_equal(tx_after_reorg["confirmations"], 2) + assert(tx_before_reorg["blockhash"] != tx_after_reorg["blockhash"]) + conflicted_after_reorg = self.nodes[1].gettransaction(conflicted_txid) + # Check that conflicted tx is confirmed again with blockhash different than previously conflicting tx + assert_equal(conflicted_after_reorg["confirmations"], 1) + assert(conflicting["blockhash"] != conflicted_after_reorg["blockhash"]) + +if __name__ == '__main__': + ReorgsRestoreTest().main()