From a19a5f92df45e9fa6419d9d8e283af70260dca1a Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 18 Dec 2024 10:42:35 +0700 Subject: [PATCH] Add waitNext() to BlockTemplate interface --- src/interfaces/mining.h | 13 +++++++ src/ipc/capnp/mining.capnp | 1 + src/node/interfaces.cpp | 79 +++++++++++++++++++++++++++++++++++++- src/test/miner_tests.cpp | 30 +++++++++++---- 4 files changed, 114 insertions(+), 9 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index bc5955ded66288..f1902043681bb9 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -56,6 +56,19 @@ class BlockTemplate * @returns if the block was processed, independent of block validity */ virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) = 0; + + /** + * Waits for fees in the next block to rise, a new tip or the timeout. + * + * @param[in] fee_threshold By how much total fees for the next block should rise. + * Default is to not monitor fee changes and only wait for a new chaintip. + * @param[in] timeout How long to wait. Default is forever. + * + * @returns a new BlockTemplate or nullptr if the timeout occurs. Immediately + * returns a new BlockTemplate if called with both fee_threshold and + * timeout set to 0. + */ + virtual std::unique_ptr waitNext(CAmount fee_threshold = MAX_MONEY, MillisecondsDouble timeout = MillisecondsDouble::max()) = 0; }; //! Interface giving clients (RPC, Stratum v2 Template Provider in the future) diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 50b07bda708b75..2b479df8b7692f 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -31,6 +31,7 @@ interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { getWitnessCommitmentIndex @7 (context: Proxy.Context) -> (result: Int32); getCoinbaseMerklePath @8 (context: Proxy.Context) -> (result: List(Data)); submitSolution @9 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (result: Bool); + waitNext @10 (context: Proxy.Context, feeThreshold: Int64, timeout: Float64) -> (result: BlockTemplate); } struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f3b8c6a072f32d..15a786080def84 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -871,7 +871,7 @@ class ChainImpl : public Chain class BlockTemplateImpl : public BlockTemplate { public: - explicit BlockTemplateImpl(std::unique_ptr block_template, NodeContext& node) : m_block_template(std::move(block_template)), m_node(node) + explicit BlockTemplateImpl(BlockAssembler::Options assemble_options, std::unique_ptr block_template, NodeContext& node) : m_assemble_options(std::move(assemble_options)), m_block_template(std::move(block_template)), m_node(node) { assert(m_block_template); } @@ -936,9 +936,84 @@ class BlockTemplateImpl : public BlockTemplate return chainman().ProcessNewBlock(block_ptr, /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/nullptr); } + std::unique_ptr waitNext(CAmount fee_threshold, MillisecondsDouble timeout) override + { + // 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{std::chrono::steady_clock::now()}; + const auto deadline = now + timeout; + const MillisecondsDouble tick{1000}; + + // Lower than OR equal is used here, so that a fee_threshold of 0 combined with + // a timeout of 0 immedidately returns a new template (rather than nullptr). + while (now <= deadline) { + bool tip_changed{false}; + { + WAIT_LOCK(notifications().m_tip_block_mutex, lock); + notifications().m_tip_block_cv.wait_until(lock, std::min(now + tick, deadline), [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) { + // Although C++17 allows safe comparison of optional with + // another T, we need to wait for m_tip_block to be set AND + // for the value to be different than the current_tip value. + tip_changed = notifications().TipBlock() && notifications().TipBlock() != m_block_template->block.hashPrevBlock; + return tip_changed || chainman().m_interrupt; + }); + } + + if (chainman().m_interrupt) return nullptr; + + // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks. + LOCK(::cs_main); + /** + * The only way to determine if fees increased compared to the previous template, + * is to generate a fresh template. Cluster Mempool may allow for a more efficient + * way to determine how much (approximate) fees for the next block increased. + * + * We'll also create a new template if the tip changed during the last tick. + */ + if (fee_threshold < MAX_MONEY || tip_changed) { + auto block_template{std::make_unique(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 (block_template->m_block_template->block.hashPrevBlock != m_block_template->block.hashPrevBlock) { + return block_template; + } + + // 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 : block_template->m_block_template->vTxFees) { + // Skip coinbase + if (fee < 0) continue; + new_fees += fee; + if (new_fees >= current_fees + fee_threshold) return block_template; + } + } + + now = std::chrono::steady_clock::now(); + } + + return nullptr; + } + + const BlockAssembler::Options m_assemble_options; + const std::unique_ptr 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; }; @@ -985,7 +1060,7 @@ class MinerImpl : public Mining { BlockAssembler::Options assemble_options{options}; ApplyArgsManOptions(*Assert(m_node.args), assemble_options); - return std::make_unique(BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(), m_node); + return std::make_unique(assemble_options, BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(), m_node); } NodeContext* context() override { return &m_node; } diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index ee1cc3aaf4efbc..784aa354052a01 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -159,7 +159,15 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const tx.vout[0].nValue = 5000000000LL - 1000 - 50000 - feeToUse; Txid hashLowFeeTx = tx.GetHash(); AddToMempool(tx_mempool, entry.Fee(feeToUse).FromTx(tx)); - block_template = miner->createNewBlock(options); + + // waitNext() should return nullptr because there is no better template + auto should_be_nullptr = block_template->waitNext(1, MillisecondsDouble{0}); + BOOST_REQUIRE(should_be_nullptr == nullptr); + + // Unless it's called with fee_threshold=0 and timeout=0 + block_template = block_template->waitNext(0, MillisecondsDouble{0}); + BOOST_REQUIRE(block_template != nullptr); + BOOST_REQUIRE(block_template); block = block_template->getBlock(); // Verify that the free tx and the low fee tx didn't get selected @@ -175,7 +183,9 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee hashLowFeeTx = tx.GetHash(); AddToMempool(tx_mempool, entry.Fee(feeToUse + 2).FromTx(tx)); - block_template = miner->createNewBlock(options); + + // waitNext() should return if fees for the new template are at least 1 sat up + block_template = block_template->waitNext(1); BOOST_REQUIRE(block_template); block = block_template->getBlock(); BOOST_REQUIRE_EQUAL(block.vtx.size(), 6U); @@ -650,7 +660,9 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) const int current_height{miner->getTip()->height}; // Simple block creation, nothing special yet: - block_template = miner->createNewBlock(options); + if (current_height % 2 == 0) { + block_template = miner->createNewBlock(options); + } // for odd heights the next template is created at the end of this loop BOOST_REQUIRE(block_template); CBlock block{block_template->getBlock()}; @@ -677,13 +689,17 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) if (current_height % 2 == 0) { BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, /*force_processing=*/true, /*min_pow_checked=*/true, nullptr)); } else { - BOOST_REQUIRE(block_template->submitSolution(block.nVersion, block.nTime, block.nNonce, MakeTransactionRef(std::move(txCoinbase)))); + BOOST_REQUIRE(block_template->submitSolution(block.nVersion, block.nTime, block.nNonce, MakeTransactionRef(txCoinbase))); } // ProcessNewBlock and submitSolution do not check if the new block is // valid. If it is, they will wait for the tip to update. Therefore the - // following check will either return immediately, or be stuck indefinately. - // It's mainly there to increase test coverage. - miner->waitTipChanged(block.hashPrevBlock); + // following checks will either return immediately, or be stuck indefinately. + // They're mainly there to increase test coverage. + if (current_height % 2 == 0) { + block_template = block_template->waitNext(); + } else { + miner->waitTipChanged(block.hashPrevBlock); + } } LOCK(cs_main);