CHIA-1525 Make PreValidationResult take SpendBundleConditions instead of NPCResult (#18647)

Make PreValidationResult take SpendBundleConditions instead of NPCResult.

Author: AmineKhaldi

chia/_tests/blockchain/test_blockchain.py

@@ -2641,7 +2641,10 @@ async def test_cost_exceeds_max(
         diff = b.constants.DIFFICULTY_STARTING
         err = (
             await b.add_block(
-                blocks[-1], PreValidationResult(None, uint64(1), npc_result, True, uint32(0)), None, sub_slot_iters=ssi
+                blocks[-1],
+                PreValidationResult(None, uint64(1), npc_result.conds, True, uint32(0)),
+                None,
+                sub_slot_iters=ssi,
             )
         )[1]
         assert err in [Err.BLOCK_COST_EXCEEDS_MAX]
@@ -2717,7 +2720,7 @@ async def test_invalid_cost_in_block(
         )
         ssi = b.constants.SUB_SLOT_ITERS_STARTING
         _, err, _ = await b.add_block(
-            block_2, PreValidationResult(None, uint64(1), npc_result, False, uint32(0)), None, sub_slot_iters=ssi
+            block_2, PreValidationResult(None, uint64(1), npc_result.conds, False, uint32(0)), None, sub_slot_iters=ssi
         )
         assert err == Err.INVALID_BLOCK_COST
 
@@ -2746,7 +2749,7 @@ async def test_invalid_cost_in_block(
             constants=bt.constants,
         )
         _, err, _ = await b.add_block(
-            block_2, PreValidationResult(None, uint64(1), npc_result, False, uint32(0)), None, sub_slot_iters=ssi
+            block_2, PreValidationResult(None, uint64(1), npc_result.conds, False, uint32(0)), None, sub_slot_iters=ssi
         )
         assert err == Err.INVALID_BLOCK_COST
 
@@ -2777,7 +2780,7 @@ async def test_invalid_cost_in_block(
         )
 
         result, err, _ = await b.add_block(
-            block_2, PreValidationResult(None, uint64(1), npc_result, False, uint32(0)), None, sub_slot_iters=ssi
+            block_2, PreValidationResult(None, uint64(1), npc_result.conds, False, uint32(0)), None, sub_slot_iters=ssi
         )
         assert err == Err.INVALID_BLOCK_COST
 

chia/_tests/core/full_node/test_full_node.py

@@ -741,9 +741,8 @@ async def test_respond_unfinished(self, wallet_nodes, self_hostname):
         assert entry is not None
         result = entry.result
         assert result is not None
-        assert result.npc_result is not None
-        assert result.npc_result.conds is not None
-        assert result.npc_result.conds.cost > 0
+        assert result.conds is not None
+        assert result.conds.cost > 0
 
         assert not full_node_1.full_node.blockchain.contains_block(block.header_hash)
         assert block.transactions_generator is not None

chia/consensus/block_body_validation.py

@@ -14,7 +14,6 @@
 from chia.consensus.blockchain_interface import BlockRecordsProtocol
 from chia.consensus.coinbase import create_farmer_coin, create_pool_coin
 from chia.consensus.constants import ConsensusConstants
-from chia.consensus.cost_calculator import NPCResult
 from chia.full_node.mempool_check_conditions import mempool_check_time_locks
 from chia.types.blockchain_format.coin import Coin
 from chia.types.blockchain_format.sized_bytes import bytes32
@@ -125,19 +124,22 @@ async def validate_block_body(
     get_coin_records: Callable[[Collection[bytes32]], Awaitable[List[CoinRecord]]],
     block: Union[FullBlock, UnfinishedBlock],
     height: uint32,
-    npc_result: Optional[NPCResult],
+    conds: Optional[SpendBundleConditions],
     fork_info: ForkInfo,
     bls_cache: Optional[BLSCache],
     *,
     validate_signature: bool = True,
-) -> Tuple[Optional[Err], Optional[NPCResult]]:
+) -> Tuple[Optional[Err], Optional[SpendBundleConditions]]:
     """
     This assumes the header block has been completely validated.
-    Validates the transactions and body of the block. Returns None for the first value if everything
-    validates correctly, or an Err if something does not validate. For the second value, returns a CostResult
-    only if validation succeeded, and there are transactions. In other cases it returns None. The NPC result is
-    the result of running the generator with the previous generators refs. It is only present for transaction
-    blocks which have spent coins.
+    Validates the transactions and body of the block.
+    Returns None for the first value if everything validates correctly, or an
+        Err if something does not validate.
+    For the second value, returns a SpendBundleConditions only if validation
+        succeeded, and there are transactions. In other cases it returns None.
+    conds is the result of running the generator with the previous generators
+        refs. It must be set for transaction blocks and must be None for
+        non-transaction blocks.
     fork_info specifies the fork context of this block. In case the block
         extends the main chain, it can be empty, but if the block extends a fork
         of the main chain, the fork info is mandatory in order to validate the block.
@@ -291,8 +293,7 @@ async def validate_block_body(
     if block.transactions_generator is not None:
         # Get List of names removed, puzzles hashes for removed coins and conditions created
 
-        assert npc_result is not None
-        cost = uint64(0 if npc_result.conds is None else npc_result.conds.cost)
+        cost = uint64(0 if conds is None else conds.cost)
 
         # 7. Check that cost <= MAX_BLOCK_COST_CLVM
         log.debug(
@@ -303,19 +304,16 @@ async def validate_block_body(
             return Err.BLOCK_COST_EXCEEDS_MAX, None
 
         # 8. The CLVM program must not return any errors
-        if npc_result.error is not None:
-            return Err(npc_result.error), None
+        assert conds is not None
 
-        assert npc_result.conds is not None
-
-        for spend in npc_result.conds.spends:
+        for spend in conds.spends:
             removals.append(bytes32(spend.coin_id))
             removals_puzzle_dic[bytes32(spend.coin_id)] = bytes32(spend.puzzle_hash)
             for puzzle_hash, amount, _ in spend.create_coin:
                 c = Coin(bytes32(spend.coin_id), bytes32(puzzle_hash), uint64(amount))
                 additions.append((c, c.name()))
     else:
-        assert npc_result is None
+        assert conds is None
 
     # 9. Check that the correct cost is in the transactions info
     if block.transactions_info.cost != cost:
@@ -459,10 +457,7 @@ async def validate_block_body(
 
     # reserve fee cannot be greater than UINT64_MAX per consensus rule.
     # run_generator() would fail
-    assert_fee_sum: uint64 = uint64(0)
-    if npc_result:
-        assert npc_result.conds is not None
-        assert_fee_sum = uint64(npc_result.conds.reserve_fee)
+    assert_fee_sum = uint64(0 if conds is None else conds.reserve_fee)
 
     # 17. Check that the assert fee sum <= fees, and that each reserved fee is non-negative
     if fees < assert_fee_sum:
@@ -483,24 +478,21 @@ async def validate_block_body(
 
     # 21. Verify conditions
     # verify absolute/relative height/time conditions
-    if npc_result is not None:
-        assert npc_result.conds is not None
-
+    if conds is not None:
         error = mempool_check_time_locks(
             removal_coin_records,
-            npc_result.conds,
+            conds,
             prev_transaction_block_height,
             prev_transaction_block_timestamp,
         )
-        if error:
+        if error is not None:
             return error, None
 
     # create hash_key list for aggsig check
     pairs_pks: List[G1Element] = []
     pairs_msgs: List[bytes] = []
-    if npc_result:
-        assert npc_result.conds is not None
-        pairs_pks, pairs_msgs = pkm_pairs(npc_result.conds, constants.AGG_SIG_ME_ADDITIONAL_DATA)
+    if conds is not None:
+        pairs_pks, pairs_msgs = pkm_pairs(conds, constants.AGG_SIG_ME_ADDITIONAL_DATA)
 
     # 22. Verify aggregated signature
     # TODO: move this to pre_validate_blocks_multiprocessing so we can sync faster
@@ -520,4 +512,4 @@ async def validate_block_body(
             if not bls_cache.aggregate_verify(pairs_pks, pairs_msgs, block.transactions_info.aggregated_signature):
                 return Err.BAD_AGGREGATE_SIGNATURE, None
 
-    return None, npc_result
+    return None, conds

chia/consensus/blockchain.py

@@ -338,7 +338,6 @@ async def add_block(
             if prev_block.height + 1 != block.height:
                 return AddBlockResult.INVALID_BLOCK, Err.INVALID_HEIGHT, None
 
-        npc_result: Optional[NPCResult] = pre_validation_result.npc_result
         required_iters = pre_validation_result.required_iters
         if pre_validation_result.error is not None:
             return AddBlockResult.INVALID_BLOCK, Err(pre_validation_result.error), None
@@ -412,7 +411,7 @@ async def add_block(
                 # main chain, we still need to re-run it to update the additions and
                 # removals in fork_info.
                 await self.advance_fork_info(block, fork_info)
-                fork_info.include_spends(None if npc_result is None else npc_result.conds, block, header_hash)
+                fork_info.include_spends(pre_validation_result.conds, block, header_hash)
                 self.add_block_record(block_rec)
                 return AddBlockResult.ALREADY_HAVE_BLOCK, None, None
 
@@ -431,7 +430,7 @@ async def add_block(
             self.coin_store.get_coin_records,
             block,
             block.height,
-            npc_result,
+            pre_validation_result.conds,
             fork_info,
             bls_cache,
             # If we did not already validate the signature, validate it now
@@ -444,7 +443,7 @@ async def add_block(
         # case we're validating blocks on a fork, the next block validation will
         # need to know of these additions and removals. Also, _reconsider_peak()
         # will need these results
-        fork_info.include_spends(None if npc_result is None else npc_result.conds, block, header_hash)
+        fork_info.include_spends(pre_validation_result.conds, block, header_hash)
 
         # block_to_block_record() require the previous block in the cache
         if not genesis and prev_block is not None:
@@ -781,7 +780,7 @@ async def validate_unfinished_block(
             self.coin_store.get_coin_records,
             block,
             uint32(prev_height + 1),
-            npc_result,
+            None if npc_result is None else npc_result.conds,
             fork_info,
             None,
             validate_signature=False,  # Signature was already validated before calling this method, no need to validate

chia/consensus/multiprocess_validation.py

@@ -43,7 +43,7 @@
 class PreValidationResult(Streamable):
     error: Optional[uint16]
     required_iters: Optional[uint64]  # Iff error is None
-    npc_result: Optional[NPCResult]  # Iff error is None and block is a transaction block
+    conds: Optional[SpendBundleConditions]  # Iff error is None and block is a transaction block
     validated_signature: bool
     timing: uint32  # the time (in milliseconds) it took to pre-validate the block
 
@@ -95,7 +95,7 @@ def batch_pre_validate_blocks(
                     validation_time = time.monotonic() - validation_start
                     results.append(
                         PreValidationResult(
-                            uint16(npc_result.error), None, npc_result, False, uint32(validation_time * 1000)
+                            uint16(npc_result.error), None, npc_result.conds, False, uint32(validation_time * 1000)
                         )
                     )
                     continue
@@ -144,7 +144,7 @@ def batch_pre_validate_blocks(
                 PreValidationResult(
                     error_int,
                     required_iters,
-                    None if conds is None else NPCResult(None, conds),
+                    conds,
                     successfully_validated_signatures,
                     uint32(validation_time * 1000),
                 )
@@ -164,7 +164,7 @@ async def pre_validate_blocks_multiprocessing(
     block_records: BlocksProtocol,
     blocks: Sequence[FullBlock],
     pool: Executor,
-    npc_results: Dict[uint32, NPCResult],
+    block_height_conds_map: Dict[uint32, SpendBundleConditions],
     *,
     sub_slot_iters: uint64,
     difficulty: uint64,
@@ -275,10 +275,8 @@ async def pre_validate_blocks_multiprocessing(
             prev_ses_block = block_rec
 
     conditions_pickled = {}
-    for k, v in npc_results.items():
-        assert v.error is None
-        assert v.conds is not None
-        conditions_pickled[k] = bytes(v.conds)
+    for k, v in block_height_conds_map.items():
+        conditions_pickled[k] = bytes(v)
     futures = []
     # Pool of workers to validate blocks concurrently
     recent_blocks_bytes = {bytes(k): bytes(v) for k, v in recent_blocks.items()}  # convert to bytes

chia/full_node/full_node.py

@@ -1846,9 +1846,9 @@ async def add_block(
                 return None
             validation_start = time.monotonic()
             # Tries to add the block to the blockchain, if we already validated transactions, don't do it again
-            npc_results = {}
-            if pre_validation_result is not None and pre_validation_result.npc_result is not None:
-                npc_results[block.height] = pre_validation_result.npc_result
+            block_height_conds_map = {}
+            if pre_validation_result is not None and pre_validation_result.conds is not None:
+                block_height_conds_map[block.height] = pre_validation_result.conds
 
             # Don't validate signatures because we want to validate them in the main thread later, since we have a
             # cache available
@@ -1868,7 +1868,7 @@ async def add_block(
                 self.blockchain,
                 [block],
                 self.blockchain.pool,
-                npc_results,
+                block_height_conds_map,
                 sub_slot_iters=ssi,
                 difficulty=diff,
                 prev_ses_block=prev_ses_block,