diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index dd5cfe5ccc66b..bc5955ded6628 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -93,31 +93,6 @@ class Mining */ virtual std::unique_ptr createNewBlock(const node::BlockCreateOptions& options = {}) = 0; - /** - * Processes new block. A valid new block is automatically relayed to peers. - * - * @param[in] block The block we want to process. - * @param[out] new_block A boolean which is set to indicate if the block was first received via this call - * @returns If the block was processed, independently of block validity - */ - virtual bool processNewBlock(const std::shared_ptr& block, bool* new_block) = 0; - - //! Return the number of transaction updates in the mempool, - //! used to decide whether to make a new block template. - virtual unsigned int getTransactionsUpdated() = 0; - - /** - * Check a block is completely valid from start to finish. - * Only works on top of our current best block. - * Does not check proof-of-work. - * - * @param[in] block the block to validate - * @param[in] check_merkle_root call CheckMerkleRoot() - * @param[out] state details of why a block failed to validate - * @returns false if it does not build on the current tip, or any of the checks fail - */ - virtual bool testBlockValidity(const CBlock& block, bool check_merkle_root, BlockValidationState& state) = 0; - //! Get internal node context. Useful for RPC and testing, //! but not accessible across processes. virtual node::NodeContext* context() { return nullptr; } diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 5af37b725ab25..50b07bda708b7 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -18,9 +18,6 @@ interface Mining $Proxy.wrap("interfaces::Mining") { getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool); waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef); createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate); - processNewBlock @5 (context :Proxy.Context, block: Data) -> (newBlock: Bool, result: Bool); - getTransactionsUpdated @6 (context :Proxy.Context) -> (result: UInt32); - testBlockValidity @7 (context :Proxy.Context, block: Data, checkMerkleRoot: Bool) -> (state: BlockValidationState, result: Bool); } interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 20562568534c8..b7dbd8c6ab184 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -979,29 +979,6 @@ class MinerImpl : public Mining return BlockRef{chainman().ActiveChain().Tip()->GetBlockHash(), chainman().ActiveChain().Tip()->nHeight}; } - bool processNewBlock(const std::shared_ptr& block, bool* new_block) override - { - return chainman().ProcessNewBlock(block, /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/new_block); - } - - unsigned int getTransactionsUpdated() override - { - return context()->mempool->GetTransactionsUpdated(); - } - - bool testBlockValidity(const CBlock& block, bool check_merkle_root, BlockValidationState& state) override - { - LOCK(cs_main); - CBlockIndex* tip{chainman().ActiveChain().Tip()}; - // Fail if the tip updated before the lock was taken - if (block.hashPrevBlock != tip->GetBlockHash()) { - state.Error("Block does not connect to current chain tip."); - return false; - } - - return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, tip, /*fCheckPOW=*/false, check_merkle_root); - } - std::unique_ptr createNewBlock(const BlockCreateOptions& options) override { BlockAssembler::Options assemble_options{options}; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 5516e1c0d9991..3b05f84eee4f6 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -131,7 +131,7 @@ static RPCHelpMan getnetworkhashps() }; } -static bool GenerateBlock(ChainstateManager& chainman, Mining& miner, CBlock&& block, uint64_t& max_tries, std::shared_ptr& block_out, bool process_new_block) +static bool GenerateBlock(ChainstateManager& chainman, CBlock&& block, uint64_t& max_tries, std::shared_ptr& block_out, bool process_new_block) { block_out.reset(); block.hashMerkleRoot = BlockMerkleRoot(block); @@ -151,7 +151,7 @@ static bool GenerateBlock(ChainstateManager& chainman, Mining& miner, CBlock&& b if (!process_new_block) return true; - if (!miner.processNewBlock(block_out, nullptr)) { + if (!chainman.ProcessNewBlock(block_out, /*force_processing=*/true, /*min_pow_checked=*/true, nullptr)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted"); } @@ -166,7 +166,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const CHECK_NONFATAL(block_template); std::shared_ptr block_out; - if (!GenerateBlock(chainman, miner, block_template->getBlock(), nMaxTries, block_out, /*process_new_block=*/true)) { + if (!GenerateBlock(chainman, block_template->getBlock(), nMaxTries, block_out, /*process_new_block=*/true)) { break; } @@ -384,16 +384,17 @@ static RPCHelpMan generateblock() RegenerateCommitments(block, chainman); { + LOCK(::cs_main); BlockValidationState state; - if (!miner.testBlockValidity(block, /*check_merkle_root=*/false, state)) { - throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("testBlockValidity failed: %s", state.ToString())); + if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/false)) { + throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString())); } } std::shared_ptr block_out; uint64_t max_tries{DEFAULT_MAX_TRIES}; - if (!GenerateBlock(chainman, miner, std::move(block), max_tries, block_out, process_new_block) || !block_out) { + if (!GenerateBlock(chainman, std::move(block), max_tries, block_out, process_new_block) || !block_out) { throw JSONRPCError(RPC_MISC_ERROR, "Failed to make block."); } @@ -709,12 +710,12 @@ static RPCHelpMan getblocktemplate() return "duplicate-inconclusive"; } - // testBlockValidity only supports blocks built on the current Tip + // TestBlockValidity only supports blocks built on the current Tip if (block.hashPrevBlock != tip) { return "inconclusive-not-best-prevblk"; } BlockValidationState state; - miner.testBlockValidity(block, /*check_merkle_root=*/true, state); + TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/true); return BIP22ValidationResult(state); } @@ -742,6 +743,7 @@ static RPCHelpMan getblocktemplate() } static unsigned int nTransactionsUpdatedLast; + const CTxMemPool& mempool = EnsureMemPool(node); if (!lpval.isNull()) { @@ -772,7 +774,7 @@ static RPCHelpMan getblocktemplate() tip = miner.waitTipChanged(hashWatchedChain, checktxtime).hash; // Timeout: Check transactions for update // without holding the mempool lock to avoid deadlocks - if (miner.getTransactionsUpdated() != nTransactionsUpdatedLastLP) + if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP) break; checktxtime = std::chrono::seconds(10); } @@ -803,13 +805,13 @@ static RPCHelpMan getblocktemplate() static int64_t time_start; static std::unique_ptr block_template; if (!pindexPrev || pindexPrev->GetBlockHash() != tip || - (miner.getTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - time_start > 5)) + (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - time_start > 5)) { // Clear pindexPrev so future calls make a new block, despite any failures from here on pindexPrev = nullptr; // Store the pindexBest used before createNewBlock, to avoid races - nTransactionsUpdatedLast = miner.getTransactionsUpdated(); + nTransactionsUpdatedLast = mempool.GetTransactionsUpdated(); CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(tip); time_start = GetTime(); @@ -1032,13 +1034,10 @@ static RPCHelpMan submitblock() } } - NodeContext& node = EnsureAnyNodeContext(request.context); - Mining& miner = EnsureMining(node); - bool new_block; auto sc = std::make_shared(block.GetHash()); CHECK_NONFATAL(chainman.m_options.signals)->RegisterSharedValidationInterface(sc); - bool accepted = miner.processNewBlock(blockptr, /*new_block=*/&new_block); + bool accepted = chainman.ProcessNewBlock(blockptr, /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/&new_block); CHECK_NONFATAL(chainman.m_options.signals)->UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { return "duplicate"; diff --git a/test/functional/rpc_generate.py b/test/functional/rpc_generate.py index 68de9006644b5..74f31f45716e0 100755 --- a/test/functional/rpc_generate.py +++ b/test/functional/rpc_generate.py @@ -87,7 +87,7 @@ def test_generateblock(self): txid1 = miniwallet.send_self_transfer(from_node=node)['txid'] utxo1 = miniwallet.get_utxo(txid=txid1) rawtx2 = miniwallet.create_self_transfer(utxo_to_spend=utxo1)['hex'] - assert_raises_rpc_error(-25, 'testBlockValidity failed: bad-txns-inputs-missingorspent', self.generateblock, node, address, [rawtx2, txid1]) + assert_raises_rpc_error(-25, 'TestBlockValidity failed: bad-txns-inputs-missingorspent', self.generateblock, node, address, [rawtx2, txid1]) self.log.info('Fail to generate block with txid not in mempool') missing_txid = '0000000000000000000000000000000000000000000000000000000000000000'