Merge bitcoin/bitcoin#26325: rpc: Return accurate results for scanblocks

5ca7a7be76 rpc: Return accurate results for scanblocks (Aurèle Oulès)

Pull request description:

  Implements #26322.
  Adds a `filter_false_positives` mode to `scanblocks` to accurately verify results from blockfilters.

  If the option is enabled, pre-results given by blockfilters will be filtered out again by checking vouts and vins of all transactions of the relevant blocks against the given descriptors.

  ### Master
  ```bash
  ./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]'
  {
    "from_height": 0,
    "to_height": 2376055,
    "relevant_blocks": [
      "000000000001bc35077dec4104e0ab1f667ae27059bd907f9a8fac55c802ae36",
      "00000000000120a9c50542d73248fb7c37640c252850f0cf273134ad9febaf61",
      "0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed",
      "0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3"
    ]
  }
  ```

  ### PR (without `filter_false_positives` mode)
  Same as master
  ```bash
  ./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]' filter_false_positives=false
  {
    "from_height": 0,
    "to_height": 2376055,
    "relevant_blocks": [
      "000000000001bc35077dec4104e0ab1f667ae27059bd907f9a8fac55c802ae36",
      "00000000000120a9c50542d73248fb7c37640c252850f0cf273134ad9febaf61",
      "0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed",
      "0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3"
    ]
  }
  ```

  ### PR (with `filter_false_positives` mode)
  ```bash
  ./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]' filter_false_positives=true
  {
    "from_height": 0,
    "to_height": 2376058,
    "relevant_blocks": [
      "0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed",
      "0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3"
    ]
  }
  ```

  Also adds a test to check that the blockhash of a transaction will be included in the `relevant_blocks` whether the `filter_false_positives` mode is enabled or not.

ACKs for top commit:
  achow101:
    ACK 5ca7a7be76
  theStack:
    re-ACK 5ca7a7be76
  furszy:
    Code review ACK 5ca7a7be

Tree-SHA512: e8f3cceddddd66f59509717b6314d89e2fef241e13cee81b18fd95e8362cbb95cc40f884342ce6cf892a86febd9e2d434afce05d51892240e67f72ae991852e7

src/rpc/blockchain.cpp

@@ -25,6 +25,7 @@
 #include <net_processing.h>
 #include <node/blockstorage.h>
 #include <node/context.h>
+#include <node/transaction.h>
 #include <node/utxo_snapshot.h>
 #include <primitives/transaction.h>
 #include <rpc/server.h>
@@ -2268,17 +2269,47 @@ public:
     }
 };
 
+static bool CheckBlockFilterMatches(BlockManager& blockman, const CBlockIndex& blockindex, const GCSFilter::ElementSet& needles)
+{
+    const CBlock block{GetBlockChecked(blockman, &blockindex)};
+    const CBlockUndo block_undo{GetUndoChecked(blockman, &blockindex)};
+
+    // Check if any of the outputs match the scriptPubKey
+    for (const auto& tx : block.vtx) {
+        if (std::any_of(tx->vout.cbegin(), tx->vout.cend(), [&](const auto& txout) {
+                return needles.count(std::vector<unsigned char>(txout.scriptPubKey.begin(), txout.scriptPubKey.end())) != 0;
+            })) {
+            return true;
+        }
+    }
+    // Check if any of the inputs match the scriptPubKey
+    for (const auto& txundo : block_undo.vtxundo) {
+        if (std::any_of(txundo.vprevout.cbegin(), txundo.vprevout.cend(), [&](const auto& coin) {
+                return needles.count(std::vector<unsigned char>(coin.out.scriptPubKey.begin(), coin.out.scriptPubKey.end())) != 0;
+            })) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 static RPCHelpMan scanblocks()
 {
     return RPCHelpMan{"scanblocks",
-        "\nReturn relevant blockhashes for given descriptors.\n"
+        "\nReturn relevant blockhashes for given descriptors (requires blockfilterindex).\n"
         "This call may take several minutes. Make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)",
         {
             scan_action_arg_desc,
             scan_objects_arg_desc,
             RPCArg{"start_height", RPCArg::Type::NUM, RPCArg::Default{0}, "Height to start to scan from"},
             RPCArg{"stop_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"chain tip"}, "Height to stop to scan"},
-            RPCArg{"filtertype", RPCArg::Type::STR, RPCArg::Default{BlockFilterTypeName(BlockFilterType::BASIC)}, "The type name of the filter"}
+            RPCArg{"filtertype", RPCArg::Type::STR, RPCArg::Default{BlockFilterTypeName(BlockFilterType::BASIC)}, "The type name of the filter"},
+            RPCArg{"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
+                {
+                    {"filter_false_positives", RPCArg::Type::BOOL, RPCArg::Default{false}, "Filter false positives (slower and may fail on pruned nodes). Otherwise they may occur at a rate of 1/M"},
+                },
+                RPCArgOptions{.oneline_description="\"options\""}},
         },
         {
             scan_result_status_none,
@@ -2338,6 +2369,9 @@ static RPCHelpMan scanblocks()
             throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Unknown filtertype");
         }
 
+        UniValue options{request.params[5].isNull() ? UniValue::VOBJ : request.params[5]};
+        bool filter_false_positives{options.exists("filter_false_positives") ? options["filter_false_positives"].get_bool() : false};
+
         BlockFilterIndex* index = GetBlockFilterIndex(filtertype);
         if (!index) {
             throw JSONRPCError(RPC_MISC_ERROR, "Index is not enabled for filtertype " + filtertype_name);
@@ -2408,6 +2442,15 @@ static RPCHelpMan scanblocks()
                     for (const BlockFilter& filter : filters) {
                         // compare the elements-set with each filter
                         if (filter.GetFilter().MatchAny(needle_set)) {
+                            if (filter_false_positives) {
+                                // Double check the filter matches by scanning the block
+                                const CBlockIndex& blockindex = *CHECK_NONFATAL(WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(filter.GetBlockHash())));
+
+                                if (!CheckBlockFilterMatches(chainman.m_blockman, blockindex, needle_set)) {
+                                    continue;
+                                }
+                            }
+
                             blocks.push_back(filter.GetBlockHash().GetHex());
                             LogPrint(BCLog::RPC, "scanblocks: found match in %s\n", filter.GetBlockHash().GetHex());
                         }

src/rpc/client.cpp

@@ -86,6 +86,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "scanblocks", 1, "scanobjects" },
     { "scanblocks", 2, "start_height" },
     { "scanblocks", 3, "stop_height" },
+    { "scanblocks", 5, "options" },
     { "scantxoutset", 1, "scanobjects" },
     { "addmultisigaddress", 0, "nrequired" },
     { "addmultisigaddress", 1, "keys" },

test/functional/feature_pruning.py

@@ -84,7 +84,7 @@ class PruneTest(BitcoinTestFramework):
             ["-maxreceivebuffer=20000", "-prune=550"],
             ["-maxreceivebuffer=20000"],
             ["-maxreceivebuffer=20000"],
-            ["-prune=550"],
+            ["-prune=550", "-blockfilterindex=1"],
         ]
         self.rpc_timeout = 120
 
@@ -356,7 +356,7 @@ class PruneTest(BitcoinTestFramework):
         self.connect_nodes(0, 5)
         nds = [self.nodes[0], self.nodes[5]]
         self.sync_blocks(nds, wait=5, timeout=300)
-        self.restart_node(5, extra_args=["-prune=550"]) # restart to trigger rescan
+        self.restart_node(5, extra_args=["-prune=550", "-blockfilterindex=1"]) # restart to trigger rescan
         self.log.info("Success")
 
     def run_test(self):
@@ -472,7 +472,20 @@ class PruneTest(BitcoinTestFramework):
         self.log.info("Test invalid pruning command line options")
         self.test_invalid_command_line_options()
 
+        self.test_scanblocks_pruned()
+
         self.log.info("Done")
 
+    def test_scanblocks_pruned(self):
+        node = self.nodes[5]
+        genesis_blockhash = node.getblockhash(0)
+        false_positive_spk = bytes.fromhex("001400000000000000000000000000000000000cadcb")
+
+        assert genesis_blockhash in node.scanblocks(
+            "start", [{"desc": f"raw({false_positive_spk.hex()})"}], 0, 0)['relevant_blocks']
+
+        assert_raises_rpc_error(-1, "Block not available (pruned data)", node.scanblocks,
+            "start", [{"desc": f"raw({false_positive_spk.hex()})"}], 0, 0, "basic", {"filter_false_positives": True})
+
 if __name__ == '__main__':
     PruneTest().main()

test/functional/rpc_scanblocks.py

@@ -62,6 +62,12 @@ class ScanblocksTest(BitcoinTestFramework):
         # make sure the blockhash is present when using the first mined block as start_height
         assert blockhash in node.scanblocks(
             "start", [f"addr({addr_1})"], height)['relevant_blocks']
+        for v in [False, True]:
+            assert blockhash in node.scanblocks(
+                action="start",
+                scanobjects=[f"addr({addr_1})"],
+                start_height=height,
+                options={"filter_false_positives": v})['relevant_blocks']
 
         # also test the stop height
         assert blockhash in node.scanblocks(
@@ -94,8 +100,11 @@ class ScanblocksTest(BitcoinTestFramework):
         assert genesis_blockhash in node.scanblocks(
             "start", [{"desc": f"raw({false_positive_spk.hex()})"}], 0, 0)['relevant_blocks']
 
-        # TODO: after an "accurate" mode for scanblocks is implemented (e.g. PR #26325)
-        # check here that it filters out the false-positive
+        # check that the filter_false_positives option works
+        assert genesis_blockhash in node.scanblocks(
+            "start", [{"desc": f"raw({genesis_coinbase_spk.hex()})"}], 0, 0, "basic", {"filter_false_positives": True})['relevant_blocks']
+        assert genesis_blockhash not in node.scanblocks(
+            "start", [{"desc": f"raw({false_positive_spk.hex()})"}], 0, 0, "basic", {"filter_false_positives": True})['relevant_blocks']
 
         # test node with disabled blockfilterindex
         assert_raises_rpc_error(-1, "Index is not enabled for filtertype basic",