From ada41c3af1727d47f6d57f6a0389c7d1165eda5e Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 27 Dec 2017 18:41:04 +0100 Subject: [PATCH 1/9] Fix crash on exit when -createwalletbackups=0 (#1810) --- src/qt/overviewpage.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 7e2ef30be..e554fdf7b 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -132,7 +132,8 @@ OverviewPage::OverviewPage(const PlatformStyle *platformStyle, QWidget *parent) currentWatchOnlyBalance(-1), currentWatchUnconfBalance(-1), currentWatchImmatureBalance(-1), - txdelegate(new TxViewDelegate(platformStyle, this)) + txdelegate(new TxViewDelegate(platformStyle, this)), + timer(nullptr) { ui->setupUi(this); QString theme = GUIUtil::getThemeName(); @@ -195,7 +196,7 @@ void OverviewPage::handleOutOfSyncWarningClicks() OverviewPage::~OverviewPage() { - if(!fLiteMode && !fMasterNode) disconnect(timer, SIGNAL(timeout()), this, SLOT(privateSendStatus())); + if(timer) disconnect(timer, SIGNAL(timeout()), this, SLOT(privateSendStatus())); delete ui; } From d6e2aa843c595c04025e35453ab65db9315e5155 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 28 Dec 2017 01:49:05 +0100 Subject: [PATCH 2/9] Swap iterations and fUseInstantSend parameters in ApproximateBestSubset (#1819) #1695 introduced a fix for a instant send related edge case. Somehow the parameters got mixed up and fUseInstantSend was passed as "iterations". --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index eadd825b5..a01dbe6bb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2410,7 +2410,7 @@ void CWallet::AvailableCoins(vector& vCoins, bool fOnlyConfirmed, const } static void ApproximateBestSubset(vector > >vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, - vector& vfBest, CAmount& nBest, int iterations = 1000, bool fUseInstantSend = false) + vector& vfBest, CAmount& nBest, bool fUseInstantSend = false, int iterations = 1000) { vector vfIncluded; From 068b20bc70c1cd0cb5b3a49d6c257c7e64689f23 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 24 Jun 2016 13:47:54 +0200 Subject: [PATCH 3/9] Merge #8256: BUG: bitcoin-qt crash d7828ab check that transactionView->selectionModel()->selectedRows(0) exists (fsb4000) --- src/qt/transactionview.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 311cece90..56de76d97 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -404,6 +404,8 @@ void TransactionView::contextualMenu(const QPoint &point) { QModelIndex index = transactionView->indexAt(point); QModelIndexList selection = transactionView->selectionModel()->selectedRows(0); + if (selection.empty()) + return; // check if transaction can be abandoned, disable context menu action in case it doesn't uint256 hash; From c9bafe154293c161cb3507600b3e69033eb5034d Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 6 Jan 2018 13:06:43 +0300 Subject: [PATCH 4/9] Vote on IS only if it was accepted to mempool (#1826) --- src/instantx.cpp | 22 ++++++++++++++++------ src/instantx.h | 1 + src/net_processing.cpp | 1 + 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/instantx.cpp b/src/instantx.cpp index 7cd95298d..f0a65fa7f 100644 --- a/src/instantx.cpp +++ b/src/instantx.cpp @@ -122,15 +122,11 @@ bool CInstantSend::ProcessTxLockRequest(const CTxLockRequest& txLockRequest, CCo } LogPrintf("CInstantSend::ProcessTxLockRequest -- accepted, txid=%s\n", txHash.ToString()); - std::map::iterator itLockCandidate = mapTxLockCandidates.find(txHash); - CTxLockCandidate& txLockCandidate = itLockCandidate->second; - Vote(txLockCandidate, connman); - ProcessOrphanTxLockVotes(connman); - // Masternodes will sometimes propagate votes before the transaction is known to the client. // If this just happened - lock inputs, resolve conflicting locks, update transaction status // forcing external script notification. - TryToFinalizeLockCandidate(txLockCandidate); + std::map::iterator itLockCandidate = mapTxLockCandidates.find(txHash); + TryToFinalizeLockCandidate(itLockCandidate->second); return true; } @@ -182,6 +178,18 @@ void CInstantSend::CreateEmptyTxLockCandidate(const uint256& txHash) mapTxLockCandidates.insert(std::make_pair(txHash, CTxLockCandidate(txLockRequest))); } +void CInstantSend::Vote(const uint256& txHash, CConnman& connman) +{ + AssertLockHeld(cs_main); + LOCK(cs_instantsend); + + std::map::iterator itLockCandidate = mapTxLockCandidates.find(txHash); + if (itLockCandidate == mapTxLockCandidates.end()) return; + Vote(itLockCandidate->second, connman); + // Let's see if our vote changed smth + TryToFinalizeLockCandidate(itLockCandidate->second); +} + void CInstantSend::Vote(CTxLockCandidate& txLockCandidate, CConnman& connman) { if(!fMasterNode) return; @@ -190,6 +198,8 @@ void CInstantSend::Vote(CTxLockCandidate& txLockCandidate, CConnman& connman) LOCK2(cs_main, cs_instantsend); uint256 txHash = txLockCandidate.GetHash(); + // We should never vote on a Transaction Lock Request that was not (yet) accepted by the mempool + if(mapLockRequestAccepted.find(txHash) == mapLockRequestAccepted.end()) return; // check if we need to vote on this candidate's outpoints, // it's possible that we need to vote for several of them std::map::iterator itOutpointLock = txLockCandidate.mapOutPointLocks.begin(); diff --git a/src/instantx.h b/src/instantx.h index 73815c99e..9f071e3b8 100644 --- a/src/instantx.h +++ b/src/instantx.h @@ -86,6 +86,7 @@ public: void ProcessMessage(CNode* pfrom, std::string& strCommand, CDataStream& vRecv, CConnman& connman); bool ProcessTxLockRequest(const CTxLockRequest& txLockRequest, CConnman& connman); + void Vote(const uint256& txHash, CConnman& connman); bool AlreadyHave(const uint256& hash); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c32256cc5..137a0462a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1635,6 +1635,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, LogPrintf("TXLOCKREQUEST -- Transaction Lock Request accepted, txid=%s, peer=%d\n", tx.GetHash().ToString(), pfrom->id); instantsend.AcceptLockRequest(txLockRequest); + instantsend.Vote(tx.GetHash(), connman); } mempool.check(pcoinsTip); From fa5fc418a0c5e180b308b5dd1b91eb608d84e3d5 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 6 Jan 2018 13:06:22 +0300 Subject: [PATCH 5/9] Fix -liquidityprovider option (#1829) revert to pre-1248 logic for liquidity providers --- src/privatesend-client.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index 58f1534d0..f044a310a 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -1002,13 +1002,24 @@ bool CPrivateSendClient::SubmitDenominate(CConnman& connman) std::vector vecTxOutRet; // Submit transaction to the pool if we get here - // Try to use only inputs with the same number of rounds starting from the highest number of rounds possible - for(int i = nPrivateSendRounds; i > 0; i--) { - if(PrepareDenominate(i - 1, i, strError, vecTxDSInRet, vecTxOutRet)) { - LogPrintf("CPrivateSendClient::SubmitDenominate -- Running PrivateSend denominate for %d rounds, success\n", i); - return SendDenominate(vecTxDSInRet, vecTxOutRet, connman); + if (nLiquidityProvider) { + // Try to use only inputs with the same number of rounds starting from the lowest number of rounds possible + for(int i = 0; i< nPrivateSendRounds; i++) { + if(PrepareDenominate(i, i + 1, strError, vecTxDSInRet, vecTxOutRet)) { + LogPrintf("CPrivateSendClient::SubmitDenominate -- Running PrivateSend denominate for %d rounds, success\n", i); + return SendDenominate(vecTxDSInRet, vecTxOutRet, connman); + } + LogPrint("privatesend", "CPrivateSendClient::SubmitDenominate -- Running PrivateSend denominate for %d rounds, error: %s\n", i, strError); + } + } else { + // Try to use only inputs with the same number of rounds starting from the highest number of rounds possible + for(int i = nPrivateSendRounds; i > 0; i--) { + if(PrepareDenominate(i - 1, i, strError, vecTxDSInRet, vecTxOutRet)) { + LogPrintf("CPrivateSendClient::SubmitDenominate -- Running PrivateSend denominate for %d rounds, success\n", i); + return SendDenominate(vecTxDSInRet, vecTxOutRet, connman); + } + LogPrint("privatesend", "CPrivateSendClient::SubmitDenominate -- Running PrivateSend denominate for %d rounds, error: %s\n", i, strError); } - LogPrint("privatesend", "CPrivateSendClient::SubmitDenominate -- Running PrivateSend denominate for %d rounds, error: %s\n", i, strError); } // We failed? That's strange but let's just make final attempt and try to mix everything From f71ab1daf63022d5b367bbf039d8b3dbe5a297f1 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 7 Jan 2018 02:37:53 +0300 Subject: [PATCH 6/9] Merge pull request #1836 from lodgepole/feature/backport-11847 [backport] Fixes compatibility with boost 1.66 --- src/txmempool.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/txmempool.h b/src/txmempool.h index 3e5c22060..ef74e7f76 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -204,7 +204,7 @@ struct mempoolentry_txid class CompareTxMemPoolEntryByDescendantScore { public: - bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const { bool fUseADescendants = UseDescendantScore(a); bool fUseBDescendants = UseDescendantScore(b); @@ -226,7 +226,7 @@ public: } // Calculate which score to use for an entry (avoiding division). - bool UseDescendantScore(const CTxMemPoolEntry &a) + bool UseDescendantScore(const CTxMemPoolEntry &a) const { double f1 = (double)a.GetModifiedFee() * a.GetSizeWithDescendants(); double f2 = (double)a.GetModFeesWithDescendants() * a.GetTxSize(); @@ -241,7 +241,7 @@ public: class CompareTxMemPoolEntryByScore { public: - bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const { double f1 = (double)a.GetModifiedFee() * b.GetTxSize(); double f2 = (double)b.GetModifiedFee() * a.GetTxSize(); @@ -255,7 +255,7 @@ public: class CompareTxMemPoolEntryByEntryTime { public: - bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const { return a.GetTime() < b.GetTime(); } From d261575b4f69a9e723f623ad0afcaaba2f1ad2c3 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 9 Jan 2018 12:17:54 +0300 Subject: [PATCH 7/9] Skip existing masternode conections on mixing (#1833) --- src/privatesend-client.cpp | 41 ++++++++++++-------------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index f044a310a..a629c5316 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -873,23 +873,19 @@ bool CPrivateSendClient::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CCon vecMasternodesUsed.push_back(dsq.vin.prevout); - CNode* pnodeFound = NULL; - bool fDisconnect = false; - connman.ForNode(infoMn.addr, CConnman::AllNodes, [&pnodeFound, &fDisconnect](CNode* pnode) { - pnodeFound = pnode; - if(pnodeFound->fDisconnect) { - fDisconnect = true; - } else { - pnodeFound->AddRef(); - } + bool fSkip = false; + connman.ForNode(infoMn.addr, CConnman::AllNodes, [&fSkip](CNode* pnode) { + fSkip = pnode->fDisconnect || pnode->fMasternode; return true; }); - if (fDisconnect) + if (fSkip) { + LogPrintf("CPrivateSendClient::JoinExistingQueue -- skipping masternode connection, addr=%s\n", infoMn.addr.ToString()); continue; + } LogPrintf("CPrivateSendClient::JoinExistingQueue -- attempt to connect to masternode from queue, addr=%s\n", infoMn.addr.ToString()); // connect to Masternode and submit the queue request - CNode* pnode = (pnodeFound && pnodeFound->fMasternode) ? pnodeFound : connman.ConnectNode(CAddress(infoMn.addr, NODE_NETWORK), NULL, true); + CNode* pnode = connman.ConnectNode(CAddress(infoMn.addr, NODE_NETWORK), NULL, true); if(pnode) { infoMixingMasternode = infoMn; nSessionDenom = dsq.nDenom; @@ -900,9 +896,6 @@ bool CPrivateSendClient::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CCon strAutoDenomResult = _("Mixing in progress..."); SetState(POOL_STATE_QUEUE); nTimeLastSuccessfulStep = GetTimeMillis(); - if(pnodeFound) { - pnodeFound->Release(); - } return true; } else { LogPrintf("CPrivateSendClient::JoinExistingQueue -- can't connect, addr=%s\n", infoMn.addr.ToString()); @@ -947,24 +940,19 @@ bool CPrivateSendClient::StartNewQueue(CAmount nValueMin, CAmount nBalanceNeedsA continue; } - CNode* pnodeFound = NULL; - bool fDisconnect = false; - connman.ForNode(infoMn.addr, CConnman::AllNodes, [&pnodeFound, &fDisconnect](CNode* pnode) { - pnodeFound = pnode; - if(pnodeFound->fDisconnect) { - fDisconnect = true; - } else { - pnodeFound->AddRef(); - } + bool fSkip = false; + connman.ForNode(infoMn.addr, CConnman::AllNodes, [&fSkip](CNode* pnode) { + fSkip = pnode->fDisconnect || pnode->fMasternode; return true; }); - if (fDisconnect) { + if (fSkip) { + LogPrintf("CPrivateSendClient::StartNewQueue -- skipping masternode connection, addr=%s\n", infoMn.addr.ToString()); nTries++; continue; } LogPrintf("CPrivateSendClient::StartNewQueue -- attempt %d connection to Masternode %s\n", nTries, infoMn.addr.ToString()); - CNode* pnode = (pnodeFound && pnodeFound->fMasternode) ? pnodeFound : connman.ConnectNode(CAddress(infoMn.addr, NODE_NETWORK), NULL, true); + CNode* pnode = connman.ConnectNode(CAddress(infoMn.addr, NODE_NETWORK), NULL, true); if(pnode) { LogPrintf("CPrivateSendClient::StartNewQueue -- connected, addr=%s\n", infoMn.addr.ToString()); infoMixingMasternode = infoMn; @@ -982,9 +970,6 @@ bool CPrivateSendClient::StartNewQueue(CAmount nValueMin, CAmount nBalanceNeedsA strAutoDenomResult = _("Mixing in progress..."); SetState(POOL_STATE_QUEUE); nTimeLastSuccessfulStep = GetTimeMillis(); - if(pnodeFound) { - pnodeFound->Release(); - } return true; } else { LogPrintf("CPrivateSendClient::StartNewQueue -- can't connect, addr=%s\n", infoMn.addr.ToString()); From 21a10057df0987873c2d3f50949ac996e4235331 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 9 Jan 2018 12:18:10 +0300 Subject: [PATCH 8/9] Protect CKeyHolderStorage via mutex (#1834) --- src/privatesend-util.cpp | 3 +++ src/privatesend-util.h | 1 + 2 files changed, 4 insertions(+) diff --git a/src/privatesend-util.cpp b/src/privatesend-util.cpp index a1ef639fc..e350ad3c8 100644 --- a/src/privatesend-util.cpp +++ b/src/privatesend-util.cpp @@ -27,12 +27,14 @@ CScript CKeyHolder::GetScriptForDestination() const const CKeyHolder& CKeyHolderStorage::AddKey(CWallet* pwallet) { + LOCK(cs_storage); storage.emplace_back(std::unique_ptr(new CKeyHolder(pwallet))); LogPrintf("CKeyHolderStorage::%s -- storage size %lld\n", __func__, storage.size()); return *storage.back(); } void CKeyHolderStorage::KeepAll(){ + LOCK(cs_storage); if (storage.size() > 0) { for (auto &key : storage) { key->KeepKey(); @@ -44,6 +46,7 @@ void CKeyHolderStorage::KeepAll(){ void CKeyHolderStorage::ReturnAll() { + LOCK(cs_storage); if (storage.size() > 0) { for (auto &key : storage) { key->ReturnKey(); diff --git a/src/privatesend-util.h b/src/privatesend-util.h index f6bfede8a..a6c4535fb 100644 --- a/src/privatesend-util.h +++ b/src/privatesend-util.h @@ -27,6 +27,7 @@ class CKeyHolderStorage { private: std::vector > storage; + mutable CCriticalSection cs_storage; public: const CKeyHolder& AddKey(CWallet* pwalletIn); From 476888683ca652a9a54c2df1212dab177392e883 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 9 Jan 2018 14:11:43 +0300 Subject: [PATCH 9/9] Avoid reference leakage in CKeyHolderStorage::AddKey (#1840) --- src/privatesend-client.cpp | 6 +++--- src/privatesend-util.cpp | 4 ++-- src/privatesend-util.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index a629c5316..dd63669e0 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -1096,7 +1096,7 @@ bool CPrivateSendClient::PrepareDenominate(int nMinRounds, int nMaxRounds, std:: vecTxDSIn.erase(it); vCoins.erase(it2); - CScript scriptDenom = keyHolderStorage.AddKey(pwalletMain).GetScriptForDestination(); + CScript scriptDenom = keyHolderStorage.AddKey(pwalletMain); // add new output CTxOut txout(nValueDenom, scriptDenom); @@ -1272,7 +1272,7 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo // ****** Add an output for mixing collaterals ************ / if(fCreateMixingCollaterals) { - CScript scriptCollateral = keyHolderStorageDenom.AddKey(pwalletMain).GetScriptForDestination(); + CScript scriptCollateral = keyHolderStorageDenom.AddKey(pwalletMain); vecSend.push_back((CRecipient){ scriptCollateral, CPrivateSend::GetMaxCollateralAmount(), false }); nValueLeft -= CPrivateSend::GetMaxCollateralAmount(); } @@ -1307,7 +1307,7 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo // add each output up to 11 times until it can't be added again while(nValueLeft - nDenomValue >= 0 && nOutputs <= 10) { - CScript scriptDenom = keyHolderStorageDenom.AddKey(pwalletMain).GetScriptForDestination(); + CScript scriptDenom = keyHolderStorageDenom.AddKey(pwalletMain); vecSend.push_back((CRecipient){ scriptDenom, nDenomValue, false }); diff --git a/src/privatesend-util.cpp b/src/privatesend-util.cpp index e350ad3c8..ebec0474c 100644 --- a/src/privatesend-util.cpp +++ b/src/privatesend-util.cpp @@ -25,12 +25,12 @@ CScript CKeyHolder::GetScriptForDestination() const } -const CKeyHolder& CKeyHolderStorage::AddKey(CWallet* pwallet) +CScript CKeyHolderStorage::AddKey(CWallet* pwallet) { LOCK(cs_storage); storage.emplace_back(std::unique_ptr(new CKeyHolder(pwallet))); LogPrintf("CKeyHolderStorage::%s -- storage size %lld\n", __func__, storage.size()); - return *storage.back(); + return storage.back()->GetScriptForDestination(); } void CKeyHolderStorage::KeepAll(){ diff --git a/src/privatesend-util.h b/src/privatesend-util.h index a6c4535fb..9040eb011 100644 --- a/src/privatesend-util.h +++ b/src/privatesend-util.h @@ -30,7 +30,7 @@ private: mutable CCriticalSection cs_storage; public: - const CKeyHolder& AddKey(CWallet* pwalletIn); + CScript AddKey(CWallet* pwalletIn); void KeepAll(); void ReturnAll();