From 4942f848f867316128febe9667a5f2c60fd966af Mon Sep 17 00:00:00 2001 From: jeffro256 Date: Mon, 19 May 2025 10:40:43 -0500 Subject: [PATCH] carrot_impl: fix sweep_all to create multiple txs with n_outputs *each* --- src/carrot_impl/tx_proposal_utils.cpp | 13 ++- src/wallet/tx_builder.cpp | 121 +++++++++---------------- src/wallet/tx_builder.h | 8 +- tests/unit_tests/wallet_tx_builder.cpp | 43 ++++----- 4 files changed, 79 insertions(+), 106 deletions(-) diff --git a/src/carrot_impl/tx_proposal_utils.cpp b/src/carrot_impl/tx_proposal_utils.cpp index 923c42f60..f9b8979bd 100644 --- a/src/carrot_impl/tx_proposal_utils.cpp +++ b/src/carrot_impl/tx_proposal_utils.cpp @@ -30,6 +30,7 @@ #include "tx_proposal_utils.h" //local headers +#include "carrot_core/exceptions.h" #include "carrot_core/output_set_finalization.h" #include "cryptonote_basic/cryptonote_format_utils.h" #include "misc_log_ex.h" @@ -169,7 +170,7 @@ void make_carrot_transaction_proposal_v1(const std::vector fee_per_input_count; - for (size_t num_ins = 1; num_ins <= CARROT_MAX_TX_INPUTS; ++num_ins) + for (size_t num_ins = CARROT_MIN_TX_INPUTS; num_ins <= CARROT_MAX_TX_INPUTS; ++num_ins) { const uint64_t tx_weight = get_fcmppp_tx_weight(num_ins, num_outs, tx_extra_size); CHECK_AND_ASSERT_THROW_MES(tx_weight != std::numeric_limits::max(), @@ -197,8 +198,14 @@ void make_carrot_transaction_proposal_v1(const std::vector= CARROT_MIN_TX_INPUTS, + too_few_inputs, "input selection returned too few inputs: " << n_inputs); + CARROT_CHECK_AND_THROW(n_inputs <= CARROT_MAX_TX_OUTPUTS, + too_few_inputs, "input selection returned too many inputs: " << n_inputs); + CARROT_CHECK_AND_THROW(fee_per_input_count.count(n_inputs), + carrot_logic_error, "BUG: fee_per_input_count populated with holes, missing: " << n_inputs); + tx_proposal_out.fee = fee_per_input_count.at(n_inputs); // calculate input amount sum boost::multiprecision::uint128_t input_amount_sum = 0; diff --git a/src/wallet/tx_builder.cpp b/src/wallet/tx_builder.cpp index 56f63ea2d..3e96ef818 100644 --- a/src/wallet/tx_builder.cpp +++ b/src/wallet/tx_builder.cpp @@ -420,16 +420,16 @@ std::vector make_carrot_transaction_proposa const std::vector &input_key_images, const cryptonote::account_public_address &address, const bool is_subaddress, - const size_t n_dests, + const size_t n_dests_per_tx, const rct::xmr_amount fee_per_weight, const std::vector &extra, const std::uint64_t top_block_index) { const size_t n_inputs = input_key_images.size(); - CHECK_AND_ASSERT_THROW_MES(n_inputs, - __func__ << ": no key images provided"); - CHECK_AND_ASSERT_THROW_MES(n_dests, - __func__ << ": n_dests is zero"); + CARROT_CHECK_AND_THROW(n_inputs, carrot::too_few_inputs, "no key images provided"); + CARROT_CHECK_AND_THROW(n_dests_per_tx, carrot::too_few_outputs, "sweep must have at least one destination"); + CARROT_CHECK_AND_THROW(n_dests_per_tx <= FCMP_PLUS_PLUS_MAX_OUTPUTS, + carrot::too_many_outputs, "too many sweep destinations per transaction"); // Check that the key image is usable and isn't spent, collect amounts, and get subaddress account index std::vector input_amounts; @@ -457,86 +457,51 @@ std::vector make_carrot_transaction_proposa = find_change_address_spend_pubkey(subaddress_map, subaddr_account); // get 1 payment proposal corresponding to (address, is_subaddres) - std::vector normal_payment_proposal; - std::vector selfsend_payment_proposal; - const bool is_selfsend_dest = build_payment_proposals(normal_payment_proposal, - selfsend_payment_proposal, - cryptonote::tx_destination_entry(/*amount=*/0, address, is_subaddress), - subaddress_map); - CHECK_AND_ASSERT_THROW_MES((is_selfsend_dest && selfsend_payment_proposal.size() == 1) - || (!is_selfsend_dest && normal_payment_proposal.size() == 1), - __func__ << ": BUG in build_payment_proposals: incorrect count for payment proposal lists"); - - // in/out/tx count calculations - const size_t max_dsts_per_tx = FCMP_PLUS_PLUS_MAX_OUTPUTS - (size_t(!is_selfsend_dest)); - const size_t min_n_dests = div_ceil(n_inputs, FCMP_PLUS_PLUS_MAX_INPUTS); - const size_t max_n_dests = n_inputs * max_dsts_per_tx; - CHECK_AND_ASSERT_THROW_MES(n_dests >= min_n_dests, - __func__ << ": not enough destinations (" << n_dests << ") for number of inputs to be spent (" - << n_inputs << ")"); - CHECK_AND_ASSERT_THROW_MES(n_dests <= max_n_dests, - __func__ << ": too many destinations (" << n_dests << ") for number of inputs to be spent (" - << n_inputs << ")"); - - const size_t n_txs = std::max(div_ceil(n_dests, max_dsts_per_tx), min_n_dests); - CHECK_AND_ASSERT_THROW_MES(n_txs, __func__ << ": BUG: calculated target num of txs to be 0"); - - struct sweep_tx_outlay_t + std::vector normal_payment_proposals; + std::vector selfsend_payment_proposals; + for (size_t i = 0; i < n_dests_per_tx; ++i) { - std::vector selected_inputs; - size_t n_tx_dests; - }; - - // build list of sweep_tx_outlay_t's - std::vector tx_outlays(n_txs); - size_t input_idx = 0; - for (size_t tx_idx = 0; tx_idx < tx_outlays.size(); ++tx_idx) - { - sweep_tx_outlay_t &tx_outlay = tx_outlays[tx_idx]; - - const size_t n_remaining_inputs = n_inputs - input_idx; - const size_t n_tx_inputs = std::min(div_ceil(n_inputs, n_txs), n_remaining_inputs); - const size_t n_tx_dests = n_dests / n_txs + ((tx_idx < (n_dests % n_txs)) ? 1 : 0); - - const size_t max_input_idx = input_idx + n_tx_inputs; - tx_outlay.selected_inputs.reserve(n_tx_inputs); - for (; input_idx < max_input_idx; ++input_idx) - tx_outlay.selected_inputs.push_back({input_amounts.at(input_idx), input_key_images.at(input_idx)}); - - tx_outlay.n_tx_dests = n_tx_dests; + const bool is_selfsend_dest = build_payment_proposals(normal_payment_proposals, + selfsend_payment_proposals, + cryptonote::tx_destination_entry(/*amount=*/0, address, is_subaddress), + subaddress_map); + CHECK_AND_ASSERT_THROW_MES((is_selfsend_dest && selfsend_payment_proposals.size() == i+1) + || (!is_selfsend_dest && normal_payment_proposals.size() == i+1), + __func__ << ": BUG in build_payment_proposals: incorrect count for payment proposal lists"); } + CARROT_CHECK_AND_THROW(normal_payment_proposals.size() < FCMP_PLUS_PLUS_MAX_OUTPUTS, + carrot::too_many_outputs, "too many *outgoing* sweep destinations per tx, we also need 1 self-send output"); - //! @TODO: sanity check tx_outlays - - // convert sweep outlays into transaction proposals - std::vector tx_proposals; - tx_proposals.reserve(tx_outlays.size()); - for (sweep_tx_outlay_t &sweep_outlay : tx_outlays) + // make `n_txs` tx proposals with `n_output` payment proposals each + const size_t n_txs = div_ceil(n_inputs, FCMP_PLUS_PLUS_MAX_INPUTS); + std::vector tx_proposals(n_txs); + size_t ki_idx = 0; + for (carrot::CarrotTransactionProposalV1 &tx_proposal : tx_proposals) { - std::vector tx_normal_payment_proposals; - std::vector tx_selfsend_payment_proposals; - if (is_selfsend_dest) - tx_selfsend_payment_proposals.resize(sweep_outlay.n_tx_dests, selfsend_payment_proposal.at(0)); - else - tx_normal_payment_proposals.resize(sweep_outlay.n_tx_dests, normal_payment_proposal.at(0)); - // if a 2-selfsend, 2-out tx, flip one of the enote types to get unique derivations - if (tx_selfsend_payment_proposals.size() == 2) - tx_selfsend_payment_proposals.back().proposal.enote_type = carrot::CarrotEnoteType::CHANGE; + if (selfsend_payment_proposals.size() == 2) + selfsend_payment_proposals.back().proposal.enote_type = carrot::CarrotEnoteType::CHANGE; - carrot::CarrotTransactionProposalV1 tx_proposal; - carrot::make_carrot_transaction_proposal_v1_sweep(tx_normal_payment_proposals, - tx_selfsend_payment_proposals, + // collect inputs for this tx + const size_t ki_idx_end = std::min(n_inputs, ki_idx + FCMP_PLUS_PLUS_MAX_INPUTS); + std::vector selected_inputs; + selected_inputs.reserve(n_inputs - ki_idx_end); + for (; ki_idx < ki_idx_end; ++ki_idx) + selected_inputs.push_back({input_amounts.at(ki_idx), input_key_images.at(ki_idx)}); + + carrot::make_carrot_transaction_proposal_v1_sweep(normal_payment_proposals, + selfsend_payment_proposals, fee_per_weight, extra, - std::move(sweep_outlay.selected_inputs), + std::move(selected_inputs), change_address_spend_pubkey, {{subaddr_account, 0}, carrot::AddressDeriveType::PreCarrot}, //! @TODO: handle Carrot keys tx_proposal); - - tx_proposals.push_back(std::move(tx_proposal)); } + CARROT_CHECK_AND_THROW(ki_idx == input_key_images.size(), + carrot::carrot_logic_error, "BUG: sweep_all did not consume the correct num of key images while iterating"); + return tx_proposals; } //------------------------------------------------------------------------------------------------------------------- @@ -545,7 +510,7 @@ std::vector make_carrot_transaction_proposa const std::vector &input_key_images, const cryptonote::account_public_address &address, const bool is_subaddress, - const size_t n_dests, + const size_t n_dests_per_tx, const std::uint32_t priority, const std::vector &extra) { @@ -565,7 +530,7 @@ std::vector make_carrot_transaction_proposa input_key_images, address, is_subaddress, - n_dests, + n_dests_per_tx, fee_per_weight, extra, top_block_index); @@ -577,7 +542,7 @@ std::vector make_carrot_transaction_proposa const rct::xmr_amount only_below, const cryptonote::account_public_address &address, const bool is_subaddress, - const size_t n_dests, + const size_t n_dests_per_tx, const rct::xmr_amount fee_per_weight, const std::vector &extra, const std::uint32_t subaddr_account, @@ -618,7 +583,7 @@ std::vector make_carrot_transaction_proposa input_key_images, address, is_subaddress, - n_dests, + n_dests_per_tx, fee_per_weight, extra, top_block_index); @@ -629,7 +594,7 @@ std::vector make_carrot_transaction_proposa const rct::xmr_amount only_below, const cryptonote::account_public_address &address, const bool is_subaddress, - const size_t n_dests, + const size_t n_dests_per_tx, const std::uint32_t priority, const std::vector &extra, const std::uint32_t subaddr_account, @@ -651,7 +616,7 @@ std::vector make_carrot_transaction_proposa only_below, address, is_subaddress, - n_dests, + n_dests_per_tx, fee_per_weight, extra, subaddr_account, diff --git a/src/wallet/tx_builder.h b/src/wallet/tx_builder.h index c897376fc..0d4239078 100644 --- a/src/wallet/tx_builder.h +++ b/src/wallet/tx_builder.h @@ -83,7 +83,7 @@ std::vector make_carrot_transaction_proposa const std::vector &input_key_images, const cryptonote::account_public_address &address, const bool is_subaddress, - const size_t n_dests, + const size_t n_dests_per_tx, const rct::xmr_amount fee_per_weight, const std::vector &extra, const std::uint64_t top_block_index); @@ -92,7 +92,7 @@ std::vector make_carrot_transaction_proposa const std::vector &input_key_images, const cryptonote::account_public_address &address, const bool is_subaddress, - const size_t n_dests, + const size_t n_dests_per_tx, const std::uint32_t priority, const std::vector &extra); @@ -102,7 +102,7 @@ std::vector make_carrot_transaction_proposa const rct::xmr_amount only_below, const cryptonote::account_public_address &address, const bool is_subaddress, - const size_t n_dests, + const size_t n_dests_per_tx, const rct::xmr_amount fee_per_weight, const std::vector &extra, const std::uint32_t subaddr_account, @@ -113,7 +113,7 @@ std::vector make_carrot_transaction_proposa const rct::xmr_amount only_below, const cryptonote::account_public_address &address, const bool is_subaddress, - const size_t n_dests, + const size_t n_dests_per_tx, const std::uint32_t priority, const std::vector &extra, const std::uint32_t subaddr_account, diff --git a/tests/unit_tests/wallet_tx_builder.cpp b/tests/unit_tests/wallet_tx_builder.cpp index 33e1e741a..991f47b17 100644 --- a/tests/unit_tests/wallet_tx_builder.cpp +++ b/tests/unit_tests/wallet_tx_builder.cpp @@ -296,7 +296,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_1) {transfers.front().m_key_image}, bob.get_keys().m_account_address, /*is_subaddress=*/false, - /*n_dests=*/1, + /*n_dests_per_tx=*/1, /*fee_per_weight=*/1, /*extra=*/{}, transfers.front().m_block_height + CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE); @@ -330,7 +330,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_2) {transfers.front().m_key_image}, bob.get_keys().m_account_address, /*is_subaddress=*/false, - /*n_dests=*/FCMP_PLUS_PLUS_MAX_OUTPUTS - 1, + /*n_dests_per_tx=*/FCMP_PLUS_PLUS_MAX_OUTPUTS - 1, /*fee_per_weight=*/1, /*extra=*/{}, transfers.front().m_block_height + CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE); @@ -372,7 +372,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_3) {transfers.front().m_key_image}, alice.get_keys().m_account_address, /*is_subaddress=*/false, - /*n_dests=*/FCMP_PLUS_PLUS_MAX_OUTPUTS, + /*n_dests_per_tx=*/FCMP_PLUS_PLUS_MAX_OUTPUTS, /*fee_per_weight=*/1, /*extra=*/{}, transfers.front().m_block_height + CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE); @@ -438,7 +438,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_4) ASSERT_EQ(n_selected_transfers, selected_key_images.size()); ASSERT_EQ(n_selected_transfers, amounts_by_ki.size()); - const size_t n_dests = 4; + const size_t n_dests_per_tx = 4; // make tx proposals const std::vector tx_proposals = tools::wallet::make_carrot_transaction_proposals_wallet2_sweep( @@ -447,7 +447,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_4) selected_key_images, bob.get_keys().m_account_address, /*is_subaddress=*/false, - /*n_dests=*/n_dests, + /*n_dests_per_tx=*/n_dests_per_tx, /*fee_per_weight=*/1, /*extra=*/{}, top_block_index); @@ -455,11 +455,10 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_4) std::set actual_seen_kis; size_t n_actual_inputs = 0; - size_t n_actual_dests = 0; for (const carrot::CarrotTransactionProposalV1 &tx_proposal : tx_proposals) { ASSERT_LE(tx_proposal.key_images_sorted.size(), FCMP_PLUS_PLUS_MAX_INPUTS); - ASSERT_EQ(1, tx_proposal.normal_payment_proposals.size()); + ASSERT_EQ(n_dests_per_tx, tx_proposal.normal_payment_proposals.size()); ASSERT_EQ(1, tx_proposal.selfsend_payment_proposals.size()); ASSERT_EQ(0, tx_proposal.selfsend_payment_proposals.at(0).proposal.amount); EXPECT_EQ(0, tx_proposal.extra.size()); @@ -472,15 +471,15 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_4) actual_seen_kis.insert(ki); tx_inputs_amount += amounts_by_ki.at(ki); } - const rct::xmr_amount tx_outputs_amount = tx_proposal.fee + tx_proposal.normal_payment_proposals.at(0).amount; + rct::xmr_amount tx_outputs_amount = tx_proposal.fee; + for (const carrot::CarrotPaymentProposalV1 &normal_payment_proposal : tx_proposal.normal_payment_proposals) + tx_outputs_amount += normal_payment_proposal.amount; ASSERT_EQ(tx_inputs_amount, tx_outputs_amount); n_actual_inputs += tx_proposal.key_images_sorted.size(); - n_actual_dests += tx_proposal.normal_payment_proposals.size(); } EXPECT_EQ(n_selected_transfers, n_actual_inputs); - EXPECT_EQ(n_dests, n_actual_dests); } //---------------------------------------------------------------------------------------------------------------------- TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_5) @@ -520,7 +519,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_5) ASSERT_EQ(n_selected_transfers, selected_key_images.size()); ASSERT_EQ(n_selected_transfers, amounts_by_ki.size()); - const size_t n_dests = 8; + const size_t n_dests_per_tx = 8; // make tx proposals const std::vector tx_proposals = tools::wallet::make_carrot_transaction_proposals_wallet2_sweep( @@ -529,7 +528,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_5) selected_key_images, alice.get_keys().m_account_address, /*is_subaddress=*/false, - /*n_dests=*/n_dests, + /*n_dests_per_tx=*/n_dests_per_tx, /*fee_per_weight=*/1, /*extra=*/{}, top_block_index); @@ -537,13 +536,15 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_5) std::set actual_seen_kis; size_t n_actual_inputs = 0; - size_t n_actual_dests = 0; for (const carrot::CarrotTransactionProposalV1 &tx_proposal : tx_proposals) { ASSERT_LE(tx_proposal.key_images_sorted.size(), FCMP_PLUS_PLUS_MAX_INPUTS); - ASSERT_EQ(1, tx_proposal.normal_payment_proposals.size()); - ASSERT_EQ(1, tx_proposal.selfsend_payment_proposals.size()); - ASSERT_EQ(0, tx_proposal.normal_payment_proposals.at(0).amount); + ASSERT_EQ(n_dests_per_tx == 1 ? 1 : 0, tx_proposal.normal_payment_proposals.size()); + ASSERT_EQ(n_dests_per_tx, tx_proposal.selfsend_payment_proposals.size()); + if (!tx_proposal.normal_payment_proposals.empty()) + { + ASSERT_EQ(0, tx_proposal.normal_payment_proposals.at(0).amount); + } EXPECT_EQ(0, tx_proposal.extra.size()); rct::xmr_amount tx_inputs_amount = 0; @@ -554,15 +555,15 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_5) actual_seen_kis.insert(ki); tx_inputs_amount += amounts_by_ki.at(ki); } - const rct::xmr_amount tx_outputs_amount = tx_proposal.fee + tx_proposal.selfsend_payment_proposals.at(0).proposal.amount; + rct::xmr_amount tx_outputs_amount = tx_proposal.fee; + for (const carrot::CarrotPaymentProposalVerifiableSelfSendV1 &selfsend_payment_proposal : tx_proposal.selfsend_payment_proposals) + tx_outputs_amount += selfsend_payment_proposal.proposal.amount; ASSERT_EQ(tx_inputs_amount, tx_outputs_amount); n_actual_inputs += tx_proposal.key_images_sorted.size(); - n_actual_dests += tx_proposal.selfsend_payment_proposals.size(); } EXPECT_EQ(n_selected_transfers, n_actual_inputs); - EXPECT_EQ(n_dests, n_actual_dests); } //---------------------------------------------------------------------------------------------------------------------- TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_6) @@ -602,7 +603,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_6) ASSERT_EQ(n_selected_transfers, selected_key_images.size()); ASSERT_EQ(n_selected_transfers, amounts_by_ki.size()); - const size_t n_dests = 2; + const size_t n_dests_per_tx = 2; // make tx proposals const std::vector tx_proposals = tools::wallet::make_carrot_transaction_proposals_wallet2_sweep( @@ -611,7 +612,7 @@ TEST(wallet_tx_builder, make_carrot_transaction_proposals_wallet2_sweep_6) selected_key_images, alice.get_keys().m_account_address, /*is_subaddress=*/false, - /*n_dests=*/n_dests, + /*n_dests_per_tx=*/n_dests_per_tx, /*fee_per_weight=*/1, /*extra=*/{}, top_block_index);