Remove redundant checks in validate_spend_bundle() (#14035)

* Simplify addition_amount calculation in validate_spend_bundle() and remove unneeded COIN_AMOUNT_NEGATIVE and COIN_AMOUNT_EXCEEDS_MAXIMUM checks.

* Create a record for the test coin, return it from get_coin_record() and split the tests case by case.

* Add comments next to the unexpected error codes.

* Move the test block record into the fixture function.

* Make it explicit that we're not calling get_coin_record in these tests.

* Also test get_name_puzzle_conditions besides pre_validate_spendbundle.

* Revert testing get_name_puzzle_conditions besides pre_validate_spendbundle.

chia/full_node/mempool_manager.py

@@ -395,19 +395,11 @@ class MempoolManager:
             return Err.INVALID_SPEND_BUNDLE, None, []
 
         additions: List[Coin] = additions_for_npc(npc_result)
-
         additions_dict: Dict[bytes32, Coin] = {}
+        addition_amount: int = 0
         for add in additions:
             additions_dict[add.name()] = add
-
-        addition_amount: int = 0
-        # Check additions for max coin amount
-        for coin in additions:
-            if coin.amount < 0:
-                return Err.COIN_AMOUNT_NEGATIVE, None, []
-            if coin.amount > self.constants.MAX_COIN_AMOUNT:
-                return Err.COIN_AMOUNT_EXCEEDS_MAXIMUM, None, []
-            addition_amount = addition_amount + coin.amount
+            addition_amount = addition_amount + add.amount
         # Check for duplicate outputs
         addition_counter = collections.Counter(_.name() for _ in additions)
         for k, v in addition_counter.items():

tests/core/mempool/test_mempool_manager.py

@@ -0,0 +1,105 @@
+from __future__ import annotations
+
+from typing import Any, Awaitable, Callable, List, Optional
+
+import pytest
+from blspy import G2Element
+
+from chia.consensus.block_record import BlockRecord
+from chia.consensus.default_constants import DEFAULT_CONSTANTS
+from chia.full_node.mempool_manager import MempoolManager
+from chia.types.blockchain_format.classgroup import ClassgroupElement
+from chia.types.blockchain_format.coin import Coin
+from chia.types.blockchain_format.program import Program
+from chia.types.blockchain_format.sized_bytes import bytes32, bytes100
+from chia.types.coin_record import CoinRecord
+from chia.types.coin_spend import CoinSpend
+from chia.types.condition_opcodes import ConditionOpcode
+from chia.types.spend_bundle import SpendBundle
+from chia.util.errors import ValidationError
+from chia.util.ints import uint8, uint32, uint64, uint128
+
+IDENTITY_PUZZLE = Program.to(1)
+IDENTITY_PUZZLE_HASH = IDENTITY_PUZZLE.get_tree_hash()
+
+TEST_TIMESTAMP = uint64(1616108400)
+TEST_COIN = Coin(IDENTITY_PUZZLE_HASH, IDENTITY_PUZZLE_HASH, uint64(1000000000))
+TEST_HEIGHT = uint32(1)
+
+
+async def zero_calls_get_coin_record(_: bytes32) -> Optional[CoinRecord]:
+    assert False
+
+
+async def instantiate_mempool_manager(
+    get_coin_record: Callable[[bytes32], Awaitable[Optional[CoinRecord]]]
+) -> MempoolManager:
+    mempool_manager = MempoolManager(get_coin_record, DEFAULT_CONSTANTS)
+    test_block_record = BlockRecord(
+        IDENTITY_PUZZLE_HASH,
+        IDENTITY_PUZZLE_HASH,
+        TEST_HEIGHT,
+        uint128(0),
+        uint128(0),
+        uint8(0),
+        ClassgroupElement(bytes100(b"0" * 100)),
+        None,
+        IDENTITY_PUZZLE_HASH,
+        IDENTITY_PUZZLE_HASH,
+        uint64(0),
+        IDENTITY_PUZZLE_HASH,
+        IDENTITY_PUZZLE_HASH,
+        uint64(0),
+        uint8(0),
+        False,
+        uint32(TEST_HEIGHT - 1),
+        TEST_TIMESTAMP,
+        None,
+        uint64(0),
+        None,
+        None,
+        None,
+        None,
+        None,
+    )
+    await mempool_manager.new_peak(test_block_record, None)
+    return mempool_manager
+
+
+def spend_bundle_from_conditions(conditions: List[List[Any]]) -> SpendBundle:
+    solution = Program.to(conditions)
+    coin_spend = CoinSpend(TEST_COIN, IDENTITY_PUZZLE, solution)
+    return SpendBundle([coin_spend], G2Element())
+
+
+@pytest.mark.asyncio
+async def test_negative_addition_amount() -> None:
+    mempool_manager = await instantiate_mempool_manager(zero_calls_get_coin_record)
+    conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, -1]]
+    sb = spend_bundle_from_conditions(conditions)
+    # chia_rs currently emits this instead of Err.COIN_AMOUNT_NEGATIVE
+    # Addressed in https://github.com/Chia-Network/chia_rs/pull/99
+    with pytest.raises(ValidationError, match="Err.INVALID_CONDITION"):
+        await mempool_manager.pre_validate_spendbundle(sb, None, sb.name())
+
+
+@pytest.mark.asyncio
+async def test_valid_addition_amount() -> None:
+    mempool_manager = await instantiate_mempool_manager(zero_calls_get_coin_record)
+    max_amount = mempool_manager.constants.MAX_COIN_AMOUNT
+    conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, max_amount]]
+    sb = spend_bundle_from_conditions(conditions)
+    npc_result = await mempool_manager.pre_validate_spendbundle(sb, None, sb.name())
+    assert npc_result.error is None
+
+
+@pytest.mark.asyncio
+async def test_too_big_addition_amount() -> None:
+    mempool_manager = await instantiate_mempool_manager(zero_calls_get_coin_record)
+    max_amount = mempool_manager.constants.MAX_COIN_AMOUNT
+    conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, max_amount + 1]]
+    sb = spend_bundle_from_conditions(conditions)
+    # chia_rs currently emits this instead of Err.COIN_AMOUNT_EXCEEDS_MAXIMUM
+    # Addressed in https://github.com/Chia-Network/chia_rs/pull/99
+    with pytest.raises(ValidationError, match="Err.INVALID_CONDITION"):
+        await mempool_manager.pre_validate_spendbundle(sb, None, sb.name())