Merge c8a3fabe40 into 51d76634fb

src/node/interfaces.cpp

@@ -929,111 +929,14 @@ public:
 
     bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) override
     {
-        CBlock block{m_block_template->block};
+        AddMerkleRootAndCoinbase(m_block_template->block, std::move(coinbase), version, timestamp, nonce);
-
+        return chainman().ProcessNewBlock(std::make_shared<const CBlock>(m_block_template->block), /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/nullptr);
-        if (block.vtx.size() == 0) {
-            block.vtx.push_back(coinbase);
-        } else {
-            block.vtx[0] = coinbase;
-        }
-
-        block.nVersion = version;
-        block.nTime = timestamp;
-        block.nNonce = nonce;
-
-        block.hashMerkleRoot = BlockMerkleRoot(block);
-
-        auto block_ptr = std::make_shared<const CBlock>(block);
-        return chainman().ProcessNewBlock(block_ptr, /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/nullptr);
     }
 
     std::unique_ptr<BlockTemplate> waitNext(BlockWaitOptions options) override
     {
-        // Delay calculating the current template fees, just in case a new block
+        auto new_template = WaitNext(chainman(), notifications(), m_node.mempool.get(), m_block_template->block.hashPrevBlock, m_block_template->vTxFees, options, m_assemble_options);
-        // comes in before the next tick.
+        if (new_template) return std::make_unique<BlockTemplateImpl>(m_assemble_options, std::move(*new_template), m_node);
-        CAmount current_fees = -1;
-
-        // Alternate waiting for a new tip and checking if fees have risen.
-        // The latter check is expensive so we only run it once per second.
-        auto now{NodeClock::now()};
-        const auto deadline = now + options.timeout;
-        const MillisecondsDouble tick{1000};
-        const bool allow_min_difficulty{chainman().GetParams().GetConsensus().fPowAllowMinDifficultyBlocks};
-
-        do {
-            bool tip_changed{false};
-            {
-                WAIT_LOCK(notifications().m_tip_block_mutex, lock);
-                // Note that wait_until() checks the predicate before waiting
-                notifications().m_tip_block_cv.wait_until(lock, std::min(now + tick, deadline), [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) {
-                    AssertLockHeld(notifications().m_tip_block_mutex);
-                    const auto tip_block{notifications().TipBlock()};
-                    // We assume tip_block is set, because this is an instance
-                    // method on BlockTemplate and no template could have been
-                    // generated before a tip exists.
-                    tip_changed = Assume(tip_block) && tip_block != m_block_template->block.hashPrevBlock;
-                    return tip_changed || chainman().m_interrupt;
-                });
-            }
-
-            if (chainman().m_interrupt) return nullptr;
-            // At this point the tip changed, a full tick went by or we reached
-            // the deadline.
-
-            // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks.
-            LOCK(::cs_main);
-
-            // On test networks return a minimum difficulty block after 20 minutes
-            if (!tip_changed && allow_min_difficulty) {
-                const NodeClock::time_point tip_time{std::chrono::seconds{chainman().ActiveChain().Tip()->GetBlockTime()}};
-                if (now > tip_time + 20min) {
-                    tip_changed = true;
-                }
-            }
-
-            /**
-             * We determine if fees increased compared to the previous template by generating
-             * a fresh template. There may be more efficient ways to determine how much
-             * (approximate) fees for the next block increased, perhaps more so after
-             * Cluster Mempool.
-             *
-             * We'll also create a new template if the tip changed during this iteration.
-             */
-            if (options.fee_threshold < MAX_MONEY || tip_changed) {
-                auto tmpl{std::make_unique<BlockTemplateImpl>(m_assemble_options,
-                                                              BlockAssembler{
-                                                                  chainman().ActiveChainstate(),
-                                                                  context()->mempool.get(),
-                                                                  m_assemble_options}
-                                                                  .CreateNewBlock(),
-                                                              m_node)};
-
-                // If the tip changed, return the new template regardless of its fees.
-                if (tip_changed) return tmpl;
-
-                // Calculate the original template total fees if we haven't already
-                if (current_fees == -1) {
-                    current_fees = 0;
-                    for (CAmount fee : m_block_template->vTxFees) {
-                        // Skip coinbase
-                        if (fee < 0) continue;
-                        current_fees += fee;
-                    }
-                }
-
-                CAmount new_fees = 0;
-                for (CAmount fee : tmpl->m_block_template->vTxFees) {
-                    // Skip coinbase
-                    if (fee < 0) continue;
-                    new_fees += fee;
-                    Assume(options.fee_threshold != MAX_MONEY);
-                    if (new_fees >= current_fees + options.fee_threshold) return tmpl;
-                }
-            }
-
-            now = NodeClock::now();
-        } while (now < deadline);
-
         return nullptr;
     }
 
@@ -1041,7 +944,6 @@ public:
 
     const std::unique_ptr<CBlockTemplate> m_block_template;
 
-    NodeContext* context() { return &m_node; }
     ChainstateManager& chainman() { return *Assert(m_node.chainman); }
     KernelNotifications& notifications() { return *Assert(m_node.notifications); }
     NodeContext& m_node;
@@ -1072,32 +974,8 @@ public:
 
     std::optional<BlockRef> waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override
     {
-        Assume(timeout >= 0ms); // No internal callers should use a negative timeout
+        if (ChainTipChanged(chainman(), notifications(), current_tip, timeout)) return getTip();
-        if (timeout < 0ms) timeout = 0ms;
+        return {};
-        if (timeout > std::chrono::years{100}) timeout = std::chrono::years{100}; // Upper bound to avoid UB in std::chrono
-        auto deadline{std::chrono::steady_clock::now() + timeout};
-        {
-            WAIT_LOCK(notifications().m_tip_block_mutex, lock);
-            // For callers convenience, wait longer than the provided timeout
-            // during startup for the tip to be non-null. That way this function
-            // always returns valid tip information when possible and only
-            // returns null when shutting down, not when timing out.
-            notifications().m_tip_block_cv.wait(lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) {
-                return notifications().TipBlock() || chainman().m_interrupt;
-            });
-            if (chainman().m_interrupt) return {};
-            // At this point TipBlock is set, so continue to wait until it is
-            // different then `current_tip` provided by caller.
-            notifications().m_tip_block_cv.wait_until(lock, deadline, [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) {
-                return Assume(notifications().TipBlock()) != current_tip || chainman().m_interrupt;
-            });
-        }
-
-        if (chainman().m_interrupt) return {};
-
-        // Must release m_tip_block_mutex before getTip() locks cs_main, to
-        // avoid deadlocks.
-        return getTip();
     }
 
     std::unique_ptr<BlockTemplate> createNewBlock(const BlockCreateOptions& options) override

src/node/miner.cpp

@@ -16,11 +16,14 @@
 #include <consensus/validation.h>
 #include <deploymentstatus.h>
 #include <logging.h>
+#include <node/context.h>
+#include <node/kernel_notifications.h>
 #include <policy/feerate.h>
 #include <policy/policy.h>
 #include <pow.h>
 #include <primitives/transaction.h>
 #include <util/moneystr.h>
+#include <util/signalinterrupt.h>
 #include <util/time.h>
 #include <validation.h>
 
@@ -433,4 +436,134 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
         nDescendantsUpdated += UpdatePackagesForAdded(mempool, ancestors, mapModifiedTx);
     }
 }
+
+void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t version, uint32_t timestamp, uint32_t nonce)
+{
+    if (block.vtx.size() == 0) {
+        block.vtx.emplace_back(coinbase);
+    } else {
+        block.vtx[0] = coinbase;
+    }
+    block.nVersion = version;
+    block.nTime = timestamp;
+    block.nNonce = nonce;
+    block.hashMerkleRoot = BlockMerkleRoot(block);
+}
+
+std::optional<std::unique_ptr<CBlockTemplate>> WaitNext(ChainstateManager& chainman, KernelNotifications& kernel_notifications, CTxMemPool* mempool, const uint256& prev_block_hash,
+                                                        const std::vector<CAmount>& tx_fees, const BlockWaitOptions& options, const BlockAssembler::Options& assemble_options)
+{
+    // Delay calculating the current template fees, just in case a new block
+    // comes in before the next tick.
+    CAmount current_fees = -1;
+
+    // Alternate waiting for a new tip and checking if fees have risen.
+    // The latter check is expensive so we only run it once per second.
+    auto now{NodeClock::now()};
+    const auto deadline = now + options.timeout;
+    const MillisecondsDouble tick{1000};
+    const bool allow_min_difficulty{chainman.GetParams().GetConsensus().fPowAllowMinDifficultyBlocks};
+
+    do {
+        bool tip_changed{false};
+        {
+            WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock);
+            // Note that wait_until() checks the predicate before waiting
+            kernel_notifications.m_tip_block_cv.wait_until(lock, std::min(now + tick, deadline), [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) {
+                AssertLockHeld(kernel_notifications.m_tip_block_mutex);
+                const auto tip_block{kernel_notifications.TipBlock()};
+                // We assume tip_block is set, because this is an instance
+                // method on BlockTemplate and no template could have been
+                // generated before a tip exists.
+                tip_changed = Assume(tip_block) && tip_block != prev_block_hash;
+                return tip_changed || chainman.m_interrupt;
+            });
+        }
+
+        if (chainman.m_interrupt) return std::nullopt;
+        // At this point the tip changed, a full tick went by or we reached
+        // the deadline.
+
+        // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks.
+        LOCK(::cs_main);
+
+        // On test networks return a minimum difficulty block after 20 minutes
+        if (!tip_changed && allow_min_difficulty) {
+            const NodeClock::time_point tip_time{std::chrono::seconds{chainman.ActiveChain().Tip()->GetBlockTime()}};
+            if (now > tip_time + 20min) {
+                tip_changed = true;
+            }
+        }
+
+        /**
+         * We determine if fees increased compared to the previous template by generating
+         * a fresh template. There may be more efficient ways to determine how much
+         * (approximate) fees for the next block increased, perhaps more so after
+         * Cluster Mempool.
+         *
+         * We'll also create a new template if the tip changed during this iteration.
+         */
+        if (options.fee_threshold < MAX_MONEY || tip_changed) {
+            auto new_tmpl{BlockAssembler{
+                chainman.ActiveChainstate(),
+                mempool,
+                assemble_options}
+                              .CreateNewBlock()};
+
+            // If the tip changed, return the new template regardless of its fees.
+            if (tip_changed) return new_tmpl;
+
+            // Calculate the original template total fees if we haven't already
+            if (current_fees == -1) {
+                current_fees = 0;
+                for (CAmount fee : tx_fees) {
+                    // Skip coinbase
+                    if (fee < 0) continue;
+                    current_fees += fee;
+                }
+            }
+
+            CAmount new_fees = 0;
+            for (CAmount fee : new_tmpl->vTxFees) {
+                // Skip coinbase
+                if (fee < 0) continue;
+                new_fees += fee;
+                Assume(options.fee_threshold != MAX_MONEY);
+                if (new_fees >= current_fees + options.fee_threshold) return new_tmpl;
+            }
+        }
+
+        now = NodeClock::now();
+    } while (now < deadline);
+
+    return std::nullopt;
+}
+
+bool ChainTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout)
+{
+    Assume(timeout >= 0ms); // No internal callers should use a negative timeout
+    if (timeout < 0ms) timeout = 0ms;
+    if (timeout > std::chrono::years{100}) timeout = std::chrono::years{100}; // Upper bound to avoid UB in std::chrono
+    auto deadline{std::chrono::steady_clock::now() + timeout};
+    {
+        WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock);
+        // For callers convenience, wait longer than the provided timeout
+        // during startup for the tip to be non-null. That way this function
+        // always returns valid tip information when possible and only
+        // returns null when shutting down, not when timing out.
+        kernel_notifications.m_tip_block_cv.wait(lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) {
+            return kernel_notifications.TipBlock() || chainman.m_interrupt;
+        });
+        if (chainman.m_interrupt) return false;
+        // At this point TipBlock is set, so continue to wait until it is
+        // different then `current_tip` provided by caller.
+        kernel_notifications.m_tip_block_cv.wait_until(lock, deadline, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) {
+            return Assume(kernel_notifications.TipBlock()) != current_tip || chainman.m_interrupt;
+        });
+    }
+
+    if (chainman.m_interrupt) return false;
+
+    return true;
+}
 } // namespace node

src/node/miner.h

@@ -32,6 +32,8 @@ class ChainstateManager;
 namespace Consensus { struct Params; };
 
 namespace node {
+class KernelNotifications;
+
 static const bool DEFAULT_PRINT_MODIFIED_FEE = false;
 
 struct CBlockTemplate
@@ -229,6 +231,16 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman);
 
 /** Apply -blockmintxfee and -blockmaxweight options from ArgsManager to BlockAssembler options. */
 void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& options);
+
+/* Compute the block's merkle root, insert the coinbase transaction and the merkle root into the block */
+void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t version, uint32_t timestamp, uint32_t nonce);
+
+/* Return a new block template when fees rise to a certain threshold or after a new tip; return nullopt if timeout is reached. */
+std::optional<std::unique_ptr<CBlockTemplate>> WaitNext(ChainstateManager& chainman, KernelNotifications& kernel_notifications, CTxMemPool* mempool, const uint256& prev_block_hash,
+                                                        const std::vector<CAmount>& tx_fees, const BlockWaitOptions& options, const BlockAssembler::Options& assemble_options);
+
+/* Waits for the connected tip to change until timeout has elapsed. During node initialization, this will wait until the tip is connected (regardless of `timeout`). */
+bool ChainTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout);
 } // namespace node
 
 #endif // BITCOIN_NODE_MINER_H