More split-outs from UTXO hasher

divi/src/ActiveChainManager.cpp

@@ -92,15 +92,15 @@ bool ActiveChainManager::DisconnectBlock(
     // undo transactions in reverse order
     for (int transactionIndex = block.vtx.size() - 1; transactionIndex >= 0; transactionIndex--) {
         const CTransaction& tx = block.vtx[transactionIndex];
+        const TransactionLocationReference txLocationReference(tx, pindex->nHeight, transactionIndex);
         const auto* undo = (transactionIndex > 0 ? &blockUndo.vtxundo[transactionIndex - 1] : nullptr);
-        const TxReversalStatus status = UpdateCoinsReversingTransaction(tx, view, undo, pindex->nHeight);
+        const TxReversalStatus status = UpdateCoinsReversingTransaction(tx, txLocationReference, view, undo);
         if(!CheckTxReversalStatus(status,fClean))
         {
             return false;
         }
         if(!pfClean)
         {
-            TransactionLocationReference txLocationReference(tx.GetHash(),pindex->nHeight,transactionIndex);
             IndexDatabaseUpdateCollector::ReverseTransaction(tx,txLocationReference,view,indexDBUpdates);
         }
     }

divi/src/BlockMemoryPoolTransactionCollector.cpp

@@ -206,7 +206,7 @@ std::vector<TxPriority> BlockMemoryPoolTransactionCollector::PrioritizeMempoolTr
             // Read prev transaction
             if (!view.HaveCoins(txin.prevout.hash)) {
                 CTransaction prevTx;
-                if(!mempool_.lookup(txin.prevout.hash, prevTx)) {
+                if(!mempool_.lookupOutpoint(txin.prevout.hash, prevTx)) {
                     // This should never happen; all transactions in the memory
                     // pool should connect to either transactions in the chain
                     // or other transactions in the memory pool.

divi/src/BlockTransactionChecker.cpp

@@ -60,7 +60,7 @@ bool BlockTransactionChecker::Check(const CBlockRewards& nExpectedMint,bool fJus
     pindex_->nMint = 0;
     for (unsigned int i = 0; i < block_.vtx.size(); i++) {
         const CTransaction& tx = block_.vtx[i];
-        const TransactionLocationReference txLocationRef(tx.GetHash(),pindex_->nHeight,i);
+        const TransactionLocationReference txLocationRef(tx, pindex_->nHeight, i);
 
         if(!txInputChecker_.TotalSigOpsAreBelowMaximum(tx))
         {

divi/src/IndexDatabaseUpdates.cpp

@@ -1,5 +1,7 @@
 #include <IndexDatabaseUpdates.h>
 
+#include <primitives/transaction.h>
+
 IndexDatabaseUpdates::IndexDatabaseUpdates(
     ): addressIndex()
     , addressUnspentIndex()
@@ -9,11 +11,11 @@ IndexDatabaseUpdates::IndexDatabaseUpdates(
 }
 
 TransactionLocationReference::TransactionLocationReference(
-    uint256 hashValue,
+    const CTransaction& tx,
     unsigned blockheightValue,
     int transactionIndexValue
-    ): hash(hashValue)
+    ): hash(tx.GetHash())
     , blockHeight(blockheightValue)
     , transactionIndex(transactionIndexValue)
 {
-}
+}

divi/src/IndexDatabaseUpdates.h

@@ -6,6 +6,8 @@
 #include <spentindex.h>
 #include <uint256.h>
 
+class CTransaction;
+
 /** One entry in the tx index, which locates transactions on disk by their txid
  *  or bare txid (both keys are possible).  */
 struct TxIndexEntry
@@ -36,7 +38,7 @@ struct TransactionLocationReference
     int transactionIndex;
 
     TransactionLocationReference(
-        uint256 hashValue,
+        const CTransaction& tx,
         unsigned blockheightValue,
         int transactionIndexValue);
 };

divi/src/UtxoCheckingAndUpdating.cpp

@@ -5,6 +5,7 @@
 #include <coins.h>
 #include <chain.h>
 #include <blockmap.h>
+#include <IndexDatabaseUpdates.h>
 #include <ValidationState.h>
 #include <utilmoneystr.h>
 #include <undo.h>
@@ -29,7 +30,7 @@ void UpdateCoinsWithTransaction(const CTransaction& tx, CCoinsViewCache& inputs,
 
 static bool RemoveTxOutputsFromCache(
     const CTransaction& tx,
-    const int blockHeight,
+    const TransactionLocationReference& txLocationReference,
     CCoinsViewCache& view)
 {
     bool outputsAvailable = true;
@@ -38,10 +39,10 @@ static bool RemoveTxOutputsFromCache(
     // have outputs available even in the block itself, so we handle that case
     // specially with outsEmpty.
     CCoins outsEmpty;
-    CCoinsModifier outs = view.ModifyCoins(tx.GetHash());
+    CCoinsModifier outs = view.ModifyCoins(txLocationReference.hash);
     outs->ClearUnspendable();
 
-    CCoins outsBlock(tx, blockHeight);
+    CCoins outsBlock(tx, txLocationReference.blockHeight);
     // The CCoins serialization does not serialize negative numbers.
     // No network rules currently depend on the version here, so an inconsistency is harmless
     // but it must be corrected before txout nversion ever influences a network rule.
@@ -86,10 +87,10 @@ static void UpdateCoinsForRestoredInputs(
     coins->vout[out.n] = undo.txout;
 }
 
-TxReversalStatus UpdateCoinsReversingTransaction(const CTransaction& tx, CCoinsViewCache& inputs, const CTxUndo* txundo, int nHeight)
+TxReversalStatus UpdateCoinsReversingTransaction(const CTransaction& tx, const TransactionLocationReference& txLocationReference, CCoinsViewCache& inputs, const CTxUndo* txundo)
 {
     bool fClean = true;
-    fClean = fClean && RemoveTxOutputsFromCache(tx,nHeight,inputs);
+    fClean = fClean && RemoveTxOutputsFromCache(tx, txLocationReference, inputs);
     if(tx.IsCoinBase()) return fClean? TxReversalStatus::OK : TxReversalStatus::CONTINUE_WITH_ERRORS;
     assert(txundo != nullptr);
     if (txundo->vprevout.size() != tx.vin.size())

divi/src/UtxoCheckingAndUpdating.h

@@ -9,6 +9,8 @@ class CTransaction;
 class CValidationState;
 class CCoinsViewCache;
 class CTxUndo;
+class TransactionLocationReference;
+
 enum class TxReversalStatus
 {
     ABORT_NO_OTHER_ERRORS,
@@ -17,7 +19,7 @@ enum class TxReversalStatus
     OK,
 };
 void UpdateCoinsWithTransaction(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo& txundo, int nHeight);
-TxReversalStatus UpdateCoinsReversingTransaction(const CTransaction& tx, CCoinsViewCache& inputs, const CTxUndo* txundo, int nHeight);
+TxReversalStatus UpdateCoinsReversingTransaction(const CTransaction& tx, const TransactionLocationReference& txLocationReference, CCoinsViewCache& inputs, const CTxUndo* txundo);
 bool CheckInputs(
     const CTransaction& tx,
     CValidationState& state,

divi/src/rpcblockchain.cpp

@@ -218,8 +218,9 @@ Value getrawmempool(const Array& params, bool fHelp)
             info.push_back(Pair("currentpriority", e.GetPriority(chainActive.Height())));
             const CTransaction& tx = e.GetTx();
             set<string> setDepends;
-            BOOST_FOREACH (const CTxIn& txin, tx.vin) {
-                if (mempool.exists(txin.prevout.hash))
+            for (const CTxIn& txin : tx.vin) {
+                CTransaction dummyResult;
+                if (mempool.lookupOutpoint(txin.prevout.hash, dummyResult))
                     setDepends.insert(txin.prevout.hash.ToString());
             }
             Array depends(setDepends.begin(), setDepends.end());

divi/src/test/mempool_tests.cpp

@@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
     removed.clear();
 }
 
-BOOST_AUTO_TEST_CASE(MempoolIndexByBareTxid)
+BOOST_AUTO_TEST_CASE(MempoolDirectLookup)
 {
     CTransaction tx;
     std::list<CTransaction> removed;
@@ -172,14 +172,62 @@ BOOST_AUTO_TEST_CASE(MempoolIndexByBareTxid)
 
     BOOST_CHECK(testPool.lookupBareTxid(txParent.GetBareTxid(), tx));
     BOOST_CHECK(tx.GetHash() == txParent.GetHash());
+    BOOST_CHECK(testPool.lookup(txParent.GetHash(), tx));
+    BOOST_CHECK(tx.GetHash() == txParent.GetHash());
+
+    BOOST_CHECK(!testPool.lookup(txParent.GetBareTxid(), tx));
     BOOST_CHECK(!testPool.lookupBareTxid(txParent.GetHash(), tx));
 
     testPool.remove(txParent, removed, true);
+    BOOST_CHECK(!testPool.lookup(txParent.GetHash(), tx));
+    BOOST_CHECK(!testPool.lookup(txChild[0].GetHash(), tx));
+    BOOST_CHECK(!testPool.lookup(txGrandChild[0].GetHash(), tx));
     BOOST_CHECK(!testPool.lookupBareTxid(txParent.GetBareTxid(), tx));
     BOOST_CHECK(!testPool.lookupBareTxid(txChild[0].GetBareTxid(), tx));
     BOOST_CHECK(!testPool.lookupBareTxid(txGrandChild[0].GetBareTxid(), tx));
 }
 
+BOOST_AUTO_TEST_CASE(MempoolOutpointLookup)
+{
+    CTransaction tx;
+    CCoins c;
+
+    AddAll();
+    CCoinsViewMemPool viewPool(&coins, testPool);
+
+    BOOST_CHECK(testPool.lookupOutpoint(txParent.GetHash(), tx));
+    BOOST_CHECK(!testPool.lookupOutpoint(txParent.GetBareTxid(), tx));
+    BOOST_CHECK(testPool.lookupOutpoint(txChild[0].GetHash(), tx));
+    BOOST_CHECK(!testPool.lookupOutpoint(txChild[0].GetBareTxid(), tx));
+
+    BOOST_CHECK(viewPool.HaveCoins(txParent.GetHash()));
+    BOOST_CHECK(viewPool.GetCoins(txParent.GetHash(), c));
+    BOOST_CHECK(!viewPool.HaveCoins(txParent.GetBareTxid()));
+    BOOST_CHECK(!viewPool.GetCoins(txParent.GetBareTxid(), c));
+
+    BOOST_CHECK(viewPool.HaveCoins(txChild[0].GetHash()));
+    BOOST_CHECK(viewPool.GetCoins(txChild[0].GetHash(), c));
+    BOOST_CHECK(!viewPool.HaveCoins(txChild[0].GetBareTxid()));
+    BOOST_CHECK(!viewPool.GetCoins(txChild[0].GetBareTxid(), c));
+}
+
+BOOST_AUTO_TEST_CASE(MempoolExists)
+{
+    CTransaction tx;
+    std::list<CTransaction> removed;
+
+    AddAll();
+
+    BOOST_CHECK(testPool.exists(txParent.GetHash()));
+    BOOST_CHECK(!testPool.exists(txParent.GetBareTxid()));
+    BOOST_CHECK(!testPool.existsBareTxid(txParent.GetHash()));
+    BOOST_CHECK(testPool.existsBareTxid(txParent.GetBareTxid()));
+
+    testPool.remove(txParent, removed, true);
+    BOOST_CHECK(!testPool.exists(txParent.GetHash()));
+    BOOST_CHECK(!testPool.existsBareTxid(txParent.GetBareTxid()));
+}
+
 BOOST_AUTO_TEST_CASE(MempoolSpentIndex)
 {
     spentIndex = true;

divi/src/txmempool.cpp

@@ -638,7 +638,7 @@ void CTxMemPool::removeCoinbaseSpends(const CCoinsViewCache* pcoins, unsigned in
         const CTransaction& tx = entry.second.GetTx();
         for (const auto& txin : tx.vin) {
             CTransaction tx2;
-            if (lookup(txin.prevout.hash, tx2))
+            if (lookupOutpoint(txin.prevout.hash, tx2))
                 continue;
             const CCoins* coins = pcoins->AccessCoins(txin.prevout.hash);
             if (fSanityCheck) assert(coins);
@@ -723,7 +723,7 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins, const BlockMap& blockIndex
         for (const auto& txin : tx.vin) {
             // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's.
             CTransaction tx2;
-            if (lookup(txin.prevout.hash, tx2)) {
+            if (lookupOutpoint(txin.prevout.hash, tx2)) {
                 assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull());
                 fDependsWait = true;
             } else {
@@ -803,6 +803,13 @@ bool CTxMemPool::lookupBareTxid(const uint256& btxid, CTransaction& result) cons
     return true;
 }
 
+bool CTxMemPool::lookupOutpoint(const uint256& hash, CTransaction& result) const
+{
+    /* For now (until we add the UTXO hasher and segwit light), the outpoint
+       is just the transaction ID.  */
+    return lookup(hash, result);
+}
+
 CFeeRate CTxMemPool::estimateFee(int nBlocks) const
 {
     LOCK(cs);
@@ -882,7 +889,7 @@ bool CCoinsViewMemPool::GetCoins(const uint256& txid, CCoins& coins) const
     // conflict with the underlying cache, and it cannot have pruned entries (as it contains full)
     // transactions. First checking the underlying cache risks returning a pruned entry instead.
     CTransaction tx;
-    if (mempool.lookup(txid, tx)) {
+    if (mempool.lookupOutpoint(txid, tx)) {
         coins = CCoins(tx, MEMPOOL_HEIGHT);
         return true;
     }
@@ -891,5 +898,9 @@ bool CCoinsViewMemPool::GetCoins(const uint256& txid, CCoins& coins) const
 
 bool CCoinsViewMemPool::HaveCoins(const uint256& txid) const
 {
-    return mempool.exists(txid) || base->HaveCoins(txid);
+    CTransaction dummy;
+    if (mempool.lookupOutpoint(txid, dummy))
+        return true;
+
+    return base->HaveCoins(txid);
 }

divi/src/txmempool.h

@@ -201,6 +201,10 @@ class CTxMemPool
     bool lookup(const uint256& hash, CTransaction& result) const;
     bool lookupBareTxid(const uint256& btxid, CTransaction& result) const;
 
+    /** Looks up a transaction by its outpoint for spending, taking potential changes
+     *  from the raw txid (e.g. segwit light) into account.  */
+    bool lookupOutpoint(const uint256& hash, CTransaction& result) const;
+
     /** Estimate fee rate needed to get into the next nBlocks */
     CFeeRate estimateFee(int nBlocks) const;