Merge pull request #746 from UdjinM6/refactorixdstx

Refactor IX/DSTX messages handling

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())){
+        // have we seen it already?
-            return;
+        if(mapTxLockReq.count(inv.hash) || mapTxLockReqRejected.count(inv.hash)) return;
-        }
+        // is it a valid one?
-
+        if(!IsIXTXValid(tx)) return;
-        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).
@@ -110,18 +107,12 @@ void ProcessMessageInstantX(CNode* pfrom, std::string& strCommand, CDataStream&
             }
 
             // resolve conflicts
-            std::map<uint256, CTransactionLock>::iterator i = mapTxLocks.find(tx.GetHash());
+            if (IsLockedIXTransaction(tx.GetHash()) && !CheckForConflictingLocks(tx)){
-            if (i != mapTxLocks.end()){
+                LogPrintf("ProcessMessageInstantX::ix - Found Existing Complete IX Lock\n");
-                //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");
 
-                        //reprocess the last 15 blocks
+                //reprocess the last 15 blocks
-                        ReprocessBlocks(15);
+                ReprocessBlocks(15);
-                        mapTxLockReq.insert(make_pair(tx.GetHash(), tx));
+                mapTxLockReq.insert(make_pair(tx.GetHash(), tx));
-                    }
-                }
             }
 
             return;
@@ -173,7 +164,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 +355,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){
+        if(nSignatures >= INSTANTX_SIGNATURES_REQUIRED){
-            LogPrint("instantx", "InstantX::ProcessConsensusVote - Transaction Lock Is Complete %s !\n", (*i).second.GetHash().ToString());
+            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 +384,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);
                 }
@@ -466,6 +463,37 @@ void CleanTransactionLocksList()
     }
 }
 
+bool IsLockedIXTransaction(uint256 txHash) {
+    std::map<uint256, CTransactionLock>::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<uint256, CTransactionLock>::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<uint256, CTransactionLock>::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));

src/instantx.h

@@ -37,7 +37,6 @@ static const int MIN_INSTANTX_PROTO_VERSION = 70103;
 extern map<uint256, CTransaction> mapTxLockReq;
 extern map<uint256, CTransaction> mapTxLockReqRejected;
 extern map<uint256, CConsensusVote> mapTxLockVote;
-extern map<uint256, CTransactionLock> mapTxLocks;
 extern std::map<COutPoint, uint256> 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

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){
+    if (nResult < 6 && IsLockedIXTransaction(nTXHash))
-        std::map<uint256, CTransactionLock>::iterator i = mapTxLocks.find(nTXHash);
+        return nInstantXDepth + nResult;
-        if (i != mapTxLocks.end()){
-            sigs = (*i).second.CountSignatures();
-        }
-        if(sigs >= INSTANTX_SIGNATURES_REQUIRED){
-            return nInstantXDepth+nResult;
-        }
-    }
 
-    return -1;
+    return nResult;
 }
 
 int GetIXConfirmations(uint256 nTXHash)
-{    
+{
-    int sigs = 0;
+    if (IsLockedIXTransaction(nTXHash))
-
-    std::map<uint256, CTransactionLock>::iterator i = mapTxLocks.find(nTXHash);
-    if (i != mapTxLocks.end()){
-        sigs = (*i).second.CountSignatures();
-    }
-    if(sigs >= INSTANTX_SIGNATURES_REQUIRED){
         return nInstantXDepth;
-    }
 
     return 0;
 }
@@ -4394,17 +4379,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 +4392,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)) {
+        return mnpayments.mapMasternodePayeeVotes.count(inv.hash);
-            //masternodeSync.AddedMasternodeWinner(inv.hash);
+
-            return true;
-        }
-        return false;
     case MSG_BUDGET_VOTE:
-        if(budget.mapSeenMasternodeBudgetVotes.count(inv.hash)) {
+        return budget.mapSeenMasternodeBudgetVotes.count(inv.hash);
-            //masternodeSync.AddedBudgetItem(inv.hash);
+
-            return true;
-        }
-        return false;
     case MSG_BUDGET_PROPOSAL:
-        if(budget.mapSeenMasternodeBudgetProposals.count(inv.hash)) {
+        return 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;
     case MSG_BUDGET_FINALIZED:
-        if(budget.mapSeenFinalizedBudgets.count(inv.hash)) {
+        return budget.mapSeenFinalizedBudgets.count(inv.hash);
-            //masternodeSync.AddedBudgetItem(inv.hash);
+
-            return true;
+    case MSG_BUDGET_FINALIZED_VOTE:
-        }
+        return budget.mapSeenFinalizedBudgetVotes.count(inv.hash);
-        return false;
+
     case MSG_MASTERNODE_ANNOUNCE:
-        if(mnodeman.mapSeenMasternodeBroadcast.count(inv.hash)) {
+        return mnodeman.mapSeenMasternodeBroadcast.count(inv.hash);
-            //masternodeSync.AddedMasternodeList(inv.hash);
+
-            return true;
-        }
-        return false;
     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 +5290,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))
         {

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)

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) */

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");

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<CRecipient>& 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
+                if (nResult == 0 && !mempool.exists(GetHash()))
-    BlockMap::iterator mi = mapBlockIndex.find(hashBlock);
+                    return -1; // Not in chain, not in mempool
-    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(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<uint256, CTransactionLock>::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<uint256, CTransactionLock>::iterator i = mapTxLocks.find(GetHash());
-    if (i != mapTxLocks.end()){
-        return GetTime() > (*i).second.nTimeout;
-    }
-
-    return false;
-}

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); }