From 57a59abde037123b9ae4fa60798dfedd24b43e76 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Tue, 2 Jan 2024 18:45:23 +0100 Subject: [PATCH] remove redundant cost field in NPCResults (#17167) --- chia/clvm/spend_sim.py | 5 +++-- chia/consensus/block_body_validation.py | 2 +- chia/consensus/block_creation.py | 2 +- chia/consensus/cost_calculator.py | 4 +--- chia/consensus/multiprocess_validation.py | 4 ++-- chia/full_node/mempool.py | 5 +++-- chia/full_node/mempool_check_conditions.py | 6 +++--- chia/full_node/mempool_manager.py | 2 +- chia/rpc/full_node_rpc_api.py | 2 +- chia/types/mempool_item.py | 2 +- tests/clvm/benchmark_costs.py | 3 ++- tests/core/full_node/test_full_node.py | 4 +++- tests/core/mempool/test_mempool.py | 8 +++++--- tests/core/mempool/test_mempool_manager.py | 17 +++++++++++------ tests/core/test_cost_calculation.py | 7 ++++--- .../test_fee_estimation_integration.py | 2 +- tests/generator/test_rom.py | 5 ++++- 17 files changed, 47 insertions(+), 33 deletions(-) diff --git a/chia/clvm/spend_sim.py b/chia/clvm/spend_sim.py index 4fc387cc58d1..71a32aa0546f 100644 --- a/chia/clvm/spend_sim.py +++ b/chia/clvm/spend_sim.py @@ -71,11 +71,12 @@ class CostLogger: height=DEFAULT_CONSTANTS.HARD_FORK_HEIGHT, constants=DEFAULT_CONSTANTS, ) - self.cost_dict[descriptor] = npc_result.cost + cost = uint64(0 if npc_result.conds is None else npc_result.conds.cost) + self.cost_dict[descriptor] = cost cost_to_subtract: int = 0 for cs in spend_bundle.coin_spends: cost_to_subtract += len(bytes(cs.puzzle_reveal)) * DEFAULT_CONSTANTS.COST_PER_BYTE - self.cost_dict_no_puzs[descriptor] = npc_result.cost - cost_to_subtract + self.cost_dict_no_puzs[descriptor] = cost - cost_to_subtract return spend_bundle def log_cost_statistics(self) -> str: diff --git a/chia/consensus/block_body_validation.py b/chia/consensus/block_body_validation.py index 1b7ee8e2df0d..dbfd380729bf 100644 --- a/chia/consensus/block_body_validation.py +++ b/chia/consensus/block_body_validation.py @@ -294,7 +294,7 @@ async def validate_block_body( # Get List of names removed, puzzles hashes for removed coins and conditions created assert npc_result is not None - cost = npc_result.cost + cost = uint64(0 if npc_result.conds is None else npc_result.conds.cost) # 7. Check that cost <= MAX_BLOCK_COST_CLVM log.debug( diff --git a/chia/consensus/block_creation.py b/chia/consensus/block_creation.py index a1b7091f1d76..eb0ecea2096e 100644 --- a/chia/consensus/block_creation.py +++ b/chia/consensus/block_creation.py @@ -39,7 +39,7 @@ def compute_block_cost(generator: BlockGenerator, constants: ConsensusConstants, result: NPCResult = get_name_puzzle_conditions( generator, constants.MAX_BLOCK_COST_CLVM, mempool_mode=True, height=height, constants=constants ) - return result.cost + return uint64(0 if result.conds is None else result.conds.cost) def compute_block_fee(additions: Sequence[Coin], removals: Sequence[Coin]) -> uint64: diff --git a/chia/consensus/cost_calculator.py b/chia/consensus/cost_calculator.py index 468c7f376360..b9426ec318db 100644 --- a/chia/consensus/cost_calculator.py +++ b/chia/consensus/cost_calculator.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from typing import Optional from chia.types.spend_bundle_conditions import SpendBundleConditions -from chia.util.ints import uint16, uint64 +from chia.util.ints import uint16 from chia.util.streamable import Streamable, streamable @@ -13,5 +13,3 @@ from chia.util.streamable import Streamable, streamable class NPCResult(Streamable): error: Optional[uint16] conds: Optional[SpendBundleConditions] - cost: uint64 # The total cost of the block, including CLVM cost, cost of - # conditions and cost of bytes diff --git a/chia/consensus/multiprocess_validation.py b/chia/consensus/multiprocess_validation.py index 1aacfaa4f245..49a26f265cb2 100644 --- a/chia/consensus/multiprocess_validation.py +++ b/chia/consensus/multiprocess_validation.py @@ -406,6 +406,6 @@ def _run_generator( ) return bytes(npc_result) except ValidationError as e: - return bytes(NPCResult(uint16(e.code.value), None, uint64(0))) + return bytes(NPCResult(uint16(e.code.value), None)) except Exception: - return bytes(NPCResult(uint16(Err.UNKNOWN.value), None, uint64(0))) + return bytes(NPCResult(uint16(Err.UNKNOWN.value), None)) diff --git a/chia/full_node/mempool.py b/chia/full_node/mempool.py index 6a4025b18161..3ec40c686f60 100644 --- a/chia/full_node/mempool.py +++ b/chia/full_node/mempool.py @@ -402,10 +402,11 @@ class Mempool: if not item_inclusion_filter(name): continue try: + cost = uint64(0 if item.npc_result.conds is None else item.npc_result.conds.cost) unique_coin_spends, cost_saving, unique_additions = eligible_coin_spends.get_deduplication_info( - bundle_coin_spends=item.bundle_coin_spends, max_cost=item.npc_result.cost + bundle_coin_spends=item.bundle_coin_spends, max_cost=cost ) - item_cost = item.npc_result.cost - cost_saving + item_cost = cost - cost_saving log.info("Cumulative cost: %d, fee per cost: %0.4f", cost_sum, fee / item_cost) if ( item_cost + cost_sum > self.mempool_info.max_block_clvm_cost diff --git a/chia/full_node/mempool_check_conditions.py b/chia/full_node/mempool_check_conditions.py index ba6e8d11cb56..a9afe4f94156 100644 --- a/chia/full_node/mempool_check_conditions.py +++ b/chia/full_node/mempool_check_conditions.py @@ -93,13 +93,13 @@ def get_name_puzzle_conditions( err, result = run_block(bytes(generator.program), block_args, max_cost, flags) assert (err is None) != (result is None) if err is not None: - return NPCResult(uint16(err), None, uint64(0)) + return NPCResult(uint16(err), None) else: assert result is not None - return NPCResult(None, result, uint64(result.cost)) + return NPCResult(None, result) except BaseException: log.exception("get_name_puzzle_condition failed") - return NPCResult(uint16(Err.GENERATOR_RUNTIME_ERROR.value), None, uint64(0)) + return NPCResult(uint16(Err.GENERATOR_RUNTIME_ERROR.value), None) def get_puzzle_and_solution_for_coin( diff --git a/chia/full_node/mempool_manager.py b/chia/full_node/mempool_manager.py index a8648536db10..62729a4a4e91 100644 --- a/chia/full_node/mempool_manager.py +++ b/chia/full_node/mempool_manager.py @@ -403,7 +403,7 @@ class MempoolManager: if npc_result.error is not None: return Err(npc_result.error), None, [] - cost = npc_result.cost + cost = uint64(0 if npc_result.conds is None else npc_result.conds.cost) log.debug(f"Cost: {cost}") assert npc_result.conds is not None diff --git a/chia/rpc/full_node_rpc_api.py b/chia/rpc/full_node_rpc_api.py index 6bebf5ce9226..6cf91e93175f 100644 --- a/chia/rpc/full_node_rpc_api.py +++ b/chia/rpc/full_node_rpc_api.py @@ -873,7 +873,7 @@ class FullNodeRpcApi: ) if npc_result.error is not None: raise RuntimeError(f"Spend Bundle failed validation: {npc_result.error}") - cost = npc_result.cost + cost = uint64(0 if npc_result.conds is None else npc_result.conds.cost) elif "cost" in request: cost = request["cost"] else: diff --git a/chia/types/mempool_item.py b/chia/types/mempool_item.py index a2a8d15b5f32..44ffa8f8b554 100644 --- a/chia/types/mempool_item.py +++ b/chia/types/mempool_item.py @@ -57,7 +57,7 @@ class MempoolItem: @property def cost(self) -> uint64: - return self.npc_result.cost + return uint64(0 if self.npc_result.conds is None else self.npc_result.conds.cost) @property def additions(self) -> List[Coin]: diff --git a/tests/clvm/benchmark_costs.py b/tests/clvm/benchmark_costs.py index 72d59c0b85a5..3f7e4f29dc2d 100644 --- a/tests/clvm/benchmark_costs.py +++ b/tests/clvm/benchmark_costs.py @@ -7,6 +7,7 @@ from chia.full_node.mempool_check_conditions import get_name_puzzle_conditions from chia.types.blockchain_format.program import INFINITE_COST from chia.types.generator_types import BlockGenerator from chia.types.spend_bundle import SpendBundle +from chia.util.ints import uint64 def cost_of_spend_bundle(spend_bundle: SpendBundle) -> int: @@ -19,4 +20,4 @@ def cost_of_spend_bundle(spend_bundle: SpendBundle) -> int: height=DEFAULT_CONSTANTS.HARD_FORK_HEIGHT, constants=DEFAULT_CONSTANTS, ) - return npc_result.cost + return uint64(0 if npc_result.conds is None else npc_result.conds.cost) diff --git a/tests/core/full_node/test_full_node.py b/tests/core/full_node/test_full_node.py index 8619155865ec..bb4ca1db742d 100644 --- a/tests/core/full_node/test_full_node.py +++ b/tests/core/full_node/test_full_node.py @@ -800,7 +800,9 @@ class TestFullNodeProtocol: assert full_node_1.full_node.full_node_store.get_unfinished_block(unf.partial_hash) is not None result = full_node_1.full_node.full_node_store.get_unfinished_block_result(unf.partial_hash) assert result is not None - assert result.npc_result is not None and result.npc_result.cost > 0 + assert result.npc_result is not None + assert result.npc_result.conds is not None + assert result.npc_result.conds.cost > 0 assert not full_node_1.full_node.blockchain.contains_block(block.header_hash) assert block.transactions_generator is not None diff --git a/tests/core/mempool/test_mempool.py b/tests/core/mempool/test_mempool.py index eec9d411e06c..8c9c8c3d1186 100644 --- a/tests/core/mempool/test_mempool.py +++ b/tests/core/mempool/test_mempool.py @@ -100,7 +100,7 @@ def make_item(idx: int, cost: uint64 = uint64(80), assert_height=100) -> Mempool return MempoolItem( SpendBundle([], G2Element()), uint64(0), - NPCResult(None, SpendBundleConditions([], 0, 0, 0, None, None, [], cost, 0, 0), cost), + NPCResult(None, SpendBundleConditions([], 0, 0, 0, None, None, [], cost, 0, 0)), spend_bundle_name, uint32(0), assert_height, @@ -2134,7 +2134,8 @@ class TestGeneratorConditions: height=softfork_height, ) assert npc_result.error is None - assert npc_result.cost == generator_base_cost + 95 * COST_PER_BYTE + ConditionCost.CREATE_COIN.value + assert npc_result.conds is not None + assert npc_result.conds.cost == generator_base_cost + 95 * COST_PER_BYTE + ConditionCost.CREATE_COIN.value assert len(npc_result.conds.spends) == 1 assert len(npc_result.conds.spends[0].create_coin) == 1 @@ -2186,7 +2187,8 @@ class TestGeneratorConditions: height=softfork_height, ) assert npc_result.error is None - assert npc_result.cost == generator_base_cost + 117 * COST_PER_BYTE + expected_cost + assert npc_result.conds is not None + assert npc_result.conds.cost == generator_base_cost + 117 * COST_PER_BYTE + expected_cost assert len(npc_result.conds.spends) == 1 # if we subtract one from max cost, this should fail diff --git a/tests/core/mempool/test_mempool_manager.py b/tests/core/mempool/test_mempool_manager.py index fbdec07f36dd..9c17c63077f8 100644 --- a/tests/core/mempool/test_mempool_manager.py +++ b/tests/core/mempool/test_mempool_manager.py @@ -698,7 +698,7 @@ def mk_item( # can_replace() spends = [make_spend(c, SerializedProgram.to(None), SerializedProgram.to(None)) for c in coins] spend_bundle = SpendBundle(spends, G2Element()) - npc_result = NPCResult(None, make_test_conds(cost=cost, spend_ids=[c.name() for c in coins]), uint64(cost)) + npc_result = NPCResult(None, make_test_conds(cost=cost, spend_ids=[c.name() for c in coins])) return MempoolItem( spend_bundle, uint64(fee), @@ -1258,9 +1258,10 @@ def test_dedup_info_nothing_to_do() -> None: ] sb = spend_bundle_from_conditions(conditions, TEST_COIN) mempool_item = mempool_item_from_spendbundle(sb) + assert mempool_item.npc_result.conds is not None eligible_coin_spends = EligibleCoinSpends() unique_coin_spends, cost_saving, unique_additions = eligible_coin_spends.get_deduplication_info( - bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.cost + bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.conds.cost ) assert unique_coin_spends == sb.coin_spends assert cost_saving == 0 @@ -1276,10 +1277,11 @@ def test_dedup_info_eligible_1st_time() -> None: ] sb = spend_bundle_from_conditions(conditions, TEST_COIN) mempool_item = mempool_item_from_spendbundle(sb) + assert mempool_item.npc_result.conds is not None eligible_coin_spends = EligibleCoinSpends() solution = SerializedProgram.to(conditions) unique_coin_spends, cost_saving, unique_additions = eligible_coin_spends.get_deduplication_info( - bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.cost + bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.conds.cost ) assert unique_coin_spends == sb.coin_spends assert cost_saving == 0 @@ -1301,9 +1303,10 @@ def test_dedup_info_eligible_but_different_solution() -> None: conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, 2]] sb = spend_bundle_from_conditions(conditions, TEST_COIN) mempool_item = mempool_item_from_spendbundle(sb) + assert mempool_item.npc_result.conds is not None with pytest.raises(ValueError, match="Solution is different from what we're deduplicating on"): eligible_coin_spends.get_deduplication_info( - bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.cost + bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.conds.cost ) @@ -1321,8 +1324,9 @@ def test_dedup_info_eligible_2nd_time_and_another_1st_time() -> None: sb2 = spend_bundle_from_conditions(second_conditions, TEST_COIN2) sb = SpendBundle.aggregate([sb1, sb2]) mempool_item = mempool_item_from_spendbundle(sb) + assert mempool_item.npc_result.conds is not None unique_coin_spends, cost_saving, unique_additions = eligible_coin_spends.get_deduplication_info( - bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.cost + bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.conds.cost ) # Only the eligible one that we encountered more than once gets deduplicated assert unique_coin_spends == sb2.coin_spends @@ -1365,8 +1369,9 @@ def test_dedup_info_eligible_3rd_time_another_2nd_time_and_one_non_eligible() -> sb3 = spend_bundle_from_conditions(sb3_conditions, TEST_COIN3) sb = SpendBundle.aggregate([sb1, sb2, sb3]) mempool_item = mempool_item_from_spendbundle(sb) + assert mempool_item.npc_result.conds is not None unique_coin_spends, cost_saving, unique_additions = eligible_coin_spends.get_deduplication_info( - bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.cost + bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.npc_result.conds.cost ) assert unique_coin_spends == sb3.coin_spends saved_cost2 = uint64(1800044) diff --git a/tests/core/test_cost_calculation.py b/tests/core/test_cost_calculation.py index f50a369e7fa2..0f44d5634598 100644 --- a/tests/core/test_cost_calculation.py +++ b/tests/core/test_cost_calculation.py @@ -105,7 +105,7 @@ async def test_basics(softfork_height: int, bt: BlockTools) -> None: # Create condition + agg_sig_condition + length + cpu_cost assert ( - npc_result.cost + npc_result.conds.cost == ConditionCost.CREATE_COIN.value + ConditionCost.AGG_SIG.value + len(bytes(program.program)) * bt.constants.COST_PER_BYTE @@ -248,7 +248,7 @@ async def test_clvm_max_cost(softfork_height: int) -> None: ) assert npc_result.error is not None - assert npc_result.cost == 0 + assert npc_result.conds is None # raise the max cost to make sure this passes # ensure we pass if the program does not exceeds the cost @@ -257,7 +257,8 @@ async def test_clvm_max_cost(softfork_height: int) -> None: ) assert npc_result.error is None - assert npc_result.cost > 10000000 + assert npc_result.conds is not None + assert npc_result.conds.cost > 10000000 @pytest.mark.anyio diff --git a/tests/fee_estimation/test_fee_estimation_integration.py b/tests/fee_estimation/test_fee_estimation_integration.py index 64463ea05b3d..4292d88ebb8d 100644 --- a/tests/fee_estimation/test_fee_estimation_integration.py +++ b/tests/fee_estimation/test_fee_estimation_integration.py @@ -47,7 +47,7 @@ def make_mempoolitem() -> MempoolItem: mempool_item = MempoolItem( spend_bundle, fee, - NPCResult(None, conds, cost), + NPCResult(None, conds), spend_bundle.name(), uint32(block_height), ) diff --git a/tests/generator/test_rom.py b/tests/generator/test_rom.py index 65cd29cca065..f98a38c73338 100644 --- a/tests/generator/test_rom.py +++ b/tests/generator/test_rom.py @@ -133,7 +133,10 @@ class TestROM: else: cost = EXPECTED_COST1 assert npc_result.error is None - assert npc_result.cost == cost + ConditionCost.CREATE_COIN.value + (len(bytes(gen.program)) * COST_PER_BYTE) + assert npc_result.conds is not None + assert npc_result.conds.cost == cost + ConditionCost.CREATE_COIN.value + ( + len(bytes(gen.program)) * COST_PER_BYTE + ) assert npc_result.conds is not None spend = Spend(