From 51d7a6921c8d6a337773f261a04301ef75ee5e3b 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 18cd0c2a6..7372d8d15 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -196,7 +196,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=]
[]"); @@ -531,7 +531,52 @@ namespace fail_msg_writer() << sw::tr("invalid format for subaddress lookahead; must be :"); return r; } -} + + static constexpr const char* 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)) // 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() + strlen(SFFD_ARG_NAME); 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) { @@ -3290,7 +3335,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), @@ -6640,6 +6685,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(); ) @@ -6736,6 +6802,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 0218c8292..96a6c6d71 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -10523,7 +10523,7 @@ bool wallet2::light_wallet_key_image_is_ours(const crypto::key_image& key_image, // 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(); @@ -10538,11 +10538,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; @@ -10552,9 +10553,11 @@ std::vector wallet2::create_transactions_2(std::vector::iterator i; @@ -10564,6 +10567,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; } @@ -10579,13 +10583,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; @@ -10608,6 +10666,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; @@ -10635,6 +10701,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) @@ -10748,7 +10815,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; @@ -10864,7 +10932,8 @@ std::vector wallet2::create_transactions_2(std::vector wallet2::create_transactions_2(std::vector 0 && !dsts.empty() && @@ -10881,7 +10951,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"); @@ -11061,10 +11159,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, */ @@ -11077,7 +11178,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, @@ -11111,23 +11212,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 877e5afd0..9dcece0af 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -598,6 +598,7 @@ private: typedef std::vector transfer_container; typedef serializable_unordered_multimap payment_container; + typedef std::set unique_index_container; struct multisig_sig { @@ -1111,11 +1112,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 fcf2ddd93..1f7e1c75d 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 cecd79368..4e42d64cd 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -988,9 +988,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) @@ -1007,6 +1007,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 @@ -1087,7 +1093,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()) { @@ -1104,7 +1110,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) @@ -1152,7 +1158,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) @@ -1541,7 +1547,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) @@ -1601,7 +1607,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) @@ -1678,7 +1684,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 3088fd9c2..3308d1751 100644 --- a/src/wallet/wallet_rpc_server.h +++ b/src/wallet/wallet_rpc_server.h @@ -263,9 +263,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 d5c6bfd6b..b6098d95c 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 931deddfb..4063911f4 100755 --- a/tests/functional_tests/transfer.py +++ b/tests/functional_tests/transfer.py @@ -62,6 +62,7 @@ class TransferTest(): self.check_rescan() self.check_is_key_image_spent() self.check_scan_tx() + self.check_subtract_fee_from_outputs() def reset(self): print('Resetting blockchain') @@ -1046,5 +1047,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 1c756e77c..110cebf6a 100644 --- a/tests/unit_tests/epee_utils.cpp +++ b/tests/unit_tests/epee_utils.cpp @@ -1836,3 +1836,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 37e91034b..1e10e1f86 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,