Merge bitcoin/bitcoin#34708: validation: refactor: remove ConnectTrace

2f8f2e900118c98699db91075bd1b72e6460c6b1 validation: remove ConnectTrace wrapper class (stickies-v)
b83de7f28e71f0b10a999327395198fbcb02f44b validation: remove sentinel block from ConnectTrace (stickies-v)

Pull request description:

  The sentinel pattern in `ConnectTrace` has been unnecessary since conflicted transaction tracking was removed in 5613f9842b4000fed088b8cf7b99674c328d15e1. Without that tracking `ConnectTrace` is a trivial wrapper around `std::vector`, so it seems better to just replace it with the vector directly.

  Also modernize/update naming along the way, renaming `PerBlockConnectTrace` to `ConnectedBlock`

  Refactor, no behaviour change.

ACKs for top commit:
  HowHsu:
    ACK 2f8f2e900118c98699db91075bd1b72e6460c6b1
  sedited:
    ACK 2f8f2e900118c98699db91075bd1b72e6460c6b1
  w0xlt:
    reACK 2f8f2e9001

Tree-SHA512: 0045fcdc1178a160e31ef9d44dcd5fddd21c30c53ed06e84beacddb0b73e7b8120fee874256d1b9ceae45da65164a2e5531992bd374f8d57b6a8455a5354fe57
This commit is contained in:
merge-script 2026-03-12 10:08:53 +00:00
commit ab64277375
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1
2 changed files with 15 additions and 51 deletions

View File

@ -2987,57 +2987,22 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra
return true;
}
struct PerBlockConnectTrace {
CBlockIndex* pindex = nullptr;
struct ConnectedBlock {
const CBlockIndex* pindex;
std::shared_ptr<const CBlock> pblock;
PerBlockConnectTrace() = default;
};
/**
* Used to track blocks whose transactions were applied to the UTXO state as a
* part of a single ActivateBestChainStep call.
*
* This class is single-use, once you call GetBlocksConnected() you have to throw
* it away and make a new one.
*/
class ConnectTrace {
private:
std::vector<PerBlockConnectTrace> blocksConnected;
public:
explicit ConnectTrace() : blocksConnected(1) {}
void BlockConnected(CBlockIndex* pindex, std::shared_ptr<const CBlock> pblock) {
assert(!blocksConnected.back().pindex);
assert(pindex);
assert(pblock);
blocksConnected.back().pindex = pindex;
blocksConnected.back().pblock = std::move(pblock);
blocksConnected.emplace_back();
}
std::vector<PerBlockConnectTrace>& GetBlocksConnected() {
// We always keep one extra block at the end of our list because
// blocks are added after all the conflicted transactions have
// been filled in. Thus, the last entry should always be an empty
// one waiting for the transactions from the next block. We pop
// the last entry here to make sure the list we return is sane.
assert(!blocksConnected.back().pindex);
blocksConnected.pop_back();
return blocksConnected;
}
};
/**
* Connect a new block to m_chain. block_to_connect is either nullptr or a pointer to a CBlock
* corresponding to pindexNew, to bypass loading it again from disk.
*
* The block is added to connectTrace if connection succeeds.
* The block is added to connected_blocks if connection succeeds.
*/
bool Chainstate::ConnectTip(
BlockValidationState& state,
CBlockIndex* pindexNew,
std::shared_ptr<const CBlock> block_to_connect,
ConnectTrace& connectTrace,
std::vector<ConnectedBlock>& connected_blocks,
DisconnectedBlockTransactions& disconnectpool)
{
AssertLockHeld(cs_main);
@ -3134,7 +3099,7 @@ bool Chainstate::ConnectTip(
Chainstate& current_cs{m_chainman.CurrentChainstate()};
m_chainman.MaybeValidateSnapshot(*this, current_cs);
connectTrace.BlockConnected(pindexNew, std::move(block_to_connect));
connected_blocks.emplace_back(pindexNew, std::move(block_to_connect));
return true;
}
@ -3219,7 +3184,7 @@ void Chainstate::PruneBlockIndexCandidates() {
*
* @returns true unless a system error occurred
*/
bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace)
bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, std::vector<ConnectedBlock>& connected_blocks)
{
AssertLockHeld(cs_main);
if (m_mempool) AssertLockHeld(m_mempool->cs);
@ -3264,7 +3229,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex*
// Connect new blocks.
for (CBlockIndex* pindexConnect : vpindexToConnect | std::views::reverse) {
if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connectTrace, disconnectpool)) {
if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connected_blocks, disconnectpool)) {
if (state.IsInvalid()) {
// The block violates a consensus rule.
if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) {
@ -3389,7 +3354,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
{
LOCK(cs_main);
{
// Lock transaction pool for at least as long as it takes for connectTrace to be consumed
// Lock transaction pool for at least as long as it takes for connected_blocks to be consumed
LOCK(MempoolMutex());
const bool was_in_ibd = m_chainman.IsInitialBlockDownload();
CBlockIndex* starting_tip = m_chain.Tip();
@ -3397,7 +3362,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
do {
// We absolutely may not unlock cs_main until we've made forward progress
// (with the exception of shutdown due to hardware issues, low disk space, etc).
ConnectTrace connectTrace; // Destructed before cs_main is unlocked
std::vector<ConnectedBlock> connected_blocks; // Destructed before cs_main is unlocked
if (pindexMostWork == nullptr) {
pindexMostWork = FindMostWorkChain();
@ -3414,7 +3379,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
// in case snapshot validation is completed during ActivateBestChainStep, the
// result of GetRole() changes from BACKGROUND to NORMAL.
const ChainstateRole chainstate_role{this->GetRole()};
if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) {
if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connected_blocks)) {
// A system error occurred
return false;
}
@ -3426,10 +3391,9 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
}
pindexNewTip = m_chain.Tip();
for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) {
assert(trace.pblock && trace.pindex);
for (const auto& [index, block] : connected_blocks) {
if (m_chainman.m_options.signals) {
m_chainman.m_options.signals->BlockConnected(chainstate_role, trace.pblock, trace.pindex);
m_chainman.m_options.signals->BlockConnected(chainstate_role, Assert(block), Assert(index));
}
}

View File

@ -455,7 +455,7 @@ enum DisconnectResult
DISCONNECT_FAILED // Something else went wrong.
};
class ConnectTrace;
struct ConnectedBlock;
/** @see Chainstate::FlushStateToDisk */
inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "FORCE_FLUSH", "FORCE_SYNC"};
@ -851,12 +851,12 @@ public:
std::pair<int, int> GetPruneRange(int last_height_can_prune) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
protected:
bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, std::vector<ConnectedBlock>& connected_blocks) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
bool ConnectTip(
BlockValidationState& state,
CBlockIndex* pindexNew,
std::shared_ptr<const CBlock> block_to_connect,
ConnectTrace& connectTrace,
std::vector<ConnectedBlock>& connected_blocks,
DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
void InvalidBlockFound(CBlockIndex* pindex, const BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);