[CHIA-3601] Port spend_clawback_coins to @marshal (#19965)

* Port `spend_clawback_coins` to `@marshal`

* fix tests

* fix test again

* how do I keep missing these?

Author: Quexington

chia/_tests/cmds/wallet/test_wallet.py

@@ -67,6 +67,8 @@
     RoyaltyAsset,
     SendTransaction,
     SendTransactionResponse,
+    SpendClawbackCoins,
+    SpendClawbackCoinsResponse,
     TakeOfferResponse,
     TransactionRecordWithMetadata,
     WalletInfoResponse,
@@ -539,23 +541,25 @@ def test_clawback(capsys: object, get_test_cli_clients: tuple[TestRpcClients, Pa
     class ClawbackWalletRpcClient(TestWalletRpcClient):
         async def spend_clawback_coins(
             self,
-            coin_ids: list[bytes32],
-            fee: int = 0,
-            force: bool = False,
-            push: bool = True,
+            request: SpendClawbackCoins,
+            tx_config: TXConfig,
+            extra_conditions: tuple[Condition, ...] = tuple(),
             timelock_info: ConditionValidTimes = ConditionValidTimes(),
-        ) -> dict[str, Any]:
-            self.add_to_log("spend_clawback_coins", (coin_ids, fee, force, push, timelock_info))
-            tx_hex_list = [get_bytes32(6).hex(), get_bytes32(7).hex(), get_bytes32(8).hex()]
-            return {
-                "transaction_ids": tx_hex_list,
-                "transactions": [STD_TX.to_json_dict()],
-            }
+        ) -> SpendClawbackCoinsResponse:
+            self.add_to_log(
+                "spend_clawback_coins", (request.coin_ids, request.fee, request.force, request.push, timelock_info)
+            )
+            tx_list = [get_bytes32(6), get_bytes32(7), get_bytes32(8)]
+            return SpendClawbackCoinsResponse(
+                transaction_ids=tx_list,
+                transactions=[STD_TX],
+                unsigned_transactions=[STD_UTX],
+            )
 
     inst_rpc_client = ClawbackWalletRpcClient()
     test_rpc_clients.wallet_rpc_client = inst_rpc_client
     tx_ids = [get_bytes32(3), get_bytes32(4), get_bytes32(5)]
-    r_tx_ids_hex = [get_bytes32(6).hex(), get_bytes32(7).hex(), get_bytes32(8).hex()]
+    r_tx_ids_hex = ["0x" + get_bytes32(6).hex(), "0x" + get_bytes32(7).hex(), "0x" + get_bytes32(8).hex()]
     command_args = [
         "wallet",
         "clawback",
@@ -569,7 +573,7 @@ async def spend_clawback_coins(
         "--expires-at",
         "150",
     ]
-    run_cli_command_and_assert(capsys, root_dir, command_args, ["transaction_ids", str(r_tx_ids_hex)])
+    run_cli_command_and_assert(capsys, root_dir, command_args, ["transaction_ids", *r_tx_ids_hex])
     # these are various things that should be in the output
     expected_calls: logType = {
         "spend_clawback_coins": [(tx_ids, 500000000000, False, True, test_condition_valid_times)],

chia/_tests/wallet/rpc/test_wallet_rpc.py

@@ -143,6 +143,7 @@
     RoyaltyAsset,
     SendTransaction,
     SetWalletResyncOnStartup,
+    SpendClawbackCoins,
     SplitCoins,
     VerifySignature,
     VerifySignatureResponse,
@@ -833,7 +834,6 @@ async def test_spend_clawback_coins(wallet_rpc_environment: WalletRpcTestEnviron
     wallet_1 = wallet_1_node.wallet_state_manager.main_wallet
     wallet_2 = wallet_2_node.wallet_state_manager.main_wallet
     full_node_api: FullNodeSimulator = env.full_node.api
-    wallet_2_api = WalletRpcApi(wallet_2_node)
 
     generated_funds = await generate_funds(full_node_api, env.wallet_1, 1)
     await generate_funds(full_node_api, env.wallet_2, 1)
@@ -876,41 +876,39 @@ async def test_spend_clawback_coins(wallet_rpc_environment: WalletRpcTestEnviron
     await time_out_assert(20, get_confirmed_balance, generated_funds - 500, wallet_1_rpc, 1)
     await time_out_assert(20, get_confirmed_balance, generated_funds - 500, wallet_2_rpc, 1)
     await asyncio.sleep(10)
-    # Test missing coin_ids
-    has_exception = False
-    try:
-        await wallet_2_api.spend_clawback_coins({})
-    except ValueError:
-        has_exception = True
-    assert has_exception
     # Test coin ID is not a Clawback coin
     invalid_coin_id = tx.removals[0].name()
-    resp = await wallet_2_rpc.spend_clawback_coins([invalid_coin_id], 500)
-    assert resp["success"]
-    assert resp["transaction_ids"] == []
+    resp = await wallet_2_rpc.spend_clawback_coins(
+        SpendClawbackCoins(coin_ids=[invalid_coin_id], fee=uint64(500), push=True), tx_config=DEFAULT_TX_CONFIG
+    )
+    assert resp.transaction_ids == []
     # Test unsupported wallet
     coin_record = await wallet_1_node.wallet_state_manager.coin_store.get_coin_record(clawback_coin_id_1)
     assert coin_record is not None
     await wallet_1_node.wallet_state_manager.coin_store.add_coin_record(
         dataclasses.replace(coin_record, wallet_type=WalletType.CAT)
     )
-    resp = await wallet_1_rpc.spend_clawback_coins([clawback_coin_id_1], 100)
-    assert resp["success"]
-    assert len(resp["transaction_ids"]) == 0
+    resp = await wallet_1_rpc.spend_clawback_coins(
+        SpendClawbackCoins(coin_ids=[clawback_coin_id_1], fee=uint64(100), push=True), tx_config=DEFAULT_TX_CONFIG
+    )
+    assert len(resp.transaction_ids) == 0
     # Test missing metadata
     await wallet_1_node.wallet_state_manager.coin_store.add_coin_record(dataclasses.replace(coin_record, metadata=None))
-    resp = await wallet_1_rpc.spend_clawback_coins([clawback_coin_id_1], 100)
-    assert resp["success"]
-    assert len(resp["transaction_ids"]) == 0
+    resp = await wallet_1_rpc.spend_clawback_coins(
+        SpendClawbackCoins(coin_ids=[clawback_coin_id_1], fee=uint64(100), push=True), tx_config=DEFAULT_TX_CONFIG
+    )
+    assert len(resp.transaction_ids) == 0
     # Test missing incoming tx
     coin_record = await wallet_1_node.wallet_state_manager.coin_store.get_coin_record(clawback_coin_id_2)
     assert coin_record is not None
     fake_coin = Coin(coin_record.coin.parent_coin_info, wallet_2_puzhash, coin_record.coin.amount)
     await wallet_1_node.wallet_state_manager.coin_store.add_coin_record(
         dataclasses.replace(coin_record, coin=fake_coin)
     )
-    resp = await wallet_1_rpc.spend_clawback_coins([fake_coin.name()], 100)
-    assert resp["transaction_ids"] == []
+    resp = await wallet_1_rpc.spend_clawback_coins(
+        SpendClawbackCoins(coin_ids=[fake_coin.name()], fee=uint64(100), push=True), tx_config=DEFAULT_TX_CONFIG
+    )
+    assert resp.transaction_ids == []
     # Test coin puzzle hash doesn't match the puzzle
     farmed_tx = (await wallet_1.wallet_state_manager.tx_store.get_farming_rewards())[0]
     await wallet_1.wallet_state_manager.tx_store.add_transaction_record(
@@ -919,8 +917,10 @@ async def test_spend_clawback_coins(wallet_rpc_environment: WalletRpcTestEnviron
     await wallet_1_node.wallet_state_manager.coin_store.add_coin_record(
         dataclasses.replace(coin_record, coin=fake_coin)
     )
-    resp = await wallet_1_rpc.spend_clawback_coins([fake_coin.name()], 100)
-    assert resp["transaction_ids"] == []
+    resp = await wallet_1_rpc.spend_clawback_coins(
+        SpendClawbackCoins(coin_ids=[fake_coin.name()], fee=uint64(100), push=True), tx_config=DEFAULT_TX_CONFIG
+    )
+    assert resp.transaction_ids == []
     # Test claim spend
     await wallet_2_rpc.set_auto_claim(
         AutoClaimSettings(
@@ -930,21 +930,23 @@ async def test_spend_clawback_coins(wallet_rpc_environment: WalletRpcTestEnviron
             batch_size=uint16(1),
         )
     )
-    resp = await wallet_2_rpc.spend_clawback_coins([clawback_coin_id_1, clawback_coin_id_2], 100)
-    assert resp["success"]
-    assert len(resp["transaction_ids"]) == 2
-    for _tx in resp["transactions"]:
-        clawback_tx = TransactionRecord.from_json_dict(_tx)
+    resp = await wallet_2_rpc.spend_clawback_coins(
+        SpendClawbackCoins(coin_ids=[clawback_coin_id_1, clawback_coin_id_2], fee=uint64(100), push=True),
+        tx_config=DEFAULT_TX_CONFIG,
+    )
+    assert len(resp.transaction_ids) == 2
+    for clawback_tx in resp.transactions:
         if clawback_tx.spend_bundle is not None:
             await time_out_assert_not_none(
                 10, full_node_api.full_node.mempool_manager.get_spendbundle, clawback_tx.spend_bundle.name()
             )
     await farm_transaction_block(full_node_api, wallet_2_node)
     await time_out_assert(20, get_confirmed_balance, generated_funds + 300, wallet_2_rpc, 1)
     # Test spent coin
-    resp = await wallet_2_rpc.spend_clawback_coins([clawback_coin_id_1], 500)
-    assert resp["success"]
-    assert resp["transaction_ids"] == []
+    resp = await wallet_2_rpc.spend_clawback_coins(
+        SpendClawbackCoins(coin_ids=[clawback_coin_id_1], fee=uint64(500), push=True), tx_config=DEFAULT_TX_CONFIG
+    )
+    assert resp.transaction_ids == []
 
 
 @pytest.mark.anyio
@@ -2861,12 +2863,11 @@ async def test_set_wallet_resync_on_startup(wallet_rpc_environment: WalletRpcTes
     await farm_transaction(full_node_api, wallet_node, tx.spend_bundle)
     await time_out_assert(20, check_client_synced, True, wc)
     await asyncio.sleep(10)
-    resp = await wc.spend_clawback_coins([clawback_coin_id], 0)
-    assert resp["success"]
-    assert len(resp["transaction_ids"]) == 1
-    await time_out_assert_not_none(
-        10, full_node_api.full_node.mempool_manager.get_spendbundle, bytes32.from_hexstr(resp["transaction_ids"][0])
+    resp = await wc.spend_clawback_coins(
+        SpendClawbackCoins(coin_ids=[clawback_coin_id], fee=uint64(0), push=True), tx_config=DEFAULT_TX_CONFIG
     )
+    assert len(resp.transaction_ids) == 1
+    await time_out_assert_not_none(10, full_node_api.full_node.mempool_manager.get_spendbundle, resp.transaction_ids[0])
     await farm_transaction_block(full_node_api, wallet_node)
     await time_out_assert(20, check_client_synced, True, wc)
     wallet_node_2._close()

chia/_tests/wallet/test_wallet.py

@@ -397,8 +397,6 @@ async def test_wallet_clawback_clawback(self, wallet_environments: WalletTestFra
         assert not txs["transactions"][0]["confirmed"]
         assert txs["transactions"][0]["metadata"]["recipient_puzzle_hash"][2:] == normal_puzhash.hex()
         assert txs["transactions"][0]["metadata"]["coin_id"] == "0x" + merkle_coin.name().hex()
-        with pytest.raises(ValueError):
-            await api_0.spend_clawback_coins({})
 
         test_fee = 10
         resp = await api_0.spend_clawback_coins(
@@ -408,7 +406,6 @@ async def test_wallet_clawback_clawback(self, wallet_environments: WalletTestFra
                 **wallet_environments.tx_config.to_json_dict(),
             }
         )
-        assert resp["success"]
         assert len(resp["transaction_ids"]) == 1
 
         await wallet_environments.process_pending_states(
@@ -541,7 +538,6 @@ async def test_wallet_clawback_sent_self(self, wallet_environments: WalletTestFr
                 **wallet_environments.tx_config.to_json_dict(),
             }
         )
-        assert resp["success"]
         assert len(resp["transaction_ids"]) == 1
         # Wait mempool update
         await wallet_environments.process_pending_states(
@@ -678,7 +674,6 @@ async def test_wallet_clawback_claim_manual(self, wallet_environments: WalletTes
                 **wallet_environments.tx_config.to_json_dict(),
             }
         )
-        assert resp["success"]
         assert len(resp["transaction_ids"]) == 1
 
         await wallet_environments.process_pending_states(
@@ -1094,10 +1089,8 @@ async def test_clawback_resync(self, self_hostname: str, wallet_environments: Wa
         await time_out_assert(20, wsm_2.coin_store.count_small_unspent, 1, 1000, CoinType.CLAWBACK)
         # clawback merkle coin
         resp = await api_1.spend_clawback_coins({"coin_ids": [clawback_coin_id_1.hex()], "fee": 0})
-        assert resp["success"]
         assert len(resp["transaction_ids"]) == 1
         resp = await api_1.spend_clawback_coins({"coin_ids": [clawback_coin_id_2.hex()], "fee": 0})
-        assert resp["success"]
         assert len(resp["transaction_ids"]) == 1
 
         await wallet_environments.process_pending_states(

chia/cmds/wallet_funcs.py

@@ -40,6 +40,7 @@
 from chia.wallet.util.puzzle_decorator_type import PuzzleDecoratorType
 from chia.wallet.util.query_filter import HashFilter, TransactionTypeFilter
 from chia.wallet.util.transaction_type import CLAWBACK_INCOMING_TRANSACTION_TYPES, TransactionType
+from chia.wallet.util.tx_config import DEFAULT_TX_CONFIG
 from chia.wallet.util.wallet_types import WalletType
 from chia.wallet.vc_wallet.vc_store import VCProofs
 from chia.wallet.wallet_coin_store import GetCoinRecords
@@ -79,6 +80,7 @@
     SignMessageByAddressResponse,
     SignMessageByID,
     SignMessageByIDResponse,
+    SpendClawbackCoins,
     VCAddProofs,
     VCGet,
     VCGetList,
@@ -1684,14 +1686,12 @@ async def spend_clawback(
             print("Batch fee cannot be negative.")
             return []
         response = await wallet_client.spend_clawback_coins(
-            tx_ids,
-            fee,
-            force,
-            push=push,
+            SpendClawbackCoins(coin_ids=tx_ids, fee=fee, force=force, push=push),
+            tx_config=DEFAULT_TX_CONFIG,
             timelock_info=condition_valid_times,
         )
         print(str(response))
-        return [TransactionRecord.from_json_dict(tx) for tx in response["transactions"]]
+        return response.transactions
 
 
 async def mint_vc(

chia/wallet/wallet_request_types.py

@@ -1146,6 +1146,20 @@ class SendTransactionResponse(TransactionEndpointResponse):
     transaction_id: bytes32
 
 
+@streamable
+@dataclass(frozen=True)
+class SpendClawbackCoins(TransactionEndpointRequest):
+    coin_ids: list[bytes32] = field(default_factory=default_raise)
+    batch_size: Optional[uint16] = None
+    force: bool = False
+
+
+@streamable
+@dataclass(frozen=True)
+class SpendClawbackCoinsResponse(TransactionEndpointResponse):
+    transaction_ids: list[bytes32]
+
+
 @streamable
 @dataclass(frozen=True)
 class PushTransactions(TransactionEndpointRequest):

chia/wallet/wallet_rpc_api.py

@@ -249,6 +249,8 @@
     SignMessageByAddressResponse,
     SignMessageByID,
     SignMessageByIDResponse,
+    SpendClawbackCoins,
+    SpendClawbackCoinsResponse,
     SplitCoins,
     SplitCoinsResponse,
     SubmitTransactions,
@@ -1655,34 +1657,33 @@ async def send_transaction_multi(self, request: dict[str, Any]) -> EndpointResul
         }
 
     @tx_endpoint(push=True, merge_spends=False)
+    @marshal
     async def spend_clawback_coins(
         self,
-        request: dict[str, Any],
+        request: SpendClawbackCoins,
         action_scope: WalletActionScope,
         extra_conditions: tuple[Condition, ...] = tuple(),
-    ) -> EndpointResult:
+    ) -> SpendClawbackCoinsResponse:
         """Spend clawback coins that were sent (to claw them back) or received (to claim them).
 
         :param coin_ids: list of coin ids to be spent
         :param batch_size: number of coins to spend per bundle
         :param fee: transaction fee in mojos
         :return:
         """
-        if "coin_ids" not in request:
-            raise ValueError("Coin IDs are required.")
-        coin_ids: list[bytes32] = [bytes32.from_hexstr(coin) for coin in request["coin_ids"]]
-        tx_fee: uint64 = uint64(request.get("fee", 0))
         # Get inner puzzle
         coin_records = await self.service.wallet_state_manager.coin_store.get_coin_records(
-            coin_id_filter=HashFilter.include(coin_ids),
+            coin_id_filter=HashFilter.include(request.coin_ids),
             coin_type=CoinType.CLAWBACK,
             wallet_type=WalletType.STANDARD_WALLET,
             spent_range=UInt32Range(stop=uint32(0)),
         )
 
         coins: dict[Coin, ClawbackMetadata] = {}
-        batch_size = request.get(
-            "batch_size", self.service.wallet_state_manager.config.get("auto_claim", {}).get("batch_size", 50)
+        batch_size = (
+            request.batch_size
+            if request.batch_size is not None
+            else self.service.wallet_state_manager.config.get("auto_claim", {}).get("batch_size", 50)
         )
         for coin_id, coin_record in coin_records.coin_id_to_record.items():
             try:
@@ -1692,9 +1693,9 @@ async def spend_clawback_coins(
                 if len(coins) >= batch_size:
                     await self.service.wallet_state_manager.spend_clawback_coins(
                         coins,
-                        tx_fee,
+                        request.fee,
                         action_scope,
-                        request.get("force", False),
+                        request.force,
                         extra_conditions=extra_conditions,
                     )
                     coins = {}
@@ -1703,17 +1704,14 @@ async def spend_clawback_coins(
         if len(coins) > 0:
             await self.service.wallet_state_manager.spend_clawback_coins(
                 coins,
-                tx_fee,
+                request.fee,
                 action_scope,
-                request.get("force", False),
+                request.force,
                 extra_conditions=extra_conditions,
             )
 
-        return {
-            "success": True,
-            "transaction_ids": None,  # tx_endpoint wrapper will take care of this
-            "transactions": None,  # tx_endpoint wrapper will take care of this
-        }
+        # tx_endpoint will fill in the default values here
+        return SpendClawbackCoinsResponse([], [], transaction_ids=[])
 
     @marshal
     async def delete_unconfirmed_transactions(self, request: DeleteUnconfirmedTransactions) -> Empty: