Merge bitcoin/bitcoin#29003: rpc: fix getrawtransaction segfault

9075a44 test: add regression test for the getrawtransaction segfault (Martin Zumsande)
494a926 rpc: fix getrawtransaction segfault (Martin Zumsande)

Pull request description:

  The crash, reported in #28986, happens when calling `getrawtransaction` for any mempool transaction with `verbosity=2`, while pruning, because the rpc calls `IsBlockPruned(const CBlockIndex* pblockindex)`, which dereferences `pblockindex` without a check.

  For ease of backporting this PR fixes it just locally in `rpc/rawtransaction.cpp` by moving the check for`!blockindex` up so that `IsBlockPruned()` will not be called with a `nullptr`. We might also want to change `IsBlockPruned()` so it doesn't crash when called with a `nullptr`, but I didn't do that here.

  Fixes #28986

ACKs for top commit:
  maflcko:
    lgtm test-was-added ACK 9075a44
  theStack:
    Tested ACK 9075a44

Tree-SHA512: 0f7ed52579487196c206e16b45582b64e4b02ecf2a2eb0a31d2f3b52415bc9c64278cb94259314ef14ab7fb393c6195f79b3027d6de471d67614e51474498b11

Author: fanquake

src/rpc/rawtransaction.cpp

@@ -396,7 +396,7 @@ static RPCHelpMan getrawtransaction()
         LOCK(cs_main);
         blockindex = chainman.m_blockman.LookupBlockIndex(hash_block);
     }
-    if (verbosity == 1) {
+    if (verbosity == 1 || !blockindex) {
         TxToJSON(*tx, hash_block, result, chainman.ActiveChainstate());
         return result;
     }
@@ -405,8 +405,7 @@ static RPCHelpMan getrawtransaction()
     CBlock block;
     const bool is_block_pruned{WITH_LOCK(cs_main, return chainman.m_blockman.IsBlockPruned(blockindex))};
 
-    if (tx->IsCoinBase() ||
-        !blockindex || is_block_pruned ||
+    if (tx->IsCoinBase() || is_block_pruned ||
         !(chainman.m_blockman.UndoReadFromDisk(blockUndo, *blockindex) && chainman.m_blockman.ReadBlockFromDisk(block, *blockindex))) {
         TxToJSON(*tx, hash_block, result, chainman.ActiveChainstate());
         return result;

test/functional/rpc_rawtransaction.py

@@ -32,6 +32,7 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
+    assert_greater_than,
     assert_raises_rpc_error,
 )
 from test_framework.wallet import (
@@ -70,7 +71,7 @@ def set_test_params(self):
         self.extra_args = [
             ["-txindex"],
             ["-txindex"],
-            [],
+            ["-fastprune", "-prune=1"],
         ]
         # whitelist all peers to speed up tx relay / mempool sync
         for args in self.extra_args:
@@ -85,7 +86,6 @@ def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])
 
         self.getrawtransaction_tests()
-        self.getrawtransaction_verbosity_tests()
         self.createrawtransaction_tests()
         self.sendrawtransaction_tests()
         self.sendrawtransaction_testmempoolaccept_tests()
@@ -94,6 +94,8 @@ def run_test(self):
         if self.is_specified_wallet_compiled() and not self.options.descriptors:
             self.import_deterministic_coinbase_privkeys()
             self.raw_multisig_transaction_legacy_tests()
+        self.getrawtransaction_verbosity_tests()
+
 
     def getrawtransaction_tests(self):
         tx = self.wallet.send_self_transfer(from_node=self.nodes[0])
@@ -243,6 +245,13 @@ def getrawtransaction_verbosity_tests(self):
         coin_base = self.nodes[1].getblock(block1)['tx'][0]
         gottx = self.nodes[1].getrawtransaction(txid=coin_base, verbosity=2, blockhash=block1)
         assert 'fee' not in gottx
+        # check that verbosity 2 for a mempool tx will fallback to verbosity 1
+        # Do this with a pruned chain, as a regression test for https://github.com/bitcoin/bitcoin/pull/29003
+        self.generate(self.nodes[2], 400)
+        assert_greater_than(self.nodes[2].pruneblockchain(250), 0)
+        mempool_tx = self.wallet.send_self_transfer(from_node=self.nodes[2])['txid']
+        gottx = self.nodes[2].getrawtransaction(txid=mempool_tx, verbosity=2)
+        assert 'fee' not in gottx
 
     def createrawtransaction_tests(self):
         self.log.info("Test createrawtransaction")