rpc, logging: add backgroundvalidation to getblockchaininfo #33259

pull polespinasa wants to merge 4 commits into bitcoin:master from polespinasa:2025-08-26-getblockchaininfo-backgroundvalidation changing 6 files +81 −8
  1. polespinasa commented at 11:02 PM on August 26, 2025: member

    getblockchaininfo returns verificationprogress=1 and initialblockdownload=false even if there's background validation. This PR adds information about background validation to rpc getblockchaininfo in a similar way to validationprogress does.

    If assume utxo was used the output of a "sync" node performing background validation:

    $ ./build/bin/bitcoin-cli getblockchaininfo
    ...
      "mediantime": 1756933740,
      "verificationprogress": 1,
      "initialblockdownload": false,
      "backgroundvalidation": {
        "snapshotheight": 880000,
        "blocks": 527589,
        "bestblockhash": "0000000000000000002326308420fa5ccd28a9155217f4d1896ab443d84148fa",
        "mediantime": 1529076654,
        "chainwork": "0000000000000000000000000000000000000000020c92fab9e5e1d8ed2d8dbc",
        "verificationprogress": 0.2815790617966284
      },
      "chainwork": "0000000000000000000000000000000000000000df97866c410b0302954919d2",
      "size_on_disk": 61198817285,
    
    ...
    

    If assume utxo was not used the progress is hidden:

    $ ./build/bin/bitcoin-cli getblockchaininfo
    ...
      "mediantime": 1756245700,
      "verificationprogress": 1,
      "initialblockdownload": false,
      "chainwork": "00000000000000000000000000000000000000000000000000000656d6bb052b",
      "size_on_disk": 3964972194,
    ...
    

    The PR also updates the way we estimate the verification progress returning a 100% on the snapshot block and not on the tip as we will stop doing background validation when reaching it.

  2. DrahtBot commented at 11:02 PM on August 26, 2025: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK sedited, fjahr, danielabrozzoni, achow101
    Concept ACK yuvicc, luke-jr
    Stale ACK nebula-21

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • and \initialblockdownload` false even though the node may still be validating blocks in the background.->and `initialblockdownload` is false even though the node may still be validating blocks in the background.` [As written, the missing verb (“is”) makes the sentence grammatically incomplete and slightly harder to parse.]

    <sup>2026-03-24 14:51:50</sup>

  3. yuvicc commented at 5:23 AM on August 27, 2025: contributor

    Concept ACK

  4. luke-jr commented at 10:50 PM on August 27, 2025: member

    Concept ACK. Would be nice to test the active state.

  5. polespinasa commented at 11:12 PM on August 27, 2025: member

    Would be nice to test the active state. @luke-jr what do you mean by active state?

  6. bitcoin deleted a comment on Aug 27, 2025
  7. luke-jr commented at 11:43 PM on August 27, 2025: member

    When there is a background sync, not just false

  8. luke-jr referenced this in commit 732ba0e131 on Aug 28, 2025
  9. ajtowns commented at 12:56 AM on August 28, 2025: contributor

    Would it be better to return an object, eg:

    {
        "background": {
            "blocks": 100000,
            "bestblockhash": "000000000003ba27aa200b1cecaad478d2b00432346c3f1f3986da1afd33e506",
            "mediantime": 1293622620,
            "chainwork": "0000000000000000000000000000000000000000000000000644cb7f5234089e"
            "verificationprogress": 0.001,
        }
        ...
    }
    
  10. polespinasa force-pushed on Aug 28, 2025
  11. polespinasa force-pushed on Aug 28, 2025
  12. polespinasa commented at 2:32 AM on August 28, 2025: member

    @ajtowns sgtm, done. Not sure about the order thought... I'm open to suggestions.

  13. in src/rpc/blockchain.cpp:None in 8efdfa9035 outdated
    1335 | @@ -1336,6 +1336,15 @@ RPCHelpMan getblockchaininfo()
    1336 |                  {RPCResult::Type::NUM_TIME, "mediantime", "The median block time expressed in " + UNIX_EPOCH_TIME},
    1337 |                  {RPCResult::Type::NUM, "verificationprogress", "estimate of verification progress [0..1]"},
    1338 |                  {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
    1339 | +                {RPCResult::Type::BOOL, "backgroundvalidation", "(debug information) estimate of whether this node is doing background validation"},
    


    ajtowns commented at 5:04 AM on August 28, 2025:

    Can just drop this field, and use the presence/absence of the object to indicate where background validation is occurring?

    If not, "estimate of" isn't accurate here -- the node knows whether it's doing background validation or not.


    polespinasa commented at 6:30 PM on August 28, 2025:

    I like to see false if it's not in b validation. Fixed the "estimate of"

  14. in src/rpc/blockchain.cpp:None in 8efdfa9035 outdated
    1335 | @@ -1336,6 +1336,15 @@ RPCHelpMan getblockchaininfo()
    1336 |                  {RPCResult::Type::NUM_TIME, "mediantime", "The median block time expressed in " + UNIX_EPOCH_TIME},
    1337 |                  {RPCResult::Type::NUM, "verificationprogress", "estimate of verification progress [0..1]"},
    1338 |                  {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
    1339 | +                {RPCResult::Type::BOOL, "backgroundvalidation", "(debug information) estimate of whether this node is doing background validation"},
    1340 | +                {RPCResult::Type::OBJ, "background", /*optional=*/true, "state info regarding background validation process",
    1341 | +                {
    1342 | +                    {RPCResult::Type::NUM, "blocks", /*optional=*/true, "the height of the most-work fully-background validated chain. The genesis block has height 0"},
    1343 | +                    {RPCResult::Type::STR, "bestblockhash", /*optional=*/true, "the hash of the currently best block validated in background"},
    1344 | +                    {RPCResult::Type::NUM_TIME, "mediantime", /*optional=*/true, "The median block time expressed in " + UNIX_EPOCH_TIME},
    


    ajtowns commented at 5:11 AM on August 28, 2025:

    Could make it the instead of The for consistency with other options. Could consider doing the same for the main mediantime help text that this was copied from too.

    As far as ordering goes, probably copy the ordering of the fields in getblockchaininfo?


    polespinasa commented at 6:29 PM on August 28, 2025:

    done

  15. in src/rpc/blockchain.cpp:None in 8efdfa9035 outdated
    1392 | +        const CBlockIndex& btip{*CHECK_NONFATAL(chainman.GetBackgroundSyncTip())};
    1393 | +        bValidation.pushKV("blocks",btip.nHeight);
    1394 | +        bValidation.pushKV("bestblockhash", btip.GetBlockHash().GetHex());
    1395 | +        bValidation.pushKV("mediantime", btip.GetMedianTimePast());
    1396 | +        bValidation.pushKV("chainwork", btip.nChainWork.GetHex());
    1397 | +        bValidation.pushKV("verificationprogress", chainman.GuessVerificationProgress(&btip));
    


    ajtowns commented at 5:24 AM on August 28, 2025:

    This is telling you how far the background has progressed if it were going to go all the way to the current tip; but it will actually stop when it hits the snapshot block. So I think this should be calling a new method chainman.GetBackgroundVerificationProgress(btip), something like:

    double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex* tip)
    {
        if (tip == nullptr) return 0.0;
        auto base = GetSnapshotBaseBlock();
        if (base == nullptr || base->m_chain_tx_count == 0) return 0.0;
        return tip->m_chain_tx_count / base->m_chain_tc_count;
    }
    

    polespinasa commented at 6:48 PM on August 28, 2025:

    That's a good point, but that's actually what we are showing in the logs. Do we want to keep consistency between both? Update maybe also the logging?

    [background validation] UpdateTip: new best=00000000000000000007b796179f5abb7eed59f736e3418eb8381c29eb78527a height=744000 version=0x20400004 log2_work=93.616004 tx=747106639 date='2022-07-07T14:47:25Z' progress=0.605933 cache=358.0MiB(2606151txo)

    In validation.cpp

    constexpr int BACKGROUND_LOG_INTERVAL = 2000;
            if (pindexNew->nHeight % BACKGROUND_LOG_INTERVAL == 0) {
                UpdateTipLog(m_chainman, coins_tip, pindexNew, __func__, "[background validation] ", "");
            }
    
    static void UpdateTipLog( ... const CBlockIndex* tip,...)
    {
    
       ...
                       FormatISO8601DateTime(tip->GetBlockTime()),
                       chainman.GuessVerificationProgress(tip),
       ...
    }
    

    polespinasa commented at 10:07 PM on September 3, 2025:

    Implemented the new method and used it also in the logs

  16. in test/functional/rpc_blockchain.py:None in 8efdfa9035 outdated
     170 | @@ -170,6 +171,10 @@ def _test_getblockchaininfo(self):
     171 |          assert res['pruned']
     172 |          assert not res['automatic_pruning']
     173 |  
     174 | +        # check background validation
     175 | +        assert not res['backgroundvalidation']
     176 | +        assert "background" not in res.keys()
    


    ajtowns commented at 5:29 AM on August 28, 2025:

    Should be able to test the field-exists case via feature_assumeutxo.py probably just prior to the invocation of n1.submitblock(snapshot_block)


    polespinasa commented at 9:19 PM on August 28, 2025:

    do we want to test that in feature_assumeutxo.py or in rpc_blockchain.py


    polespinasa commented at 10:36 PM on September 3, 2025:

    done in a48d92a5fb4c443dbf615715c8f25d1de9cabc9e

  17. polespinasa force-pushed on Aug 28, 2025
  18. polespinasa force-pushed on Aug 28, 2025
  19. polespinasa force-pushed on Aug 28, 2025
  20. polespinasa commented at 9:00 PM on August 28, 2025: member

    2d48902999a7ad1fcf19a8e630c69d6228dbd373 added snapshot height

    83a1721817fda4db88ba12e0395ed63ba064c41b modify the way verification progress is calculated for background validation -> context: #33259 (review)

  21. polespinasa force-pushed on Aug 28, 2025
  22. polespinasa force-pushed on Aug 28, 2025
  23. in src/rpc/blockchain.cpp:None in 2d48902999 outdated
    1340 |                  {RPCResult::Type::NUM, "verificationprogress", "estimate of verification progress [0..1]"},
    1341 |                  {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
    1342 | +                {RPCResult::Type::BOOL, "backgroundvalidation", "(debug information) whether this node is doing background validation"},
    1343 | +                {RPCResult::Type::OBJ, "background", /*optional=*/true, "state info regarding background validation process",
    1344 | +                {
    1345 | +                    {RPCResult::Type::NUM, "snapshotheight", /*optional=*/true, "the heigh of the snapshot block"},
    


    maflcko commented at 10:56 AM on August 29, 2025:
    "the heigh of the snapshot block" -> "the height of the snapshot block" [spelling error: "heigh" should be "height"]
  24. in src/validation.h:None in 2d48902999 outdated
    1162 | @@ -1163,6 +1163,9 @@ class ChainstateManager
    1163 |      /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip). */
    1164 |      double GuessVerificationProgress(const CBlockIndex* pindex) const EXCLUSIVE_LOCKS_REQUIRED(GetMutex());
    1165 |  
    1166 | +    /** Guess background verification progress in case assume utxo was used (as a fraction between 0.0=genesis and 1.0=snapshot blocks). */
    1167 | +    double GetBackgroundVerificationProgress(const CBlockIndex* pindex) const EXCLUSIVE_LOCKS_REQUIRED(GetMutex());
    


    maflcko commented at 10:59 AM on August 29, 2025:
        /** Guess background verification progress in case assume-utxo was used (as a fraction between 0.0=genesis and 1.0=snapshot blocks). */
        double GetBackgroundVerificationProgress(const CBlockIndex& block) const EXCLUSIVE_LOCKS_REQUIRED(GetMutex());
    

    can remove the unused nullptr handling here, as all call-sites already dereferenced the pointer


    polespinasa commented at 7:49 PM on August 29, 2025:

    Done

  25. polespinasa force-pushed on Aug 29, 2025
  26. danielabrozzoni commented at 2:35 PM on September 2, 2025: member

    Concept ACK

    I did a very rough first pass and the code looks good to me. As others have said, I would like to see a functional test testing the newly introduced fields when the background validation is in progress

  27. polespinasa force-pushed on Sep 3, 2025
  28. polespinasa commented at 10:08 PM on September 3, 2025: member

    @danielabrozzoni @ajtowns @luke-jr

    Tests implemented, not the cleanest way for sure, but I couldn't figure out anything simpler. Open to suggestions :)

  29. polespinasa requested review from danielabrozzoni on Sep 3, 2025
  30. polespinasa requested review from ajtowns on Sep 3, 2025
  31. polespinasa requested review from luke-jr on Sep 3, 2025
  32. in test/functional/feature_assumeutxo.py:None in a48d92a5fb outdated
     518 | @@ -518,6 +519,28 @@ def check_dump_output(output):
     519 |          assert_equal(utxo_info['height'], SNAPSHOT_BASE_HEIGHT)
     520 |          assert_equal(utxo_info['bestblock'], snapshot_hash)
     521 |  
     522 | +        self.log.info("Check that getblockchaininfo returns information about the background validaiton process")
    


    BinaryIgor commented at 5:17 PM on September 9, 2025:

    nit: typo, should be "validation"


    polespinasa commented at 5:22 PM on September 15, 2025:

    thanks 👍

  33. in test/functional/feature_assumeutxo.py:None in a48d92a5fb outdated
     527 | +            "mediantime",
     528 | +            "chainwork",
     529 | +            "verificationprogress"
     530 | +        ]
     531 | +        res = n1.getblockchaininfo()
     532 | +        assert "background" in res.keys()
    


    BinaryIgor commented at 5:24 PM on September 9, 2025:

    As this already is a rather too long test, it would be nice to move these assertions into a separate method, similarly as it done with check_tx_counts


    polespinasa commented at 5:23 PM on September 15, 2025:

    I don't think that's useful. The test is not much longer than others and check_tx_counts is separated in a function because it's called multiple times with different values of final. In this case there's no duplicated code.

  34. polespinasa force-pushed on Sep 15, 2025
  35. polespinasa commented at 5:23 PM on September 15, 2025: member

    21fb4c949cebf3510dad3dc3827ccb9451e4828d correct typos

  36. DrahtBot added the label Needs rebase on Dec 16, 2025
  37. polespinasa force-pushed on Dec 21, 2025
  38. polespinasa commented at 1:28 PM on December 21, 2025: member

    Rebased on top of master to solve conflicts with #30214

  39. DrahtBot removed the label Needs rebase on Dec 21, 2025
  40. in src/rpc/blockchain.cpp:1407 in bb0bc4d09c outdated
    1400 | @@ -1391,6 +1401,21 @@ RPCHelpMan getblockchaininfo()
    1401 |      obj.pushKV("mediantime", tip.GetMedianTimePast());
    1402 |      obj.pushKV("verificationprogress", chainman.GuessVerificationProgress(&tip));
    1403 |      obj.pushKV("initialblockdownload", chainman.IsInitialBlockDownload());
    1404 | +    auto historical_blocks{chainman.GetHistoricalBlockRange()};
    1405 | +    bool background_sync = historical_blocks.has_value();
    1406 | +    obj.pushKV("backgroundvalidation", background_sync);
    1407 | +    if(historical_blocks) {
    


    nebula-21 commented at 6:42 PM on January 16, 2026:

    Nit: missing space after ifif (historical_blocks)

  41. in src/rpc/blockchain.cpp:1411 in bb0bc4d09c outdated
    1406 | +    obj.pushKV("backgroundvalidation", background_sync);
    1407 | +    if(historical_blocks) {
    1408 | +        UniValue bValidation(UniValue::VOBJ);
    1409 | +        const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
    1410 | +        const std::optional<int> bHeight{active_chainstate.SnapshotBase()->nHeight};
    1411 | +        if(bHeight) bValidation.pushKV("snapshotheight", *bHeight);
    


    nebula-21 commented at 6:44 PM on January 16, 2026:

    Nit: missing space after ifif (bHeight)

  42. in src/rpc/blockchain.cpp:1412 in bb0bc4d09c outdated
    1407 | +    if(historical_blocks) {
    1408 | +        UniValue bValidation(UniValue::VOBJ);
    1409 | +        const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
    1410 | +        const std::optional<int> bHeight{active_chainstate.SnapshotBase()->nHeight};
    1411 | +        if(bHeight) bValidation.pushKV("snapshotheight", *bHeight);
    1412 | +        bValidation.pushKV("blocks",btip.nHeight);
    


    nebula-21 commented at 6:53 PM on January 16, 2026:

    Nit: missing space after comma → bValidation.pushKV("blocks", btip.nHeight);

  43. nebula-21 commented at 7:50 PM on January 16, 2026: none

    ACK ea374564f2421df1883d16c19ac13d466adc1221

    Built from source, tested, and reviewed the code. New RPC fields behave as expected.

    Without loading UTXO snapshot: backgroundvalidation=false

    <details> <pre><code> bitcoin-cli getblockchaininfo ... "mediantime": 1231006505, "verificationprogress": 7.692763291753853e-10, "initialblockdownload": true, "backgroundvalidation": false, "chainwork": "0000000000000000000000000000000000000000000000000000000100010001", "size_on_disk": 293, "pruned": false, ... </code></pre> </details>

    With loadtxoutset: backgroundvalidation=true and background progress shown.

    <details> <pre><code> bitcoin-cli getblockchaininfo ... "mediantime": 1737334898, "verificationprogress": 0.8812817484397629, "initialblockdownload": true, "backgroundvalidation": true, "background": { "snapshotheight": 880000, "blocks": 34031, "bestblockhash": "00000000b2d3088131445fb70bda85e90663c36a66d429a2089c05014b8aac2b", "mediantime": 1263124911, "chainwork": "000000000000000000000000000000000000000000000000000086355a943ee0", "verificationprogress": 2.990124328575242e-05 }, "chainwork": "0000000000000000000000000000000000000000a879619cfddf806d3bdf607b", "size_on_disk": 9569248, "pruned": false, ... </code></pre> </details>

    Left a few nits in comments.

  44. polespinasa force-pushed on Jan 19, 2026
  45. in src/validation.cpp:2912 in 17d9606090
    2907 | @@ -2889,7 +2908,8 @@ static void UpdateTipLog(
    2908 |      const CBlockIndex* tip,
    2909 |      const std::string& func_name,
    2910 |      const std::string& prefix,
    2911 | -    const std::string& warning_messages) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
    2912 | +    const std::string& warning_messages,
    2913 | +    const bool &background_validation) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
    


    sedited commented at 10:09 AM on March 16, 2026:

    Taking a const reference of a bool does not make sense to me.


    polespinasa commented at 12:20 PM on March 16, 2026:

    True, removed the reference

  46. in src/rpc/blockchain.cpp:1354 in e79df7bc1d
    1352 | -                {RPCResult::Type::NUM_TIME, "mediantime", "The median block time expressed in " + UNIX_EPOCH_TIME},
    1353 | +                {RPCResult::Type::NUM_TIME, "time", "the block time expressed in " + UNIX_EPOCH_TIME},
    1354 | +                {RPCResult::Type::NUM_TIME, "mediantime", "the median block time expressed in " + UNIX_EPOCH_TIME},
    1355 |                  {RPCResult::Type::NUM, "verificationprogress", "estimate of verification progress [0..1]"},
    1356 |                  {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
    1357 | +                {RPCResult::Type::BOOL, "backgroundvalidation", "(debug information) whether this node is doing background validation"},
    


    sedited commented at 10:13 AM on March 16, 2026:

    Is this useful to include if we already have the background object if we are doing background validation in the first place? Seems like callers of this can just check for its presence instead?


    polespinasa commented at 12:14 PM on March 16, 2026:

    Same comment as: https://github.com/bitcoin/bitcoin/pull/33259/#discussion_r2306180631 I like to see the backgroundvalidation: false but given that the comment is repeated twice I will drop it.

  47. in src/rpc/blockchain.cpp:1408 in e79df7bc1d
    1400 | @@ -1391,6 +1401,21 @@ RPCHelpMan getblockchaininfo()
    1401 |      obj.pushKV("mediantime", tip.GetMedianTimePast());
    1402 |      obj.pushKV("verificationprogress", chainman.GuessVerificationProgress(&tip));
    1403 |      obj.pushKV("initialblockdownload", chainman.IsInitialBlockDownload());
    1404 | +    auto historical_blocks{chainman.GetHistoricalBlockRange()};
    1405 | +    bool background_sync = historical_blocks.has_value();
    1406 | +    obj.pushKV("backgroundvalidation", background_sync);
    1407 | +    if (historical_blocks) {
    1408 | +        UniValue bValidation(UniValue::VOBJ);
    


    sedited commented at 10:17 AM on March 16, 2026:

    Can use snake case for variables as per the dev-notes?


    polespinasa commented at 12:15 PM on March 16, 2026:

    done

  48. in test/functional/feature_assumeutxo.py:543 in 8e4a0b16f1 outdated
     538 | +        assert_equal(res["background"]["mediantime"], block["mediantime"])
     539 | +        assert_equal(res["background"]["chainwork"], block["chainwork"])
     540 | +        assert_greater_than(res["background"]["verificationprogress"], 0)
     541 | +        assert_greater_than(1, res["background"]["verificationprogress"])
     542 | +
     543 | +
    


    sedited commented at 10:18 AM on March 16, 2026:

    Nit: Redundant new line.

  49. polespinasa force-pushed on Mar 16, 2026
  50. polespinasa commented at 12:30 PM on March 16, 2026: member

    Thanks for the review @sedited.

    Because of removing backgroundvalidation camp and checking only the presence of the info box, I renamed the box background to backgroundvalidation to make it clearer and more self-explanatory.

    Force pushed with all suggestions + the name change:

    <details>

    $ git diff 8e4a0b16f16235cf86969ae3b1c094e2b54ce58b..f2e97c82487f556b98041a3d0007a98e2374b1d7
    diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
    index 9e95b4d200..60cfa71f99 100644
    --- a/src/rpc/blockchain.cpp
    +++ b/src/rpc/blockchain.cpp
    @@ -1351,8 +1351,7 @@ RPCHelpMan getblockchaininfo()
                     {RPCResult::Type::NUM_TIME, "mediantime", "the median block time expressed in " + UNIX_EPOCH_TIME},
                     {RPCResult::Type::NUM, "verificationprogress", "estimate of verification progress [0..1]"},
                     {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
    -                {RPCResult::Type::BOOL, "backgroundvalidation", "(debug information) whether this node is doing background validation"},
    -                {RPCResult::Type::OBJ, "background", /*optional=*/true, "state info regarding background validation process",
    +                {RPCResult::Type::OBJ, "backgroundvalidation", /*optional=*/true, "state info regarding background validation process",
                     {
                         {RPCResult::Type::NUM, "snapshotheight", /*optional=*/true, "the height of the snapshot block"},
                         {RPCResult::Type::NUM, "blocks", /*optional=*/true, "the height of the most-work fully-background validated chain. The genesis block has height 0"},
    @@ -1402,19 +1401,17 @@ RPCHelpMan getblockchaininfo()
         obj.pushKV("verificationprogress", chainman.GuessVerificationProgress(&tip));
         obj.pushKV("initialblockdownload", chainman.IsInitialBlockDownload());
         auto historical_blocks{chainman.GetHistoricalBlockRange()};
    -    bool background_sync = historical_blocks.has_value();
    -    obj.pushKV("backgroundvalidation", background_sync);
         if (historical_blocks) {
    -        UniValue bValidation(UniValue::VOBJ);
    +        UniValue background_validation(UniValue::VOBJ);
             const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
             const std::optional<int> bHeight{active_chainstate.SnapshotBase()->nHeight};
    -        if (bHeight) bValidation.pushKV("snapshotheight", *bHeight);
    -        bValidation.pushKV("blocks", btip.nHeight);
    -        bValidation.pushKV("bestblockhash", btip.GetBlockHash().GetHex());
    -        bValidation.pushKV("mediantime", btip.GetMedianTimePast());
    -        bValidation.pushKV("chainwork", btip.nChainWork.GetHex());
    -        bValidation.pushKV("verificationprogress", chainman.GetBackgroundVerificationProgress(btip));
    -        obj.pushKV("background", std::move(bValidation));
    +        if (bHeight) background_validation.pushKV("snapshotheight", *bHeight);
    +        background_validation.pushKV("blocks", btip.nHeight);
    +        background_validation.pushKV("bestblockhash", btip.GetBlockHash().GetHex());
    +        background_validation.pushKV("mediantime", btip.GetMedianTimePast());
    +        background_validation.pushKV("chainwork", btip.nChainWork.GetHex());
    +        background_validation.pushKV("verificationprogress", chainman.GetBackgroundVerificationProgress(btip));
    +        obj.pushKV("backgroundvalidation", std::move(background_validation));
         }
         obj.pushKV("chainwork", tip.nChainWork.GetHex());
         obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage());
    diff --git a/src/validation.cpp b/src/validation.cpp
    index 9608f8cc1e..75722ef04b 100644
    --- a/src/validation.cpp
    +++ b/src/validation.cpp
    @@ -2909,7 +2909,7 @@ static void UpdateTipLog(
         const std::string& func_name,
         const std::string& prefix,
         const std::string& warning_messages,
    -    const bool &background_validation) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
    +    const bool background_validation) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
     {
     
         AssertLockHeld(::cs_main);
    diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
    index da48a3a1a2..6f094eec32 100755
    --- a/test/functional/feature_assumeutxo.py
    +++ b/test/functional/feature_assumeutxo.py
    @@ -529,17 +529,16 @@ class AssumeutxoTest(BitcoinTestFramework):
                 "verificationprogress"
             ]
             res = n1.getblockchaininfo()
    -        assert "background" in res.keys()
    -        assert_equal(sorted(expected_keys), sorted(res["background"].keys()))
    -        assert_equal(res["background"]["snapshotheight"], SNAPSHOT_BASE_HEIGHT)
    -        assert_equal(res["background"]["blocks"], START_HEIGHT)
    -        assert_equal(res["background"]["bestblockhash"], n1.getblockhash(START_HEIGHT))
    -        block = n1.getblockheader(res["background"]["bestblockhash"])
    -        assert_equal(res["background"]["mediantime"], block["mediantime"])
    -        assert_equal(res["background"]["chainwork"], block["chainwork"])
    -        assert_greater_than(res["background"]["verificationprogress"], 0)
    -        assert_greater_than(1, res["background"]["verificationprogress"])
    -
    +        assert "backgroundvalidation" in res.keys()
    +        assert_equal(sorted(expected_keys), sorted(res["backgroundvalidation"].keys()))
    +        assert_equal(res["backgroundvalidation"]["snapshotheight"], SNAPSHOT_BASE_HEIGHT)
    +        assert_equal(res["backgroundvalidation"]["blocks"], START_HEIGHT)
    +        assert_equal(res["backgroundvalidation"]["bestblockhash"], n1.getblockhash(START_HEIGHT))
    +        block = n1.getblockheader(res["backgroundvalidation"]["bestblockhash"])
    +        assert_equal(res["backgroundvalidation"]["mediantime"], block["mediantime"])
    +        assert_equal(res["backgroundvalidation"]["chainwork"], block["chainwork"])
    +        assert_greater_than(res["backgroundvalidation"]["verificationprogress"], 0)
    +        assert_greater_than(1, res["backgroundvalidation"]["verificationprogress"])
     
             # find coinbase output at snapshot height on node0 and scan for it on node1,
             # where the block is not available, but the snapshot was loaded successfully
    diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py
    index c4e9472b7a..97bc16a9d7 100755
    --- a/test/functional/rpc_blockchain.py
    +++ b/test/functional/rpc_blockchain.py
    @@ -136,7 +136,6 @@ class BlockchainTest(BitcoinTestFramework):
             self.log.info("Test getblockchaininfo")
     
             keys = [
    -            'backgroundvalidation',
                 'bestblockhash',
                 'bits',
                 'blocks',
    @@ -171,9 +170,8 @@ class BlockchainTest(BitcoinTestFramework):
             assert res['pruned']
             assert not res['automatic_pruning']
     
    -        # check background validation
    -        assert not res['backgroundvalidation']
    -        assert "background" not in res.keys()
    +        # check background validation is not present when we are not using assumeutxo
    +        assert "backgroundvalidation" not in res.keys()
     
             self.restart_node(0, ['-stopatheight=207'])
             res = self.nodes[0].getblockchaininfo()
    
  51. in src/rpc/blockchain.cpp:1361 in f2e97c8248
    1359 | +                    {RPCResult::Type::NUM, "snapshotheight", /*optional=*/true, "the height of the snapshot block"},
    1360 | +                    {RPCResult::Type::NUM, "blocks", /*optional=*/true, "the height of the most-work fully-background validated chain. The genesis block has height 0"},
    1361 | +                    {RPCResult::Type::STR, "bestblockhash", /*optional=*/true, "the hash of the currently best block validated in background"},
    1362 | +                    {RPCResult::Type::NUM_TIME, "mediantime", /*optional=*/true, "the median block time expressed in " + UNIX_EPOCH_TIME},
    1363 | +                    {RPCResult::Type::NUM, "verificationprogress", /*optional=*/true, "estimate of background validation progress [0..1]"},
    1364 | +                    {RPCResult::Type::STR_HEX, "chainwork", /*optional=*/true, "total amount of work in background validated chain, in hexadecimal"},
    


    sedited commented at 5:30 PM on March 16, 2026:

    Don't we always populate these fields if we have backgroundvalidation? From what I can tell we usually don't mark these nested fields of an optional as optional too.


    polespinasa commented at 6:01 PM on March 16, 2026:

    You are right, they are not. Didn't know optional are inherited in nested fields. Good catch!

  52. in src/rpc/blockchain.cpp:1407 in f2e97c8248
    1399 | @@ -1420,6 +1400,19 @@ RPCHelpMan getblockchaininfo()
    1400 |      obj.pushKV("mediantime", tip.GetMedianTimePast());
    1401 |      obj.pushKV("verificationprogress", chainman.GuessVerificationProgress(&tip));
    1402 |      obj.pushKV("initialblockdownload", chainman.IsInitialBlockDownload());
    1403 | +    auto historical_blocks{chainman.GetHistoricalBlockRange()};
    1404 | +    if (historical_blocks) {
    1405 | +        UniValue background_validation(UniValue::VOBJ);
    1406 | +        const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
    1407 | +        const std::optional<int> bHeight{active_chainstate.SnapshotBase()->nHeight};
    


    sedited commented at 5:30 PM on March 16, 2026:

    Nit: Another non-snake case variable.


    polespinasa commented at 6:05 PM on March 16, 2026:

    done

  53. in src/rpc/blockchain.cpp:1360 in f2e97c8248
    1358 | +                {
    1359 | +                    {RPCResult::Type::NUM, "snapshotheight", /*optional=*/true, "the height of the snapshot block"},
    1360 | +                    {RPCResult::Type::NUM, "blocks", /*optional=*/true, "the height of the most-work fully-background validated chain. The genesis block has height 0"},
    1361 | +                    {RPCResult::Type::STR, "bestblockhash", /*optional=*/true, "the hash of the currently best block validated in background"},
    1362 | +                    {RPCResult::Type::NUM_TIME, "mediantime", /*optional=*/true, "the median block time expressed in " + UNIX_EPOCH_TIME},
    1363 | +                    {RPCResult::Type::NUM, "verificationprogress", /*optional=*/true, "estimate of background validation progress [0..1]"},
    


    sedited commented at 5:33 PM on March 16, 2026:

    Nit: Maybe call this verification progress for symmetry with the other field. Seems like we usually call it 'verification' instead of 'validation' in the user facing strings of this file.


    polespinasa commented at 6:04 PM on March 16, 2026:

    done

  54. in src/rpc/blockchain.cpp:1358 in f2e97c8248
    1356 |                  {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
    1357 | +                {RPCResult::Type::OBJ, "backgroundvalidation", /*optional=*/true, "state info regarding background validation process",
    1358 | +                {
    1359 | +                    {RPCResult::Type::NUM, "snapshotheight", /*optional=*/true, "the height of the snapshot block"},
    1360 | +                    {RPCResult::Type::NUM, "blocks", /*optional=*/true, "the height of the most-work fully-background validated chain. The genesis block has height 0"},
    1361 | +                    {RPCResult::Type::STR, "bestblockhash", /*optional=*/true, "the hash of the currently best block validated in background"},
    


    sedited commented at 5:36 PM on March 16, 2026:

    Nit (typo): "validated in the background".


    polespinasa commented at 6:03 PM on March 16, 2026:

    done

  55. in src/rpc/blockchain.cpp:1357 in f2e97c8248
    1355 |                  {RPCResult::Type::NUM, "verificationprogress", "estimate of verification progress [0..1]"},
    1356 |                  {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
    1357 | +                {RPCResult::Type::OBJ, "backgroundvalidation", /*optional=*/true, "state info regarding background validation process",
    1358 | +                {
    1359 | +                    {RPCResult::Type::NUM, "snapshotheight", /*optional=*/true, "the height of the snapshot block"},
    1360 | +                    {RPCResult::Type::NUM, "blocks", /*optional=*/true, "the height of the most-work fully-background validated chain. The genesis block has height 0"},
    


    sedited commented at 5:38 PM on March 16, 2026:

    Nit: "background fully-validated chain" reads a bit clearer to me.


    polespinasa commented at 6:03 PM on March 16, 2026:

    done

  56. sedited commented at 5:38 PM on March 16, 2026: contributor

    I think this still needs a release note.

  57. polespinasa force-pushed on Mar 16, 2026
  58. polespinasa force-pushed on Mar 16, 2026
  59. DrahtBot added the label CI failed on Mar 16, 2026
  60. polespinasa commented at 6:24 PM on March 16, 2026: member

    Force pushed to apply @sedited nits + added release note:

    <details>

    $ git diff f2e97c82487f556b98041a3d0007a98e2374b1d7..b05466d172d6ae03e33f30c6734c920cfe1b85ca
    diff --git a/doc/release-notes-33259.md b/doc/release-notes-33259.md
    new file mode 100644
    index 0000000000..640aaf9498
    --- /dev/null
    +++ b/doc/release-notes-33259.md
    @@ -0,0 +1,4 @@
    +RPC
    +---
    +
    +The `getblockchaininfo` RPC exposes progress for background validation if the `assumeutxo` feature is used. Previously, `verificationprogress` could return 1.0 and `ìnitialblockdownload` false even if the node was still validating blocks in the background. A new object, `backgroundvalidation`, provides details about the snapshot being validated, including snapshot height, number of blocks processed, best block hash, chainwork, median time, and verification progress.
    diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
    index 60cfa71f99..cf8d91adf3 100644
    --- a/src/rpc/blockchain.cpp
    +++ b/src/rpc/blockchain.cpp
    @@ -1353,12 +1353,12 @@ RPCHelpMan getblockchaininfo()
                     {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
                     {RPCResult::Type::OBJ, "backgroundvalidation", /*optional=*/true, "state info regarding background validation process",
                     {
    -                    {RPCResult::Type::NUM, "snapshotheight", /*optional=*/true, "the height of the snapshot block"},
    -                    {RPCResult::Type::NUM, "blocks", /*optional=*/true, "the height of the most-work fully-background validated chain. The genesis block has height 0"},
    -                    {RPCResult::Type::STR, "bestblockhash", /*optional=*/true, "the hash of the currently best block validated in background"},
    -                    {RPCResult::Type::NUM_TIME, "mediantime", /*optional=*/true, "the median block time expressed in " + UNIX_EPOCH_TIME},
    -                    {RPCResult::Type::NUM, "verificationprogress", /*optional=*/true, "estimate of background validation progress [0..1]"},
    -                    {RPCResult::Type::STR_HEX, "chainwork", /*optional=*/true, "total amount of work in background validated chain, in hexadecimal"},
    +                    {RPCResult::Type::NUM, "snapshotheight", "the height of the snapshot block"},
    +                    {RPCResult::Type::NUM, "blocks", "the height of the most-work background fully-validated chain. The genesis block has height 0"},
    +                    {RPCResult::Type::STR, "bestblockhash", "the hash of the currently best block validated in the background"},
    +                    {RPCResult::Type::NUM_TIME, "mediantime", "the median block time expressed in " + UNIX_EPOCH_TIME},
    +                    {RPCResult::Type::NUM, "verificationprogress", "estimate of background verification progress [0..1]"},
    +                    {RPCResult::Type::STR_HEX, "chainwork", "total amount of work in background validated chain, in hexadecimal"},
                     }},
                     {RPCResult::Type::STR_HEX, "chainwork", "total amount of work in active chain, in hexadecimal"},
                     {RPCResult::Type::NUM, "size_on_disk", "the estimated size of the block and undo files on disk"},
    @@ -1404,8 +1404,8 @@ RPCHelpMan getblockchaininfo()
         if (historical_blocks) {
             UniValue background_validation(UniValue::VOBJ);
             const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
    -        const std::optional<int> bHeight{active_chainstate.SnapshotBase()->nHeight};
    -        if (bHeight) background_validation.pushKV("snapshotheight", *bHeight);
    +        const std::optional<int> background_height{active_chainstate.SnapshotBase()->nHeight};
    +        if (background_height) background_validation.pushKV("snapshotheight", *background_height);
             background_validation.pushKV("blocks", btip.nHeight);
             background_validation.pushKV("bestblockhash", btip.GetBlockHash().GetHex());
             background_validation.pushKV("mediantime", btip.GetMedianTimePast());
    
    
  61. DrahtBot removed the label CI failed on Mar 16, 2026
  62. sedited approved
  63. sedited commented at 8:35 PM on March 16, 2026: contributor

    ACK b05466d172d6ae03e33f30c6734c920cfe1b85ca

  64. DrahtBot requested review from nebula-21 on Mar 16, 2026
  65. sedited removed review request from nebula-21 on Mar 16, 2026
  66. sedited requested review from fjahr on Mar 16, 2026
  67. polespinasa force-pushed on Mar 16, 2026
  68. polespinasa commented at 9:43 PM on March 16, 2026: member

    Sorry for breaking your ACK @sedited , there was a typo in the release note 😅

  69. in src/validation.cpp:5528 in 64b3fcc765 outdated
    5520 | @@ -5520,6 +5521,17 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c
    5521 |      return std::min<double>(pindex->m_chain_tx_count / fTxTotal, 1.0);
    5522 |  }
    5523 |  
    5524 | +double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex& pindex) const
    5525 | +{
    5526 | +    AssertLockHeld(GetMutex());
    5527 | +    auto snapshot_base_block = CurrentChainstate().SnapshotBase();
    5528 | +    if (pindex.m_chain_tx_count == 0 || snapshot_base_block == nullptr || snapshot_base_block->m_chain_tx_count == 0) {
    


    fjahr commented at 10:48 PM on March 16, 2026:

    Feels like snapshot_base_block == nullptr should either get a different log or instead CHECK_NONFATAL.


    polespinasa commented at 5:04 PM on March 17, 2026:

    Added the CHECK_NONFATAL also added !CHECK_NONFATAL(HistoricalChainstate() != nullptr) as if in background validation we should also have a HistoricalChainstate() if I understood correctly.

  70. in src/validation.cpp:5529 in 64b3fcc765 outdated
    5520 | @@ -5520,6 +5521,17 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c
    5521 |      return std::min<double>(pindex->m_chain_tx_count / fTxTotal, 1.0);
    5522 |  }
    5523 |  
    5524 | +double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex& pindex) const
    5525 | +{
    5526 | +    AssertLockHeld(GetMutex());
    5527 | +    auto snapshot_base_block = CurrentChainstate().SnapshotBase();
    5528 | +    if (pindex.m_chain_tx_count == 0 || snapshot_base_block == nullptr || snapshot_base_block->m_chain_tx_count == 0) {
    5529 | +        LogDebug(BCLog::VALIDATION, "Block %d has unset m_chain_tx_count. Unable to estimate verification progress.\n", pindex.nHeight);
    


    fjahr commented at 10:50 PM on March 16, 2026:

    nit: \n isn't needed. You could make it explicit in the log message that this is coming from a background chainstate too.


    polespinasa commented at 4:41 PM on March 17, 2026:

    done

  71. in test/functional/feature_assumeutxo.py:564 in 3f154e7d79 outdated
     559 | +        assert_equal(res["backgroundvalidation"]["bestblockhash"], n1.getblockhash(START_HEIGHT))
     560 | +        block = n1.getblockheader(res["backgroundvalidation"]["bestblockhash"])
     561 | +        assert_equal(res["backgroundvalidation"]["mediantime"], block["mediantime"])
     562 | +        assert_equal(res["backgroundvalidation"]["chainwork"], block["chainwork"])
     563 | +        assert_greater_than(res["backgroundvalidation"]["verificationprogress"], 0)
     564 | +        assert_greater_than(1, res["backgroundvalidation"]["verificationprogress"])
    


    fjahr commented at 10:56 PM on March 16, 2026:

    Can you check for a more narrow range for verificationprogress? Also seems like a good place to use assert_approx instead of these two lines.


    polespinasa commented at 5:48 PM on March 17, 2026:

    Done I think now is more accurate.

  72. in src/validation.cpp:5527 in 64b3fcc765 outdated
    5520 | @@ -5520,6 +5521,17 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c
    5521 |      return std::min<double>(pindex->m_chain_tx_count / fTxTotal, 1.0);
    5522 |  }
    5523 |  
    5524 | +double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex& pindex) const
    5525 | +{
    5526 | +    AssertLockHeld(GetMutex());
    5527 | +    auto snapshot_base_block = CurrentChainstate().SnapshotBase();
    


    fjahr commented at 11:02 PM on March 16, 2026:

    I don't think this makes sense although in practice it seems to not make a big difference. SnapshotBase() is the base of the assumed valid (snapshot) chainstate and CurrentChainstate() is the chainstate targetting the tip. For the historic chainstate SnapshotBase() should always return a nullptr which is why it's confusing to see it here in this context. HistoricalChainstate().TargetBlock() is what you actually want to use I think.


    polespinasa commented at 4:40 PM on March 17, 2026:

    Makes totally sense. Change applied.

    Do you think is worth to add a check ensuring HistoricalChainstate()->TargetBlock() == CurrentChainstate().SnapshotBase() ?


    fjahr commented at 5:19 PM on March 17, 2026:

    I don't think that needs to be the responsibility of this function, it doesn't do anything critical and this would add some unnecessary overhead IMO.

  73. in src/rpc/blockchain.cpp:1437 in d2fc59fcef outdated
    1428 | @@ -1420,6 +1429,19 @@ RPCHelpMan getblockchaininfo()
    1429 |      obj.pushKV("mediantime", tip.GetMedianTimePast());
    1430 |      obj.pushKV("verificationprogress", chainman.GuessVerificationProgress(&tip));
    1431 |      obj.pushKV("initialblockdownload", chainman.IsInitialBlockDownload());
    1432 | +    auto historical_blocks{chainman.GetHistoricalBlockRange()};
    1433 | +    if (historical_blocks) {
    1434 | +        UniValue background_validation(UniValue::VOBJ);
    1435 | +        const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
    1436 | +        const std::optional<int> background_height{active_chainstate.SnapshotBase()->nHeight};
    1437 | +        if (background_height) background_validation.pushKV("snapshotheight", *background_height);
    


    fjahr commented at 11:05 PM on March 16, 2026:

    Just a note: Related to the other comment, it would be more consistent with our internals if TargetBlock() was reported here as targetblock but it might be confusing to users so let's leave it at that.

  74. fjahr commented at 11:36 PM on March 16, 2026: contributor

    Concept ACK

  75. polespinasa force-pushed on Mar 17, 2026
  76. polespinasa force-pushed on Mar 17, 2026
  77. DrahtBot added the label CI failed on Mar 17, 2026
  78. polespinasa force-pushed on Mar 17, 2026
  79. polespinasa force-pushed on Mar 17, 2026
  80. polespinasa commented at 6:09 PM on March 17, 2026: member

    Thanks for your review @fjahr :)

    Force pushed to apply suggestions.

    The two main changes are test improvement and GetBackgroundVerificationProgress()

    <details>

    $ git diff cf227d21836dc0724758a5ea01d8336a8e06a4ce..4e2c956ce8d24e74e7856d1ce35c44cc78968912 test/functional/feature_assumeutxo.py
    diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
    index 21151db65e..544496daa3 100755
    --- a/test/functional/feature_assumeutxo.py
    +++ b/test/functional/feature_assumeutxo.py
    @@ -36,7 +36,6 @@ from test_framework.test_framework import BitcoinTestFramework
     from test_framework.util import (
         assert_approx,
         assert_equal,
    -    assert_greater_than,
         assert_not_equal,
         assert_raises_rpc_error,
         ensure_for,
    @@ -560,8 +559,10 @@ class AssumeutxoTest(BitcoinTestFramework):
             block = n1.getblockheader(res["backgroundvalidation"]["bestblockhash"])
             assert_equal(res["backgroundvalidation"]["mediantime"], block["mediantime"])
             assert_equal(res["backgroundvalidation"]["chainwork"], block["chainwork"])
    -        assert_greater_than(res["backgroundvalidation"]["verificationprogress"], 0)
    -        assert_greater_than(1, res["backgroundvalidation"]["verificationprogress"])
    +        background_tx_count = n1.getchaintxstats(blockhash=res["backgroundvalidation"]["bestblockhash"])["txcount"]
    +        snapshot_tx_count = n1.getchaintxstats(blockhash=snapshot_hash)["txcount"]
    +        expected_verification_progress = background_tx_count / snapshot_tx_count
    +        assert_approx(res["backgroundvalidation"]["verificationprogress"], expected_verification_progress, vspan=0.01)
     
             # find coinbase output at snapshot height on node0 and scan for it on node1,
             # where the block is not available, but the snapshot was loaded successfully
    
    $ git diff cf227d21836dc0724758a5ea01d8336a8e06a4ce..4e2c956ce8d24e74e7856d1ce35c44cc78968912 src/validation.cpp
    diff --git a/src/validation.cpp b/src/validation.cpp
    index c1873d546a..c562796da6 100644
    --- a/src/validation.cpp
    +++ b/src/validation.cpp
    @@ -5524,12 +5524,15 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c
     double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex& pindex) const
     {
         AssertLockHeld(GetMutex());
    -    auto snapshot_base_block = CurrentChainstate().SnapshotBase();
    -    if (pindex.m_chain_tx_count == 0 || snapshot_base_block == nullptr || snapshot_base_block->m_chain_tx_count == 0) {
    -        LogDebug(BCLog::VALIDATION, "Block %d has unset m_chain_tx_count. Unable to estimate verification progress.\n", pindex.nHeight);
    +    if (!CHECK_NONFATAL(HistoricalChainstate() != nullptr)) return 0.0;
    +    auto target_block = HistoricalChainstate()->TargetBlock();
    +    if (!CHECK_NONFATAL(target_block != nullptr)) return 0.0;
    +
    +    if (pindex.m_chain_tx_count == 0 || target_block->m_chain_tx_count == 0) {
    +        LogDebug(BCLog::VALIDATION, "[background validation] Block %d has unset m_chain_tx_count. Unable to estimate verification progress.", pindex.nHeight);
             return 0.0;
         }
    -    return static_cast<double>(pindex.m_chain_tx_count) / static_cast<double>(snapshot_base_block->m_chain_tx_count);
    +    return static_cast<double>(pindex.m_chain_tx_count) / static_cast<double>(target_block->m_chain_tx_count);
     }
     
     Chainstate& ChainstateManager::InitializeChainstate(CTxMemPool* mempool)
    

    </details>

  81. DrahtBot removed the label CI failed on Mar 17, 2026
  82. in src/validation.cpp:5527 in f534b25e70 outdated
    5520 | @@ -5520,6 +5521,20 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c
    5521 |      return std::min<double>(pindex->m_chain_tx_count / fTxTotal, 1.0);
    5522 |  }
    5523 |  
    5524 | +double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex& pindex) const
    5525 | +{
    5526 | +    AssertLockHeld(GetMutex());
    5527 | +    if (!CHECK_NONFATAL(HistoricalChainstate() != nullptr)) return 0.0;
    


    fjahr commented at 9:32 AM on March 20, 2026:

    nit: (here and below) just CHECK_NONFATAL(HistoricalChainstate()) should work as well without the explicit nullptr.


    polespinasa commented at 12:14 PM on March 20, 2026:

    done

  83. in src/validation.h:1197 in f534b25e70 outdated
    1196 | @@ -1197,6 +1197,9 @@ class ChainstateManager
    1197 |      /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip). */
    


    fjahr commented at 9:36 AM on March 20, 2026:

    nit: Could have clarified this comment a bit with what this does in the assumeutxo context.


    polespinasa commented at 12:16 PM on March 20, 2026:

    I think now is more clear, lmk what you think:

        /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip).
        * In the assumeutxo context, reflects the active chain progress as a fraction
        * between 0.0=snapshot base block and 1.0=current tip. */
        double GuessVerificationProgress(const CBlockIndex* pindex) const EXCLUSIVE_LOCKS_REQUIRED(GetMutex());
    

    fjahr commented at 11:04 PM on March 20, 2026:

    Thanks but I don't think the comment is correct. My vague recollection was that this should report with the genesis block as the base and so I actually expected you to write that the number can be a bit deceiving for the snapshot chainstate. We are using the m_chain_tx_count from the assume utxo data in the params for the snaphot block and then the tx count is extrapolated for the next blocks but there is no normalization of that number happening in the function or outside afaict.

    I made some experiment to verify: https://github.com/fjahr/bitcoin/commit/35397907ae5e73b8d3b99f87a5b2fff4c91ada76

    If you run modified functional test with master code you should see:

    verificationprogress after snapshot load: 0.9964200477326969
    verificationprogress after snapshot + 1: 0.9982121573301549
    verificationprogress after snapshot + 2: 1
    

    And compiled with the (super whacky) normalization added to GuessVerificationProgress in that commit:

    verificationprogress after snapshot load: 0
    verificationprogress after snapshot + 1: 0.6249999999999911
    verificationprogress after snapshot + 2: 1
    

    I have experimented a bit with what it would take to add the normalization but so far I am not sure it's worth the effort. It would probably have to come with some refactoring and I am contemplating if the progress functions now shouldn't rather be chainstate functions rather than chainstatemanager. I also need to check if ryanofsky doesn't have some related refactoring already laying around somewhere. But I am certain there isn't an easy fix and it's out of scope for this PR.

    The rambling above is not meant to block the progress here. As is, the PR is already a good improvement and can be merged as when the comment is fixed.

    I would suggest something like:

        /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip).
        * This is also the case in the assumeutxo context, meaning that the progress reported for
        * the snapshot chainstate may suggest that all historical blocks have already been verified
        * even though that may not actually be the case. */
    

    EDIT: Fun edge case to think about: The snapshot could have been loaded while the node was already synced almost up until the snapshot height and so the reported progress number from genesis for the snapshot chainstate may actually be basically correct. 🙃


    polespinasa commented at 8:07 AM on March 23, 2026:

    Oh you are right, I didn't know that. I saw now that m_chain_tx_count is hardcoded in src/kernel/chainparams.cpp. I missunderstood how this works, definitely the comment is wrong, sorry. I will cherry-pick your comment.

    I am contemplating if the progress functions now shouldn't rather be chainstate functions rather than chainstatemanager

    Why? If the progress functions has to be aware of more than one chainstate why wouldn't it be better inside the chainstatemanager? Wouldn't that add dependencies on specific chainstates knowing about other ones?

    EDIT: Fun edge case to think about: The snapshot could have been loaded while the node was already synced almost up until the snapshot height and so the reported progress number from genesis for the snapshot chainstate may actually be basically correct. 🙃

    Definitely a weird thing to do lol 😅


    fjahr commented at 8:46 AM on March 23, 2026:

    Why? If the progress functions has to be aware of more than one chainstate why wouldn't it be better inside the chainstatemanager? Wouldn't that add dependencies on specific chainstates knowing about other ones?

    I don't think it needs to be aware of the other ones. From a high level: Right now if you ask the ChainstateManager "what's your progress?" It needs to figure out what progress you mean, you could have one or two chainstates and it has to figure out which one is which etc. It just seems cleaner that the call site knows which progress it wants (normal, snapshot, background) and then there is just one progress function on chainstate that answers the question depending on what chainstate this is. But so far this is just impression from a narrow view, I didn't go through all uses of the progress etc.


    fjahr commented at 8:50 AM on March 23, 2026:

    Just see the other comment thread: You ask the chainstatemanager for the background progress but then the chainstatemanager first needs to check if there is actually some background chainstate. This wouldn't be necessary if the progress function was on the background chainstate and the callsite would need to handle it correctly. I'm not saying it would save us a lot of LOCs but it seems a cleaner interface in my mind.


    polespinasa commented at 1:43 PM on March 23, 2026:

    I see what you mean, yes could make sense.

  84. in test/functional/feature_assumeutxo.py:554 in e06017e1bf outdated
     549 | +            "mediantime",
     550 | +            "chainwork",
     551 | +            "verificationprogress"
     552 | +        ]
     553 | +        res = n1.getblockchaininfo()
     554 | +        assert "backgroundvalidation" in res.keys()
    


    fjahr commented at 9:45 AM on March 20, 2026:

    nit: bv_res = res["backgroundvalidation"] could make many of the lines below a bit quicker to parse


    polespinasa commented at 12:16 PM on March 20, 2026:

    done

  85. fjahr commented at 9:51 AM on March 20, 2026: contributor

    utACK 4e2c956ce8d24e74e7856d1ce35c44cc78968912

    Thanks for addressing comments. Feel free to leave my remaining nits unaddressed if you don't have to push again, they are pretty minor.

  86. DrahtBot requested review from nebula-21 on Mar 20, 2026
  87. DrahtBot requested review from sedited on Mar 20, 2026
  88. sedited commented at 9:55 AM on March 20, 2026: contributor

    I'd prefer the nits to be addressed, they are minor and I'm sure we'd re-approve quickly.

  89. polespinasa force-pushed on Mar 20, 2026
  90. in src/rpc/blockchain.cpp:1385 in cfd07f7959
    1383 | +                {RPCResult::Type::NUM_TIME, "mediantime", "the median block time expressed in " + UNIX_EPOCH_TIME},
    1384 |                  {RPCResult::Type::NUM, "verificationprogress", "estimate of verification progress [0..1]"},
    1385 |                  {RPCResult::Type::BOOL, "initialblockdownload", "(debug information) estimate of whether this node is in Initial Block Download mode"},
    1386 | +                {RPCResult::Type::OBJ, "backgroundvalidation", /*optional=*/true, "state info regarding background validation process",
    1387 | +                {
    1388 | +                    {RPCResult::Type::NUM, "snapshotheight", "the height of the snapshot block"},
    


    danielabrozzoni commented at 4:03 PM on March 20, 2026:

    nit: maybe this could be expanded to make it clearer, something like "the height of the snapshot block; background validation is catching up to this block from genesis".

    However...my understanding of assumeutxo is very limited, and I don't know how much context the average user has about this. "the height of the snapshot block" made me a bit confused at first, I had to research what we mean by snapshot etc. Feel free to discard if our average user would understand :)


    polespinasa commented at 8:18 AM on March 23, 2026:

    I think it's fine to add more details; it's not too verbose, so I just extended it to make it more explanatory. 🫡

  91. in src/validation.cpp:5527 in 94e9419d5e
    5520 | @@ -5523,6 +5521,20 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c
    5521 |      return std::min<double>(pindex->m_chain_tx_count / fTxTotal, 1.0);
    5522 |  }
    5523 |  
    5524 | +double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex& pindex) const
    5525 | +{
    5526 | +    AssertLockHeld(GetMutex());
    5527 | +    if (!CHECK_NONFATAL(HistoricalChainstate())) return 0.0;
    


    sedited commented at 8:18 AM on March 23, 2026:

    Sorry, should have commented when it was first suggested, but I don't understand the purpose of this. This throws when there is no historical chainstate, so when does this ever return anything? Can we assert here instead? If there is no target block anymore could we return 1 instead of throwing?


    polespinasa commented at 8:34 AM on March 23, 2026:

    Can we assert here instead?

    Yes, I think we could assert here, we should never reach this function if there's no historical chainstate. Added.

    If there is no target block anymore could we return 1 instead of throwing?

    I am not sure about this one tho. If we call this function without a target block, then something ¿is wrong?. If the reason to not have a target block is because validation=1 means that we are not anymore in background validation so this function should never be called. If we are in background validation and we don't have a target block then something is wrong. I think the CHECK_NONFATAL is not wrong here so it reports that something is in a weird state.


    fjahr commented at 8:36 AM on March 23, 2026:

    Since I suggested this: No historical chainstate or no target block would imply that this function was called when assumeutxo wasn't used or the chainstate is broken somehow, so probably that should be a developer error. I don't think there is a case where we could have no historical chainstate or one with no target block where we do use assumeutxo. So if this is the case this function shouldn't have been called in the first place. I may be missing some edge case though.


    polespinasa commented at 8:38 AM on March 23, 2026:

    I added the Assert of HistoricalChainstate but I'm happy to take it back to check_nonfatal if we don't want to crash the node because of that.


    sedited commented at 9:12 AM on March 23, 2026:

    My issue is more with adding a return condition that can seemingly never be reached. We should either return something, or just throw on this condition. Doing both is what does not make sense to me.


    polespinasa commented at 1:38 PM on March 23, 2026:

    Done, I didn't take back the throw on HistoricalChainstate, left the Assert. As said, happy to take it back, I'm pretty concept~0 on what to use here.

  92. polespinasa force-pushed on Mar 23, 2026
  93. polespinasa commented at 8:37 AM on March 23, 2026: member

    Force pushed to fix the mistake in the code comment. Improve the rpc call helper without being to verbose and changing the check_nonfatal to Assert in case we don't have a HistoricalChainstate.

    Diff:

    https://github.com/bitcoin/bitcoin/compare/b05f8b0076f78e1649a6f0ba8e7ec110f7973e44..e42aed69d0127e6d58b90b49b7ac638391f92b71

  94. polespinasa force-pushed on Mar 23, 2026
  95. polespinasa force-pushed on Mar 23, 2026
  96. DrahtBot added the label CI failed on Mar 23, 2026
  97. DrahtBot removed the label CI failed on Mar 23, 2026
  98. in doc/release-notes-33259.md:4 in 297d24faa8
       0 | @@ -0,0 +1,4 @@
       1 | +RPC
       2 | +---
       3 | +
       4 | +The `getblockchaininfo` RPC exposes progress for background validation if the `assumeutxo` feature is used. Previously, `verificationprogress` could return 1.0 and `initialblockdownload` false even if the node was still validating blocks in the background. A new object, `backgroundvalidation`, provides details about the snapshot being validated, including snapshot height, number of blocks processed, best block hash, chainwork, median time, and verification progress.
    


    fjahr commented at 4:15 PM on March 23, 2026:

    Previously, verificationprogress could return 1.0 and initialblockdownload false even if the node was still validating blocks in the background.

    mini-nit: This kind of suggests that this behavior has changed although it hasn't. The sentence could either be removed completely or reworked, with something like this:

    The `getblockchaininfo` RPC now exposes progress for background validation if the `assumeutxo` feature is used. Once a node has synced from snapshot to tip, `verificationprogress` returns 1.0 and `initialblockdownload` false even though the node may still be validating blocks in the background. A new object, `backgroundvalidation`, provides details about the snapshot being validated, including snapshot height, number of blocks processed, best block hash, chainwork, median time, and verification progress.
    

    But the release notes can still be edited later.


    polespinasa commented at 6:39 PM on March 23, 2026:

    Thanks for the suggestion, I picked your comment.

    Force pushed with other nits.

  99. fjahr commented at 4:16 PM on March 23, 2026: contributor

    ACK 297d24faa84080891e091906b83a99b5032d0159

  100. DrahtBot requested review from sedited on Mar 23, 2026
  101. DrahtBot requested review from danielabrozzoni on Mar 23, 2026
  102. in src/validation.cpp:5532 in 3557d29f85 outdated
    5527 | +    Assert(HistoricalChainstate());
    5528 | +    auto target_block = HistoricalChainstate()->TargetBlock();
    5529 | +    CHECK_NONFATAL(target_block);
    5530 | +
    5531 | +    if (pindex.m_chain_tx_count == 0 || target_block->m_chain_tx_count == 0) {
    5532 | +        LogDebug(BCLog::VALIDATION, "[background validation] Block %d has unset m_chain_tx_count. Unable to estimate verification progress.", pindex.nHeight);
    


    danielabrozzoni commented at 4:36 PM on March 23, 2026:

    nit: the log might be misleading, because it could be that it's target_block with an unset m_chain_tx_count

    -        LogDebug(BCLog::VALIDATION, "[background validation] Block %d has unset m_chain_tx_count. Unable to estimate verification progress.", pindex.nHeight);
    +        LogDebug(BCLog::VALIDATION, "[background validation] Block %d has unset m_chain_tx_count. Unable to estimate verification progress.", pindex.m_chain_tx_count == 0 ? pindex.nHeight : target_block->nHeight);
    

    polespinasa commented at 6:22 PM on March 23, 2026:

    I think this cannot happen. The value is hardcoded in src/kernel/chainparams.cpp. Actually I think I can even drop the target_block->m_chain_tx_count == 0 because it can't be 0.

    What do you think? @fjahr

    <details> <summary>kernel/chainparams.cpp</summary>

            m_assumeutxo_data = {
                {
                    .height = 840'000,
                    .hash_serialized = AssumeutxoHash{uint256{"a2a5521b1b5ab65f67818e5e8eccabb7171a517f9e2382208f77687310768f96"}},
                    .m_chain_tx_count = 991032194,
                    .blockhash = uint256{"0000000000000000000320283a032748cef8227873ff4872689bf23f1cda83a5"},
                },
                {
                    .height = 880'000,
                    .hash_serialized = AssumeutxoHash{uint256{"dbd190983eaf433ef7c15f78a278ae42c00ef52e0fd2a54953782175fbadcea9"}},
                    .m_chain_tx_count = 1145604538,
                    .blockhash = uint256{"000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"},
                },
                {
                    .height = 910'000,
                    .hash_serialized = AssumeutxoHash{uint256{"4daf8a17b4902498c5787966a2b51c613acdab5df5db73f196fa59a4da2f1568"}},
                    .m_chain_tx_count = 1226586151,
                    .blockhash = uint256{"0000000000000000000108970acb9522ffd516eae17acddcb1bd16469194a821"},
                },
                {
                    .height = 935'000,
                    .hash_serialized = AssumeutxoHash{uint256{"e4b90ef9eae834f56c4b64d2d50143cee10ad87994c614d7d04125e2a6025050"}},
                    .m_chain_tx_count = 1305397408,
                    .blockhash = uint256{"0000000000000000000147034958af1652b2b91bba607beacc5e72a56f0fb5ee"},
                }
            };
    

    </details>

  103. sedited approved
  104. sedited commented at 4:39 PM on March 23, 2026: contributor

    ACK 297d24faa84080891e091906b83a99b5032d0159

  105. in src/rpc/blockchain.cpp:1436 in 4a0c2b7177
    1428 | @@ -1420,6 +1429,19 @@ RPCHelpMan getblockchaininfo()
    1429 |      obj.pushKV("mediantime", tip.GetMedianTimePast());
    1430 |      obj.pushKV("verificationprogress", chainman.GuessVerificationProgress(&tip));
    1431 |      obj.pushKV("initialblockdownload", chainman.IsInitialBlockDownload());
    1432 | +    auto historical_blocks{chainman.GetHistoricalBlockRange()};
    1433 | +    if (historical_blocks) {
    1434 | +        UniValue background_validation(UniValue::VOBJ);
    1435 | +        const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
    1436 | +        const std::optional<int> background_height{active_chainstate.SnapshotBase()->nHeight};
    


    danielabrozzoni commented at 5:02 PM on March 23, 2026:

    What's the reason for the optional here? I thought at first it was to protect from SnapshotBase returning null, but if that happened, ->nHeight would be a null pointer dereference.

    (I don't think it can happen that SnapshotBase returns null if GetHistoricalBlockRange is non-null...)

    Also, I tried reviewing with Claude, and it pointed out that instead of calling active_chainstate.SnapshotBase(), you should be able to use historical_blocks->second, since the active chainstate snapshot base is always the same as the historical chainstate target block:

    https://github.com/bitcoin/bitcoin/blob/1a1f584360ce053f9f832e310d063d686f86c6d7/src/validation.cpp#L6162

    I think claude's suggestion makes sense, but don't trust verify pls :)


    polespinasa commented at 6:36 PM on March 23, 2026:

    What's the reason for the optional here? I thought at first it was to protect from SnapshotBase returning null, but if that happened, ->nHeight would be a null pointer dereference.

    Nice catch!

    active_chainstate.SnapshotBase(), you should be able to use historical_blocks->second

    This is also true :) Fixed See #33259 (review) for a similar discussion.

    I modified a bit the code to make it more readable and self explanatory:

            const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
            const CBlockIndex& btarget{*CHECK_NONFATAL(historical_blocks->second)};
            background_validation.pushKV("snapshotheight", btarget.nHeight);
            background_validation.pushKV("blocks", btip.nHeight);
    
  106. danielabrozzoni commented at 5:18 PM on March 23, 2026: member

    I left a question :) I'm almost done reviewing and testing!

  107. DrahtBot requested review from danielabrozzoni on Mar 23, 2026
  108. polespinasa force-pushed on Mar 23, 2026
  109. polespinasa commented at 6:42 PM on March 23, 2026: member

    Sorry for breaking the ACKs @fjahr @sedited

    Force pushed to apply @danielabrozzoni nits and updated the release notes

    $ git diff 297d24faa84080891e091906b83a99b5032d0159..07384c577acbbd9b8a65c0906608b429892aa5cf
    diff --git a/doc/release-notes-33259.md b/doc/release-notes-33259.md
    index a635738e30..46f339fe7b 100644
    --- a/doc/release-notes-33259.md
    +++ b/doc/release-notes-33259.md
    @@ -1,4 +1,4 @@
     RPC
     ---
     
    -The `getblockchaininfo` RPC exposes progress for background validation if the `assumeutxo` feature is used. Previously, `verificationprogress` could return 1.0 and `initialblockdownload` false even if the node was still validating blocks in the background. A new object, `backgroundvalidation`, provides details about the snapshot being validated, including snapshot height, number of blocks processed, best block hash, chainwork, median time, and verification progress.
    +The `getblockchaininfo` RPC now exposes progress for background validation if the `assumeutxo` feature is used. Once a node has synced from snapshot to tip, `verificationprogress` returns 1.0 and `initialblockdownload` false even though the node may still be validating blocks in the background. A new object, `backgroundvalidation`, provides details about the snapshot being validated, including snapshot height, number of blocks processed, best block hash, chainwork, median time, and verification progress.
    diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
    index 8b1df2ab26..dc6473b927 100644
    --- a/src/rpc/blockchain.cpp
    +++ b/src/rpc/blockchain.cpp
    @@ -1433,8 +1433,8 @@ RPCHelpMan getblockchaininfo()
         if (historical_blocks) {
             UniValue background_validation(UniValue::VOBJ);
             const CBlockIndex& btip{*CHECK_NONFATAL(historical_blocks->first)};
    -        const std::optional<int> background_height{active_chainstate.SnapshotBase()->nHeight};
    -        if (background_height) background_validation.pushKV("snapshotheight", *background_height);
    +        const CBlockIndex& btarget{*CHECK_NONFATAL(historical_blocks->second)};
    +        background_validation.pushKV("snapshotheight", btarget.nHeight);
             background_validation.pushKV("blocks", btip.nHeight);
             background_validation.pushKV("bestblockhash", btip.GetBlockHash().GetHex());
             background_validation.pushKV("mediantime", btip.GetMedianTimePast());
    
    
  110. fjahr commented at 10:53 PM on March 23, 2026: contributor

    ACK 07384c577acbbd9b8a65c0906608b429892aa5cf

  111. DrahtBot requested review from sedited on Mar 23, 2026
  112. danielabrozzoni approved
  113. danielabrozzoni commented at 12:23 PM on March 24, 2026: member

    light tACK 07384c577a

    Code looks good to me, I tested locally by starting up a new node, loading a utxo set, and verifying the getblockchaininfo output.

    I'm only light acking since I'm very unfamiliar with this part of the codebase :)

  114. in src/validation.cpp:5529 in 3557d29f85
    5520 | @@ -5523,6 +5521,20 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c
    5521 |      return std::min<double>(pindex->m_chain_tx_count / fTxTotal, 1.0);
    5522 |  }
    5523 |  
    5524 | +double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex& pindex) const
    5525 | +{
    5526 | +    AssertLockHeld(GetMutex());
    5527 | +    Assert(HistoricalChainstate());
    5528 | +    auto target_block = HistoricalChainstate()->TargetBlock();
    5529 | +    CHECK_NONFATAL(target_block);
    


    sedited commented at 1:04 PM on March 24, 2026:

    I think CHECK_NONFATAL only really makes sense in the RPC code and similar contexts. If we throw here and the exception bubbles up to other validation code, it will be fatal. Are we really sure this cannot be null? We do set the target block to nullptr if we detect an invalid snapshot on validation.


    danielabrozzoni commented at 1:42 PM on March 24, 2026:

    I don't think it can be null, because HistoricalChainstate() checks the presence of cs->m_target_blockhash.

    One thing to note is that we are calling HistoricalChainstate() twice: we first check that it's not null, then we use it to get the target block. Can HistoricalChainstate pass the first check, and become null once we use it? I don't think so, because we are holding the mutex, but I'm not 100% sure.

    In any case, I'm happy to re-ACK if this gets changed!


    sedited commented at 1:54 PM on March 24, 2026:

    One thing to note is that we are calling HistoricalChainstate() twice: we first check that it's not null, then we use it to get the target block. Can HistoricalChainstate pass the first check, and become null once we use it? I don't think so, because we are holding the mutex, but I'm not 100% sure.

    I also think it's probably fine, it just feels brittle.


    polespinasa commented at 2:50 PM on March 24, 2026:

    I don't think it can be null, because HistoricalChainstate() checks the presence of cs->m_target_blockhash.

    I think we can maybe remove the CHECK_NONFATAL because of this. We already asserted HistoricalChainstate() and it should not get modified as we hold the mutex.

    Will remove it, I agree it is a bit weird to use here, and checking the code we mostly use it in RPC code and some wallet stuff.

  115. DrahtBot requested review from sedited on Mar 24, 2026
  116. log: update progress calculations for background validation
    updates estimations to the block snapshot instead of the main chain tip as it will stop validation after reaching that height
    5b2e4c4a88
  117. rpc, log: add backgroundvalidation to getblockchaininfo a3d6f32a39
  118. test: add background validation test for getblockchaininfo af629821cf
  119. release note 25f69d970a
  120. polespinasa force-pushed on Mar 24, 2026
  121. polespinasa commented at 2:53 PM on March 24, 2026: member

    Force pushed to drop a "useless" CHECK_NONFATAL instruction. See #33259 (review) for context.

    $ git diff 07384c577acbbd9b8a65c0906608b429892aa5cf..25f69d970a411057681b1792c1c32084f4e49cf1
    diff --git a/src/validation.cpp b/src/validation.cpp
    index e4974fc580..59654c9d19 100644
    --- a/src/validation.cpp
    +++ b/src/validation.cpp
    @@ -5526,7 +5526,6 @@ double ChainstateManager::GetBackgroundVerificationProgress(const CBlockIndex& p
         AssertLockHeld(GetMutex());
         Assert(HistoricalChainstate());
         auto target_block = HistoricalChainstate()->TargetBlock();
    -    CHECK_NONFATAL(target_block);
     
         if (pindex.m_chain_tx_count == 0 || target_block->m_chain_tx_count == 0) {
             LogDebug(BCLog::VALIDATION, "[background validation] Block %d has unset m_chain_tx_count. Unable to estimate verification progress.", pindex.nHeight);
    
    
  122. sedited approved
  123. sedited commented at 3:34 PM on March 24, 2026: contributor

    ACK 25f69d970a411057681b1792c1c32084f4e49cf1

  124. DrahtBot requested review from fjahr on Mar 24, 2026
  125. DrahtBot requested review from danielabrozzoni on Mar 24, 2026
  126. fjahr commented at 4:11 PM on March 24, 2026: contributor

    ACK 25f69d970a411057681b1792c1c32084f4e49cf1

  127. danielabrozzoni commented at 5:16 PM on March 24, 2026: member

    ACK 25f69d970a411057681b1792c1c32084f4e49cf1

  128. achow101 commented at 9:22 PM on March 24, 2026: member

    ACK 25f69d970a411057681b1792c1c32084f4e49cf1

  129. achow101 merged this on Mar 24, 2026
  130. achow101 closed this on Mar 24, 2026

  131. ismaelsadeeq commented at 3:02 PM on March 25, 2026: member

    Concept ACK

  132. polespinasa deleted the branch on Mar 25, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-02 18:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me