UIP-0024 Implementation (CTOR/LTOR) (#440)

UIP-0024 implementation (Canonical Transactions Ordering Rule,
specifically LTOR, Lexicographical Transactions Ordering Rule).

Signed-off-by: Andres Correa Casablanca andres@thirdhash.com

src/hash.cpp

@@ -165,7 +165,7 @@ uint64_t CSipHasher::Finalize() const
     return v0 ^ v1 ^ v2 ^ v3;
 }
 
-uint64_t SipHashUint256(uint64_t k0, uint64_t k1, const uint256& val)
+uint64_t SipHashUint256(const uint64_t k0, const uint64_t k1, const uint256& val)
 {
     /* Specialized implementation for efficiency */
     uint64_t d = val.GetUint64(0);

src/miner.cpp

@@ -1,5 +1,7 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
 // Copyright (c) 2009-2017 The Bitcoin Core developers
+// Copyright (c) 2018 The Bitcoin ABC developers
+// Copyright (c) 2018-2019 The Unit-e developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -156,6 +158,18 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     int nDescendantsUpdated = 0;
     addPackageTxs(nPackagesSelected, nDescendantsUpdated);
 
+    // LTOR/CTOR: We ensure that all transactions (except the 0th, coinbase) are
+    // sorted in lexicographical order. Notice that we don't sort
+    // blocktemplate->vTxFees nor blocktemplate->vTxSigOpsCost because they are
+    // not used at all. These two vectors are just a "residue" from Bitcoin's
+    // PoW mining pools code.
+    std::sort(
+        std::begin(pblock->vtx) + 1, std::end(pblock->vtx),
+        [](const CTransactionRef &a, const CTransactionRef &b) -> bool {
+            return a->GetHash().CompareAsNumber(b->GetHash()) < 0;
+        }
+    );
+
     int64_t nTime1 = GetTimeMicros();
 
     nLastBlockTx = nBlockTx;

src/test/mempool_tests.cpp

@@ -571,4 +571,94 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
     SetMockTime(0);
 }
 
+BOOST_AUTO_TEST_CASE(DisconnectionTopologicalOrderTest)
+{
+    std::vector<CTransactionRef> vtx;
+    vtx.reserve(13);
+
+    CMutableTransaction first_mtx;
+    first_mtx.vout.resize(2);
+    first_mtx.vout[0].scriptPubKey = CScript();
+    first_mtx.vout[0].nValue = 10 * UNIT;
+    first_mtx.vout[1].scriptPubKey = CScript();
+    first_mtx.vout[1].nValue = 10 * UNIT;
+    auto first_tx_ref = MakeTransactionRef(std::move(first_mtx));
+    vtx.push_back(first_tx_ref);
+
+
+    //      TX1       TX3
+    // TX0       TX2         ···
+    //      TX1'      TX3'
+    for (unsigned int i=0; i < 8; ++i) {
+        if (i % 2 == 0) {
+            CMutableTransaction mtx_a;
+            CMutableTransaction mtx_b;
+
+            mtx_a.vin.resize(1);
+            mtx_a.vin[0].prevout = COutPoint(vtx.back()->GetHash(), 0);
+            mtx_a.vin[0].scriptSig = CScript();
+            mtx_a.vout.resize(1);
+            mtx_a.vout[0].nValue = 10 * UNIT;
+            mtx_a.vout[0].scriptPubKey = CScript();
+
+            mtx_b.vin.resize(1);
+            mtx_b.vin[0].prevout = COutPoint(vtx.back()->GetHash(), 1);
+            mtx_b.vin[0].scriptSig = CScript();
+            mtx_b.vout.resize(1);
+            mtx_b.vout[0].nValue = 10 * UNIT;
+            mtx_b.vout[0].scriptPubKey = CScript();
+
+            vtx.emplace_back(MakeTransactionRef(std::move(mtx_a)));
+            vtx.emplace_back(MakeTransactionRef(std::move(mtx_b)));
+        } else {
+            CMutableTransaction mtx;
+
+            mtx.vin.resize(2);
+            auto prev_ptx = vtx.rbegin();
+            mtx.vin[0].prevout = COutPoint((*prev_ptx)->GetHash(), 0);
+            mtx.vin[0].scriptSig = CScript();
+            ++prev_ptx;
+            mtx.vin[1].prevout = COutPoint((*prev_ptx)->GetHash(), 0);
+            mtx.vin[1].scriptSig = CScript();
+            mtx.vout.resize(2);
+            mtx.vout[0].scriptPubKey = CScript();
+            mtx.vout[0].nValue = 10 * UNIT;
+            mtx.vout[1].scriptPubKey = CScript();
+            mtx.vout[1].nValue = 10 * UNIT;
+
+            vtx.emplace_back(MakeTransactionRef(std::move(mtx)));
+        }
+    }
+
+    // We sort transactions in lexicographical order to remove the previous
+    // topological order.
+    std::sort(
+        std::begin(vtx) + 1, std::end(vtx),
+        [](const CTransactionRef &a, const CTransactionRef &b) -> bool {
+          return a->GetHash().CompareAsNumber(b->GetHash()) < 0;
+        }
+    );
+
+    DisconnectedBlockTransactions disconnectpool;
+    disconnectpool.LoadFromBlockInTopologicalOrder(vtx); // System-Under-Test
+
+    std::unordered_set<uint256, SaltedTxidHasher> processed_tx_hashes;
+    processed_tx_hashes.insert(first_tx_ref->GetHash());
+
+    for (
+        auto it = disconnectpool.queuedTx.get<insertion_order>().rbegin();
+        it != disconnectpool.queuedTx.get<insertion_order>().rend();
+        ++it
+    ) {
+        CTransactionRef tx_ref = (*it);
+        for (CTxIn tx_in : tx_ref->vin) {
+            // We check that transactions have been ordered topologically
+            BOOST_CHECK(processed_tx_hashes.count(tx_in.prevout.hash) > 0);
+        }
+        processed_tx_hashes.insert(tx_ref->GetHash());
+    }
+
+    disconnectpool.clear();
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/uint256_tests.cpp

@@ -156,6 +156,29 @@ BOOST_AUTO_TEST_CASE( comparison ) // <= >= < >
     BOOST_CHECK( R2S < MaxS );
 }
 
+BOOST_AUTO_TEST_CASE( number_comparison )
+{
+    const uint160 R1Sp = uint160(std::vector<unsigned char>(R1Array + 12, R1Array + 32));
+    const uint160 R2Sp = uint160(std::vector<unsigned char>(R2Array + 12, R2Array + 32));
+    uint160 nums160[] = {ZeroS, OneS, R1Sp, R2Sp, MaxS};
+    uint256 nums256[] = {ZeroL, OneL, R1L, R2L, MaxL};
+
+    for (size_t i=0; i < 5; ++i) {
+        for (size_t j=0; j < 5; ++j) {
+            if (i == j) {
+                BOOST_CHECK(nums160[i].CompareAsNumber(nums160[i]) == 0);
+                BOOST_CHECK(nums256[i].CompareAsNumber(nums256[i]) == 0);
+            } else if (i > j) {
+                BOOST_CHECK(nums160[i].CompareAsNumber(nums160[j]) > 0);
+                BOOST_CHECK(nums256[i].CompareAsNumber(nums256[j]) > 0);
+            } else {
+                BOOST_CHECK(nums160[i].CompareAsNumber(nums160[j]) < 0);
+                BOOST_CHECK(nums256[i].CompareAsNumber(nums256[j]) < 0);
+            }
+        }
+    }
+}
+
 BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex begin() end() size() GetLow64 GetSerializeSize, Serialize, Unserialize
 {
     BOOST_CHECK(R1L.GetHex() == R1L.ToString());

src/txmempool.cpp

@@ -564,18 +564,27 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
 {
     LOCK(cs);
     std::vector<const CTxMemPoolEntry*> entries;
-    for (const auto& tx : vtx)
-    {
-        uint256 hash = tx->GetHash();
+
+    DisconnectedBlockTransactions disconnectpool;
+    disconnectpool.LoadFromBlockInTopologicalOrder(vtx);
+
+    // We take advantage from LoadFromBlockInTopologicalOrder sorting txns for
+    // us to ensure that transactions are removed from the mempool in a
+    // "correct" order, so the mempool is always consistent.
+    const auto& txns_queue = disconnectpool.GetQueuedTx().get<insertion_order>();
+    for (auto ptx = txns_queue.rbegin(); ptx != txns_queue.rend(); ++ptx) {
+        const uint256& hash = (*ptx)->GetHash();
 
         indexed_transaction_set::iterator i = mapTx.find(hash);
         if (i != mapTx.end())
             entries.push_back(&*i);
     }
+
     // Before the txs in the new block have been removed from the mempool, update policy estimates
     if (minerPolicyEstimator) {minerPolicyEstimator->processBlock(nBlockHeight, entries);}
-    for (const auto& tx : vtx)
-    {
+
+    for (auto ptx = txns_queue.rbegin(); ptx != txns_queue.rend(); ++ptx) {
+        const auto& tx = *ptx;
         txiter it = mapTx.find(tx->GetHash());
         if (it != mapTx.end()) {
             setEntries stage;
@@ -585,6 +594,15 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
         removeConflicts(*tx);
         ClearPrioritisation(tx->GetHash());
     }
+
+    // Although one would expect that this call is not really needed, the
+    // DisconnectedBlockTransactions struct has been designed to assert in its
+    // destructor that all tx have been removed from its queue before, and it
+    // does not do that by itself, in order to enforce some system-broad
+    // properties. Here we are using this struct in a different way, so we have
+    // to take care on our own.
+    disconnectpool.clear();
+
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;
 }
@@ -1089,3 +1107,60 @@ bool CTxMemPool::TransactionWithinChainLimit(const uint256& txid, size_t chainLi
 }
 
 SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
+
+void DisconnectedBlockTransactions::LoadFromBlockInTopologicalOrder(
+    const std::vector<CTransactionRef> &vtx
+) {
+    // Save transactions to re-add to mempool at end of reorg
+    for (const auto &tx : vtx) {
+        const auto it = queuedTx.find(tx->GetHash());
+        if (it != queuedTx.end()) {
+            continue;
+        }
+
+        // Queue transaction to be re-inserted into the mempool
+        addTransaction(tx);
+
+        // Fill in the set of parents.
+        std::unordered_set<uint256, SaltedTxidHasher> parents;
+        for (const CTxIn &in : tx->vin) {
+            parents.insert(in.prevout.hash);
+        }
+
+        // In order to make sure we keep things in topological order, we check
+        // if we already know of the parent of the current transaction. If so,
+        // we remove them from the set and then add them back.
+        while (!parents.empty()) {
+            std::unordered_set<uint256, SaltedTxidHasher> worklist;
+            parents.swap(worklist);
+
+            for (const uint256 &txid : worklist) {
+                // If we do not have that txid in the set, nothing needs to be
+                // done.
+                auto pit = queuedTx.find(txid);
+                if (pit == queuedTx.end()) {
+                    continue;
+                }
+
+                // We have parent in our set, we reinsert them at the right
+                // position
+                const CTransactionRef ptx = *pit;
+                queuedTx.erase(pit);
+                queuedTx.insert(ptx);
+
+                // And we make sure ancestors are covered.
+                for (const CTxIn &in : ptx->vin) {
+                    parents.insert(in.prevout.hash);
+                }
+            }
+        }
+
+    }
+
+    while (DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) {
+        // Drop the earliest entry, and remove its children from the mempool.
+        auto it = queuedTx.get<insertion_order>().begin();
+        mempool.removeRecursive(**it, MemPoolRemovalReason::REORG);
+        removeEntry(it);
+    }
+}

src/txmempool.h

@@ -354,11 +354,19 @@ class SaltedTxidHasher
 {
 private:
     /** Salt */
-    const uint64_t k0, k1;
+    uint64_t k0, k1;
 
 public:
     SaltedTxidHasher();
 
+    SaltedTxidHasher(const SaltedTxidHasher&) = default;
+    SaltedTxidHasher(SaltedTxidHasher&&) = default;
+
+    SaltedTxidHasher& operator=(const SaltedTxidHasher&) = default;
+    SaltedTxidHasher& operator=(SaltedTxidHasher&&) = default;
+
+    ~SaltedTxidHasher() = default;
+
     size_t operator()(const uint256& txid) const {
         return SipHashUint256(k0, k1, txid);
     }
@@ -768,12 +776,20 @@ struct DisconnectedBlockTransactions {
         return memusage::MallocUsage(sizeof(CTransactionRef) + 6 * sizeof(void*)) * queuedTx.size() + cachedInnerUsage;
     }
 
+    const indexed_disconnected_transactions &GetQueuedTx() const {
+        return queuedTx;
+    }
+
     void addTransaction(const CTransactionRef& tx)
     {
         queuedTx.insert(tx);
         cachedInnerUsage += RecursiveDynamicUsage(tx);
     }
 
+    // Add entries for a block while reconstructing the topological ordering so
+    // they can be added back to the mempool simply.
+    void LoadFromBlockInTopologicalOrder(const std::vector<CTransactionRef> &vtx);
+
     // Remove entries based on txid_index, and update memory usage.
     void removeForBlock(const std::vector<CTransactionRef>& vtx)
     {

src/uint256.h

@@ -1,5 +1,7 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
 // Copyright (c) 2009-2017 The Bitcoin Core developers
+// Copyright (c) 2018 The Bitcoin ABC developers
+// Copyright (c) 2018-2019 The Unit-e developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -45,6 +47,21 @@ class base_blob
 
     inline int Compare(const base_blob& other) const { return memcmp(data, other.data, sizeof(data)); }
 
+    inline int CompareAsNumber(const base_blob& other) const {
+        for (int i = WIDTH - 1; i >= 0; --i) {
+            uint8_t a = data[i];
+            uint8_t b = other.data[i];
+            if (a > b) {
+                return 1;
+            }
+            if (a < b) {
+                return -1;
+            }
+        }
+
+        return 0;
+    }
+
     friend inline bool operator==(const base_blob& a, const base_blob& b) { return a.Compare(b) == 0; }
     friend inline bool operator!=(const base_blob& a, const base_blob& b) { return a.Compare(b) != 0; }
     friend inline bool operator<(const base_blob& a, const base_blob& b) { return a.Compare(b) < 0; }