Fix FullNodeAPI's request_block_headers returned filter and simplify …

…this method.

chia/_tests/wallet/sync/test_wallet_sync.py

@@ -11,7 +11,8 @@
 
 import pytest
 from aiosqlite import Error as AIOSqliteError
-from chia_rs import confirm_not_included_already_hashed
+from chia_rs import G2Element, confirm_not_included_already_hashed
+from chiabip158 import PyBIP158
 from colorlog import getLogger
 
 from chia._tests.connection_utils import disconnect_all, disconnect_all_and_reconnect
@@ -34,17 +35,25 @@
     CoinState,
     RequestAdditions,
     RespondAdditions,
+    RespondBlockHeader,
     RespondBlockHeaders,
     SendTransaction,
 )
 from chia.server.outbound_message import Message, make_msg
+from chia.server.server import ChiaServer
 from chia.server.ws_connection import WSChiaConnection
 from chia.simulator.add_blocks_in_batches import add_blocks_in_batches
+from chia.simulator.block_tools import BlockTools
+from chia.simulator.full_node_simulator import FullNodeSimulator
 from chia.simulator.simulator_protocol import FarmNewBlockProtocol
 from chia.types.blockchain_format.program import Program
+from chia.types.blockchain_format.serialized_program import SerializedProgram
 from chia.types.blockchain_format.sized_bytes import bytes32
+from chia.types.coin_spend import make_spend
+from chia.types.condition_opcodes import ConditionOpcode
 from chia.types.full_block import FullBlock
 from chia.types.peer_info import PeerInfo
+from chia.types.spend_bundle import SpendBundle
 from chia.types.validation_state import ValidationState
 from chia.util.augmented_chain import AugmentedBlockchain
 from chia.util.hash import std_hash
@@ -94,7 +103,6 @@ async def test_request_block_headers(
     assert len(bh) == 6
     assert [x.reward_chain_block.height for x in default_400_blocks[10:16]] == [x.reward_chain_block.height for x in bh]
     assert [x.foliage for x in default_400_blocks[10:16]] == [x.foliage for x in bh]
-    assert [x.transactions_filter for x in bh] == [b"\x00"] * 6
 
     num_blocks = 20
     new_blocks = bt.get_consecutive_blocks(num_blocks, block_list_input=default_400_blocks, pool_reward_puzzle_hash=ph)
@@ -152,6 +160,71 @@ async def test_request_block_headers_rejected(
     assert msg.type == ProtocolMessageTypes.reject_block_headers.value
 
 
+@pytest.mark.limit_consensus_modes(reason="save time")
+@pytest.mark.anyio
+async def test_request_block_headers_transactions_filter(
+    one_node_one_block: tuple[FullNodeSimulator, ChiaServer, BlockTools], benchmark_runner: BenchmarkRunner
+) -> None:
+    """
+    Tests that `request_block_headers` returns a transactions filter that
+        correctly reflects the blocks transactions.
+    For completeness, we're also comparing the outcome of
+        `request_block_headers` in this regard, to `request_header_blocks` as
+        well as `request_block_header`.
+    """
+    full_node_api, _, bt = one_node_one_block
+    ph = SerializedProgram.to(1).get_tree_hash()
+    for _ in range(2):
+        await full_node_api.farm_new_transaction_block(FarmNewBlockProtocol(ph))
+    # Generate a block with our test spend
+    coins = await full_node_api.full_node.coin_store.get_coin_records_by_puzzle_hash(False, ph)
+    [parent_coin] = [c.coin for c in coins if c.coin.amount == 250_000_000_000]
+    sb = SpendBundle(
+        [
+            make_spend(
+                parent_coin, SerializedProgram.to(1), SerializedProgram.to([[ConditionOpcode.CREATE_COIN, ph, 42]])
+            )
+        ],
+        G2Element(),
+    )
+    blocks = await full_node_api.get_all_full_blocks()
+    blocks = bt.get_consecutive_blocks(1, blocks, guarantee_transaction_block=True, transaction_data=sb)
+    new_block = blocks[-1]
+    await full_node_api.full_node.add_block(new_block)
+    # Compute the expected transactions filter
+    [test_spend] = sb.additions()
+    byte_array_tx = (
+        [bytearray(test_spend.puzzle_hash)]
+        + [bytearray(coin.puzzle_hash) for coin in new_block.get_included_reward_coins()]
+        + [bytearray(parent_coin.name())]
+    )
+    expected_transactions_filter = bytes(PyBIP158(byte_array_tx).GetEncoded())
+    # Perform the request and check the transactions filter
+    msg = await full_node_api.request_block_headers(
+        wallet_protocol.RequestBlockHeaders(uint32(new_block.height), uint32(new_block.height), True)
+    )
+    assert msg is not None
+    res_block_headers = RespondBlockHeaders.from_bytes(msg.data)
+    block_headers = res_block_headers.header_blocks
+    assert len(block_headers) == 1
+    block_header = block_headers[0]
+    assert block_header.transactions_filter == expected_transactions_filter
+    # Go further and compare this to the outcome of request_header_blocks
+    msg = await full_node_api.request_header_blocks(
+        wallet_protocol.RequestHeaderBlocks(uint32(new_block.height), uint32(new_block.height))
+    )
+    assert msg is not None
+    block_headers_res = RespondBlockHeaders.from_bytes(msg.data)
+    assert block_headers_res.header_blocks == block_headers
+    assert block_headers_res.header_blocks[0].transactions_filter == expected_transactions_filter
+    # Go even further and compare this to the outcome of request_block_header
+    msg = await full_node_api.request_block_header(wallet_protocol.RequestBlockHeader(uint32(new_block.height)))
+    assert msg is not None
+    block_header_res = RespondBlockHeader.from_bytes(msg.data)
+    assert block_header_res.header_block == block_header
+    assert block_header_res.header_block.transactions_filter == expected_transactions_filter
+
+
 @pytest.mark.parametrize(
     "two_wallet_nodes",
     [dict(disable_capabilities=[Capability.BLOCK_HEADERS]), dict(disable_capabilities=[Capability.BASE])],

chia/full_node/full_node_api.py

@@ -67,7 +67,6 @@
 from chia.types.unfinished_block import UnfinishedBlock
 from chia.util.batches import to_batches
 from chia.util.db_wrapper import SQLITE_MAX_VARIABLE_NUMBER
-from chia.util.full_block_utils import header_block_from_block
 from chia.util.generator_tools import get_block_header
 from chia.util.hash import std_hash
 from chia.util.ints import uint8, uint32, uint64, uint128
@@ -1426,50 +1425,28 @@ async def request_puzzle_solution(self, request: wallet_protocol.RequestPuzzleSo
 
     @metadata.request()
     async def request_block_headers(self, request: wallet_protocol.RequestBlockHeaders) -> Optional[Message]:
-        """Returns header blocks by directly streaming bytes into Message
-
+        """
         This method should be used instead of RequestHeaderBlocks
         """
         reject = RejectBlockHeaders(request.start_height, request.end_height)
-
         if request.end_height < request.start_height or request.end_height - request.start_height > 128:
             return make_msg(ProtocolMessageTypes.reject_block_headers, reject)
-        if self.full_node.block_store.db_wrapper.db_version == 2:
-            try:
-                blocks_bytes = await self.full_node.block_store.get_block_bytes_in_range(
-                    request.start_height, request.end_height
-                )
-            except ValueError:
-                return make_msg(ProtocolMessageTypes.reject_block_headers, reject)
-
-        else:
-            height_to_hash = self.full_node.blockchain.height_to_hash
-            header_hashes: list[bytes32] = []
-            for i in range(request.start_height, request.end_height + 1):
-                header_hash: Optional[bytes32] = height_to_hash(uint32(i))
-                if header_hash is None:
-                    return make_msg(ProtocolMessageTypes.reject_header_blocks, reject)
-                header_hashes.append(header_hash)
-
-            blocks_bytes = await self.full_node.block_store.get_block_bytes_by_hash(header_hashes)
-        if len(blocks_bytes) != (request.end_height - request.start_height + 1):  # +1 because interval is inclusive
+        try:
+            header_blocks_map = await self.full_node.blockchain.get_header_blocks_in_range(
+                request.start_height, request.end_height, request.return_filter
+            )
+        except ValueError:
+            return make_msg(ProtocolMessageTypes.reject_block_headers, reject)
+        if len(header_blocks_map) != (
+            request.end_height - request.start_height + 1
+        ):  # +1 because interval is inclusive
             return make_msg(ProtocolMessageTypes.reject_block_headers, reject)
-        return_filter = request.return_filter
-        header_blocks_bytes: list[bytes] = [header_block_from_block(memoryview(b), return_filter) for b in blocks_bytes]
-
-        # we're building the RespondHeaderBlocks manually to avoid cost of
-        # dynamic serialization
-        # ---
-        # we start building RespondBlockHeaders response (start_height, end_height)
-        # and then need to define size of list object
-        respond_header_blocks_manually_streamed: bytes = (
-            uint32(request.start_height).stream_to_bytes()
-            + uint32(request.end_height).stream_to_bytes()
-            + uint32(len(header_blocks_bytes)).stream_to_bytes()
+        return make_msg(
+            ProtocolMessageTypes.respond_block_headers,
+            wallet_protocol.RespondBlockHeaders(
+                request.start_height, request.end_height, list(header_blocks_map.values())
+            ),
         )
-        # and now stream the whole list in bytes
-        respond_header_blocks_manually_streamed += b"".join(header_blocks_bytes)
-        return make_msg(ProtocolMessageTypes.respond_block_headers, respond_header_blocks_manually_streamed)
 
     @metadata.request()
     async def request_header_blocks(self, request: wallet_protocol.RequestHeaderBlocks) -> Optional[Message]: