[CHIA-1933] Remove get_new methods from CATWallet (#18965)

* Remove `get_new` methods from cat wallet

* Fix an instance in `tails.py`

* Fix an instance in `tails.py`

* Fix `test_dao_wallets.py`

* Test coverage

chia/_tests/wallet/cat_wallet/test_cat_wallet.py

@@ -6,7 +6,12 @@
 import pytest
 
 from chia._tests.conftest import ConsensusMode
-from chia._tests.environments.wallet import WalletEnvironment, WalletStateTransition, WalletTestFramework
+from chia._tests.environments.wallet import (
+    NewPuzzleHashError,
+    WalletEnvironment,
+    WalletStateTransition,
+    WalletTestFramework,
+)
 from chia._tests.util.time_out_assert import time_out_assert, time_out_assert_not_none
 from chia.protocols.wallet_protocol import CoinState
 from chia.rpc.wallet_request_types import GetTransactionMemo, PushTX
@@ -318,7 +323,7 @@ async def test_cat_spend(wallet_environments: WalletTestFramework) -> None:
 
     assert cat_wallet.cat_info.limitations_program_hash == cat_wallet_2.cat_info.limitations_program_hash
 
-    cat_2_hash = await cat_wallet_2.get_new_inner_hash()
+    cat_2_hash = await cat_wallet_2.standard_wallet.get_puzzle_hash(new=False)
     async with cat_wallet.wallet_state_manager.new_action_scope(DEFAULT_TX_CONFIG, push=True) as action_scope:
         await cat_wallet.generate_signed_transaction([uint64(60)], [cat_2_hash], action_scope, fee=uint64(1))
     tx_id = None
@@ -408,7 +413,7 @@ async def test_cat_spend(wallet_environments: WalletTestFramework) -> None:
     memos = await env_2.rpc_client.get_transaction_memo(GetTransactionMemo(transaction_id=tx_id))
     assert len(memos.coins_with_memos) == 2
     assert memos.coins_with_memos[1].memos[0] == cat_2_hash
-    cat_hash = await cat_wallet.get_new_inner_hash()
+    cat_hash = await cat_wallet.standard_wallet.get_puzzle_hash(new=False)
     async with cat_wallet_2.wallet_state_manager.new_action_scope(DEFAULT_TX_CONFIG, push=True) as action_scope:
         await cat_wallet_2.generate_signed_transaction([uint64(15)], [cat_hash], action_scope)
 
@@ -610,7 +615,7 @@ async def test_cat_doesnt_see_eve(wallet_environments: WalletTestFramework) -> N
 
     assert cat_wallet.cat_info.limitations_program_hash == cat_wallet_2.cat_info.limitations_program_hash
 
-    cat_2_hash = await cat_wallet_2.get_new_inner_hash()
+    cat_2_hash = await cat_wallet_2.standard_wallet.get_puzzle_hash(new=False)
     async with cat_wallet.wallet_state_manager.new_action_scope(
         wallet_environments.tx_config, push=True
     ) as action_scope:
@@ -684,7 +689,7 @@ async def test_cat_doesnt_see_eve(wallet_environments: WalletTestFramework) -> N
         ]
     )
 
-    cc2_ph = await cat_wallet_2.get_new_cat_puzzle_hash()
+    cc2_ph = await cat_wallet_2.get_cat_puzzle_hash(new=False)
     async with wallet.wallet_state_manager.new_action_scope(wallet_environments.tx_config, push=True) as action_scope:
         await wallet.wallet_state_manager.main_wallet.generate_signed_transaction(uint64(10), cc2_ph, action_scope)
 
@@ -817,8 +822,8 @@ async def test_cat_spend_multiple(wallet_environments: WalletTestFramework) -> N
     assert cat_wallet_0.cat_info.limitations_program_hash == cat_wallet_1.cat_info.limitations_program_hash
     assert cat_wallet_0.cat_info.limitations_program_hash == cat_wallet_2.cat_info.limitations_program_hash
 
-    cat_1_hash = await cat_wallet_1.get_new_inner_hash()
-    cat_2_hash = await cat_wallet_2.get_new_inner_hash()
+    cat_1_hash = await cat_wallet_1.standard_wallet.get_puzzle_hash(new=False)
+    cat_2_hash = await cat_wallet_2.standard_wallet.get_puzzle_hash(new=False)
 
     async with cat_wallet_0.wallet_state_manager.new_action_scope(DEFAULT_TX_CONFIG, push=True) as action_scope:
         await cat_wallet_0.generate_signed_transaction([uint64(60), uint64(20)], [cat_1_hash, cat_2_hash], action_scope)
@@ -900,7 +905,7 @@ async def test_cat_spend_multiple(wallet_environments: WalletTestFramework) -> N
         ]
     )
 
-    cat_hash = await cat_wallet_0.get_new_inner_hash()
+    cat_hash = await cat_wallet_0.standard_wallet.get_puzzle_hash(new=False)
 
     async with cat_wallet_1.wallet_state_manager.new_action_scope(DEFAULT_TX_CONFIG, push=True) as action_scope:
         await cat_wallet_1.generate_signed_transaction([uint64(15)], [cat_hash], action_scope)
@@ -1107,7 +1112,7 @@ async def test_cat_max_amount_send(wallet_environments: WalletTestFramework) ->
 
     assert cat_wallet.cat_info.limitations_program_hash is not None
 
-    cat_2 = await cat_wallet.get_new_inner_puzzle()
+    cat_2 = await cat_wallet.standard_wallet.get_puzzle(new=False)
     cat_2_hash = cat_2.get_tree_hash()
     amounts = []
     puzzle_hashes = []
@@ -1382,7 +1387,7 @@ async def test_cat_hint(wallet_environments: WalletTestFramework) -> None:
     cat_wallet_2 = wallet_node_2.wallet_state_manager.wallets[uint32(2)]
     assert isinstance(cat_wallet_2, CATWallet)
 
-    cat_hash = await cat_wallet.get_new_inner_hash()
+    cat_hash = await cat_wallet.standard_wallet.get_puzzle_hash(new=False)
     async with cat_wallet_2.wallet_state_manager.new_action_scope(
         wallet_environments.tx_config, push=True
     ) as action_scope:
@@ -1730,3 +1735,92 @@ async def test_cat_melt_balance(wallet_environments: WalletTestFramework) -> Non
                 )
             ]
         )
+
+
+@pytest.mark.parametrize(
+    "wallet_environments",
+    [
+        {
+            "num_environments": 1,
+            "blocks_needed": [1],
+            "trusted": True,  # Parameter doesn't matter for this test
+            "reuse_puzhash": True,  # Important to test this is ignored in the duplicate change scenario
+        }
+    ],
+    indirect=True,
+)
+@pytest.mark.limit_consensus_modes([ConsensusMode.PLAIN], reason="irrelevant")
+@pytest.mark.anyio
+async def test_cat_puzzle_hashes(wallet_environments: WalletTestFramework) -> None:
+    env = wallet_environments.environments[0]
+    wallet = env.xch_wallet
+
+    env.wallet_aliases = {
+        "xch": 1,
+        "cat": 2,
+    }
+
+    async with env.wallet_state_manager.new_action_scope(wallet_environments.tx_config, push=True) as action_scope:
+        cat_wallet = await CATWallet.create_new_cat_wallet(
+            env.node.wallet_state_manager,
+            wallet,
+            {"identifier": "genesis_by_id"},
+            uint64(100),
+            action_scope,
+        )
+
+    await wallet_environments.process_pending_states(
+        [
+            WalletStateTransition(
+                pre_block_balance_updates={
+                    "xch": {"set_remainder": True},
+                    "cat": {"init": True, "set_remainder": True},
+                },
+                post_block_balance_updates={
+                    "xch": {"set_remainder": True},
+                    "cat": {
+                        "confirmed_wallet_balance": 100,
+                        "unconfirmed_wallet_balance": 0,
+                        "spendable_balance": 100,
+                        "max_send_amount": 100,
+                        "pending_change": -100,
+                        "pending_coin_removal_count": -1,
+                        "unspent_coin_count": 1,
+                    },
+                },
+            ),
+        ]
+    )
+
+    # Test that we attempt a new puzzle hash here even though everything says we shouldn't
+    with pytest.raises(NewPuzzleHashError):
+        async with env.wallet_state_manager.new_action_scope(wallet_environments.tx_config, push=True) as action_scope:
+            await cat_wallet.generate_signed_transaction(
+                [uint64(50)], [await cat_wallet.standard_wallet.get_puzzle_hash(new=False)], action_scope
+            )
+
+    # Test new puzzle hash getting
+    current_derivation_index = await env.wallet_state_manager.puzzle_store.get_current_derivation_record_for_wallet(
+        uint32(env.wallet_aliases["cat"])
+    )
+    assert current_derivation_index is not None
+    await cat_wallet.get_cat_puzzle_hash(new=True)
+    next_derivation_index = await env.wallet_state_manager.puzzle_store.get_current_derivation_record_for_wallet(
+        uint32(env.wallet_aliases["cat"])
+    )
+    assert next_derivation_index is not None
+    assert current_derivation_index.index < next_derivation_index.index
+
+    # Test a weird edge case where a new puzzle hash needs to get generated
+    # First, we reset the used status of all puzzle hashes by re-adding them
+    for puzhash in await env.wallet_state_manager.puzzle_store.get_all_puzzle_hashes():
+        dr = await env.wallet_state_manager.puzzle_store.get_derivation_record_for_puzzle_hash(puzhash)
+        assert dr is not None
+        await env.wallet_state_manager.puzzle_store.add_derivation_paths([dr])
+
+    # Then we make sure that even though we asked for a used puzzle hash, it still gives us an unused one
+    unused_count = await env.wallet_state_manager.puzzle_store.get_unused_count(uint32(env.wallet_aliases["cat"]))
+    await cat_wallet.get_cat_puzzle_hash(new=False)
+    assert unused_count < await env.wallet_state_manager.puzzle_store.get_unused_count(
+        uint32(env.wallet_aliases["cat"])
+    )

chia/_tests/wallet/dao_wallet/test_dao_wallets.py

@@ -1032,7 +1032,7 @@ async def test_dao_proposal_partial_vote(
     await full_node_api.wait_for_wallets_synced(wallet_nodes=[wallet_node_0, wallet_node_1], timeout=30)
 
     # Create a mint proposal
-    recipient_puzzle_hash = await cat_wallet_1.get_new_inner_hash()
+    recipient_puzzle_hash = await cat_wallet_1.standard_wallet.get_puzzle_hash(new=False)
     new_mint_amount = uint64(500)
     mint_proposal_inner = await generate_mint_proposal_innerpuz(
         treasury_id,
@@ -1104,7 +1104,7 @@ async def test_dao_proposal_partial_vote(
     await time_out_assert(20, cat_wallet_1.get_spendable_balance, balance + new_mint_amount)
     # Can we spend the newly minted CATs?
     old_balance = await cat_wallet_0.get_spendable_balance()
-    ph_0 = await cat_wallet_0.get_new_inner_hash()
+    ph_0 = await cat_wallet_0.standard_wallet.get_puzzle_hash(new=False)
     async with cat_wallet_1.wallet_state_manager.new_action_scope(DEFAULT_TX_CONFIG, push=True) as action_scope:
         await cat_wallet_1.generate_signed_transaction([balance + new_mint_amount], [ph_0], action_scope)
     await full_node_api.process_transaction_records(records=action_scope.side_effects.transactions)

chia/rpc/wallet_rpc_api.py

@@ -1133,7 +1133,12 @@ async def split_coins(
             raise ValueError("Cannot split coins from non-fungible wallet types")
 
         outputs = [
-            Payment(await wallet.get_puzzle_hash(new=True), request.amount_per_coin)
+            Payment(
+                await wallet.get_puzzle_hash(new=True)
+                if isinstance(wallet, Wallet)
+                else await wallet.standard_wallet.get_puzzle_hash(new=True),
+                request.amount_per_coin,
+            )
             for _ in range(request.number_of_coins)
         ]
         if len(outputs) == 0:
@@ -1268,7 +1273,7 @@ async def combine_coins(
             assert isinstance(wallet, CATWallet)
             await wallet.generate_signed_transaction(
                 [primary_output_amount],
-                [await wallet.get_puzzle_hash(new=not action_scope.config.tx_config.reuse_puzhash)],
+                [await wallet.standard_wallet.get_puzzle_hash(new=not action_scope.config.tx_config.reuse_puzhash)],
                 action_scope,
                 request.fee,
                 coins=set(coins),

chia/wallet/cat_wallet/cat_wallet.py

@@ -422,33 +422,6 @@ async def puzzle_solution_received(self, coin: Coin, parent_coin_data: Optional[
                         # We also need to make sure there's no record of the transaction
                         await self.wallet_state_manager.tx_store.delete_transaction_record(record.coin.name())
 
-    async def get_inner_puzzle(self, new: bool) -> Program:
-        return await self.standard_wallet.get_puzzle(new=new)
-
-    async def get_inner_puzzle_hash(self, new: bool) -> bytes32:
-        return await self.standard_wallet.get_puzzle_hash(new=new)
-
-    async def get_new_inner_hash(self) -> bytes32:
-        puzzle = await self.get_new_inner_puzzle()
-        return puzzle.get_tree_hash()
-
-    async def get_new_inner_puzzle(self) -> Program:
-        return await self.standard_wallet.get_new_puzzle()
-
-    async def get_new_puzzlehash(self) -> bytes32:
-        return await self.standard_wallet.get_new_puzzlehash()
-
-    async def get_puzzle_hash(self, new: bool) -> bytes32:
-        if new:
-            return await self.get_new_puzzlehash()
-        else:
-            record: Optional[
-                DerivationRecord
-            ] = await self.wallet_state_manager.get_current_derivation_record_for_wallet(self.standard_wallet.id())
-            if record is None:
-                return await self.get_new_puzzlehash()
-            return record.puzzle_hash
-
     def require_derivation_paths(self) -> bool:
         return True
 
@@ -462,8 +435,15 @@ def puzzle_hash_for_pk(self, pubkey: G1Element) -> bytes32:
         limitations_program_hash_hash = Program.to(self.cat_info.limitations_program_hash).get_tree_hash()
         return curry_and_treehash(QUOTED_MOD_HASH, CAT_MOD_HASH_HASH, limitations_program_hash_hash, inner_puzzle_hash)
 
-    async def get_new_cat_puzzle_hash(self) -> bytes32:
-        return (await self.wallet_state_manager.get_unused_derivation_record(self.id())).puzzle_hash
+    async def get_cat_puzzle_hash(self, new: bool) -> bytes32:
+        if new:
+            return (await self.wallet_state_manager.get_unused_derivation_record(self.id())).puzzle_hash
+        else:
+            derivation_record = await self.wallet_state_manager.get_current_derivation_record_for_wallet(self.id())
+            if derivation_record is None:
+                return (await self.wallet_state_manager.get_unused_derivation_record(self.id())).puzzle_hash
+
+            return derivation_record.puzzle_hash
 
     async def get_spendable_balance(self, records: Optional[set[WalletCoinRecord]] = None) -> uint128:
         coins = await self.get_cat_spendable_coins(records)
@@ -679,10 +659,10 @@ async def generate_unsigned_spendbundle(
                 for payment in payments:
                     if change_puzhash == payment.puzzle_hash and change == payment.amount:
                         # We cannot create two coins has same id, create a new puzhash for the change
-                        change_puzhash = await self.get_new_inner_hash()
+                        change_puzhash = await self.standard_wallet.get_puzzle_hash(new=True)
                         break
             else:
-                change_puzhash = await self.get_new_inner_hash()
+                change_puzhash = await self.standard_wallet.get_puzzle_hash(new=True)
             primaries.append(Payment(change_puzhash, uint64(change), [change_puzhash]))
 
         # Loop through the coins we've selected and gather the information we need to spend them

chia/wallet/puzzles/tails.py

@@ -97,7 +97,9 @@ async def generate_issuance_bundle(
         origin = coins.copy().pop()
         origin_id = origin.name()
 
-        cat_inner: Program = await wallet.get_inner_puzzle(new=not action_scope.config.tx_config.reuse_puzhash)
+        cat_inner: Program = await wallet.standard_wallet.get_puzzle(
+            new=not action_scope.config.tx_config.reuse_puzhash
+        )
         tail: Program = cls.construct([Program.to(origin_id)])
 
         wallet.lineage_store = await CATLineageStore.create(
@@ -264,7 +266,9 @@ async def generate_issuance_bundle(
             origin = coins.copy().pop()
             origin_id = origin.name()
 
-        cat_inner: Program = await wallet.get_new_inner_puzzle()
+        cat_inner: Program = await wallet.standard_wallet.get_puzzle(
+            new=not action_scope.config.tx_config.reuse_puzhash
+        )
         # GENESIS_ID
         # TREASURY_SINGLETON_STRUCT  ; (SINGLETON_MOD_HASH, (LAUNCHER_ID, LAUNCHER_PUZZLE_HASH))
         launcher_puzhash = create_cat_launcher_for_singleton_id(tail_info["treasury_id"]).get_tree_hash()

chia/wallet/trade_manager.py

@@ -506,9 +506,14 @@ async def _create_offer_for_ids(
                         wallet_id = uint32(id)
                         wallet = self.wallet_state_manager.wallets.get(wallet_id)
                         assert isinstance(wallet, (CATWallet, Wallet))
-                        p2_ph: bytes32 = await wallet.get_puzzle_hash(
-                            new=not action_scope.config.tx_config.reuse_puzhash
-                        )
+                        if isinstance(wallet, Wallet):
+                            p2_ph: bytes32 = await wallet.get_puzzle_hash(
+                                new=not action_scope.config.tx_config.reuse_puzhash
+                            )
+                        else:
+                            p2_ph = await wallet.standard_wallet.get_puzzle_hash(
+                                new=not action_scope.config.tx_config.reuse_puzhash
+                            )
                         if wallet.type() != WalletType.STANDARD_WALLET:
                             if callable(getattr(wallet, "get_asset_id", None)):  # ATTENTION: new wallets
                                 assert isinstance(wallet, CATWallet)

chia/wallet/vc_wallet/cr_cat_wallet.py

@@ -458,10 +458,10 @@ async def _generate_unsigned_spendbundle(
                 for payment in payments:
                     if change_puzhash == payment.puzzle_hash and change == payment.amount:
                         # We cannot create two coins has same id, create a new puzhash for the change
-                        change_puzhash = await self.get_new_inner_hash()
+                        change_puzhash = await self.standard_wallet.get_puzzle_hash(new=True)
                         break
             else:
-                change_puzhash = await self.get_new_inner_hash()
+                change_puzhash = await self.standard_wallet.get_puzzle_hash(new=True)
             primaries.append(Payment(change_puzhash, uint64(change), [change_puzhash]))
 
         # Find the VC Wallet