p2p: Allow block downloads from peers without snapshot block after assumeutxo validation #33604
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33604. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste ConflictsNo conflicts as of last run. |
|
Nice, Concept ACK. |
|
cc @mzumsande |
|
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 |
|
Concept ACK approach seems fine |
stratospher
left a comment
There was a problem hiding this comment.
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
|
Thanks for the links @stratospher! |
|
Concept ACK |
9f946a7 to
6b92a69
Compare
…sumeutxo validation After assumeutxo background validation completes, allow block downloads from peers that don't have the snapshot block in their best chain. Previously, these peers were skipped until restart because `m_chainman.CurrentChainstate().SnapshotBase()` continued returning non-null even after validation finished. Add `m_chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED` check to only apply the restriction while background validation is ongoing.
Add test coverage to ensure peers without the snapshot block in their chain can be used for block downloads after background validation completes. The test fails without the fix in the previous commit.
6b92a69 to
5cd0968
Compare
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
m_chainman.CurrentChainstate().SnapshotBase()continues to return the snapshot base block until restart, even after validation completes. Addedm_chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATEDcheck to only apply the peer restriction while background validation is ongoing.Also added test coverage in
feature_assumeutxo.pythat verifies peers without the snapshot block can be used for block downloads after background validation completes. The test fails without this fix.