Merge bitcoin/bitcoin#24629: Bugfix: RPC/blockchain: pruneblockchain: Return the height of the actual last pruned block

e593ae0 Bugfix: RPC/blockchain: pruneblockchain: Return the height of the actual last pruned block (Luke Dashjr)

Pull request description:

  From 0.14 (2017 Mar) until before 0.19 (2019 Nov), the height of the last block pruned was returned, subject to a bug if there were blocks left unpruned due to sharing files with later blocks.

  In #15991, this was "fixed" to the current implementation, introducing a new bug: now, it returns the first *unpruned* block.

  Since the user provides the parameter as a block to include in pruning, it makes more sense to fix the behaviour to match the documentation.

  ~~(Additionally, the description of "pruneheight" in getblockchaininfo is fixed to be technically correct)~~

ACKs for top commit:
  fjahr:
    utACK e593ae0
  ryanofsky:
    Code review ACK e593ae0. Just rebased since last review. Maybe some of the original reviewers of #15991 will want to take a look at this to correct the mistake that was introduced there!

Tree-SHA512: c2d511df80682d57260aae8af1665f9d7eaed16448f185f4c9f23c78fa9b8289a02053da7a0b83643fef57610d601ea63b59ff39661a51f4827f1eb27cc30594

Author: MacroFake

src/rpc/blockchain.cpp

@@ -790,7 +790,7 @@ static RPCHelpMan pruneblockchain()
     const CBlockIndex& block{*CHECK_NONFATAL(active_chain.Tip())};
     const CBlockIndex* last_block{active_chainstate.m_blockman.GetFirstStoredBlock(block)};
 
-    return static_cast<uint64_t>(last_block->nHeight);
+    return static_cast<int64_t>(last_block->nHeight - 1);
 },
     };
 }

test/functional/feature_index_prune.py

@@ -73,7 +73,7 @@ def run_test(self):
                 pruneheight_new = node.pruneblockchain(400)
                 # the prune heights used here and below are magic numbers that are determined by the
                 # thresholds at which block files wrap, so they depend on disk serialization and default block file size.
-                assert_equal(pruneheight_new, 249)
+                assert_equal(pruneheight_new, 248)
 
         self.log.info("check if we can access the tips blockfilter and coinstats when we have pruned some blocks")
         tip = self.nodes[0].getbestblockhash()
@@ -108,7 +108,7 @@ def run_test(self):
         self.log.info("prune exactly up to the indices best blocks while the indices are disabled")
         for i in range(3):
             pruneheight_2 = self.nodes[i].pruneblockchain(1000)
-            assert_equal(pruneheight_2, 751)
+            assert_equal(pruneheight_2, 750)
             # Restart the nodes again with the indices activated
             self.restart_node(i, extra_args=self.extra_args[i])
 
@@ -142,7 +142,7 @@ def run_test(self):
         for node in self.nodes[:2]:
             with node.assert_debug_log(['limited pruning to height 2489']):
                 pruneheight_new = node.pruneblockchain(2500)
-                assert_equal(pruneheight_new, 2006)
+                assert_equal(pruneheight_new, 2005)
 
         self.log.info("ensure that prune locks don't prevent indices from failing in a reorg scenario")
         with self.nodes[0].assert_debug_log(['basic block filter index prune lock moved back to 2480']):

test/functional/feature_pruning.py

@@ -291,7 +291,7 @@ def height(index):
 
         def prune(index):
             ret = node.pruneblockchain(height=height(index))
-            assert_equal(ret, node.getblockchaininfo()['pruneheight'])
+            assert_equal(ret + 1, node.getblockchaininfo()['pruneheight'])
 
         def has_block(index):
             return os.path.isfile(os.path.join(self.nodes[node_number].datadir, self.chain, "blocks", f"blk{index:05}.dat"))