Fix wallet sweep

src/cpp/wallet/py_monero_wallet_model.cpp

@@ -240,11 +240,13 @@ void PyMoneroTxWallet::init_sent(const monero::monero_tx_config &config, std::sh
   outgoing_transfer->m_tx = tx;
 
   if (config.m_subaddress_indices.size() == 1) {
+    // we know src subaddress indices iff request specifies 1
     outgoing_transfer->m_subaddress_indices = config.m_subaddress_indices;
   }
 
   if (copy_destinations) {
-    for(const auto &conf_dest : config.m_destinations) {
+    auto conf_dests = config.get_normalized_destinations();
+    for(const auto &conf_dest : conf_dests) {
       auto dest = std::make_shared<monero::monero_destination>();
       conf_dest->copy(conf_dest, dest);
       outgoing_transfer->m_destinations.push_back(dest);
@@ -462,15 +464,13 @@ void PyMoneroTxWallet::from_property_tree_with_transfer(const boost::property_tr
         }
       }
 
-      size_t num_destinations = config.m_destinations.size();
-      if (num_destinations == 0 && config.m_address != boost::none){
-        num_destinations++;
-      }
+      auto destinations = config.get_normalized_destinations();
+      size_t num_destinations = destinations.size();
       if (num_destinations != amounts_by_dest.size()) throw std::runtime_error("Expected destinations size equal to amounts by dest size");
 
-      for(uint64_t i = 0; i < config.m_destinations.size(); i++) {
+      for(uint64_t i = 0; i < num_destinations; i++) {
         auto dest = std::make_shared<monero::monero_destination>();
-        dest->m_address = config.m_destinations[i]->m_address;
+        dest->m_address = destinations[i]->m_address;
         dest->m_amount = amounts_by_dest[i];
         outgoing_transfer->m_destinations.push_back(dest);
       }
@@ -492,6 +492,10 @@ void PyMoneroTxWallet::from_property_tree_with_transfer(const boost::property_tr
     tx->m_is_outgoing = true;
 
     if (tx->m_outgoing_transfer != boost::none) {
+      // overwrite to avoid reconcile error TODO: remove after >18.3.1 when amounts_by_dest supported
+      if (tx->m_outgoing_transfer.get()->m_destinations.size() != 0) {
+        tx->m_outgoing_transfer.get()->m_destinations.clear();
+      }
       tx->m_outgoing_transfer.get()->merge(tx->m_outgoing_transfer.get(), outgoing_transfer);
     }
     else tx->m_outgoing_transfer = outgoing_transfer;
@@ -636,9 +640,9 @@ void PyMoneroTxWallet::merge_tx(const std::shared_ptr<monero_tx_wallet>& tx, std
 void PyMoneroTxSet::from_property_tree(const boost::property_tree::ptree& node, const std::shared_ptr<monero::monero_tx_set>& set) {
   for (boost::property_tree::ptree::const_iterator it = node.begin(); it != node.end(); ++it) {
     std::string key = it->first;
-    if (key == std::string("multisig_txset")) set->m_multisig_tx_hex = it->second.data();
-    else if (key == std::string("unsigned_txset")) set->m_unsigned_tx_hex = it->second.data();
-    else if (key == std::string("signed_txset")) set->m_signed_tx_hex = it->second.data();
+    if (key == std::string("multisig_txset") && !it->second.data().empty()) set->m_multisig_tx_hex = it->second.data();
+    else if (key == std::string("unsigned_txset") && !it->second.data().empty()) set->m_unsigned_tx_hex = it->second.data();
+    else if (key == std::string("signed_txset") && !it->second.data().empty()) set->m_signed_tx_hex = it->second.data();
   }
 }
 

src/cpp/wallet/py_monero_wallet_rpc.cpp

@@ -1851,9 +1851,10 @@ std::vector<std::shared_ptr<monero_tx_wallet>> PyMoneroWalletRpc::sweep_account(
     if (config.m_subaddress_indices.size() == 1) {
       transfer->m_subaddress_indices = config.m_subaddress_indices;
     }
-    auto destination = std::make_shared<monero_destination>();
+    auto destination = std::make_shared<monero::monero_destination>();
     destination->m_address = destinations[0]->m_address;
     destination->m_amount = transfer->m_amount;
+    transfer->m_destinations.clear();
     transfer->m_destinations.push_back(destination);
     tx->m_payment_id = config.m_payment_id;
     if (tx->m_unlock_time == boost::none) tx->m_unlock_time = 0;

tests/test_monero_wallet_common.py

@@ -423,6 +423,132 @@ def test_create_then_relay(self, wallet: MoneroWallet) -> None:
     def test_create_then_relay_split(self, wallet: MoneroWallet) -> None:
         WalletUtils.test_send_to_single(wallet, True, False)
 
+    # Can sweep individual outputs identified by their key images
+    @pytest.mark.skipif(TestUtils.TEST_RELAYS is False, reason="TEST_RELAYS disabled")
+    def test_sweep_outputs(self, wallet: MoneroWallet) -> None:
+        TestUtils.WALLET_TX_TRACKER.wait_for_txs_to_clear_pool(wallet)
+
+        # test config
+        num_outputs: int = 3
+
+        # get outputs to sweep (not spent, unlocked, and amount >= fee)
+        query: MoneroOutputQuery = MoneroOutputQuery()
+        query.is_spent = False
+        tx_query: MoneroTxQuery = MoneroTxQuery()
+        tx_query.is_locked = False
+        query.set_tx_query(tx_query, True)
+        spendable_unlocked_outputs: list[MoneroOutputWallet] = wallet.get_outputs(query)
+        outputs_to_sweep: list[MoneroOutputWallet] = []
+        for spendable_output in spendable_unlocked_outputs:
+            if len(outputs_to_sweep) >= num_outputs:
+                break
+            assert spendable_output.amount is not None
+            if spendable_output.amount > TxUtils.MAX_FEE:
+                outputs_to_sweep.append(spendable_output)
+
+        assert len(outputs_to_sweep) >= num_outputs, "Wallet does not have enough sweepable outputs; run send tests"
+
+        # sweep each output by key image
+        for output in outputs_to_sweep:
+            TxUtils.test_output_wallet(output)
+            assert output.is_spent is False
+            assert output.tx is not None
+            assert isinstance(output.tx, MoneroTxWallet)
+            assert output.tx.is_locked is False
+            assert output.amount is not None
+
+            if output.amount <= TxUtils.MAX_FEE:
+                continue
+
+            # sweep output to address
+            assert output.account_index is not None
+            assert output.subaddress_index is not None
+            assert output.key_image is not None
+            assert output.key_image.hex is not None
+            address: str = wallet.get_address(output.account_index, output.subaddress_index)
+            config: MoneroTxConfig = MoneroTxConfig()
+            config.address = address
+            config.key_image = output.key_image.hex
+            config.relay = True
+            tx: MoneroTxWallet = wallet.sweep_output(config)
+
+            # test result tx
+            ctx: TxContext = TxContext()
+            ctx.wallet = wallet
+            ctx.config = config
+            ctx.config.can_split = False
+            ctx.is_send_response = True
+            ctx.is_sweep_response = True
+            ctx.is_sweep_output_response = True
+            TxUtils.test_tx_wallet(tx, ctx)
+
+        # get outputs after sweeping
+        after_outputs: list[MoneroOutputWallet] = wallet.get_outputs()
+
+        # swept outputs are now spent
+        for after_output in after_outputs:
+            assert after_output.key_image is not None
+
+            for output in outputs_to_sweep:
+                assert output.key_image is not None
+                if output.key_image.hex == after_output.key_image.hex:
+                    assert after_output.is_spent is True, "Output should be spent"
+
+    # Can sweep individual outputs identified by their key images
+    @pytest.mark.skipif(TestUtils.TEST_RELAYS is False, reason="TEST_RELAYS disabled")
+    def test_sweep_dust_no_relay(self, wallet: MoneroWallet) -> None:
+        TestUtils.WALLET_TX_TRACKER.wait_for_txs_to_clear_pool(wallet)
+
+        # sweep dust which returns empty list if no dust to sweep (dust does not exist after rct)
+        txs: list[MoneroTxWallet] = wallet.sweep_dust(False)
+        if len(txs) == 0:
+            return
+
+        # test txs
+        ctx: TxContext = TxContext()
+        ctx.is_send_response = True
+        ctx.config = MoneroTxConfig()
+        ctx.config.relay = False
+        ctx.is_sweep_response = True
+        for tx in txs:
+            TxUtils.test_tx_wallet(tx, ctx)
+
+        # relay txs
+        metadatas: list[str] = []
+        for tx in txs:
+            assert tx.metadata is not None
+            metadatas.append(tx.metadata)
+
+        tx_hashes: list[str] = wallet.relay_txs(metadatas)
+        assert len(tx_hashes) == len(txs)
+        for tx_hash in tx_hashes:
+            assert len(tx_hash) == 64
+
+        # fetch and test txs
+        tx_query: MoneroTxQuery = MoneroTxQuery()
+        tx_query.hashes = tx_hashes
+        txs = wallet.get_txs(tx_query)
+        ctx.config.relay = True
+        for tx in txs:
+            TxUtils.test_tx_wallet(tx, ctx)
+
+    # Can sweep dust
+    @pytest.mark.skipif(TestUtils.TEST_RELAYS is False, reason="TEST_RELAYS disabled")
+    def test_sweep_dust(self, wallet: MoneroWallet) -> None:
+        TestUtils.WALLET_TX_TRACKER.wait_for_txs_to_clear_pool(wallet)
+
+        # sweep dust which returns empty list if no dust to sweep (dust does not exist after rct)
+        txs: list[MoneroTxWallet] = wallet.sweep_dust(True)
+
+        # test any txs
+        ctx: TxContext = TxContext()
+        ctx.wallet = wallet
+        ctx.config = None
+        ctx.is_send_response = True
+        ctx.is_sweep_response = True
+        for tx in txs:
+            TxUtils.test_tx_wallet(tx, ctx)
+
     #endregion
 
     #region Non Relays Tests
@@ -3202,6 +3328,181 @@ def test_get_default_fee_priority(self, wallet: MoneroWallet) -> None:
 
     #region Reset Tests
 
+    # Can sweep subaddresses
+    @pytest.mark.skipif(TestUtils.TEST_RESETS is False, reason="TEST_RESETS disabled")
+    def test_sweep_subaddresses(self, wallet: MoneroWallet) -> None:
+        TestUtils.WALLET_TX_TRACKER.wait_for_txs_to_clear_pool(wallet)
+        NUM_SUBADDRESSES_TO_SWEEP: int = 2
+
+        # collect subaddresses with balance and unlocked balance
+        subaddresses: list[MoneroSubaddress] = []
+        subaddresses_balance: list[MoneroSubaddress] = []
+        subaddresses_unlocked: list[MoneroSubaddress] = []
+        for account in wallet.get_accounts(True):
+            if account.index == 0:
+                # skip default account
+                continue
+            for subaddress in account.subaddresses:
+                subaddresses.append(subaddress)
+                assert subaddress.balance is not None
+                if subaddress.balance > TxUtils.MAX_FEE:
+                    subaddresses_balance.append(subaddress)
+                assert subaddress.unlocked_balance is not None
+                if subaddress.unlocked_balance > TxUtils.MAX_FEE:
+                    subaddresses_unlocked.append(subaddress)
+
+        # test requires at least one more subaddresses than the number being swept to verify it does not change
+        msg: str = f"Test requires balance in at least {NUM_SUBADDRESSES_TO_SWEEP + 1} subaddresses from non-default acccount; run send-to-multiple tests"
+        assert len(subaddresses_balance) >= NUM_SUBADDRESSES_TO_SWEEP + 1, msg
+        assert len(subaddresses_unlocked) >= NUM_SUBADDRESSES_TO_SWEEP + 1, "Wallet is waiting on unlocked funds"
+
+        # sweep from first unlocked subaddresses
+        for i in range(NUM_SUBADDRESSES_TO_SWEEP):
+            # sweep unlocked account
+            unlocked_subaddress: MoneroSubaddress = subaddresses_unlocked[i]
+            config: MoneroTxConfig = MoneroTxConfig()
+            config.address = wallet.get_primary_address()
+            assert unlocked_subaddress.account_index is not None
+            assert unlocked_subaddress.index is not None
+            config.account_index = unlocked_subaddress.account_index
+            config.subaddress_indices.append(unlocked_subaddress.index)
+            config.relay = True
+            txs: list[MoneroTxWallet] = wallet.sweep_unlocked(config)
+
+            # test transactions
+            assert len(txs) > 0
+            for tx in txs:
+                assert tx.tx_set is not None
+                assert tx in tx.tx_set.txs
+                ctx: TxContext = TxContext()
+                ctx.wallet = wallet
+                ctx.config = config
+                ctx.is_send_response = True
+                ctx.is_sweep_response = True
+                TxUtils.test_tx_wallet(tx, ctx)
+
+            # assert unlocked balance is less than max fee
+            subaddress: MoneroSubaddress = wallet.get_subaddress(unlocked_subaddress.account_index, unlocked_subaddress.index)
+            assert subaddress.unlocked_balance is not None
+            assert subaddress.unlocked_balance < TxUtils.MAX_FEE
+
+        # test subaddresses after sweeping
+        subaddresses_after: list[MoneroSubaddress] = []
+        for account in wallet.get_accounts(True):
+            if account.index == 0:
+                # skip default account
+                continue
+            for subaddress in account.subaddresses:
+                subaddresses_after.append(subaddress)
+
+        assert len(subaddresses) == len(subaddresses_after)
+        for i, subaddress_before in enumerate(subaddresses):
+            subaddress_after: MoneroSubaddress = subaddresses_after[i]
+            assert subaddress_after.unlocked_balance is not None
+
+            # determine if subaddress was swept
+            swept: bool = False
+            for j in range(NUM_SUBADDRESSES_TO_SWEEP):
+                subaddress_unlocked: MoneroSubaddress = subaddresses_unlocked[j]
+                account_eq: bool = subaddress_unlocked.account_index == subaddress_before.account_index
+                subaddr_eq: bool = subaddress_unlocked.index == subaddress_before.index
+                if account_eq and subaddr_eq:
+                    swept = True
+                    break
+
+            # assert unlocked balance is less than max fee if swept, unchanged otherwise
+            if swept:
+                assert subaddress_after.unlocked_balance < TxUtils.MAX_FEE
+            else:
+                assert subaddress_before.unlocked_balance == subaddress_after.unlocked_balance
+
+    # Can sweep accounts
+    @pytest.mark.skipif(TestUtils.TEST_RESETS is False, reason="TEST_RESETS disabled")
+    def test_sweep_accounts(self, wallet: MoneroWallet) -> None:
+        TestUtils.WALLET_TX_TRACKER.wait_for_txs_to_clear_pool(wallet)
+        NUM_ACCOUNTS_TO_SWEEP: int = 1
+
+        # collect accounts with sufficient balance and unlocked balance to cover the fee
+        accounts: list[MoneroAccount] = wallet.get_accounts(True)
+        accounts_balance: list[MoneroAccount] = []
+        accounts_unlocked: list[MoneroAccount] = []
+        for account in accounts:
+            if account.index == 0:
+                # skip default account
+                continue
+            assert account.balance is not None
+            assert account.unlocked_balance is not None
+            if account.balance > TxUtils.MAX_FEE:
+                accounts_balance.append(account)
+            if account.unlocked_balance > TxUtils.MAX_FEE:
+                accounts_unlocked.append(account)
+
+        # test requires at least one more accounts than the number being swept to verify it does not change
+
+        msg: str =  f"Test requires balance greater than the fee in at least {NUM_ACCOUNTS_TO_SWEEP + 1} non-default accounts; run send-to-multiple tests"
+        assert len(accounts_balance) >= NUM_ACCOUNTS_TO_SWEEP + 1, msg
+        assert len(accounts_unlocked) >= NUM_ACCOUNTS_TO_SWEEP + 1, "Wallet is waiting on unlocked funds"
+
+        # sweep from first unlocked accounts
+        for i in range(NUM_ACCOUNTS_TO_SWEEP):
+            # sweep unlocked account
+            unlocked_account: MoneroAccount = accounts_unlocked[i]
+            assert unlocked_account.index is not None
+            config: MoneroTxConfig = MoneroTxConfig()
+            config.address = wallet.get_primary_address()
+            config.account_index = unlocked_account.index
+            config.relay = True
+            txs: list[MoneroTxWallet] = wallet.sweep_unlocked(config)
+
+            # test transactions
+            assert len(txs) > 0
+            for tx in txs:
+                ctx: TxContext = TxContext()
+                ctx.wallet = wallet
+                ctx.config = config
+                ctx.is_send_response = True
+                ctx.is_sweep_response = True
+                TxUtils.test_tx_wallet(tx, ctx)
+                assert tx.tx_set is not None
+                assert tx in tx.tx_set.txs
+
+            # assert unlocked account balance less than max fee
+            account: MoneroAccount = wallet.get_account(unlocked_account.index)
+
+        # test accounts after sweeping
+        accounts_after: list[MoneroAccount] = wallet.get_accounts(True)
+        assert len(accounts) == len(accounts_after)
+        for i, account_before in enumerate(accounts):
+            account_after: MoneroAccount = accounts_after[i]
+            assert account_after.unlocked_balance is not None
+
+            # determine if account was swept
+            swept: bool = False
+            for j in range(NUM_ACCOUNTS_TO_SWEEP):
+                account_unlocked = accounts_unlocked[j]
+                if account_unlocked.index == account_before.index:
+                    swept = True
+                    break
+
+            # assert unlocked balance is less than max fee if swept, unchanged otherwise
+            if swept:
+                assert account_after.unlocked_balance < TxUtils.MAX_FEE
+            else:
+                assert account_after.unlocked_balance == account_after.unlocked_balance
+
+    # Can sweep the whole wallet by accounts
+    @pytest.mark.skipif(TestUtils.TEST_RESETS is False, reason="TEST_RESETS disabled")
+    def test_sweep_wallet_by_accounts(self, wallet: MoneroWallet) -> None:
+        IntegrationTestUtils.fund_wallet_and_wait_for_unlocked(wallet)
+        WalletUtils.test_sweep_wallet(wallet, None)
+
+    # Can sweep the whole wallet by subaddresses
+    #@pytest.mark.skipif(TestUtils.TEST_RESETS is False, reason="TEST_RESETS disabled")
+    @pytest.mark.xfail(reason="TODO wallet2 error: No unlocked balance in the specified subaddress(es)")
+    def test_sweep_wallet_by_subaddresses(self, wallet: MoneroWallet) -> None:
+        IntegrationTestUtils.fund_wallet_and_wait_for_unlocked(wallet)
+        WalletUtils.test_sweep_wallet(wallet, True)
+
     # Can scan transactions by id
     @pytest.mark.skipif(TestUtils.TEST_RESETS is False, reason="TEST_RESETS disabled")
     def test_scan_txs(self, wallet: MoneroWallet) -> None: