From b13c5f6669f9612bef7cba18be7189bf6dfa6e61 Mon Sep 17 00:00:00 2001 From: jeffro256 Date: Sun, 14 May 2023 11:41:59 -0500 Subject: [PATCH] wallet: feature: transfer amount with fee included To transfer ~5 XMR to an address such that your balance drops by exactly 5 XMR, provide a `subtractfeefrom` flag to the `transfer` command. For example: transfer 76bDHojqFYiFCCYYtzTveJ8oFtmpNp3X1TgV2oKP7rHmZyFK1RvyE4r8vsJzf7SyNohMnbKT9wbcD3XUTgsZLX8LU5JBCfm 5 subtractfeefrom=all If my walet balance was exactly 30 XMR before this transaction, it will be exactly 25 XMR afterwards and the destination address will receive slightly less than 5 XMR. You can manually select which destinations fund the transaction fee and which ones do not by providing the destination index. For example: transfer 75sr8AAr... 3 74M7W4eg... 4 7AbWqDZ6... 5 subtractfeefrom=0,2 This will drop your balance by exactly 12 XMR including fees and will spread the fee cost proportionally (3:5 ratio) over destinations with addresses `75sr8AAr...` and `7AbWqDZ6...`, respectively. Disclaimer: This feature was paid for by @LocalMonero. --- src/simplewallet/simplewallet.cpp | 83 +++++++++- src/wallet/wallet2.cpp | 164 ++++++++++++++++--- src/wallet/wallet2.h | 5 +- src/wallet/wallet_errors.h | 10 ++ src/wallet/wallet_rpc_server.cpp | 22 ++- src/wallet/wallet_rpc_server.h | 4 +- src/wallet/wallet_rpc_server_commands_defs.h | 19 ++- tests/functional_tests/transfer.py | 81 +++++++++ tests/unit_tests/epee_utils.cpp | 57 +++++++ utils/python-rpc/framework/wallet.py | 3 +- 10 files changed, 405 insertions(+), 43 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 6e5d0b1ec..bff62f1a8 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -188,7 +188,7 @@ namespace const char* USAGE_INCOMING_TRANSFERS("incoming_transfers [available|unavailable] [verbose] [uses] [index=[,[,...]]]"); const char* USAGE_PAYMENTS("payments [ ... ]"); const char* USAGE_PAYMENT_ID("payment_id"); - const char* USAGE_TRANSFER("transfer [index=[,,...]] [] [] ( |
) []"); + const char* USAGE_TRANSFER("transfer [index=[,,...]] [] [] ( |
) [subtractfeefrom=[,,all,...]] []"); const char* USAGE_LOCKED_TRANSFER("locked_transfer [index=[,,...]] [] [] ( | ) []"); const char* USAGE_LOCKED_SWEEP_ALL("locked_sweep_all [index=[,,...] | index=all] [] []
[]"); const char* USAGE_SWEEP_ALL("sweep_all [index=[,,...] | index=all] [] [] [outputs=]
[]"); @@ -520,7 +520,52 @@ namespace fail_msg_writer() << sw::tr("invalid format for subaddress lookahead; must be :"); return r; } -} + + static constexpr std::string_view SFFD_ARG_NAME{"subtractfeefrom="}; + + bool parse_subtract_fee_from_outputs + ( + const std::string& arg, + tools::wallet2::unique_index_container& subtract_fee_from_outputs, + bool& subtract_fee_from_all, + bool& matches + ) + { + matches = false; + if (!boost::string_ref{arg}.starts_with(SFFD_ARG_NAME.data())) // if arg doesn't match + return true; + matches = true; + + const char* arg_end = arg.c_str() + arg.size(); + for (const char* p = arg.c_str() + SFFD_ARG_NAME.size(); p < arg_end;) + { + const char* new_p = nullptr; + const unsigned long dest_index = strtoul(p, const_cast(&new_p), 10); + if (dest_index == 0 && new_p == p) // numerical conversion failed + { + if (0 != strncmp(p, "all", 3)) + { + fail_msg_writer() << tr("Failed to parse subtractfeefrom list"); + return false; + } + subtract_fee_from_all = true; + break; + } + else if (dest_index > std::numeric_limits::max()) + { + fail_msg_writer() << tr("Destination index is too large") << ": " << dest_index; + return false; + } + else + { + subtract_fee_from_outputs.insert(dest_index); + p = new_p + 1; // skip the comma + } + } + + return true; + } +} // anonymous namespace void simple_wallet::handle_transfer_exception(const std::exception_ptr &e, bool trusted_daemon) { @@ -3080,7 +3125,7 @@ simple_wallet::simple_wallet() tr("Show the blockchain height.")); m_cmd_binder.set_handler("transfer", boost::bind(&simple_wallet::on_command, this, &simple_wallet::transfer, _1), tr(USAGE_TRANSFER), - tr("Transfer to
. If the parameter \"index=[,,...]\" is specified, the wallet uses outputs received by addresses of those indices. If omitted, the wallet randomly chooses address indices to be used. In any case, it tries its best not to combine outputs across multiple addresses. is the priority of the transaction. The higher the priority, the higher the transaction fee. Valid values in priority order (from lowest to highest) are: unimportant, normal, elevated, priority. If omitted, the default value (see the command \"set priority\") is used. is the number of inputs to include for untraceability. Multiple payments can be made at once by adding URI_2 or etcetera (before the payment ID, if it's included)")); + tr("Transfer to
. If the parameter \"index=[,,...]\" is specified, the wallet uses outputs received by addresses of those indices. If omitted, the wallet randomly chooses address indices to be used. In any case, it tries its best not to combine outputs across multiple addresses. is the priority of the transaction. The higher the priority, the higher the transaction fee. Valid values in priority order (from lowest to highest) are: unimportant, normal, elevated, priority. If omitted, the default value (see the command \"set priority\") is used. is the number of inputs to include for untraceability. Multiple payments can be made at once by adding URI_2 or etcetera (before the payment ID, if it's included). The \"subtractfeefrom=\" list allows you to choose which destinations to fund the tx fee from instead of the change output. The fee will be split across the chosen destinations proportionally equally. For example, to make 3 transfers where the fee is taken from the first and third destinations, one could do: \"transfer 3 0.5 1 subtractfeefrom=0,2\". Let's say the tx fee is 0.1. The balance would drop by exactly 4.5 XMR including fees, and addr1 & addr3 would receive 2.925 & 0.975 XMR, respectively. Use \"subtractfeefrom=all\" to spread the fee across all destinations.")); m_cmd_binder.set_handler("locked_transfer", boost::bind(&simple_wallet::on_command, this, &simple_wallet::locked_transfer,_1), tr(USAGE_LOCKED_TRANSFER), @@ -6349,6 +6394,27 @@ bool simple_wallet::transfer_main(int transfer_type, const std::vector dsts_info; vector dsts; for (size_t i = 0; i < local_args.size(); ) @@ -6445,6 +6511,13 @@ bool simple_wallet::transfer_main(int transfer_type, const std::vectorcreate_transactions_2(dsts, fake_outs_count, unlock_block /* unlock_time */, priority, extra, m_current_subaddress_account, subaddr_indices); + ptx_vector = m_wallet->create_transactions_2(dsts, fake_outs_count, unlock_block /* unlock_time */, priority, extra, m_current_subaddress_account, subaddr_indices, subtract_fee_from_outputs); break; default: LOG_ERROR("Unknown transfer method, using default"); /* FALLTHRU */ case Transfer: - ptx_vector = m_wallet->create_transactions_2(dsts, fake_outs_count, 0 /* unlock_time */, priority, extra, m_current_subaddress_account, subaddr_indices); + ptx_vector = m_wallet->create_transactions_2(dsts, fake_outs_count, 0 /* unlock_time */, priority, extra, m_current_subaddress_account, subaddr_indices, subtract_fee_from_outputs); break; } diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 336f4e159..f45acb175 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -9851,7 +9851,7 @@ static uint32_t get_count_above(const std::vector &tr // This system allows for sending (almost) the entire balance, since it does // not generate spurious change in all txes, thus decreasing the instantaneous // usable balance. -std::vector wallet2::create_transactions_2(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices) +std::vector wallet2::create_transactions_2(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices, const unique_index_container& subtract_fee_from_outputs) { //ensure device is let in NONE mode in any case hw::device &hwdev = m_account.get_device(); @@ -9862,11 +9862,12 @@ std::vector wallet2::create_transactions_2(std::vector>> unused_transfers_indices_per_subaddr; std::vector>> unused_dust_indices_per_subaddr; - uint64_t needed_money; + uint64_t needed_money, total_needed_money; // 'needed_money' is the sum of the destination amounts, while 'total_needed_money' includes 'needed_money' plus the fee if not 'subtract_fee_from_outputs' uint64_t accumulated_fee, accumulated_change; struct TX { std::vector selected_transfers; std::vector dsts; + std::vector dsts_are_fee_subtractable; cryptonote::transaction tx; pending_tx ptx; size_t weight; @@ -9876,9 +9877,11 @@ std::vector wallet2::create_transactions_2(std::vector::iterator i; @@ -9888,6 +9891,7 @@ std::vector wallet2::create_transactions_2(std::vector= max_dsts) return false; dsts.push_back(de); + dsts_are_fee_subtractable.push_back(subtracting_fee); i = dsts.end() - 1; i->amount = 0; } @@ -9903,13 +9907,67 @@ std::vector wallet2::create_transactions_2(std::vector get_adjusted_dsts(uint64_t needed_fee) const + { + uint64_t dest_total = 0; + uint64_t subtractable_dest_total = 0; + std::vector subtractable_indices; + subtractable_indices.reserve(dsts.size()); + for (size_t i = 0; i < dsts.size(); ++i) + { + dest_total += dsts[i].amount; + if (dsts_are_fee_subtractable[i]) + { + subtractable_dest_total += dsts[i].amount; + subtractable_indices.push_back(i); + } + } + + if (subtractable_indices.empty()) // if subtract_fee_from_outputs is not enabled for this tx + return dsts; + + THROW_WALLET_EXCEPTION_IF(subtractable_dest_total < needed_fee, error::tx_not_possible, + subtractable_dest_total, dest_total, needed_fee); + + std::vector res = dsts; + + // subtract fees from destinations equally, rounded down, until dust is left where we subtract 1 + uint64_t subtractable_remaining = needed_fee; + auto si_it = subtractable_indices.cbegin(); + uint64_t amount_to_subtract = 0; + while (subtractable_remaining) + { + // Set the amount to subtract iterating at the beginning of the list so equal amounts are + // subtracted throughout the list of destinations. We use max(x, 1) so that we we still step + // forwards even when the amount remaining is less than the number of subtractable indices + if (si_it == subtractable_indices.cbegin()) + amount_to_subtract = std::max(subtractable_remaining / subtractable_indices.size(), 1); + + cryptonote::tx_destination_entry& d = res[*si_it]; + THROW_WALLET_EXCEPTION_IF(d.amount <= amount_to_subtract, error::zero_amount); + + subtractable_remaining -= amount_to_subtract; + d.amount -= amount_to_subtract; + ++si_it; + + // Wrap around to first subtractable index once we hit the end of the list + if (si_it == subtractable_indices.cend()) + si_it = subtractable_indices.cbegin(); + } + + return res; + } }; + std::vector txes; bool adding_fee; // true if new outputs go towards fee, rather than destinations uint64_t needed_fee, available_for_fee = 0; @@ -9932,6 +9990,14 @@ std::vector wallet2::create_transactions_2(std::vector= dsts.size(), + error::subtract_fee_from_bad_index, *subtract_fee_from_outputs.crbegin()); + + // throw if subtract_fee_from_outputs is enabled and we have too many outputs to fit into one tx + THROW_WALLET_EXCEPTION_IF(subtract_fee_from_outputs.size() && dsts.size() > BULLETPROOF_MAX_OUTPUTS - 1, + error::wallet_internal_error, "subtractfeefrom transfers cannot be split over multiple transactions yet"); + // calculate total amount being sent to all destinations // throw if total amount overflows uint64_t needed_money = 0; @@ -9959,6 +10025,7 @@ std::vector wallet2::create_transactions_2(std::vector wallet2::create_transactions_2(std::vector balance_subtotal, error::not_enough_money, + THROW_WALLET_EXCEPTION_IF(total_needed_money > balance_subtotal || min_fee > balance_subtotal, error::not_enough_money, balance_subtotal, needed_money, 0); // first check overall balance is enough, then unlocked one, so we throw distinct exceptions - THROW_WALLET_EXCEPTION_IF(needed_money + min_fee > unlocked_balance_subtotal, error::not_enough_unlocked_money, + THROW_WALLET_EXCEPTION_IF(total_needed_money > unlocked_balance_subtotal || min_fee > unlocked_balance_subtotal, error::not_enough_unlocked_money, unlocked_balance_subtotal, needed_money, 0); for (uint32_t i : subaddr_indices) @@ -10072,7 +10139,8 @@ std::vector wallet2::create_transactions_2(std::vector wallet2::create_transactions_2(std::vector* unused_transfers_indices = &unused_transfers_indices_per_subaddr[0].second; std::vector* unused_dust_indices = &unused_dust_indices_per_subaddr[0].second; @@ -10188,7 +10256,8 @@ std::vector wallet2::create_transactions_2(std::vector wallet2::create_transactions_2(std::vector 0 && !dsts.empty() && @@ -10205,7 +10275,8 @@ std::vector wallet2::create_transactions_2(std::vector wallet2::create_transactions_2(std::vector wallet2::create_transactions_2(std::vector wallet2::create_transactions_2(std::vector test_ptx.fee; ++fee_tries) { + tx_dsts = tx.get_adjusted_dsts(needed_fee); + if (use_rct) - transfer_selected_rct(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra, + transfer_selected_rct(tx_dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra, test_tx, test_ptx, rct_config, use_view_tags); else - transfer_selected(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra, + transfer_selected(tx_dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra, detail::digit_split_strategy, tx_dust_policy(::config::DEFAULT_DUST_THRESHOLD), test_tx, test_ptx, use_view_tags); txBlob = t_serializable_object_to_blob(test_ptx.tx); needed_fee = calculate_fee(use_per_byte_fee, test_ptx.tx, txBlob.size(), base_fee, fee_quantization_mask); LOG_PRINT_L2("Made an attempt at a final " << get_weight_string(test_ptx.tx, txBlob.size()) << " tx, with " << print_money(test_ptx.fee) << " fee and " << print_money(test_ptx.change_dts.amount) << " change"); - } while (needed_fee > test_ptx.fee); + }; + + THROW_WALLET_EXCEPTION_IF(fee_tries == 10, error::wallet_internal_error, + "Too many attempts to raise pending tx fee to level of needed fee"); LOG_PRINT_L2("Made a final " << get_weight_string(test_ptx.tx, txBlob.size()) << " tx, with " << print_money(test_ptx.fee) << " fee and " << print_money(test_ptx.change_dts.amount) << " change"); @@ -10385,10 +10483,13 @@ skip_tx: for (std::vector::iterator i = txes.begin(); i != txes.end(); ++i) { TX &tx = *i; + + const auto tx_dsts = tx.get_adjusted_dsts(tx.needed_fee); + cryptonote::transaction test_tx; pending_tx test_ptx; if (use_rct) { - transfer_selected_rct(tx.dsts, /* NOMOD std::vector dsts,*/ + transfer_selected_rct(tx_dsts, /* NOMOD std::vector dsts,*/ tx.selected_transfers, /* const std::list selected_transfers */ fake_outs_count, /* CONST size_t fake_outputs_count, */ tx.outs, /* MOD std::vector> &outs, */ @@ -10401,7 +10502,7 @@ skip_tx: rct_config, use_view_tags); /* const bool use_view_tags */ } else { - transfer_selected(tx.dsts, + transfer_selected(tx_dsts, tx.selected_transfers, fake_outs_count, tx.outs, @@ -10435,23 +10536,38 @@ skip_tx: ptx_vector.push_back(tx.ptx); } - THROW_WALLET_EXCEPTION_IF(!sanity_check(ptx_vector, original_dsts), error::wallet_internal_error, "Created transaction(s) failed sanity check"); + THROW_WALLET_EXCEPTION_IF(!sanity_check(ptx_vector, original_dsts, subtract_fee_from_outputs), error::wallet_internal_error, "Created transaction(s) failed sanity check"); // if we made it this far, we're OK to actually send the transactions return ptx_vector; } -bool wallet2::sanity_check(const std::vector &ptx_vector, std::vector dsts) const +bool wallet2::sanity_check(const std::vector &ptx_vector, const std::vector& dsts, const unique_index_container& subtract_fee_from_outputs) const { - MDEBUG("sanity_check: " << ptx_vector.size() << " txes, " << dsts.size() << " destinations"); + MDEBUG("sanity_check: " << ptx_vector.size() << " txes, " << dsts.size() << " destinations, subtract_fee_from_outputs " << + (subtract_fee_from_outputs.size() ? "enabled" : "disabled")); THROW_WALLET_EXCEPTION_IF(ptx_vector.empty(), error::wallet_internal_error, "No transactions"); + THROW_WALLET_EXCEPTION_IF(!subtract_fee_from_outputs.empty() && ptx_vector.size() != 1, + error::wallet_internal_error, "feature subtractfeefrom not supported for split transactions"); + + // For destinations from where the fee is subtracted, the required amount has to be at least + // target amount - (tx fee / num_subtractable + 1). +1 since fee might not be evenly divisble by + // the number of subtractble destinations. For non-subtractable destinations, we need at least + // the target amount. + const size_t num_subtractable_dests = subtract_fee_from_outputs.size(); + const uint64_t fee0 = ptx_vector[0].fee; + const uint64_t subtractable_fee_deduction = fee0 / std::max(num_subtractable_dests, 1) + 1; // check every party in there does receive at least the required amount std::unordered_map> required; - for (const auto &d: dsts) + for (size_t i = 0; i < dsts.size(); ++i) { - required[d.addr].first += d.amount; + const cryptonote::tx_destination_entry& d = dsts[i]; + const bool dest_is_subtractable = subtract_fee_from_outputs.count(i); + const uint64_t fee_deduction = dest_is_subtractable ? subtractable_fee_deduction : 0; + const uint64_t required_amount = d.amount - std::min(fee_deduction, d.amount); + required[d.addr].first += required_amount; required[d.addr].second = d.is_subaddress; } diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 21ca48464..c5d2dc90c 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -591,6 +591,7 @@ private: typedef std::vector transfer_container; typedef std::unordered_multimap payment_container; + typedef std::set unique_index_container; struct multisig_sig { @@ -1096,11 +1097,11 @@ private: bool parse_unsigned_tx_from_str(const std::string &unsigned_tx_st, unsigned_tx_set &exported_txs) const; bool load_tx(const std::string &signed_filename, std::vector &ptx, std::function accept_func = NULL); bool parse_tx_from_str(const std::string &signed_tx_st, std::vector &ptx, std::function accept_func); - std::vector create_transactions_2(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices); // pass subaddr_indices by value on purpose + std::vector create_transactions_2(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices, const unique_index_container& subtract_fee_from_outputs = {}); // pass subaddr_indices by value on purpose std::vector create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices); std::vector create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra); std::vector create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector unused_transfers_indices, std::vector unused_dust_indices, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra); - bool sanity_check(const std::vector &ptx_vector, std::vector dsts) const; + bool sanity_check(const std::vector &ptx_vector, const std::vector& dsts, const unique_index_container& subtract_fee_from_outputs = {}) const; void cold_tx_aux_import(const std::vector& ptx, const std::vector& tx_device_aux); void cold_sign_tx(const std::vector& ptx_vector, signed_tx_set &exported_txs, std::vector &dsts_info, std::vector & tx_device_aux); uint64_t cold_key_image_sync(uint64_t &spent, uint64_t &unspent); diff --git a/src/wallet/wallet_errors.h b/src/wallet/wallet_errors.h index 6706e77ff..5166f868f 100644 --- a/src/wallet/wallet_errors.h +++ b/src/wallet/wallet_errors.h @@ -85,6 +85,7 @@ namespace tools // tx_too_big // zero_amount // zero_destination + // subtract_fee_from_bad_index // wallet_rpc_error * // daemon_busy // no_connection_to_daemon @@ -779,6 +780,15 @@ namespace tools } }; //---------------------------------------------------------------------------------------------------- + struct subtract_fee_from_bad_index : public transfer_error + { + explicit subtract_fee_from_bad_index(std::string&& loc, long bad_index) + : transfer_error(std::move(loc), + "subtractfeefrom: bad index: " + std::to_string(bad_index) + " (indexes are 0-based)") + { + } + }; + //---------------------------------------------------------------------------------------------------- struct wallet_rpc_error : public wallet_logic_error { const std::string& request() const { return m_request; } diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 0cc42ae3f..c43745dc6 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -987,9 +987,9 @@ namespace tools return amount; } //------------------------------------------------------------------------------------------------------------------------------ - template + template bool wallet_rpc_server::fill_response(std::vector &ptx_vector, - bool get_tx_key, Ts& tx_key, Tu &amount, Tu &fee, Tu &weight, std::string &multisig_txset, std::string &unsigned_txset, bool do_not_relay, + bool get_tx_key, Ts& tx_key, Tu &amount, Ta &amounts_by_dest, Tu &fee, Tu &weight, std::string &multisig_txset, std::string &unsigned_txset, bool do_not_relay, Ts &tx_hash, bool get_tx_hex, Ts &tx_blob, bool get_tx_metadata, Ts &tx_metadata, Tk &spent_key_images, epee::json_rpc::error &er) { for (const auto & ptx : ptx_vector) @@ -1006,6 +1006,12 @@ namespace tools fill(fee, ptx.fee); fill(weight, cryptonote::get_transaction_weight(ptx.tx)); + // add amounts by destination + tools::wallet_rpc::amounts_list abd; + for (const auto& dst : ptx.dests) + abd.amounts.push_back(dst.amount); + fill(amounts_by_dest, abd); + // add spent key images tools::wallet_rpc::key_image_list key_image_list; bool all_are_txin_to_key = std::all_of(ptx.tx.vin.begin(), ptx.tx.vin.end(), [&](const cryptonote::txin_v& s_e) -> bool @@ -1086,7 +1092,7 @@ namespace tools { uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0); uint32_t priority = m_wallet->adjust_priority(req.priority); - std::vector ptx_vector = m_wallet->create_transactions_2(dsts, mixin, req.unlock_time, priority, extra, req.account_index, req.subaddr_indices); + std::vector ptx_vector = m_wallet->create_transactions_2(dsts, mixin, req.unlock_time, priority, extra, req.account_index, req.subaddr_indices, req.subtract_fee_from_outputs); if (ptx_vector.empty()) { @@ -1103,7 +1109,7 @@ namespace tools return false; } - return fill_response(ptx_vector, req.get_tx_key, res.tx_key, res.amount, res.fee, res.weight, res.multisig_txset, res.unsigned_txset, req.do_not_relay, + return fill_response(ptx_vector, req.get_tx_key, res.tx_key, res.amount, res.amounts_by_dest, res.fee, res.weight, res.multisig_txset, res.unsigned_txset, req.do_not_relay, res.tx_hash, req.get_tx_hex, res.tx_blob, req.get_tx_metadata, res.tx_metadata, res.spent_key_images, er); } catch (const std::exception& e) @@ -1151,7 +1157,7 @@ namespace tools return false; } - return fill_response(ptx_vector, req.get_tx_keys, res.tx_key_list, res.amount_list, res.fee_list, res.weight_list, res.multisig_txset, res.unsigned_txset, req.do_not_relay, + return fill_response(ptx_vector, req.get_tx_keys, res.tx_key_list, res.amount_list, res.amounts_by_dest_list, res.fee_list, res.weight_list, res.multisig_txset, res.unsigned_txset, req.do_not_relay, res.tx_hash_list, req.get_tx_hex, res.tx_blob_list, req.get_tx_metadata, res.tx_metadata_list, res.spent_key_images_list, er); } catch (const std::exception& e) @@ -1540,7 +1546,7 @@ namespace tools { std::vector ptx_vector = m_wallet->create_unmixable_sweep_transactions(); - return fill_response(ptx_vector, req.get_tx_keys, res.tx_key_list, res.amount_list, res.fee_list, res.weight_list, res.multisig_txset, res.unsigned_txset, req.do_not_relay, + return fill_response(ptx_vector, req.get_tx_keys, res.tx_key_list, res.amount_list, res.amounts_by_dest_list, res.fee_list, res.weight_list, res.multisig_txset, res.unsigned_txset, req.do_not_relay, res.tx_hash_list, req.get_tx_hex, res.tx_blob_list, req.get_tx_metadata, res.tx_metadata_list, res.spent_key_images_list, er); } catch (const std::exception& e) @@ -1600,7 +1606,7 @@ namespace tools uint32_t priority = m_wallet->adjust_priority(req.priority); std::vector ptx_vector = m_wallet->create_transactions_all(req.below_amount, dsts[0].addr, dsts[0].is_subaddress, req.outputs, mixin, req.unlock_time, priority, extra, req.account_index, subaddr_indices); - return fill_response(ptx_vector, req.get_tx_keys, res.tx_key_list, res.amount_list, res.fee_list, res.weight_list, res.multisig_txset, res.unsigned_txset, req.do_not_relay, + return fill_response(ptx_vector, req.get_tx_keys, res.tx_key_list, res.amount_list, res.amounts_by_dest_list, res.fee_list, res.weight_list, res.multisig_txset, res.unsigned_txset, req.do_not_relay, res.tx_hash_list, req.get_tx_hex, res.tx_blob_list, req.get_tx_metadata, res.tx_metadata_list, res.spent_key_images_list, er); } catch (const std::exception& e) @@ -1677,7 +1683,7 @@ namespace tools return false; } - return fill_response(ptx_vector, req.get_tx_key, res.tx_key, res.amount, res.fee, res.weight, res.multisig_txset, res.unsigned_txset, req.do_not_relay, + return fill_response(ptx_vector, req.get_tx_key, res.tx_key, res.amount, res.amounts_by_dest, res.fee, res.weight, res.multisig_txset, res.unsigned_txset, req.do_not_relay, res.tx_hash, req.get_tx_hex, res.tx_blob, req.get_tx_metadata, res.tx_metadata, res.spent_key_images, er); } catch (const std::exception& e) diff --git a/src/wallet/wallet_rpc_server.h b/src/wallet/wallet_rpc_server.h index 97fc0a278..bfb7013e2 100644 --- a/src/wallet/wallet_rpc_server.h +++ b/src/wallet/wallet_rpc_server.h @@ -262,9 +262,9 @@ namespace tools bool not_open(epee::json_rpc::error& er); void handle_rpc_exception(const std::exception_ptr& e, epee::json_rpc::error& er, int default_error_code); - template + template bool fill_response(std::vector &ptx_vector, - bool get_tx_key, Ts& tx_key, Tu &amount, Tu &fee, Tu &weight, std::string &multisig_txset, std::string &unsigned_txset, bool do_not_relay, + bool get_tx_key, Ts& tx_key, Tu &amount, Ta &amounts_by_dest, Tu &fee, Tu &weight, std::string &multisig_txset, std::string &unsigned_txset, bool do_not_relay, Ts &tx_hash, bool get_tx_hex, Ts &tx_blob, bool get_tx_metadata, Ts &tx_metadata, Tk &spent_key_images, epee::json_rpc::error &er); bool validate_transfer(const std::list& destinations, const std::string& payment_id, std::vector& dsts, std::vector& extra, bool at_least_one_destination, epee::json_rpc::error& er); diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h index 33afefb80..5d5579ced 100644 --- a/src/wallet/wallet_rpc_server_commands_defs.h +++ b/src/wallet/wallet_rpc_server_commands_defs.h @@ -47,7 +47,7 @@ // advance which version they will stop working with // Don't go over 32767 for any of these #define WALLET_RPC_VERSION_MAJOR 1 -#define WALLET_RPC_VERSION_MINOR 26 +#define WALLET_RPC_VERSION_MINOR 27 #define MAKE_WALLET_RPC_VERSION(major,minor) (((major)<<16)|(minor)) #define WALLET_RPC_VERSION MAKE_WALLET_RPC_VERSION(WALLET_RPC_VERSION_MAJOR, WALLET_RPC_VERSION_MINOR) namespace tools @@ -530,11 +530,23 @@ namespace wallet_rpc END_KV_SERIALIZE_MAP() }; + struct amounts_list + { + std::list amounts; + + bool operator==(const amounts_list& other) const { return amounts == other.amounts; } + + BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE(amounts) + END_KV_SERIALIZE_MAP() + }; + struct single_transfer_response { std::string tx_hash; std::string tx_key; uint64_t amount; + amounts_list amounts_by_dest; uint64_t fee; uint64_t weight; std::string tx_blob; @@ -547,6 +559,7 @@ namespace wallet_rpc KV_SERIALIZE(tx_hash) KV_SERIALIZE(tx_key) KV_SERIALIZE(amount) + KV_SERIALIZE_OPT(amounts_by_dest, decltype(amounts_by_dest)()) KV_SERIALIZE(fee) KV_SERIALIZE(weight) KV_SERIALIZE(tx_blob) @@ -564,6 +577,7 @@ namespace wallet_rpc std::list destinations; uint32_t account_index; std::set subaddr_indices; + std::set subtract_fee_from_outputs; uint32_t priority; uint64_t ring_size; uint64_t unlock_time; @@ -577,6 +591,7 @@ namespace wallet_rpc KV_SERIALIZE(destinations) KV_SERIALIZE(account_index) KV_SERIALIZE(subaddr_indices) + KV_SERIALIZE_OPT(subtract_fee_from_outputs, decltype(subtract_fee_from_outputs)()) KV_SERIALIZE(priority) KV_SERIALIZE_OPT(ring_size, (uint64_t)0) KV_SERIALIZE(unlock_time) @@ -598,6 +613,7 @@ namespace wallet_rpc std::list tx_hash_list; std::list tx_key_list; std::list amount_list; + std::list amounts_by_dest_list; std::list fee_list; std::list weight_list; std::list tx_blob_list; @@ -610,6 +626,7 @@ namespace wallet_rpc KV_SERIALIZE(tx_hash_list) KV_SERIALIZE(tx_key_list) KV_SERIALIZE(amount_list) + KV_SERIALIZE_OPT(amounts_by_dest_list, decltype(amounts_by_dest_list)()) KV_SERIALIZE(fee_list) KV_SERIALIZE(weight_list) KV_SERIALIZE(tx_blob_list) diff --git a/tests/functional_tests/transfer.py b/tests/functional_tests/transfer.py index ddc930eb0..56a2514d9 100755 --- a/tests/functional_tests/transfer.py +++ b/tests/functional_tests/transfer.py @@ -63,6 +63,7 @@ class TransferTest(): self.check_is_key_image_spent() self.check_multiple_submissions() self.check_scan_tx() + self.check_subtract_fee_from_outputs() def reset(self): print('Resetting blockchain') @@ -1081,5 +1082,85 @@ class TransferTest(): diff_transfers(receiver_wallet.get_transfers(), res) assert receiver_wallet.get_balance().balance == expected_receiver_balance + def check_subtract_fee_from_outputs(self): + daemon = Daemon() + + print('Testing fee-included transfers') + + def inner_test_external_transfer(dsts, subtract_fee_from_outputs): + # refresh wallet and get balance + self.wallet[0].refresh() + balance1 = self.wallet[0].get_balance().balance + + # Check that this transaction is possible with our current balance + other preconditions + dst_sum = sum(map(lambda x: x['amount'], dsts)) + assert balance1 >= dst_sum + if subtract_fee_from_outputs: + assert max(subtract_fee_from_outputs) < len(dsts) + + # transfer with subtractfeefrom=all + transfer_res = self.wallet[0].transfer(dsts, subtract_fee_from_outputs = subtract_fee_from_outputs, get_tx_metadata = True) + tx_hex = transfer_res.tx_metadata + tx_fee = transfer_res.fee + amount_spent = transfer_res.amount + amounts_by_dest = transfer_res.amounts_by_dest.amounts + + # Assert that fee and amount spent to outputs adds up + assert tx_fee != 0 + if subtract_fee_from_outputs: + assert tx_fee + amount_spent == dst_sum + else: + assert amount_spent == dst_sum + + # Check the amounts by each destination that only the destinations set as subtractable + # got subtracted and that the subtracted dests are approximately correct + assert len(amounts_by_dest) == len(dsts) # if this fails... idk + for i in range(len(dsts)): + if i in subtract_fee_from_outputs: # dest is subtractable + approx_subtraction = tx_fee // len(subtract_fee_from_outputs) + assert amounts_by_dest[i] < dsts[i]['amount'] + assert dsts[i]['amount'] - amounts_by_dest[i] - approx_subtraction <= 1 + else: + assert amounts_by_dest[i] == dsts[i]['amount'] + + # relay tx and generate block (not to us, to simplify balance change calculations) + relay_res = self.wallet[0].relay_tx(tx_hex) + daemon.generateblocks('44Kbx4sJ7JDRDV5aAhLJzQCjDz2ViLRduE3ijDZu3osWKBjMGkV1XPk4pfDUMqt1Aiezvephdqm6YD19GKFD9ZcXVUTp6BW', 1) + + # refresh and get balance again + self.wallet[0].refresh() + balance2 = self.wallet[0].get_balance().balance + + # Check that the wallet balance dropped by the correct amount + balance_drop = balance1 - balance2 + if subtract_fee_from_outputs: + assert balance_drop == dst_sum + else: + assert balance_drop == dst_sum + tx_fee + + dst1 = {'address': '44Kbx4sJ7JDRDV5aAhLJzQCjDz2ViLRduE3ijDZu3osWKBjMGkV1XPk4pfDUMqt1Aiezvephdqm6YD19GKFD9ZcXVUTp6BW', 'amount': 1100000000001} + dst2 = {'address': '46r4nYSevkfBUMhuykdK3gQ98XDqDTYW1hNLaXNvjpsJaSbNtdXh1sKMsdVgqkaihChAzEy29zEDPMR3NHQvGoZCLGwTerK', 'amount': 1200000000000} + dst3 = {'address': '46r4nYSevkfBUMhuykdK3gQ98XDqDTYW1hNLaXNvjpsJaSbNtdXh1sKMsdVgqkaihChAzEy29zEDPMR3NHQvGoZCLGwTerK', 'amount': 1} + + inner_test_external_transfer([dst1, dst2], [0, 1]) + inner_test_external_transfer([dst1, dst2], [0]) + inner_test_external_transfer([dst1, dst2], [1]) + inner_test_external_transfer([dst1, dst2], []) + inner_test_external_transfer([dst1], [0]) + inner_test_external_transfer([dst1], []) + inner_test_external_transfer([dst3], []) + try: + inner_test_external_transfer([dst1, dst3], [0, 1]) # Test subtractfeefrom if one of the outputs would underflow w/o good checks + raise ValueError('transfer request with tiny subtractable destination should have thrown') + except: + pass + + # Check for JSONRPC error on bad index + try: + transfer_res = self.wallet[0].transfer([dst1], subtract_fee_from_outputs = [1]) + raise ValueError('transfer request with index should have thrown') + except AssertionError: + pass + if __name__ == '__main__': TransferTest().run_test() diff --git a/tests/unit_tests/epee_utils.cpp b/tests/unit_tests/epee_utils.cpp index c776fbcc9..dd3aafaa6 100644 --- a/tests/unit_tests/epee_utils.cpp +++ b/tests/unit_tests/epee_utils.cpp @@ -1852,3 +1852,60 @@ TEST(parsing, unicode) epee::misc_utils::parse::match_string2(si, s.end(), bs); EXPECT_EQ(bs, "あまやかす"); } + +TEST(parsing, strtoul) +{ + long ul; + const char* p; + const char* endp; + + errno = 0; // Some libc's only set errno on failure, some set it to 0 on success + + p = "0"; + endp = nullptr; + ul = std::strtoul(p, const_cast(&endp), 10); + EXPECT_EQ(0, errno); + EXPECT_EQ(0, ul); + EXPECT_EQ(p + 1, endp); + + p = "000000"; + endp = nullptr; + ul = std::strtoul(p, const_cast(&endp), 10); + EXPECT_EQ(0, errno); + EXPECT_EQ(0, ul); + EXPECT_EQ(p + 6, endp); + + p = "1"; + endp = nullptr; + ul = std::strtoul(p, const_cast(&endp), 10); + EXPECT_EQ(0, errno); + EXPECT_EQ(1, ul); + EXPECT_EQ(p + 1, endp); + + p = "0q"; + endp = nullptr; + ul = std::strtoul(p, const_cast(&endp), 10); + EXPECT_EQ(0, errno); + EXPECT_EQ(0, ul); + EXPECT_EQ(p + 1, endp); + + p = " \t 0"; + endp = nullptr; + ul = std::strtoul(p, const_cast(&endp), 10); + EXPECT_EQ(0, errno); + EXPECT_EQ(0, ul); + EXPECT_EQ(p + 9, endp); + + p = "q"; + endp = nullptr; + ul = std::strtoul(p, const_cast(&endp), 10); + EXPECT_EQ(0, errno); + EXPECT_EQ(0, ul); + EXPECT_EQ(p, endp); + + p = "999999999999999999999999999999999999999"; + endp = nullptr; + ul = std::strtoul(p, const_cast(&endp), 10); + EXPECT_EQ(ERANGE, errno); + EXPECT_EQ(ULLONG_MAX, ul); +} diff --git a/utils/python-rpc/framework/wallet.py b/utils/python-rpc/framework/wallet.py index 1c42f7f04..18af4edc4 100644 --- a/utils/python-rpc/framework/wallet.py +++ b/utils/python-rpc/framework/wallet.py @@ -38,13 +38,14 @@ class Wallet(object): self.port = port self.rpc = JSONRPC('{protocol}://{host}:{port}'.format(protocol=protocol, host=host, port=port if port else 18090+idx)) - def transfer(self, destinations, account_index = 0, subaddr_indices = [], priority = 0, ring_size = 0, unlock_time = 0, payment_id = '', get_tx_key = True, do_not_relay = False, get_tx_hex = False, get_tx_metadata = False): + def transfer(self, destinations, account_index = 0, subaddr_indices = [], priority = 0, ring_size = 0, unlock_time = 0, payment_id = '', get_tx_key = True, do_not_relay = False, get_tx_hex = False, get_tx_metadata = False, subtract_fee_from_outputs = []): transfer = { 'method': 'transfer', 'params': { 'destinations': destinations, 'account_index': account_index, 'subaddr_indices': subaddr_indices, + 'subtract_fee_from_outputs': subtract_fee_from_outputs, 'priority': priority, 'ring_size' : ring_size, 'unlock_time' : unlock_time,