[CHIA-2947] Actually don't commit puzzle hashes when push=False (#19647)

* Actually dont commit puzzle hashes when push=False

* Fix tests

* Fix tests

* Fix test

Author: Quexington

chia/_tests/wallet/cat_wallet/test_trades.py

@@ -188,6 +188,7 @@ async def test_cat_trades(
             wallet_node_maker,
             client_maker,
             full_node,
+            wallet_environments.tx_config,
             authorized_providers,
             tail_maker,
             proofs_checker_maker,
@@ -198,6 +199,7 @@ async def test_cat_trades(
             wallet_node_taker,
             client_taker,
             full_node,
+            wallet_environments.tx_config,
             authorized_providers,
             tail_taker,
             proofs_checker_taker,

chia/_tests/wallet/test_signer_protocol.py

@@ -76,7 +76,6 @@
     json_deserialize_with_clvm_streamable,
     json_serialize_with_clvm_streamable,
 )
-from chia.wallet.util.tx_config import DEFAULT_TX_CONFIG
 from chia.wallet.wallet import Wallet
 from chia.wallet.wallet_spend_bundle import WalletSpendBundle
 from chia.wallet.wallet_state_manager import WalletStateManager
@@ -614,7 +613,9 @@ async def test_signer_commands(wallet_environments: WalletTestFramework) -> None
     )
 
     AMOUNT = uint64(1)
-    async with wallet_state_manager.new_action_scope(DEFAULT_TX_CONFIG, sign=False, push=False) as action_scope:
+    async with wallet_state_manager.new_action_scope(
+        wallet_environments.tx_config, sign=False, push=False
+    ) as action_scope:
         await wallet.generate_signed_transaction([AMOUNT], [bytes32.zeros], action_scope)
     [tx] = action_scope.side_effects.transactions
 

chia/_tests/wallet/test_wallet.py

@@ -401,7 +401,11 @@ async def test_wallet_clawback_clawback(self, wallet_environments: WalletTestFra
 
         test_fee = 10
         resp = await api_0.spend_clawback_coins(
-            {"coin_ids": [normal_puzhash.hex(), merkle_coin.name().hex()], "fee": test_fee}
+            {
+                "coin_ids": [normal_puzhash.hex(), merkle_coin.name().hex()],
+                "fee": test_fee,
+                **wallet_environments.tx_config.to_json_dict(),
+            }
         )
         assert resp["success"]
         assert len(resp["transaction_ids"]) == 1
@@ -530,7 +534,11 @@ async def test_wallet_clawback_sent_self(self, wallet_environments: WalletTestFr
         merkle_coin = tx.additions[0] if tx.additions[0].amount == tx_amount else tx.additions[1]
         test_fee = 10
         resp = await api_0.spend_clawback_coins(
-            {"coin_ids": [merkle_coin.name().hex(), normal_puzhash.hex()], "fee": test_fee}
+            {
+                "coin_ids": [merkle_coin.name().hex(), normal_puzhash.hex()],
+                "fee": test_fee,
+                **wallet_environments.tx_config.to_json_dict(),
+            }
         )
         assert resp["success"]
         assert len(resp["transaction_ids"]) == 1
@@ -663,7 +671,11 @@ async def test_wallet_clawback_claim_manual(self, wallet_environments: WalletTes
         merkle_coin = tx.additions[0] if tx.additions[0].amount == tx_amount else tx.additions[1]
         test_fee = 10
         resp = await api_1.spend_clawback_coins(
-            {"coin_ids": [merkle_coin.name().hex(), normal_puzhash.hex()], "fee": test_fee}
+            {
+                "coin_ids": [merkle_coin.name().hex(), normal_puzhash.hex()],
+                "fee": test_fee,
+                **wallet_environments.tx_config.to_json_dict(),
+            }
         )
         assert resp["success"]
         assert len(resp["transaction_ids"]) == 1
@@ -2133,6 +2145,24 @@ async def test_forced_new_puzzle_hash(self, wallet_environments: WalletTestFrame
         assert tx.spend_bundle is not None
         assert len(list(set(coin.puzzle_hash for coin in tx.spend_bundle.additions()))) == 2
 
+    @pytest.mark.parametrize(
+        "wallet_environments",
+        [{"num_environments": 1, "blocks_needed": [1], "reuse_puzhash": True, "trusted": True}],
+        indirect=True,
+    )
+    @pytest.mark.limit_consensus_modes
+    @pytest.mark.anyio
+    async def test_puzzle_hashes_not_committed(self, wallet_environments: WalletTestFramework) -> None:
+        env = wallet_environments.environments[0]
+        wallet = env.xch_wallet
+
+        # Our framework
+        async with wallet.wallet_state_manager.new_action_scope(
+            wallet_environments.tx_config,
+            push=False,
+        ) as action_scope:
+            await action_scope.get_puzzle_hash(wallet.wallet_state_manager, override_reuse_puzhash_with=False)
+
 
 def test_get_wallet_db_path_v2_r1() -> None:
     root_path: Path = Path("/x/y/z/.chia/mainnet").resolve()

chia/_tests/wallet/vc_wallet/test_vc_wallet.py

@@ -25,7 +25,7 @@
 from chia.wallet.did_wallet.did_wallet import DIDWallet
 from chia.wallet.util.query_filter import TransactionTypeFilter
 from chia.wallet.util.transaction_type import TransactionType
-from chia.wallet.util.tx_config import DEFAULT_TX_CONFIG
+from chia.wallet.util.tx_config import DEFAULT_TX_CONFIG, TXConfig
 from chia.wallet.util.wallet_types import WalletType
 from chia.wallet.vc_wallet.cr_cat_drivers import ProofsChecker, construct_cr_layer
 from chia.wallet.vc_wallet.cr_cat_wallet import CRCATWallet
@@ -41,11 +41,12 @@ async def mint_cr_cat(
     wallet_node_0: WalletNode,
     client_0: WalletRpcClient,
     full_node_api: FullNodeSimulator,
+    tx_config: TXConfig,
     authorized_providers: list[bytes32] = [],
     tail: Program = Program.to(None),
     proofs_checker: ProofsChecker = ProofsChecker(["foo", "bar"]),
 ) -> None:
-    async with wallet_0.wallet_state_manager.new_action_scope(DEFAULT_TX_CONFIG, push=True) as action_scope:
+    async with wallet_0.wallet_state_manager.new_action_scope(tx_config, push=True) as action_scope:
         our_puzzle = await action_scope.get_puzzle(wallet_0.wallet_state_manager)
     cat_puzzle: Program = construct_cat_puzzle(
         CAT_MOD,
@@ -63,7 +64,7 @@ async def mint_cr_cat(
                     "amount": CAT_AMOUNT_0,
                 }
             ],
-            DEFAULT_TX_CONFIG,
+            tx_config,
             wallet_id=1,
         )
     ).signed_tx
@@ -314,7 +315,7 @@ async def test_vc_lifecycle(wallet_environments: WalletTestFramework) -> None:
     assert get_list_reponse.proof_dict[proof_root] == proofs.key_value_pairs
 
     # Mint CR-CAT
-    await mint_cr_cat(1, wallet_0, wallet_node_0, client_0, full_node_api, [did_id])
+    await mint_cr_cat(1, wallet_0, wallet_node_0, client_0, full_node_api, wallet_environments.tx_config, [did_id])
     await wallet_environments.process_pending_states(
         [
             WalletStateTransition(
@@ -823,7 +824,7 @@ async def test_cat_wallet_conversion(
     )
 
     did_id = bytes32.zeros
-    await mint_cr_cat(num_blocks, wallet_0, wallet_node_0, client_0, full_node_api, [did_id])
+    await mint_cr_cat(num_blocks, wallet_0, wallet_node_0, client_0, full_node_api, DEFAULT_TX_CONFIG, [did_id])
     await full_node_api.farm_blocks_to_wallet(count=num_blocks, wallet=wallet_0)
     await full_node_api.wait_for_wallet_synced(wallet_node=wallet_node_0, timeout=20)
 

chia/wallet/trade_manager.py

@@ -862,6 +862,16 @@ async def respond_to_offer(
             complete_offer, valid_spend_solver = await self.check_for_final_modifications(
                 Offer.aggregate([offer, take_offer]), solver, inner_action_scope
             )
+
+        async with action_scope.use() as interface:
+            if interface.side_effects.get_unused_derivation_record_result is not None:
+                # This error is of a protection against potential misues of this band-aid solution.
+                # We should put more thought into how sub-action scopes are generated and what effects
+                # we might want to push. A ticket to this respect can be found [CHIA-2984].
+                raise ValueError("Cannot use `respond_to_offer` with existing puzzle hash generation")
+            interface.side_effects.get_unused_derivation_record_result = (
+                inner_action_scope.side_effects.get_unused_derivation_record_result
+            )
         self.log.info("COMPLETE OFFER: %s", complete_offer.to_bech32())
         assert complete_offer.is_valid()
         final_spend_bundle: WalletSpendBundle = complete_offer.to_valid_spend(

chia/wallet/wallet_action_scope.py

@@ -165,5 +165,5 @@ async def new_wallet_action_scope(
         extra_spends=self.side_effects.extra_spends,
         singleton_records=self.side_effects.singleton_records,
     )
-    if self.side_effects.get_unused_derivation_record_result is not None:
+    if push and self.side_effects.get_unused_derivation_record_result is not None:
         await self.side_effects.get_unused_derivation_record_result.to_standard().commit(wallet_state_manager)

chia/wallet/wallet_state_manager.py

@@ -368,6 +368,16 @@ def get_wallet(self, id: uint32, required_type: type[TWalletType]) -> TWalletTyp
 
         return wallet
 
+    @asynccontextmanager
+    async def puzzle_hash_db_writer(self) -> AsyncIterator[None]:
+        async with self.db_wrapper.writer():
+            old_cache = self.puzzle_store.last_wallet_derivation_index.copy()
+            try:
+                yield
+            except Exception:
+                self.puzzle_store.last_wallet_derivation_index = old_cache
+                raise
+
     async def create_more_puzzle_hashes(
         self,
         from_zero: bool = False,
@@ -382,7 +392,7 @@ async def create_more_puzzle_hashes(
         that we can restore the wallet from only the private keys.
         """
         try:
-            async with self.db_wrapper.writer():
+            async with self.puzzle_hash_db_writer():
                 if previous_result is not None:
                     if previous_result.mark_existing_as_used is not mark_existing_as_used:
                         raise ValueError(
@@ -562,7 +572,7 @@ async def _get_unused_derivation_record(
         same public key more than once (for privacy).
         """
         try:
-            async with self.db_wrapper.writer():
+            async with self.puzzle_hash_db_writer():
                 if previous_result is not None:
                     await previous_result.commit(self)
                     create_more_puzzle_hashes_result: Optional[CreateMorePuzzleHashesResult] = (