From fa62c8b1f04a5386ffa171aeff713d55bd874cbe Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 24 Dec 2024 15:55:22 +0100 Subject: [PATCH 1/2] rpc: Extend scope of validation mutex in generateblock The mutex (required by TestBlockValidity) must be held after creating the block, until TestBlockValidity is called. Otherwise, it is possible that the chain advances in the meantime and leads to a crash in TestBlockValidity: Assertion failed: pindexPrev && pindexPrev == chainstate.m_chain.Tip() (validation.cpp: TestBlockValidity: 4338) The diff can be reviewed with the git options --ignore-all-space --function-context --- src/rpc/mining.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 3b05f84eee..6cadd324c9 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2010 Satoshi Nakamoto -// Copyright (c) 2009-2022 The Bitcoin Core developers +// Copyright (c) 2009-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -371,22 +371,22 @@ static RPCHelpMan generateblock() ChainstateManager& chainman = EnsureChainman(node); { - std::unique_ptr block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script})}; - CHECK_NONFATAL(block_template); + LOCK(chainman.GetMutex()); + { + std::unique_ptr block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script})}; + CHECK_NONFATAL(block_template); - block = block_template->getBlock(); - } + block = block_template->getBlock(); + } - CHECK_NONFATAL(block.vtx.size() == 1); + CHECK_NONFATAL(block.vtx.size() == 1); - // Add transactions - block.vtx.insert(block.vtx.end(), txs.begin(), txs.end()); - RegenerateCommitments(block, chainman); + // Add transactions + block.vtx.insert(block.vtx.end(), txs.begin(), txs.end()); + RegenerateCommitments(block, chainman); - { - LOCK(::cs_main); BlockValidationState state; - if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/false)) { + 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())); } } From fa63b8232f38e78d3c6413fa7d51809f376de75c Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 25 Dec 2024 04:45:56 +0100 Subject: [PATCH 2/2] test: generateblocks called by multiple threads Co-Authored-By: David Gumberg --- test/functional/rpc_generate.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/functional/rpc_generate.py b/test/functional/rpc_generate.py index 74f31f4571..618bbb2e00 100755 --- a/test/functional/rpc_generate.py +++ b/test/functional/rpc_generate.py @@ -1,9 +1,11 @@ #!/usr/bin/env python3 -# Copyright (c) 2020-2022 The Bitcoin Core developers +# Copyright (c) 2020-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test generate* RPCs.""" +from concurrent.futures import ThreadPoolExecutor + from test_framework.test_framework import BitcoinTestFramework from test_framework.wallet import MiniWallet from test_framework.util import ( @@ -83,6 +85,13 @@ class RPCGenerateTest(BitcoinTestFramework): txid = block['tx'][1] assert_equal(node.getrawtransaction(txid=txid, verbose=False, blockhash=hash), rawtx) + # Ensure that generateblock can be called concurrently by many threads. + self.log.info('Generate blocks in parallel') + generate_50_blocks = lambda n: [n.generateblock(output=address, transactions=[]) for _ in range(50)] + rpcs = [node.cli for _ in range(6)] + with ThreadPoolExecutor(max_workers=len(rpcs)) as threads: + list(threads.map(generate_50_blocks, rpcs)) + self.log.info('Fail to generate block with out of order txs') txid1 = miniwallet.send_self_transfer(from_node=node)['txid'] utxo1 = miniwallet.get_utxo(txid=txid1)