Skip to content

Commit

Permalink
Merge #2209: Killing wallet and GUI cs_main locks
Browse files Browse the repository at this point in the history
ca3edc5 GUI: Update worker type if task already exist. (furszy)
a1390c3 wallet balances, cache total delegated balance and calculate it only once. (furszy)
02eb781 GUI: settings information, do not update block num and block hash if the screen is not visible. (furszy)
cbc5021 Masternode-sync read only function to get the "IsBlockchainSynced" state. (furszy)
ef77fdf GUI: topbar, removing not used block source. (furszy)
e9c160a wallet: single loop to calculate the currently required balances. (furszy)
32538ae wallet: Disallow abandon of conflicted txes (furszy)
62cd35f GUI: MN model, remove now unneeded cs_main locks. (furszy)
5658844 wallet: removing unnecessary `mempool.cs` lock from ReacceptWalletTransactions() (furszy)
63f3fa7 GUI: add missing qt metatype declarations. (furszy)
bc76186 wallet model: update delay increased to 5 seconds. (furszy)
3fb3f34 GUI dashboard: do not update the staking filter if it's not needed. (furszy)
7cb8276 init: move "Done loading" message and rpc warm up finished after the wallet post initialization process (furszy)
8762e81 Refactoring with QString::toNSString (Hennadii Stepanov)
69b3330 GUI: txmodel, do not lock cs_wallet if no wtx status update is needed. (furszy)
b4ab286 GUI: dashboard, charts update delay time increased for IBD. (furszy)
0f197ca Wallet interface: do not load not used cold staking balances. (furszy)
dee4224 Wallet:GetLockedCoins() loop only over the locked coins, not over the entire wallet txes map. (furszy)
0a61f7f GUI: cold staking, do not refresh delegations if the screen is not visible. (furszy)
c2d66d0 GUI: cold staking screen, do not initialize coin control dialog at startup. (furszy)
44d9cbe GUI: dashboard screen, remove stakes filter source model if chart is not being presented. (furszy)
6e49a11 GUI: send screen, initialize coinControlDialog view only when it's being called. (furszy)
379e5f2 GUI: send screen, move refresh amounts calculation to a background worker thread. (furszy)
76bc08b GUI: balance polling update moved to a background worker thread. (furszy)
b73500a wallet:BlockConnected do not lock cs_wallet for its entire process. (furszy)
208a292 wallet: remove last cs_main locks from every signal handler function :) . (furszy)
9720579 SSPKM: remove redundant ReadBlockFromDisk from IncrementNoteWitnesses. (furszy)
4ccec74 wallet: Add IsSpent() cs_wallet lock assertion. (furszy)
e0a0d2d sapling_wallet_tests: locking order refactor, solving inconsistent orders for cs_main and cs_wallet. (furszy)
c69e7e8 Adapt sapling_wallet_listreceived.py to the new wtx confirmation structure. (furszy)
c4952a2 Wallet::CreateWalletFromFile guard direct chainActive access (furszy)
f889dcb test : updating wallet's last block processed manually. (furszy)
45c9471 wallet: split CheckTXAvailability in two. (furszy)
5109c8d Wallet: remove cs_main from IsSaplingSpent() and GetFilteredNotes() (furszy)
a7f6ab1 Wallet: remove cs_main requirement from RelayWalletTransaction and FundTransaction. (furszy)
1e7ffc2 Wallet: remove cs_main requirement from CreateCoinStake (furszy)
2e7cdb2 Wallet: added max value out filter for AvailableCoins. (furszy)
b9220f4 Wallet: Do not add P2CS utxo to autocombine flow and discard them later. (furszy)
59ed47d Wallet: remove cs_main lock from AutoCombineDust plus a redundant maturity check. (furszy)
fcc4c83 Move AutoCombineDust functionality to the wallet background thread (furszy)
fcb20c2 GUI: remove cs_main lock dependency from CWalletModel::isSpent, CWalletModel::isLockedCoin, CWalletModel::lockCoin, CWalletModel::unlockCoin (furszy)
65fbad1 GUI: remove cs_main lock dependency from transaction model update. (furszy)
0b61857 GUI: fix coinstake tx ordering. (furszy)
33588fe GUI: Remove cs_main lock and chain dependency from transaction update status (furszy)
3dede64 GUI: Remove cs_main lock from balance polling timer (furszy)
5e06330 Removed IsFinalTx() cs_main lock requirement. (furszy)
1bd97ca wallet::MarkConflicted remove blockIndex and there by cs_main lock dependency. (furszy)
239d6a2 wallet: remove the now unneeded cs_main locks (furszy)
1386ab7 Use CWallet::m_last_block_processed_height in GetDepthInMainChain (furszy)
9adeb61 Only return early from BlockUntilSyncedToCurrentChain if current tip is exact match (furszy)
8aa2d31 Add block_height field in struct Confirmation (furszy)
4405ac0 Replace CWalletTx::SetConf by Confirmation initialization list (furszy)
6320efb Update wallet last processed index in every unit test that needs it (furszy)
4957051 wallet: cache block hash, height and time inside the wallet. (furszy)
33f4788 wallet: refactor GetDepthInMainChain to not return the block index. (furszy)
e7c8ca6 wallet: remove unused IsInMainChain method (furszy)
9e51a48 Add a test wallet_reorgsrestore (Antoine Riard)
370c200 wallet: simplifying pindexRescan set + added an AssertLockHeld on FindForkInGlobalIndex (furszy)
46f0e30 Fixing reindex problem, use mapBlockIndex.find() and not mapBlockIndex[] (furszy)
da78039 [Wallet] Adapting TransactionAddedToMempool and BlockDisconnected to the new wtx confirmation status (furszy)
ff04fa6 Modify wallet tx status if has been reorged out (furszy)
f7baeaf Remove SyncTransaction for conflicted txn in CWallet::BlockConnected (Antoine Riard)
8d8928e Encapsulate tx status in a Confirmation struct (Antoine Riard)

Pull request description:

  This is the conclusion of a deep deep rabbit hole: #1726, #2150, #2082, #2118, #2150, #2179, #2191, #2192, #2195, #2201, #2203..

  Effectively removing every `cs_main` lock from the wallet and GUI processing threads. Completely decoupling the wallet state and the visual interface update procedures from the main message and validation handler thread.

  Meaning that the messages & validation handler thread will be largely more active, accepting and verifying way more data in less time, not being affected by several other threads accessing to the main critical section. And, at the same time, the wallet and GUI worker threads will be able to perform and process their tasks concurrently, without waiting for the `cs_main` mutex acquisition.
  Improving the overall software performance, the GUI responsiveness and decreasing the synchronization time.

  To make this possible, the wallet is maintaining in memory a view of the chain and updating it only via the validation interface signals. Using the view to perform all of the chain related calculations.

ACKs for top commit:
  Fuzzbawls:
    All good now, ACK ca3edc5
  random-zebra:
    ACK ca3edc5 and merging...

Tree-SHA512: 6d4268730941822942b0df0aab200683a1fabaf6801618cf34955b43b29bc2beb694567635f731a724abf7b73cec3edfe506e0428de6710c0fcf603613f5614a
  • Loading branch information
random-zebra committed Mar 12, 2021
2 parents 6438931 + ca3edc5 commit b6d5146
Show file tree
Hide file tree
Showing 47 changed files with 991 additions and 574 deletions.
3 changes: 0 additions & 3 deletions src/consensus/tx_verify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion src/consensus/tx_verify.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 4 additions & 3 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -1985,5 +1983,8 @@ bool AppInitMain()
}
#endif

SetRPCWarmupFinished();
uiInterface.InitMessage(_("Done loading"));

return true;
}
17 changes: 10 additions & 7 deletions src/interfaces/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions src/interfaces/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
5 changes: 5 additions & 0 deletions src/masternode-sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ bool CMasternodeSync::IsBlockchainSynced()
return true;
}

bool CMasternodeSync::IsBlockchainSyncedReadOnly() const
{
return fBlockchainSynced;
}

void CMasternodeSync::Reset()
{
fBlockchainSynced = false;
Expand Down
2 changes: 2 additions & 0 deletions src/masternode-sync.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
20 changes: 20 additions & 0 deletions src/qt/clientmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion src/qt/clientmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -114,7 +118,7 @@ class ClientModel : public QObject
QString cachedMasternodeCountString;
bool cachedReindexing;
bool cachedImporting;
bool cachedInitialSync;
std::atomic<bool> cachedInitialSync{false};

int numBlocksAtStartup;

Expand Down
1 change: 1 addition & 0 deletions src/qt/coincontroldialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down
23 changes: 4 additions & 19 deletions src/qt/macnotificationhandler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
}
Expand Down
13 changes: 10 additions & 3 deletions src/qt/pivx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#ifdef ENABLE_WALLET
#include "paymentserver.h"
#include "walletmodel.h"
#include "interfaces/wallet.h"
#endif
#include "masternodeconfig.h"

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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>("CAmount");
qRegisterMetaType<CAmount>("interfaces::WalletBalances");
qRegisterMetaType<size_t>("size_t");

/// 3. Application identification
// must be set before OptionsModel is initialized or translations are loaded,
Expand Down
14 changes: 9 additions & 5 deletions src/qt/pivx/coldstakingwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand All @@ -225,8 +224,6 @@ void ColdStakingWidget::loadWalletModel()
ui->containerHistoryLabel->setVisible(false);
ui->emptyContainer->setVisible(false);
ui->listView->setVisible(false);

tryRefreshDelegations();
}

}
Expand All @@ -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) {
Expand Down Expand Up @@ -533,6 +536,7 @@ void ColdStakingWidget::onCoinControlClicked()
{
if (isInDelegation) {
if (walletModel->getBalance() > 0) {
if (!coinControlDialog->hasModel()) coinControlDialog->setModel(walletModel);
coinControlDialog->refreshDialog();
setCoinControlPayAmounts();
coinControlDialog->exec();
Expand Down
2 changes: 2 additions & 0 deletions src/qt/pivx/coldstakingwidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading

0 comments on commit b6d5146

Please sign in to comment.