when asking the full_node_store for an unfinished block, only by its …

…partial hash; return the highest ranking (lowest foliage tx block hash)

chia/full_node/full_node_store.py

@@ -206,8 +206,22 @@ def get_unfinished_block(self, unfinished_reward_hash: bytes32) -> Optional[Unfi
         if result is None:
             return None
         # The old API doesn't distinguish between duplicate UnfinishedBlocks,
-        # just return the first one
-        return next(iter(result.values())).unfinished_block
+        # return the *best* UnfinishedBlock. This is the path taken when the
+        # timelord sends us an infusion point with this specific reward block
+        # hash. We pick one of the unfinished blocks based on an arbitrary but
+        # deterministic property.
+        # this sorts the UnfinishedBlocks by the foliage hash, and picks the
+        # smallest hash
+        all_blocks = list(result.items())
+        if len(all_blocks) == 1:
+            return all_blocks[0][1].unfinished_block
+
+        # if there are unfinished blocks with foliage (i.e. not None) we prefer
+        # those, so drop the first element
+        all_blocks = [e for e in all_blocks if e[0] is not None]
+        all_blocks = sorted(all_blocks)
+
+        return all_blocks[0][1].unfinished_block
 
     def get_unfinished_block2(
         self, unfinished_reward_hash: bytes32, unfinished_foliage_hash: Optional[bytes32]
@@ -216,10 +230,10 @@ def get_unfinished_block2(
         if result is None:
             return None, 0
         if unfinished_foliage_hash is None:
-            return next(iter(result.values())).unfinished_block, len(result)
-        else:
-            entry = result.get(unfinished_foliage_hash)
-            return (None if entry is None else entry.unfinished_block), len(result)
+            return self.get_unfinished_block(unfinished_reward_hash), len(result)
+
+        entry = result.get(unfinished_foliage_hash)
+        return (None if entry is None else entry.unfinished_block), len(result)
 
     def get_unfinished_block_result(self, unfinished_reward_hash: bytes32) -> Optional[PreValidationResult]:
         result = self.unfinished_blocks.get(unfinished_reward_hash, None)

tests/core/full_node/stores/test_full_node_store.py

@@ -18,14 +18,15 @@
 from chia.full_node.signage_point import SignagePoint
 from chia.protocols import timelord_protocol
 from chia.protocols.timelord_protocol import NewInfusionPointVDF
-from chia.simulator.block_tools import BlockTools, create_block_tools_async, get_signage_point
+from chia.simulator.block_tools import BlockTools, create_block_tools_async, get_signage_point, make_unfinished_block
 from chia.simulator.keyring import TempKeyring
 from chia.types.blockchain_format.sized_bytes import bytes32
 from chia.types.full_block import FullBlock
 from chia.types.unfinished_block import UnfinishedBlock
 from chia.util.block_cache import BlockCache
 from chia.util.hash import std_hash
 from chia.util.ints import uint8, uint32, uint64, uint128
+from chia.util.recursive_replace import recursive_replace
 from tests.blockchain.blockchain_test_utils import _validate_and_add_block, _validate_and_add_block_no_error
 from tests.util.blockchain import create_blockchain
 
@@ -62,6 +63,59 @@ async def empty_blockchain_with_original_constants(
         yield bc1
 
 
+@pytest.mark.anyio
+@pytest.mark.parametrize("num_duplicates", [0, 1, 3, 10])
+@pytest.mark.parametrize("include_none", [True, False])
+async def test_unfinished_block_rank(
+    empty_blockchain: Blockchain,
+    custom_block_tools: BlockTools,
+    seeded_random: random.Random,
+    num_duplicates: int,
+    include_none: bool,
+) -> None:
+    blocks = custom_block_tools.get_consecutive_blocks(
+        1,
+        guarantee_transaction_block=True,
+    )
+
+    assert blocks[-1].is_transaction_block()
+    store = FullNodeStore(custom_block_tools.constants)
+    unf: UnfinishedBlock = make_unfinished_block(blocks[-1], custom_block_tools.constants)
+
+    # create variants of the unfinished block, where all we do is to change
+    # the foliage_transaction_block_hash. As if they all had different foliage,
+    # but the same reward block hash (i.e. the same proof-of-space)
+    unfinished: List[UnfinishedBlock] = [
+        recursive_replace(unf, "foliage.foliage_transaction_block_hash", bytes32([idx + 4] * 32))
+        for idx in range(num_duplicates)
+    ]
+
+    if include_none:
+        unfinished.append(recursive_replace(unf, "foliage.foliage_transaction_block_hash", None))
+
+    # shuffle them to ensure the order we add them to the store isn't relevant
+    seeded_random.shuffle(unfinished)
+    for new_unf in unfinished:
+        store.add_unfinished_block(
+            uint32(2), new_unf, PreValidationResult(None, uint64(123532), None, False, uint32(0))
+        )
+
+    # now ask for "the" unfinished block given the proof-of-space.
+    # the FullNodeStore should return the one with the lowest foliage tx block
+    # hash. We prefer a block with foliage over one without (i.e. where foliage
+    # is None)
+    if num_duplicates == 0 and not include_none:
+        assert store.get_unfinished_block(unf.partial_hash) is None
+    else:
+        best_unf = store.get_unfinished_block(unf.partial_hash)
+        assert best_unf is not None
+        if num_duplicates == 0:
+            # if a block without foliage is our only option, that's what we get
+            assert best_unf.foliage.foliage_transaction_block_hash is None
+        else:
+            assert best_unf.foliage.foliage_transaction_block_hash == bytes32([4] * 32)
+
+
 @pytest.mark.limit_consensus_modes(reason="save time")
 @pytest.mark.anyio
 @pytest.mark.parametrize("normalized_to_identity", [False, True])
@@ -171,7 +225,8 @@ async def test_basic_store(
             uint32(height), unf_block, PreValidationResult(None, uint64(val), None, False, uint32(0))
         )
 
-    # when not specifying a foliage hash, you get the first one
+    # when not specifying a foliage hash, you get the "best" one
+    # best is defined as the lowest foliage hash
     assert store.get_unfinished_block(unf1.partial_hash) == unf1
     assert store.get_unfinished_block2(unf1.partial_hash, unf1.foliage.foliage_transaction_block_hash) == (unf1, 2)
     # unf4 overwrote unf2 and unf3 (that's why there are only 2 blocks stored).

tests/core/full_node/test_full_node.py

@@ -1568,6 +1568,10 @@ async def test_request_unfinished_block2(self, wallet_nodes, self_hostname):
 
         peer = await connect_and_get_peer(server_1, server_2, self_hostname)
 
+        # the "best" unfinished block according to the metric we use to pick one
+        # deterministically
+        best_unf: Optional[UnfinishedBlock] = None
+
         for idx in range(0, 6):
             # we include a different transaction in each block. This makes the
             # foliage different in each of them, but the reward block (plot) the same
@@ -1580,6 +1584,14 @@ async def test_request_unfinished_block2(self, wallet_nodes, self_hostname):
             unf = make_unfinished_block(block, bt.constants)
             assert unf.foliage.foliage_transaction_block_hash is not None
 
+            if best_unf is None:
+                best_unf = unf
+            elif (
+                unf.foliage.foliage_transaction_block_hash is not None
+                and unf.foliage.foliage_transaction_block_hash < best_unf.foliage.foliage_transaction_block_hash
+            ):
+                best_unf = unf
+
             # Don't have
             res = await full_node_1.request_unfinished_block2(
                 fnp.RequestUnfinishedBlock2(unf.partial_hash, unf.foliage.foliage_transaction_block_hash)
@@ -1593,6 +1605,12 @@ async def test_request_unfinished_block2(self, wallet_nodes, self_hostname):
             )
             assert res.data == bytes(fnp.RespondUnfinishedBlock(unf))
 
+            res = await full_node_1.request_unfinished_block(fnp.RequestUnfinishedBlock(unf.partial_hash))
+            assert res.data == bytes(fnp.RespondUnfinishedBlock(best_unf))
+
+            res = await full_node_1.request_unfinished_block2(fnp.RequestUnfinishedBlock2(unf.partial_hash, None))
+            assert res.data == bytes(fnp.RespondUnfinishedBlock(best_unf))
+
     @pytest.mark.anyio
     async def test_new_signage_point_or_end_of_sub_slot(self, wallet_nodes, self_hostname):
         full_node_1, full_node_2, server_1, server_2, wallet_a, wallet_receiver, bt = wallet_nodes