ZIP 317 block construction algorithm.

This breaks tests which are repaired in subsequent commits.

Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>

doc/release-notes.md

@@ -45,11 +45,13 @@ Changes to Transaction Fee Selection
 Changes to Block Template Construction
 --------------------------------------
 
-- The block template construction algorithm no longer favours transactions that
-  were previously considered "high priority" because they spent older inputs. The
-  `-blockprioritysize` config option, which configured the portion of the block
-  reserved for these transactions, has been removed and will now cause a warning
-  if used.
+We now use a new block template construction algorithm documented in
+[ZIP 317](https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction).
+
+- This algorithm no longer favours transactions that were previously considered
+  "high priority" because they spent older inputs. The `-blockprioritysize` config
+  option, which configured the portion of the block reserved for these transactions,
+  has been removed and will now cause a warning if used.
 
 Removal of Priority Estimation
 ------------------------------

src/miner.cpp

@@ -35,6 +35,7 @@
 #include "util/system.h"
 #include "util/moneystr.h"
 #include "validationinterface.h"
+#include "zip317.h"
 
 #include <librustzcash.h>
 #include <rust/bridge.h>
@@ -437,7 +438,7 @@ CBlockTemplate* BlockAssembler::CreateNewBlock(
         }
     }
 
-    addScoreTxs();
+    constructZIP317BlockTemplate();
 
     last_block_num_txs = nBlockTx;
     last_block_size = nBlockSize;
@@ -641,6 +642,94 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter)
     }
 }
 
+void BlockAssembler::constructZIP317BlockTemplate()
+{
+    CTxMemPool::weightedCandidates candidatesPayingConventionalFee;
+    CTxMemPool::weightedCandidates candidatesNotPayingConventionalFee;
+
+    for (auto mi = mempool.mapTx.begin(); mi != mempool.mapTx.end(); ++mi)
+    {
+        int128_t weightRatio = mi->GetWeightRatio();
+        if (weightRatio >= WEIGHT_RATIO_SCALE) {
+            candidatesPayingConventionalFee.add(mi->GetTx().GetHash(), mi, weightRatio);
+        } else {
+            candidatesNotPayingConventionalFee.add(mi->GetTx().GetHash(), mi, weightRatio);
+        }
+    }
+
+    CTxMemPool::queueEntries waiting;
+    CTxMemPool::queueEntries cleared;
+    addTransactions(candidatesPayingConventionalFee, waiting, cleared);
+    addTransactions(candidatesNotPayingConventionalFee, waiting, cleared);
+}
+
+void BlockAssembler::addTransactions(
+    CTxMemPool::weightedCandidates& candidates,
+    CTxMemPool::queueEntries& waiting,
+    CTxMemPool::queueEntries& cleared)
+{
+    size_t nBlockUnpaidActions = 0;
+
+    while (!blockFinished && !(candidates.empty() && cleared.empty()))
+    {
+        CTxMemPool::txiter iter;
+        if (cleared.empty()) {
+            // If no txs that were previously postponed are available to try
+            // again, then select the next transaction randomly by weight ratio
+            // from the candidate set.
+            assert(!candidates.empty());
+            iter = std::get<1>(candidates.takeRandom().value());
+        } else {
+            // If a previously postponed tx is available to try again, then it
+            // has already been randomly sampled, so just take it in order.
+            iter = cleared.front();
+            cleared.pop_front();
+        }
+
+        // The tx should never already be in the block for ZIP 317.
+        assert(inBlock.count(iter) == 0);
+
+        // If the tx would cause the block to exceed the unpaid action limit, skip it.
+        // A tx that pays at least the conventional fee will have no unpaid actions.
+        size_t txUnpaidActions = iter->GetUnpaidActionCount();
+        if (nBlockUnpaidActions + txUnpaidActions > BLOCK_UNPAID_ACTION_LIMIT) {
+            continue;
+        }
+
+        // If tx is dependent on other mempool transactions that haven't yet been
+        // included then put it in the waiting queue.
+        if (isStillDependent(iter)) {
+            waiting.push_back(iter);
+            continue;
+        }
+
+        // If this tx fits in the block add it, otherwise keep looping.
+        if (TestForBlock(iter)) {
+            AddToBlock(iter);
+            nBlockUnpaidActions += txUnpaidActions;
+
+            // This tx was successfully added, so add waiting transactions that
+            // depend on this one to the cleared queue to try again.
+            //
+            // TODO: This makes the overall algorithm O(n^2 log n) in the worst case
+            // of a linear dependency chain. (children is a std::map; its count method
+            // is O(log n) given the maximum number of children at each step, and is
+            // called O(n^2) times.) Daira conjectures that O(n log n) is possible.
+            auto children = mempool.GetMemPoolChildren(iter);
+            CTxMemPool::queueEntries stillWaiting;
+            for (CTxMemPool::txiter maybeChild : waiting)
+            {
+                if (children.count(maybeChild) > 0) {
+                    cleared.push_back(maybeChild);
+                } else {
+                    stillWaiting.push_back(maybeChild);
+                }
+            }
+            waiting.swap(stillWaiting);
+        }
+    }
+}
+
 void BlockAssembler::addScoreTxs()
 {
     std::priority_queue<CTxMemPool::txiter, std::vector<CTxMemPool::txiter>, ScoreCompare> clearedTxs;

src/miner.h

@@ -9,6 +9,7 @@
 
 #include "primitives/block.h"
 #include "txmempool.h"
+#include "weighted_map.h"
 
 #include <stdint.h>
 #include <memory>
@@ -139,6 +140,12 @@ public:
         const std::optional<CMutableTransaction>& next_coinbase_mtx = std::nullopt);
 
 private:
+    void constructZIP317BlockTemplate();
+    void addTransactions(
+        CTxMemPool::weightedCandidates& candidates,
+        CTxMemPool::queueEntries& waiting,
+        CTxMemPool::queueEntries& cleared);
+
     // utility functions
     /** Clear the block's state and prepare for assembling a new block */
     void resetBlock(const MinerAddress& minerAddress);

src/random.cpp

@@ -40,19 +40,19 @@ void GetRandBytes(unsigned char* buf, size_t num)
     librustzcash_getrandom(buf, num);
 }
 
+uint128_t GetRandUInt128(uint128_t nMax)
+{
+    return GetRandGeneric(nMax);
+}
+
+int128_t GetRandInt128(int128_t nMax)
+{
+    return GetRandUInt128(nMax);
+}
+
 uint64_t GetRand(uint64_t nMax)
 {
-    if (nMax == 0)
-        return 0;
-
-    // The range of the random source must be a multiple of the modulus
-    // to give every possible output value an equal possibility
-    uint64_t nRange = (std::numeric_limits<uint64_t>::max() / nMax) * nMax;
-    uint64_t nRand = 0;
-    do {
-        GetRandBytes((unsigned char*)&nRand, sizeof(nRand));
-    } while (nRand >= nRange);
-    return (nRand % nMax);
+    return GetRandGeneric(nMax);
 }
 
 int64_t GetRandInt64(int64_t nMax)

src/random.h

@@ -13,12 +13,33 @@
 
 #include <functional>
 #include <limits>
-#include <stdint.h>
+
+#include "int128.h"
 
 /**
  * Functions to gather random data via the rand_core OsRng
  */
 void GetRandBytes(unsigned char* buf, size_t num);
+
+template <typename I>
+I GetRandGeneric(I nMax)
+{
+    static_assert(std::numeric_limits<I>::min() == 0);
+    if (nMax == I())
+        return I();
+
+    // The range of the random source must be a multiple of the modulus
+    // to give every possible output value an equal possibility
+    I nRange = (std::numeric_limits<I>::max() / nMax) * nMax;
+    I nRand = I();
+    do {
+        GetRandBytes((unsigned char*)&nRand, sizeof(nRand));
+    } while (nRand >= nRange);
+    return (nRand % nMax);
+}
+
+uint128_t GetRandUInt128(uint128_t nMax);
+int128_t GetRandInt128(int128_t nMax);
 uint64_t GetRand(uint64_t nMax);
 int64_t GetRandInt64(int64_t nMax);
 int GetRandInt(int nMax);

src/txmempool.h

@@ -21,6 +21,7 @@
 #include "addressindex.h"
 #include "spentindex.h"
 #include "util/time.h"
+#include "weighted_map.h"
 
 #undef foreach
 #include "boost/multi_index_container.hpp"
@@ -433,6 +434,9 @@ public:
         }
     };
     typedef std::set<txiter, CompareIteratorByHash> setEntries;
+    typedef std::deque<txiter> queueEntries;
+    // Type of a set of candidate transactions to be added to a block template.
+    typedef WeightedMap<uint256, txiter, int128_t, GetRandInt128> weightedCandidates;
 
     const setEntries & GetMemPoolParents(txiter entry) const;
     const setEntries & GetMemPoolChildren(txiter entry) const;

src/weighted_map.h

@@ -13,7 +13,8 @@
 // A WeightedMap represents a map from keys (of type K) to values (of type V),
 // each entry having a weight (of type W). Elements can be randomly selected and
 // removed from the map with probability in proportion to their weight. This is
-// used to implement mempool limiting specified in ZIP 401.
+// used to implement mempool limiting specified in ZIP 401, and the block template
+// construction algorithm specified in ZIP 317.
 //
 // In order to efficiently implement random selection by weight, we keep track
 // of the total weight of all keys in the map. For performance reasons, the