Incrementally update sv2 block template

Author: Sjors

src/sv2/template_provider.cpp

@@ -74,6 +74,31 @@ void Sv2TemplateProvider::StopThreads()
     }
 }
 
+class Timer {
+private:
+    std::chrono::seconds m_interval;
+    std::chrono::seconds m_last_triggered;
+
+public:
+    Timer(std::chrono::seconds interval) : m_interval(interval) {
+        reset();
+    }
+
+    bool trigger() {
+        auto now{GetTime<std::chrono::seconds>()};
+        if (now - m_last_triggered >= m_interval) {
+            m_last_triggered = now;
+            return true;
+        }
+        return false;
+    }
+
+    void reset() {
+        auto now{GetTime<std::chrono::seconds>()};
+        m_last_triggered = now;
+    }
+};
+
 void Sv2TemplateProvider::ThreadSv2Handler()
 {
     // Wait for the node chainstate to be ready if needed.
@@ -96,6 +121,8 @@ void Sv2TemplateProvider::ThreadSv2Handler()
         std::this_thread::sleep_for(1000ms);
     }
 
+    Timer timer(m_options.fee_check_interval);
+
     while (!m_flag_interrupt_sv2) {
         // We start with one template per client, which has an interface through
         // which we monitor for better templates.
@@ -144,23 +171,27 @@ void Sv2TemplateProvider::ThreadSv2Handler()
             }
         });
 
+        // Do not send templates with improved fees more frequently than the fee check interval
+        const bool check_fees{timer.trigger()};
         bool new_template{false};
 
         // Delay event loop is no client if fully connected
         if (!first_client_id) std::this_thread::sleep_for(1000ms);
 
-        // For the first connected client, wait for a new chaintip
-        m_connman->ForEachClient([this, first_client_id, &new_template](Sv2Client& client) {
-            if (!first_client_id || client.m_id != first_client_id.value()) return;
+        // For the first connected client, wait for fees to rise.
+        m_connman->ForEachClient([this, first_client_id, check_fees, &new_template](Sv2Client& client) {
+            if (!first_client_id || client.m_id != first_client_id) return;
             Assert(client.m_coinbase_output_data_size_recv);
 
             LOCK(m_tp_mutex);
             auto block_template_entry{m_block_template_cache.find(client.m_best_template_id)};
 
+            CAmount fee_delta{check_fees ? m_options.fee_delta : MAX_MONEY};
+
             // We give waitNext() a timeout of 1 second to prevent it from generating
             // new templates too quickly. During this wait we're not serving newly connected clients.
             // This can be cleaned up by having every client run its own thread.
-            auto block_template = block_template_entry->second->waitNext(MAX_MONEY, MillisecondsDouble{1000});
+            auto block_template = block_template_entry->second->waitNext(fee_delta, MillisecondsDouble{1000});
             if (block_template) {
                 new_template = true;
 
@@ -322,6 +353,7 @@ void Sv2TemplateProvider::SubmitSolution(node::Sv2SubmitSolutionMsg solution)
         block_template->submitSolution(solution.m_version, solution.m_header_timestamp, solution.m_header_nonce, solution.m_coinbase_tx);
 }
 
+// TODO: get rid of this helper function
 Sv2TemplateProvider::NewWorkSet Sv2TemplateProvider::BuildNewWorkSet(bool future_template, unsigned int coinbase_output_max_additional_size)
 {
     AssertLockHeld(m_tp_mutex);

src/sv2/template_provider.h

@@ -24,6 +24,16 @@ struct Sv2TemplateProviderOptions
      * The listening port for the server.
      */
     uint16_t port{8336};
+
+    /**
+     * Minimum fee delta to send new template upstream
+     */
+    CAmount fee_delta{1000};
+
+    /**
+     * Block template update interval (to check for increased fees)
+     */
+    std::chrono::seconds fee_check_interval{30};
 };
 
 /**

src/test/sv2_template_provider_tests.cpp

@@ -1,11 +1,15 @@
+#include <addresstype.h>
 #include <boost/test/unit_test.hpp>
 #include <interfaces/mining.h>
 #include <node/miner.h>
+#include <node/transaction.h>
 #include <sv2/messages.h>
 #include <sv2/template_provider.h>
 #include <test/util/net.h>
 #include <test/util/setup_common.h>
+#include <test/util/transaction_utils.h>
 #include <util/sock.h>
+#include <util/strencodings.h>
 
 #include <memory>
 
@@ -164,7 +168,47 @@ BOOST_AUTO_TEST_CASE(client_tests)
     // There should now be one template
     BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 1);
 
-    // Get the template id
+    // Move mock time
+    // If the mempool doesn't change, no new template is generated.
+    SetMockTime(GetMockTime() + std::chrono::seconds{10});
+    BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 1);
+
+    // Create a transaction with a large fee
+    size_t tx_size;
+    CKey key = GenerateRandomKey();
+    CScript locking_script = GetScriptForDestination(PKHash(key.GetPubKey()));
+    // Don't hold on to the transaction
+    {
+        LOCK(cs_main);
+        BOOST_REQUIRE_EQUAL(m_node.mempool->size(), 0);
+
+        auto mtx = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0,
+                                                        /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
+                                                        /*output_destination=*/locking_script,
+                                                        /*output_amount=*/CAmount(49 * COIN), /*submit=*/true);
+        CTransactionRef tx = MakeTransactionRef(mtx);
+
+        // Get serialized transaction size
+        DataStream ss;
+        ss << TX_WITH_WITNESS(tx);
+        tx_size = ss.size();
+
+        BOOST_REQUIRE_EQUAL(m_node.mempool->size(), 1);
+    }
+
+    // Move mock time
+    SetMockTime(GetMockTime() + std::chrono::seconds{tester.m_tp_options.fee_check_interval});
+
+    // Briefly wait for block creation
+    UninterruptibleSleep(std::chrono::milliseconds{200});
+
+    // Expect our peer to receive a NewTemplate message
+    // This time it should contain the 32 byte prevhash (unchanged)
+    constexpr size_t expected_len = SV2_HEADER_ENCRYPTED_SIZE + 91 + 32 + Poly1305::TAGLEN;
+    BOOST_TEST_MESSAGE("Receive NewTemplate");
+    BOOST_REQUIRE_EQUAL(tester.PeerReceiveBytes(), expected_len);
+
+    // Get the latest template id
     uint64_t template_id = 0;
     {
         LOCK(tester.m_tp->m_tp_mutex);
@@ -175,6 +219,10 @@ BOOST_AUTO_TEST_CASE(client_tests)
         }
     }
 
+    BOOST_REQUIRE_EQUAL(template_id, 2);
+
+    UninterruptibleSleep(std::chrono::milliseconds{200});
+
     // Have the peer send us RequestTransactionData
     // We should reply with RequestTransactionData.Success
     node::Sv2NetHeader req_tx_data_header{node::Sv2MsgType::REQUEST_TRANSACTION_DATA, 8};
@@ -188,15 +236,49 @@ BOOST_AUTO_TEST_CASE(client_tests)
     tester.receiveMessage(msg);
     const size_t template_id_size = 8;
     const size_t excess_data_size = 2 + 32;
-    size_t tx_list_size = 2; // no transactions, so transaction_list is 0x0100
+    size_t tx_list_size = 2 + 3 + tx_size;
+    BOOST_TEST_MESSAGE("Receive RequestTransactionData.Success");
+    BOOST_REQUIRE_EQUAL(tester.PeerReceiveBytes(), SV2_HEADER_ENCRYPTED_SIZE + template_id_size + excess_data_size + tx_list_size + Poly1305::TAGLEN);
+    {
+        LOCK(cs_main);
+
+        // RBF the transaction with with > DEFAULT_SV2_FEE_DELTA
+        CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0,
+                                                    /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
+                                                    /*output_destination=*/locking_script,
+                                                    /*output_amount=*/CAmount(48 * COIN), /*submit=*/true);
+
+        BOOST_REQUIRE_EQUAL(m_node.mempool->size(), 1);
+    }
+
+    // Move mock time
+    SetMockTime(GetMockTime() + std::chrono::seconds{tester.m_tp_options.fee_check_interval});
+
+    // Briefly wait for the timer in ThreadSv2Handler and block creation
+    UninterruptibleSleep(std::chrono::milliseconds{200});
+
+    // Wait a bit more for macOS native CI
+    UninterruptibleSleep(std::chrono::milliseconds{1000});
+
+    // Expect our peer to receive a NewTemplate message
+    BOOST_REQUIRE_EQUAL(tester.PeerReceiveBytes(), SV2_HEADER_ENCRYPTED_SIZE + 91 + 32 + Poly1305::TAGLEN);
+
+    // Check that there's a new template
+    BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 3);
+
+    // Have the peer send us RequestTransactionData for the old template
+    // We should reply with RequestTransactionData.Success, and the original
+    // (replaced) transaction
+    tester.receiveMessage(msg);
+    tx_list_size = 2 + 3 + tx_size;
     BOOST_REQUIRE_EQUAL(tester.PeerReceiveBytes(), SV2_HEADER_ENCRYPTED_SIZE + template_id_size + excess_data_size + tx_list_size + Poly1305::TAGLEN);
 
-    // Create a new block
+    BOOST_TEST_MESSAGE("Create a new block");
     mineBlocks(1);
 
     // We should send out another NewTemplate and SetNewPrevHash
     // The new template contains the new prevhash.
-    BOOST_REQUIRE_EQUAL(tester.PeerReceiveBytes(), 2 * SV2_HEADER_ENCRYPTED_SIZE + 91 + 80 + 2 * Poly1305::TAGLEN);
+    BOOST_REQUIRE_EQUAL(tester.PeerReceiveBytes(), 2 * SV2_HEADER_ENCRYPTED_SIZE + 91 + 32 + 80 + 2 * Poly1305::TAGLEN);
     // The SetNewPrevHash message is redundant
     // TODO: don't send it?
     // Background: in the future we want to send an empty or optimistic template
@@ -205,7 +287,7 @@ BOOST_AUTO_TEST_CASE(client_tests)
     //             a new block, and construct a better template _after_ that.
 
     // Templates are briefly preserved
-    BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 2);
+    BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 4);
 
     // Do not provide transactions for stale templates
     // TODO
@@ -217,6 +299,9 @@ BOOST_AUTO_TEST_CASE(client_tests)
     SetMockTime(GetMockTime() + std::chrono::seconds{15});
     UninterruptibleSleep(std::chrono::milliseconds{1100});
     BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 1);
+
+    // Mine a block in order to interrupt waitNext()
+    mineBlocks(1);
 }
 
 BOOST_AUTO_TEST_SUITE_END()