Unit test for mempool with UTXO hasher.

This extends the mempool unit tests to explicitly verify that
adding transactions, removing transactions, checking the pool
and looking up coins / transactions still works even if we use
the bare txid for some transactions as UTXO hash (as will be
the case with segwit-light in the future).

Author: domob1812

divi/src/test/MockUtxoHasher.cpp

@@ -11,6 +11,11 @@ uint256 MockUtxoHasher::Add(const CTransaction& tx)
   return value;
 }
 
+void MockUtxoHasher::UseBareTxid(const CTransaction& tx)
+{
+  customHashes.emplace(tx.GetHash(), tx.GetBareTxid());
+}
+
 uint256 MockUtxoHasher::GetUtxoHash(const CTransaction& tx) const
 {
   const uint256 txid = tx.GetHash();

divi/src/test/MockUtxoHasher.h

@@ -32,6 +32,10 @@ class MockUtxoHasher : public TransactionUtxoHasher
    *  is generated uniquely and returned from the function.  */
   uint256 Add(const CTransaction& tx);
 
+  /** Marks the given transaction for using the bare txid rather than
+   *  the normal txid.  */
+  void UseBareTxid(const CTransaction& tx);
+
   uint256 GetUtxoHash(const CTransaction& tx) const override;
 
 };

divi/src/test/UtxoCheckingAndUpdating_tests.cpp

@@ -41,8 +41,8 @@ BOOST_AUTO_TEST_CASE(addsCorrectOutputs)
   const auto id2 = utxoHasher.Add(tx2);
 
   CTxUndo txundo;
-  UpdateCoins(tx1, coins, txundo, utxoHasher, 101);
-  UpdateCoins(tx2, coins, txundo, utxoHasher, 102);
+  UpdateCoinsWithTransaction(tx1, coins, txundo, utxoHasher, 101);
+  UpdateCoinsWithTransaction(tx2, coins, txundo, utxoHasher, 102);
 
   BOOST_CHECK(coins.HaveCoins(tx1.GetHash()));
   BOOST_CHECK(!coins.HaveCoins(tx2.GetHash()));

divi/src/test/mempool_tests.cpp

@@ -2,39 +2,67 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include "main.h"
 #include "txmempool.h"
 
+#include "blockmap.h"
+#include "MockUtxoHasher.h"
+
 #include <boost/test/unit_test.hpp>
 #include <list>
 
+extern CChain chainActive;
+extern BlockMap mapBlockIndex;
+
 class MempoolTestFixture
 {
 
+private:
+
+  /** Empty coins view used to back the cached view we actually use.  */
+  CCoinsView emptyView;
+
+  /** Tip of our fake chain.  */
+  CBlockIndex tip;
+
 protected:
 
   /** A parent transaction.  */
   CMutableTransaction txParent;
 
-  /** Three children of the parent.  */
+  /** Three children of the parent.  They use the bare txid for their UTXOs
+   *  in our UTXO hasher.  */
   CMutableTransaction txChild[3];
 
   /** Three grand children.  */
   CMutableTransaction txGrandChild[3];
 
+  /** Coins view with the parent inputs.  */
+  CCoinsViewCache view;
+
   /** The test mempool instance.  */
   CTxMemPool testPool;
 
 public:
 
   MempoolTestFixture()
-    : testPool(CFeeRate(0))
+    : view(&emptyView), testPool(CFeeRate(0))
   {
-    txParent.vin.resize(2);
+    std::unique_ptr<MockUtxoHasher> utxoHasher(new MockUtxoHasher());
+
+    CMutableTransaction base;
+    base.vout.emplace_back(99000LL, CScript() << OP_11 << OP_EQUAL);
+    view.ModifyCoins(base.GetHash())->FromTx(base, 0);
+
+    tip.pprev = nullptr;
+    tip.nHeight = 0;
+    mapBlockIndex[tip.GetBlockHeader().GetHash()] = &tip;
+    view.SetBestBlock(tip.GetBlockHeader().GetHash());
+    chainActive.SetTip(&tip);
+
+    txParent.vin.resize(1);
     txParent.vin[0].scriptSig = CScript() << OP_11;
-    /* Add a second input to make sure the transaction does not qualify as
-       coinbase and thus has a bare txid unequal to its normal hash.  */
-    txParent.vin[1].scriptSig = CScript() << OP_12;
+    txParent.vin[0].prevout.hash = base.GetHash();
+    txParent.vin[0].prevout.n = 0;
     txParent.vout.resize(3);
     for (int i = 0; i < 3; i++)
     {
@@ -52,18 +80,39 @@ class MempoolTestFixture
         txChild[i].vout.resize(1);
         txChild[i].vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
         txChild[i].vout[0].nValue = 11000LL;
+        utxoHasher->UseBareTxid(txChild[i]);
     }
 
     for (int i = 0; i < 3; i++)
     {
         txGrandChild[i].vin.resize(1);
         txGrandChild[i].vin[0].scriptSig = CScript() << OP_11;
-        txGrandChild[i].vin[0].prevout.hash = txChild[i].GetHash();
+        txGrandChild[i].vin[0].prevout.hash = txChild[i].GetBareTxid();
         txGrandChild[i].vin[0].prevout.n = 0;
         txGrandChild[i].vout.resize(1);
         txGrandChild[i].vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
         txGrandChild[i].vout[0].nValue = 11000LL;
     }
+
+    testPool.setSanityCheck(true);
+    testPool.SetUtxoHasherForTesting(std::move(utxoHasher));
+    testPool.clear();
+  }
+
+  ~MempoolTestFixture()
+  {
+    mapBlockIndex.clear();
+  }
+
+  /** Adds the parent, childs and grandchilds to the mempool.  */
+  void AddAll()
+  {
+      testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1));
+      for (int i = 0; i < 3; i++)
+      {
+          testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1));
+          testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1));
+      }
   }
 
 };
@@ -87,12 +136,10 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
     removed.clear();
 
     // Parent, children, grandchildren:
-    testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1));
-    for (int i = 0; i < 3; i++)
-    {
-        testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1));
-        testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1));
-    }
+    AddAll();
+
+    testPool.check(&view);
+
     // Remove Child[0], GrandChild[0] should be removed:
     testPool.remove(txChild[0], removed, true);
     BOOST_CHECK_EQUAL(removed.size(), 2);
@@ -127,12 +174,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexByBareTxid)
     CTransaction tx;
     std::list<CTransaction> removed;
 
-    testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1));
-    for (int i = 0; i < 3; ++i)
-    {
-        testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1));
-        testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1));
-    }
+    AddAll();
 
     BOOST_CHECK(testPool.lookupBareTxid(txParent.GetBareTxid(), tx));
     BOOST_CHECK(tx.GetHash() == txParent.GetHash());
@@ -144,4 +186,28 @@ BOOST_AUTO_TEST_CASE(MempoolIndexByBareTxid)
     BOOST_CHECK(!testPool.lookupBareTxid(txGrandChild[0].GetBareTxid(), tx));
 }
 
+BOOST_AUTO_TEST_CASE(MempoolOutpointLookup)
+{
+    CTransaction tx;
+    CCoins coins;
+
+    AddAll();
+    CCoinsViewMemPool viewPool(&view, 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(), coins));
+    BOOST_CHECK(!viewPool.HaveCoins(txParent.GetBareTxid()));
+    BOOST_CHECK(!viewPool.GetCoins(txParent.GetBareTxid(), coins));
+
+    BOOST_CHECK(!viewPool.HaveCoins(txChild[0].GetHash()));
+    BOOST_CHECK(!viewPool.GetCoins(txChild[0].GetHash(), coins));
+    BOOST_CHECK(viewPool.HaveCoins(txChild[0].GetBareTxid()));
+    BOOST_CHECK(viewPool.GetCoins(txChild[0].GetBareTxid(), coins));
+}
+
 BOOST_AUTO_TEST_SUITE_END()

divi/src/txmempool.cpp

@@ -451,6 +451,11 @@ const TransactionUtxoHasher& CTxMemPool::GetUtxoHasher() const
     return *utxoHasher;
 }
 
+void CTxMemPool::SetUtxoHasherForTesting(std::unique_ptr<TransactionUtxoHasher> hasher)
+{
+    utxoHasher = std::move(hasher);
+}
+
 void CTxMemPool::addAddressIndex(const CTxMemPoolEntry &entry, const CCoinsViewCache &view)
 {
     LOCK(cs);

divi/src/txmempool.h

@@ -158,6 +158,10 @@ class CTxMemPool
     /** Returns the UTXO hasher instance used in the mempool.  */
     const TransactionUtxoHasher& GetUtxoHasher() const;
 
+    /** Replaces the UTXO hasher used in the mempool with the given instance,
+     *  which allows dependency injection for unit tests.  */
+    void SetUtxoHasherForTesting(std::unique_ptr<TransactionUtxoHasher> hasher);
+
     void addAddressIndex(const CTxMemPoolEntry &entry, const CCoinsViewCache &view);
     bool getAddressIndex(std::vector<std::pair<uint160, int> > &addresses,
                          std::vector<std::pair<CMempoolAddressDeltaKey, CMempoolAddressDelta> > &results);