validation: fix UB in LoadChainTip

diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index b3e9753e..ff3de298 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -126,6 +126,13 @@ static ChainstateLoadResult CompleteChainstateInitialization(
         }
     }
 
+    // Populate setBlockIndexCandidates in a separate loop, after all LoadChainTip()
+    // calls have finished modifying nSequenceId. Because nSequenceId is used in the
+    // set's comparator, changing it while blocks are in the set would be UB.
+    for (const auto& chainstate : chainman.m_chainstates) {
+        chainstate->PopulateBlockIndexCandidates();
+    }
+
     const auto& chainstates{chainman.m_chainstates};
     if (std::any_of(chainstates.begin(), chainstates.end(),
                     [](const auto& cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->NeedsRedownload(); })) {
diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h
index 1e0d41cd..3ceed569 100644
--- a/src/test/util/chainstate.h
+++ b/src/test/util/chainstate.h
@@ -84,7 +84,6 @@ CreateAndActivateUTXOSnapshot(
             chain.InitCoinsDB(1 << 20, /*in_memory=*/true, /*should_wipe=*/false);
             chain.InitCoinsCache(1 << 20);
             chain.CoinsTip().SetBestBlock(gen_hash);
-            chain.setBlockIndexCandidates.insert(node.chainman->m_blockman.LookupBlockIndex(gen_hash));
             chain.LoadChainTip();
             node.chainman->MaybeRebalanceCaches();
 
@@ -106,6 +105,7 @@ CreateAndActivateUTXOSnapshot(
                 pindex->nSequenceId = 0;
                 pindex = pindex->pprev;
             }
+            chain.PopulateBlockIndexCandidates();
         }
         BlockValidationState state;
         if (!node.chainman->ActiveChainstate().ActivateBestChain(state)) {
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 40f99690..31740f8d 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -76,8 +76,13 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
         LOCK(::cs_main);
         c2.InitCoinsCache(1 << 23);
         c2.CoinsTip().SetBestBlock(active_tip->GetBlockHash());
-        c2.setBlockIndexCandidates.insert(manager.m_blockman.LookupBlockIndex(active_tip->GetBlockHash()));
+        for (const auto& cs : manager.m_chainstates) {
+            cs->ClearBlockIndexCandidates();
+        }
         c2.LoadChainTip();
+        for (const auto& cs : manager.m_chainstates) {
+            cs->PopulateBlockIndexCandidates();
+        }
     }
     BlockValidationState _;
     BOOST_CHECK(c2.ActivateBestChain(_, nullptr));
@@ -492,6 +497,9 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
             BOOST_CHECK(cs->setBlockIndexCandidates.empty());
         }
         chainman.LoadBlockIndex();
+        for (const auto& cs : chainman.m_chainstates) {
+            cs->PopulateBlockIndexCandidates();
+        }
     };
 
     // Ensure that without any assumed-valid BlockIndex entries, only the current tip is
diff --git a/src/validation.cpp b/src/validation.cpp
index 3ed46c1a..a2571787 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4604,6 +4604,12 @@ bool Chainstate::LoadChainTip()
     m_chainman.UpdateIBDStatus();
     tip = m_chain.Tip();
 
+    // nSequenceId is one of the keys used to sort setBlockIndexCandidates. Ensure all
+    // candidate sets are empty to avoid UB, as nSequenceId is about to be modified.
+    for (const auto& cs : m_chainman.m_chainstates) {
+        assert(cs->setBlockIndexCandidates.empty());
+    }
+
     // Make sure our chain tip before shutting down scores better than any other candidate
     // to maintain a consistent best tip over reboots in case of a tie.
     auto target = tip;
@@ -4612,13 +4618,6 @@ bool Chainstate::LoadChainTip()
         target = target->pprev;
     }
 
-    // Block index candidates are loaded before the chain tip, so we need to replace this entry
-    // Otherwise the scoring will be based on the memory address location instead of the nSequenceId
-    setBlockIndexCandidates.erase(tip);
-    TryAddBlockIndexCandidate(tip);
-    PruneBlockIndexCandidates();
-
-    tip = m_chain.Tip();
     LogInfo("Loaded best chain: hashBestChain=%s height=%d date=%s progress=%f",
               tip->GetBlockHash().ToString(),
               m_chain.Height(),
@@ -4923,6 +4922,22 @@ void Chainstate::ClearBlockIndexCandidates()
     setBlockIndexCandidates.clear();
 }
 
+void Chainstate::PopulateBlockIndexCandidates()
+{
+    AssertLockHeld(::cs_main);
+
+    for (CBlockIndex* pindex : m_blockman.GetAllBlockIndices()) {
+        // With assumeutxo, the snapshot block is a candidate for the tip, but it
+        // may not have BLOCK_VALID_TRANSACTIONS (e.g. if we haven't yet downloaded
+        // the block), so we special-case it here.
+        if (pindex == SnapshotBase() || pindex == TargetBlock() ||
+                (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) &&
+                 (pindex->HaveNumChainTxs() || pindex->pprev == nullptr))) {
+            TryAddBlockIndexCandidate(pindex);
+        }
+    }
+}
+
 bool ChainstateManager::LoadBlockIndex()
 {
     AssertLockHeld(cs_main);
@@ -4939,19 +4954,6 @@ bool ChainstateManager::LoadBlockIndex()
 
         for (CBlockIndex* pindex : vSortedByHeight) {
             if (m_interrupt) return false;
-            // If we have an assumeutxo-based chainstate, then the snapshot
-            // block will be a candidate for the tip, but it may not be
-            // VALID_TRANSACTIONS (eg if we haven't yet downloaded the block),
-            // so we special-case the snapshot block as a potential candidate
-            // here.
-            if (pindex == CurrentChainstate().SnapshotBase() ||
-                    (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) &&
-                     (pindex->HaveNumChainTxs() || pindex->pprev == nullptr))) {
-
-                for (const auto& chainstate : m_chainstates) {
-                    chainstate->TryAddBlockIndexCandidate(pindex);
-                }
-            }
             if (pindex->nStatus & BLOCK_FAILED_VALID && (!m_best_invalid || pindex->nChainWork > m_best_invalid->nChainWork)) {
                 m_best_invalid = pindex;
             }
@@ -5743,6 +5745,8 @@ util::Result<CBlockIndex*> ChainstateManager::ActivateSnapshot(
     Chainstate& chainstate{AddChainstate(std::move(snapshot_chainstate))};
     m_blockman.m_snapshot_height = Assert(chainstate.SnapshotBase())->nHeight;
 
+    chainstate.PopulateBlockIndexCandidates();
+
     LogInfo("[snapshot] successfully activated snapshot %s", base_blockhash.ToString());
     LogInfo("[snapshot] (%.2f MB)",
               chainstate.CoinsTip().DynamicMemoryUsage() / (1000 * 1000));
@@ -5971,7 +5975,6 @@ util::Result<void> ChainstateManager::PopulateAndValidateSnapshot(
     assert(index);
     assert(index == snapshot_start_block);
     index->m_chain_tx_count = au_data.m_chain_tx_count;
-    snapshot_chainstate.setBlockIndexCandidates.insert(snapshot_start_block);
 
     LogInfo("[snapshot] validated snapshot (%.2f MB)",
         coins_cache.DynamicMemoryUsage() / (1000 * 1000));
diff --git a/src/validation.h b/src/validation.h
index 675d7061..83f6f04c 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -811,12 +811,16 @@ public:
     /** Ensures we have a genesis block in the block tree, possibly writing one to disk. */
     bool LoadGenesisBlock();
 
+    /** Add a block to the candidate set if it has as much work as the current tip. */
     void TryAddBlockIndexCandidate(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
     void PruneBlockIndexCandidates();
 
     void ClearBlockIndexCandidates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    /** Populate the candidate set by calling TryAddBlockIndexCandidate on all valid block indices. */
+    void PopulateBlockIndexCandidates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
     /** Find the last common block of this chain and a locator. */
     const CBlockIndex* FindForkInGlobalIndex(const CBlockLocator& locator) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);