p2p: Allow block downloads from peers without snapshot block after assumeutxo validation

[stringintech]

Currently, after assumeutxo background validation finishes, the node continues to skip peers that don't have the snapshot block in their best chain until restart. This unnecessarily excludes peers from block downloads even though the background sync has completed and undo data is available.

The restriction persists because GetSnapshotBaseBlock() continues to return the snapshot base block even after validation completes. While m_ibd_chainstate->m_disabled is set to true when validation finishes, the active chainstate remains the snapshot chainstate until the next restart, when LoadChainstate() tears down both chainstates and reinitializes a single normal (non-snapshot) chainstate.

Added !m_chainman.IsSnapshotValidated() check to only apply the peer restriction while background validation is ongoing. Also added test coverage in feature_assumeutxo.py that verifies peers without the snapshot block can be used for block downloads after background validation completes. The test fails without this fix.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33604.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	stratospher
Concept ACK	TheCharlatan, Raimo33, mzumsande

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

[TheCharlatan]

Nice, Concept ACK.

[fanquake]

cc @mzumsande

[stringintech]

This change allows the snapshot chainstate to reorg to chains that don't include the snapshot block after validation completes (not sure yet if this is the only mechanism that would allow such reorgs), so we must not assume anywhere in the codebase that SnapshotBase() is always an ancestor of the active tip. I'll take a closer look to see if any such assumptions exist, but I thought this was worth noting here.

[Raimo33]

Concept ACK

approach seems fine

[stratospher]

ACK 9f946a7.

nice! I think this aligns with the original intention to restrict the peer only when background sync isn't over in #29519 (comment) where this code was introduced.

looks like we might incorrectly set pindexLastCommonBlock in L1401 to the previous best known block tip again in this scenario. but it's not a problem since we immediately reset it back to the correct value + comments suggest it's ok to set it incorrectly and that's way better than introducing more conditions.

also slightly related: #30288

[stringintech]

Thanks for the links @stratospher!
FYI, calculating pindexLastCommonBlock is undergoing some improvements in #32180, and I will probably have to rebase onto that change. But either way, as you mentioned, it shouldn't cause any issues.

[mzumsande]

Concept ACK