From f679e2431669d8fce270b2627b1908e1201f13e5 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 21 Mar 2016 23:23:45 +0300 Subject: [PATCH 1/2] Refactor IX/DSTX messages handling: - Fix CreateTransaction and GetDepthInMainChain for IX compatibility - Prepare IsIXTXValid for the next update (see FIXME in CreateTransaction) - Unify processing of TX/IX/DSTX a bit more - Clean up some code --- src/instantx.cpp | 29 ++++++++++-------- src/main.cpp | 71 ++++++++++++++++--------------------------- src/net.cpp | 20 +++--------- src/net.h | 1 - src/wallet/wallet.cpp | 50 +++++++++++++++--------------- 5 files changed, 71 insertions(+), 100 deletions(-) diff --git a/src/instantx.cpp b/src/instantx.cpp index 351d0fe413..331a2a4bda 100644 --- a/src/instantx.cpp +++ b/src/instantx.cpp @@ -51,13 +51,10 @@ void ProcessMessageInstantX(CNode* pfrom, std::string& strCommand, CDataStream& CInv inv(MSG_TXLOCK_REQUEST, tx.GetHash()); pfrom->AddInventoryKnown(inv); - if(mapTxLockReq.count(tx.GetHash()) || mapTxLockReqRejected.count(tx.GetHash())){ - return; - } - - if(!IsIXTXValid(tx)){ - return; - } + // have we seen it already? + if(mapTxLockReq.count(inv.hash) || mapTxLockReqRejected.count(inv.hash)) return; + // is it a valid one? + if(!IsIXTXValid(tx)) return; BOOST_FOREACH(const CTxOut o, tx.vout){ // IX supports normal scripts and unspendable scripts (used in DS collateral and Budget collateral). @@ -173,7 +170,11 @@ void ProcessMessageInstantX(CNode* pfrom, std::string& strCommand, CDataStream& bool IsIXTXValid(const CTransaction& txCollateral){ if(txCollateral.vout.size() < 1) return false; - if(txCollateral.nLockTime != 0) return false; + + if(!CheckFinalTx(txCollateral)) { + LogPrint("instantx", "IsIXTXValid - Transaction is not final - %s\n", txCollateral.ToString()); + return false; + } int64_t nValueIn = 0; int64_t nValueOut = 0; @@ -360,17 +361,19 @@ bool ProcessConsensusVote(CNode* pnode, CConsensusVote& ctx) } #endif - LogPrint("instantx", "InstantX::ProcessConsensusVote - Transaction Lock Votes %d - %s !\n", (*i).second.CountSignatures(), ctx.GetHash().ToString()); + int nSignatures = (*i).second.CountSignatures(); + LogPrint("instantx", "InstantX::ProcessConsensusVote - Transaction Lock Votes %d - %s !\n", nSignatures, ctx.GetHash().ToString()); - if((*i).second.CountSignatures() >= INSTANTX_SIGNATURES_REQUIRED){ - LogPrint("instantx", "InstantX::ProcessConsensusVote - Transaction Lock Is Complete %s !\n", (*i).second.GetHash().ToString()); + if(nSignatures >= INSTANTX_SIGNATURES_REQUIRED){ + LogPrint("instantx", "InstantX::ProcessConsensusVote - Transaction Lock Is Complete %s !\n", ctx.txHash.ToString()); CTransaction& tx = mapTxLockReq[ctx.txHash]; if(!CheckForConflictingLocks(tx)){ #ifdef ENABLE_WALLET if(pwalletMain){ - if(pwalletMain->UpdatedTransaction((*i).second.txHash)){ + if(pwalletMain->UpdatedTransaction(ctx.txHash)){ + // bumping this to update UI nCompleteTXLocks++; } } @@ -387,7 +390,7 @@ bool ProcessConsensusVote(CNode* pnode, CConsensusVote& ctx) // resolve conflicts //if this tx lock was rejected, we need to remove the conflicting blocks - if(mapTxLockReqRejected.count((*i).second.txHash)){ + if(mapTxLockReqRejected.count(ctx.txHash)){ //reprocess the last 15 blocks ReprocessBlocks(15); } diff --git a/src/main.cpp b/src/main.cpp index 91dfeedffd..a6f391a5cc 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4394,17 +4394,9 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) mapOrphanTransactions.count(inv.hash) || pcoinsTip->HaveCoins(inv.hash); } - case MSG_DSTX: - return mapDarksendBroadcastTxes.count(inv.hash); + case MSG_BLOCK: return mapBlockIndex.count(inv.hash); - case MSG_TXLOCK_REQUEST: - return mapTxLockReq.count(inv.hash) || - mapTxLockReqRejected.count(inv.hash); - case MSG_TXLOCK_VOTE: - return mapTxLockVote.count(inv.hash); - case MSG_SPORK: - return mapSporks.count(inv.hash); /* Dash Related Inventory Messages @@ -4415,44 +4407,38 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) We're going to be asking many nodes upfront for the full inventory list, so we'll get duplicates of these. We want to only update the time on new hits, so that we can time out appropriately if needed. */ + case MSG_TXLOCK_REQUEST: + return mapTxLockReq.count(inv.hash) || mapTxLockReqRejected.count(inv.hash); + + case MSG_TXLOCK_VOTE: + return mapTxLockVote.count(inv.hash); + + case MSG_SPORK: + return mapSporks.count(inv.hash); + case MSG_MASTERNODE_WINNER: - if(mnpayments.mapMasternodePayeeVotes.count(inv.hash)) { - //masternodeSync.AddedMasternodeWinner(inv.hash); - return true; - } - return false; + return mnpayments.mapMasternodePayeeVotes.count(inv.hash); + case MSG_BUDGET_VOTE: - if(budget.mapSeenMasternodeBudgetVotes.count(inv.hash)) { - //masternodeSync.AddedBudgetItem(inv.hash); - return true; - } - return false; + return budget.mapSeenMasternodeBudgetVotes.count(inv.hash); + case MSG_BUDGET_PROPOSAL: - if(budget.mapSeenMasternodeBudgetProposals.count(inv.hash)) { - //masternodeSync.AddedBudgetItem(inv.hash); - return true; - } - return false; - case MSG_BUDGET_FINALIZED_VOTE: - if(budget.mapSeenFinalizedBudgetVotes.count(inv.hash)) { - //masternodeSync.AddedBudgetItem(inv.hash); - return true; - } - return false; + return budget.mapSeenMasternodeBudgetProposals.count(inv.hash); + case MSG_BUDGET_FINALIZED: - if(budget.mapSeenFinalizedBudgets.count(inv.hash)) { - //masternodeSync.AddedBudgetItem(inv.hash); - return true; - } - return false; + return budget.mapSeenFinalizedBudgets.count(inv.hash); + + case MSG_BUDGET_FINALIZED_VOTE: + return budget.mapSeenFinalizedBudgetVotes.count(inv.hash); + case MSG_MASTERNODE_ANNOUNCE: - if(mnodeman.mapSeenMasternodeBroadcast.count(inv.hash)) { - //masternodeSync.AddedMasternodeList(inv.hash); - return true; - } - return false; + return mnodeman.mapSeenMasternodeBroadcast.count(inv.hash); + case MSG_MASTERNODE_PING: return mnodeman.mapSeenMasternodePing.count(inv.hash); + + case MSG_DSTX: + return mapDarksendBroadcastTxes.count(inv.hash); } // Don't know what it is, just say we already got one return true; @@ -5319,11 +5305,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } } - if(strCommand == NetMsgType::DSTX){ - CInv inv(MSG_DSTX, tx.GetHash()); - RelayInv(inv); - } - int nDoS = 0; if (state.IsInvalid(nDoS)) { diff --git a/src/net.cpp b/src/net.cpp index 6928d83931..ad96982923 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -20,6 +20,7 @@ #include "scheduler.h" #include "ui_interface.h" #include "darksend.h" +#include "instantx.h" #include "wallet/wallet.h" #include "utilstrencodings.h" @@ -2075,7 +2076,9 @@ void RelayTransaction(const CTransaction& tx) void RelayTransaction(const CTransaction& tx, const CDataStream& ss) { - CInv inv(MSG_TX, tx.GetHash()); + int nInv = mapDarksendBroadcastTxes.count(tx.GetHash()) ? MSG_DSTX : + (mapTxLockReq.count(tx.GetHash()) ? MSG_TXLOCK_REQUEST : MSG_TX); + CInv inv(nInv, tx.GetHash()); { LOCK(cs_mapRelay); // Expire old relay messages @@ -2104,21 +2107,6 @@ void RelayTransaction(const CTransaction& tx, const CDataStream& ss) } } -void RelayTransactionLockReq(const CTransaction& tx, bool relayToAll) -{ - CInv inv(MSG_TXLOCK_REQUEST, tx.GetHash()); - - //broadcast the new lock - LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodes) - { - if(!relayToAll && !pnode->fRelayTxes) - continue; - - pnode->PushMessage(NetMsgType::IX, tx); - } -} - void RelayInv(CInv &inv, const int minProtoVersion) { LOCK(cs_vNodes); BOOST_FOREACH(CNode* pnode, vNodes) diff --git a/src/net.h b/src/net.h index 959c9f013d..d46ee67695 100644 --- a/src/net.h +++ b/src/net.h @@ -850,7 +850,6 @@ public: class CTransaction; void RelayTransaction(const CTransaction& tx); void RelayTransaction(const CTransaction& tx, const CDataStream& ss); -void RelayTransactionLockReq(const CTransaction& tx, bool relayToAll=false); void RelayInv(CInv &inv, const int minProtoVersion = MIN_PEER_PROTO_VERSION); /** Access to the (IP) address database (peers.dat) */ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6c5e3639c6..b1c01540c5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1485,10 +1485,8 @@ bool CWalletTx::RelayWalletTransaction(std::string strCommand) if(strCommand == NetMsgType::IX){ mapTxLockReq.insert(make_pair(hash, (CTransaction)*this)); CreateNewLock(((CTransaction)*this)); - RelayTransactionLockReq((CTransaction)*this, true); - } else { - RelayTransaction((CTransaction)*this); } + RelayTransaction((CTransaction)*this); return true; } } @@ -2845,7 +2843,9 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt // enough, that fee sniping isn't a problem yet, but by implementing a fix // now we ensure code won't be written that makes assumptions about // nLockTime that preclude a fix later. - txNew.nLockTime = chainActive.Height(); + + // FIXME: "compatibility mode" for 12.0 IX, make it "txNew.nLockTime = chainActive.Height();" again in 12.2 + txNew.nLockTime = useIX ? 0 : chainActive.Height(); // Secondly occasionally randomly pick a nLockTime even further back, so // that transactions that are delayed after signing for whatever reason, @@ -4055,34 +4055,34 @@ int CMerkleTx::SetMerkleBranch(const CBlock& block) int CMerkleTx::GetDepthInMainChain(const CBlockIndex* &pindexRet, bool enableIX) const { + int nResult; + if (hashUnset()) - return 0; + nResult = 0; + else { + AssertLockHeld(cs_main); - AssertLockHeld(cs_main); + // 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); - // Find the block it claims to be in - BlockMap::iterator mi = mapBlockIndex.find(hashBlock); - if (mi == mapBlockIndex.end()) - return 0; - CBlockIndex* pindex = (*mi).second; - if (!pindex || !chainActive.Contains(pindex)) - return 0; - - pindexRet = pindex; - int nResult = ((nIndex == -1) ? (-1) : 1) * (chainActive.Height() - pindex->nHeight + 1); - - if (nResult == 0 && !mempool.exists(GetHash())) - return -1; // Not in chain, not in mempool - - if(enableIX){ - if (nResult < 6){ - int signatures = GetTransactionLockSignatures(); - if(signatures >= INSTANTX_SIGNATURES_REQUIRED){ - return nInstantXDepth+nResult; + if (nResult == 0 && !mempool.exists(GetHash())) + return -1; // Not in chain, not in mempool } } } + if(enableIX && nResult < 6 && GetTransactionLockSignatures() >= INSTANTX_SIGNATURES_REQUIRED) + return nInstantXDepth + nResult; + return nResult; } From 5e7496ee5ee870b7c30ac5ed1624a847f3236cb9 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 23 Mar 2016 17:49:35 +0300 Subject: [PATCH 2/2] More refactoring for IX: - move ix internal logic out of CMerkleTx - fix bug in GetInputAgeIX --- src/instantx.cpp | 47 +++++++++++++++++++++++++++++--------- src/instantx.h | 10 +++++++- src/main.cpp | 29 ++++++----------------- src/qt/transactiondesc.cpp | 4 ++-- src/wallet/wallet.cpp | 29 +---------------------- src/wallet/wallet.h | 2 -- 6 files changed, 55 insertions(+), 66 deletions(-) diff --git a/src/instantx.cpp b/src/instantx.cpp index 331a2a4bda..8ddbbcde96 100644 --- a/src/instantx.cpp +++ b/src/instantx.cpp @@ -107,18 +107,12 @@ void ProcessMessageInstantX(CNode* pfrom, std::string& strCommand, CDataStream& } // resolve conflicts - std::map::iterator i = mapTxLocks.find(tx.GetHash()); - if (i != mapTxLocks.end()){ - //we only care if we have a complete tx lock - if((*i).second.CountSignatures() >= INSTANTX_SIGNATURES_REQUIRED){ - if(!CheckForConflictingLocks(tx)){ - LogPrintf("ProcessMessageInstantX::ix - Found Existing Complete IX Lock\n"); + if (IsLockedIXTransaction(tx.GetHash()) && !CheckForConflictingLocks(tx)){ + LogPrintf("ProcessMessageInstantX::ix - Found Existing Complete IX Lock\n"); - //reprocess the last 15 blocks - ReprocessBlocks(15); - mapTxLockReq.insert(make_pair(tx.GetHash(), tx)); - } - } + //reprocess the last 15 blocks + ReprocessBlocks(15); + mapTxLockReq.insert(make_pair(tx.GetHash(), tx)); } return; @@ -469,6 +463,37 @@ void CleanTransactionLocksList() } } +bool IsLockedIXTransaction(uint256 txHash) { + std::map::iterator i = mapTxLocks.find(txHash); + return i != mapTxLocks.end() && (*i).second.CountSignatures() >= INSTANTX_SIGNATURES_REQUIRED; +} + +int GetTransactionLockSignatures(uint256 txHash) +{ + if(fLargeWorkForkFound || fLargeWorkInvalidChainFound) return -2; + if(!IsSporkActive(SPORK_2_INSTANTX)) return -3; + if(!fEnableInstantX) return -1; + + std::map::iterator i = mapTxLocks.find(txHash); + if (i != mapTxLocks.end()){ + return (*i).second.CountSignatures(); + } + + return -1; +} + +bool IsTransactionLockTimedOut(uint256 txHash) +{ + if(!fEnableInstantX) return 0; + + std::map::iterator i = mapTxLocks.find(txHash); + if (i != mapTxLocks.end()){ + return GetTime() > (*i).second.nTimeout; + } + + return false; +} + uint256 CConsensusVote::GetHash() const { return ArithToUint256(UintToArith256(vinMasternode.prevout.hash) + vinMasternode.prevout.n + UintToArith256(txHash)); diff --git a/src/instantx.h b/src/instantx.h index 9f281e5a47..19adad7eef 100644 --- a/src/instantx.h +++ b/src/instantx.h @@ -37,7 +37,6 @@ static const int MIN_INSTANTX_PROTO_VERSION = 70103; extern map mapTxLockReq; extern map mapTxLockReqRejected; extern map mapTxLockVote; -extern map mapTxLocks; extern std::map mapLockedInputs; extern int nCompleteTXLocks; @@ -60,6 +59,15 @@ bool ProcessConsensusVote(CNode *pnode, CConsensusVote& ctx); // keep transaction locks in memory for an hour void CleanTransactionLocksList(); +// verify if transaction is currently locked +bool IsLockedIXTransaction(uint256 txHash); + +// get the actual uber og accepted lock signatures +int GetTransactionLockSignatures(uint256 txHash); + +// verify if transaction lock timed out +bool IsTransactionLockTimedOut(uint256 txHash); + int64_t GetAverageVoteTime(); class CConsensusVote diff --git a/src/main.cpp b/src/main.cpp index a6f391a5cc..ea2eee0764 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -769,35 +769,20 @@ int GetInputAge(CTxIn& vin) } int GetInputAgeIX(uint256 nTXHash, CTxIn& vin) -{ - int sigs = 0; +{ int nResult = GetInputAge(vin); - if(nResult < 0) nResult = 0; + if(nResult < 0) return -1; - if (nResult < 6){ - std::map::iterator i = mapTxLocks.find(nTXHash); - if (i != mapTxLocks.end()){ - sigs = (*i).second.CountSignatures(); - } - if(sigs >= INSTANTX_SIGNATURES_REQUIRED){ - return nInstantXDepth+nResult; - } - } + if (nResult < 6 && IsLockedIXTransaction(nTXHash)) + return nInstantXDepth + nResult; - return -1; + return nResult; } int GetIXConfirmations(uint256 nTXHash) -{ - int sigs = 0; - - std::map::iterator i = mapTxLocks.find(nTXHash); - if (i != mapTxLocks.end()){ - sigs = (*i).second.CountSignatures(); - } - if(sigs >= INSTANTX_SIGNATURES_REQUIRED){ +{ + if (IsLockedIXTransaction(nTXHash)) return nInstantXDepth; - } return 0; } diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index e9b25a5a30..3c23319695 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -34,7 +34,7 @@ QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx) } else { - int signatures = wtx.GetTransactionLockSignatures(); + int signatures = GetTransactionLockSignatures(wtx.GetHash()); QString strUsingIX = ""; if(signatures >= 0){ @@ -49,7 +49,7 @@ QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx) else return tr("%1 confirmations (verified via instantx)").arg(nDepth); } else { - if(!wtx.IsTransactionLockTimedOut()){ + if(!IsTransactionLockTimedOut(wtx.GetHash())){ int nDepth = wtx.GetDepthInMainChain(); if (nDepth < 0) return tr("conflicted"); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b1c01540c5..a1d3c0be73 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4080,7 +4080,7 @@ int CMerkleTx::GetDepthInMainChain(const CBlockIndex* &pindexRet, bool enableIX) } } - if(enableIX && nResult < 6 && GetTransactionLockSignatures() >= INSTANTX_SIGNATURES_REQUIRED) + if(enableIX && nResult < 6 && IsLockedIXTransaction(GetHash())) return nInstantXDepth + nResult; return nResult; @@ -4099,30 +4099,3 @@ bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectAbsurdFee) CValidationState state; return ::AcceptToMemoryPool(mempool, state, *this, fLimitFree, NULL, false, fRejectAbsurdFee); } - -int CMerkleTx::GetTransactionLockSignatures() const -{ - if(fLargeWorkForkFound || fLargeWorkInvalidChainFound) return -2; - if(!IsSporkActive(SPORK_2_INSTANTX)) return -3; - if(!fEnableInstantX) return -1; - - //compile consessus vote - std::map::iterator i = mapTxLocks.find(GetHash()); - if (i != mapTxLocks.end()){ - return (*i).second.CountSignatures(); - } - - return -1; -} -bool CMerkleTx::IsTransactionLockTimedOut() const -{ - if(!fEnableInstantX) return 0; - - //compile consessus vote - std::map::iterator i = mapTxLocks.find(GetHash()); - if (i != mapTxLocks.end()){ - return GetTime() > (*i).second.nTimeout; - } - - return false; -} diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c1e95294f9..f474ee772d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -228,8 +228,6 @@ public: int GetDepthInMainChain(bool enableIX = true) const { const CBlockIndex *pindexRet; return GetDepthInMainChain(pindexRet, enableIX); } bool IsInMainChain() const { const CBlockIndex *pindexRet; return GetDepthInMainChain(pindexRet) > 0; } int GetBlocksToMaturity() const; - int GetTransactionLockSignatures() const; - bool IsTransactionLockTimedOut() const; bool AcceptToMemoryPool(bool fLimitFree=true, bool fRejectAbsurdFee=true); bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } bool isAbandoned() const { return (hashBlock == ABANDON_HASH); }