Skip to content

Backport critical fixes from develop #1828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
UdjinM6 merged 9 commits into from
Jan 10, 2018
Merged

Backport critical fixes from develop #1828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ada41c3
Fix crash on exit when -createwalletbackups=0 (#1810)
codablock Dec 27, 2017
d6e2aa8
Swap iterations and fUseInstantSend parameters in ApproximateBestSubs…
codablock Dec 28, 2017
068b20b
Merge #8256: BUG: bitcoin-qt crash
laanwj Jun 24, 2016
c9bafe1
Vote on IS only if it was accepted to mempool (#1826)
UdjinM6 Jan 6, 2018
fa5fc41
Fix -liquidityprovider option (#1829)
UdjinM6 Jan 6, 2018
f71ab1d
Merge pull request #1836 from lodgepole/feature/backport-11847
UdjinM6 Jan 6, 2018
d261575
Skip existing masternode conections on mixing (#1833)
UdjinM6 Jan 9, 2018
21a1005
Protect CKeyHolderStorage via mutex (#1834)
UdjinM6 Jan 9, 2018
4768886
Avoid reference leakage in CKeyHolderStorage::AddKey (#1840)
UdjinM6 Jan 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions src/instantx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,11 @@ bool CInstantSend::ProcessTxLockRequest(const CTxLockRequest& txLockRequest, CCo
}
LogPrintf("CInstantSend::ProcessTxLockRequest -- accepted, txid=%s\n", txHash.ToString());

std::map<uint256, CTxLockCandidate>::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<uint256, CTxLockCandidate>::iterator itLockCandidate = mapTxLockCandidates.find(txHash);
TryToFinalizeLockCandidate(itLockCandidate->second);

return true;
}
Expand Down Expand Up @@ -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<uint256, CTxLockCandidate>::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;
Expand All @@ -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<COutPoint, COutPointLock>::iterator itOutpointLock = txLockCandidate.mapOutPointLocks.begin();
Expand Down
1 change: 1 addition & 0 deletions src/instantx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class CInstantSend
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);

Expand Down
1 change: 1 addition & 0 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
70 changes: 33 additions & 37 deletions src/privatesend-client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
Expand Down Expand Up @@ -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;
Expand All @@ -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());
Expand All @@ -1002,13 +987,24 @@ bool CPrivateSendClient::SubmitDenominate(CConnman& connman)
std::vector<CTxOut> 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
Expand Down Expand Up @@ -1100,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);
Expand Down Expand Up @@ -1276,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();
}
Expand Down Expand Up @@ -1311,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 });

Expand Down
7 changes: 5 additions & 2 deletions src/privatesend-util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@ CScript CKeyHolder::GetScriptForDestination() const
}


const CKeyHolder& CKeyHolderStorage::AddKey(CWallet* pwallet)
CScript CKeyHolderStorage::AddKey(CWallet* pwallet)
{
LOCK(cs_storage);
storage.emplace_back(std::unique_ptr<CKeyHolder>(new CKeyHolder(pwallet)));
LogPrintf("CKeyHolderStorage::%s -- storage size %lld\n", __func__, storage.size());
return *storage.back();
return storage.back()->GetScriptForDestination();
}

void CKeyHolderStorage::KeepAll(){
LOCK(cs_storage);
if (storage.size() > 0) {
for (auto &key : storage) {
key->KeepKey();
Expand All @@ -44,6 +46,7 @@ void CKeyHolderStorage::KeepAll(){

void CKeyHolderStorage::ReturnAll()
{
LOCK(cs_storage);
if (storage.size() > 0) {
for (auto &key : storage) {
key->ReturnKey();
Expand Down
3 changes: 2 additions & 1 deletion src/privatesend-util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ class CKeyHolderStorage
{
private:
std::vector<std::unique_ptr<CKeyHolder> > storage;
mutable CCriticalSection cs_storage;

public:
const CKeyHolder& AddKey(CWallet* pwalletIn);
CScript AddKey(CWallet* pwalletIn);
void KeepAll();
void ReturnAll();

Expand Down
5 changes: 3 additions & 2 deletions src/qt/overviewpage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link

@willwray willwray Jan 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timer should be initialised first in the list.
gcc 7.1 gives this warning:

In file included from ../../src/qt/overviewpage.cpp:6:0:
../../src/qt/overviewpage.h: In constructor ‘OverviewPage::OverviewPage(const PlatformStyle*, QWidget*)’:
../../src/qt/overviewpage.h:64:21: warning: ‘OverviewPage::txdelegate’ will be initialized after [-Wreorder]
     TxViewDelegate *txdelegate;
                     ^~~~~~~~~~
../../src/qt/overviewpage.h:50:13: warning:   ‘QTimer* OverviewPage::timer’ [-Wreorder]
     QTimer *timer;
             ^~~~~
../../src/qt/overviewpage.cpp:124:1: warning:   when initialized here [-Wreorder]
 OverviewPage::OverviewPage(const PlatformStyle *platformStyle, QWidget *parent) :
 ^~~~~~~~~~~~

Copy link
Author

@UdjinM6 UdjinM6 Jan 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, noticed this as well. However, a) this is not critical imo and b) should be fixed in develop, not here.

{
ui->setupUi(this);
QString theme = GUIUtil::getThemeName();
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions src/qt/transactionview.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions src/txmempool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -226,7 +226,7 @@ class CompareTxMemPoolEntryByDescendantScore
}

// 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();
Expand All @@ -241,7 +241,7 @@ class CompareTxMemPoolEntryByDescendantScore
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();
Expand All @@ -255,7 +255,7 @@ class CompareTxMemPoolEntryByScore
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();
}
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2410,7 +2410,7 @@ void CWallet::AvailableCoins(vector<COutput>& vCoins, bool fOnlyConfirmed, const
}

static void ApproximateBestSubset(vector<pair<CAmount, pair<const CWalletTx*,unsigned int> > >vValue, const CAmount& nTotalLower, const CAmount& nTargetValue,
vector<char>& vfBest, CAmount& nBest, int iterations = 1000, bool fUseInstantSend = false)
vector<char>& vfBest, CAmount& nBest, bool fUseInstantSend = false, int iterations = 1000)
{
vector<char> vfIncluded;

Expand Down