Merge bitcoin/bitcoin#31563: rpc: Extend scope of validation mutex in generateblock

fa63b8232f test: generateblocks called by multiple threads (MarcoFalke)
fa62c8b1f0 rpc: Extend scope of validation mutex in generateblock (MarcoFalke)

Pull request description:

  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)`

  Fixes #31562

ACKs for top commit:
  davidgumberg:
    reACK fa63b8232f
  achow101:
    ACK fa63b8232f
  ismaelsadeeq:
    re-ACK fa63b8232f
  mzumsande:
    utACK fa63b8232f

Tree-SHA512: 3dfda1192af52546ab11fbffe44af8713073763863f4a63fbcdbdf95b1c6cbeb003dc4b8b29e7ec67362238ad15e07d8f6855832a0c68dc5370254f8cbf9445c
This commit is contained in:
Ava Chow 2024-12-30 14:49:21 -05:00
commit 87c9ebd889
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
2 changed files with 22 additions and 13 deletions

View file

@ -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.
@ -370,6 +370,8 @@ static RPCHelpMan generateblock()
CBlock block;
ChainstateManager& chainman = EnsureChainman(node);
{
LOCK(chainman.GetMutex());
{
std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script})};
CHECK_NONFATAL(block_template);
@ -383,8 +385,6 @@ static RPCHelpMan generateblock()
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)) {
throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString()));

View file

@ -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)