From 31abac4daf74f82cf9cd7c779ebb2ed8ded1d598 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 25 Oct 2016 21:19:47 +0100 Subject: [PATCH] wallet: fix pre-rct cold wallet signing not splitting change Re-creating the transaction on the cold wallet was not splitting the change, causing the transaction to be rejected by the network. This worked on testnet since amounts do not have to be split. Also add selected_transfers, which can now be saved since they're size_t rather than iterators. This allows the view wallet to properly set the sent outputs as spent and update balance. Bump transfer file version numbers to match. --- src/simplewallet/simplewallet.cpp | 22 ++++++++++++++++------ src/wallet/wallet2.cpp | 27 +++++++++++++-------------- src/wallet/wallet2.h | 9 ++++++--- 3 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 3f494f51..c8484dd5 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -3128,9 +3128,9 @@ bool simple_wallet::accept_loaded_tx(const tools::wallet2::unsigned_tx_set &txs) if (mixin < min_mixin) min_mixin = mixin; } - for (size_t d = 0; d < cd.destinations.size(); ++d) + for (size_t d = 0; d < cd.splitted_dsts.size(); ++d) { - const tx_destination_entry &entry = cd.destinations[d]; + const tx_destination_entry &entry = cd.splitted_dsts[d]; std::string address = get_account_address_as_str(m_wallet->testnet(), entry.addr); std::unordered_map::iterator i = dests.find(address); if (i == dests.end()) @@ -3141,9 +3141,19 @@ bool simple_wallet::accept_loaded_tx(const tools::wallet2::unsigned_tx_set &txs) } if (cd.change_dts.amount > 0) { - dests.insert(std::make_pair(get_account_address_as_str(m_wallet->testnet(), cd.change_dts.addr), cd.change_dts.amount)); - amount_to_dests += cd.change_dts.amount; - change += cd.change_dts.amount; + std::unordered_map::iterator it = dests.find(get_account_address_as_str(m_wallet->testnet(), cd.change_dts.addr)); + if (it == dests.end()) + { + fail_msg_writer() << tr("Claimed change does not go to a paid address"); + return false; + } + if (it->second < cd.change_dts.amount) + { + fail_msg_writer() << tr("Claimed change is larger than payment to the change address"); + return false; + } + change = cd.change_dts.amount; + it->second -= cd.change_dts.amount; } } std::string dest_string; @@ -3158,7 +3168,7 @@ bool simple_wallet::accept_loaded_tx(const tools::wallet2::unsigned_tx_set &txs) dest_string = tr("with no destinations"); uint64_t fee = amount - amount_to_dests; - std::string prompt_str = (boost::format(tr("Loaded %lu transactions, for %s, fee %s, change %s, %s, with min mixin %lu (full details in log file). Is this okay? (Y/Yes/N/No)")) % (unsigned long)txs.txes.size() % print_money(amount) % print_money(fee) % print_money(change) % dest_string % (unsigned long)min_mixin).str(); + std::string prompt_str = (boost::format(tr("Loaded %lu transactions, for %s, fee %s, change %s, %s, with min mixin %lu. Is this okay? (Y/Yes/N/No)")) % (unsigned long)txs.txes.size() % print_money(amount) % print_money(fee) % print_money(change) % dest_string % (unsigned long)min_mixin).str(); std::string accepted = command_line::input_line(prompt_str); return is_it_true(accepted); } diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 23e016f7..55e4fa4d 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -74,8 +74,8 @@ using namespace cryptonote; // arbitrary, used to generate different hashes from the same input #define CHACHA8_KEY_TAIL 0x8c -#define UNSIGNED_TX_PREFIX "Monero unsigned tx set\001" -#define SIGNED_TX_PREFIX "Monero signed tx set\001" +#define UNSIGNED_TX_PREFIX "Monero unsigned tx set\002" +#define SIGNED_TX_PREFIX "Monero signed tx set\002" #define RECENT_OUTPUT_RATIO (0.25) // 25% of outputs are from the recent zone #define RECENT_OUTPUT_ZONE (5 * 86400) // last 5 days are the recent zone @@ -2630,11 +2630,8 @@ bool wallet2::sign_tx(const std::string &unsigned_filename, const std::string &s signed_txes.ptx.push_back(pending_tx()); tools::wallet2::pending_tx &ptx = signed_txes.ptx.back(); crypto::secret_key tx_key; - std::vector dests = sd.destinations; - if (sd.change_dts.amount > 0) - dests.push_back(sd.change_dts); - bool r = cryptonote::construct_tx_and_get_tx_key(m_account.get_keys(), sd.sources, dests, sd.extra, ptx.tx, sd.unlock_time, tx_key, sd.use_rct); - THROW_WALLET_EXCEPTION_IF(!r, error::tx_not_constructed, sd.sources, sd.destinations, sd.unlock_time, m_testnet); + bool r = cryptonote::construct_tx_and_get_tx_key(m_account.get_keys(), sd.sources, sd.splitted_dsts, sd.extra, ptx.tx, sd.unlock_time, tx_key, sd.use_rct); + THROW_WALLET_EXCEPTION_IF(!r, error::tx_not_constructed, sd.sources, sd.splitted_dsts, sd.unlock_time, m_testnet); // we don't test tx size, because we don't know the current limit, due to not having a blockchain, // and it's a bit pointless to fail there anyway, since it'd be a (good) guess only. We sign anyway, // and if we really go over limit, the daemon will reject when it gets submitted. Chances are it's @@ -2661,13 +2658,13 @@ bool wallet2::sign_tx(const std::string &unsigned_filename, const std::string &s ptx.key_images = key_images; ptx.fee = 0; for (const auto &i: sd.sources) ptx.fee += i.amount; - for (const auto &i: dests) ptx.fee -= i.amount; + for (const auto &i: sd.splitted_dsts) ptx.fee -= i.amount; ptx.dust = 0; ptx.dust_added_to_fee = false; ptx.change_dts = sd.change_dts; -// ptx.selected_transfers = selected_transfers; + ptx.selected_transfers = sd.selected_transfers; ptx.tx_key = rct::rct2sk(rct::identity()); // don't send it back to the untrusted view wallet - ptx.dests = sd.destinations; + ptx.dests = sd.splitted_dsts; ptx.construction_data = sd; } @@ -3188,8 +3185,9 @@ void wallet2::transfer_selected(const std::vector wallet2::create_transactions_2(std::vector(i) + "(" + print_money(m_transfers[i].amount()) + ") "; LOG_PRINT_L1("Found prefered rct inputs for rct tx: " << s); } } @@ -3551,7 +3550,7 @@ std::vector wallet2::create_transactions_2(std::vector sources; - std::vector destinations; cryptonote::tx_destination_entry change_dts; + std::vector splitted_dsts; + std::list selected_transfers; std::vector extra; uint64_t unlock_time; bool use_rct; BEGIN_SERIALIZE_OBJECT() FIELD(sources) - FIELD(destinations) FIELD(change_dts) + FIELD(splitted_dsts) + FIELD(selected_transfers) FIELD(extra) VARINT_FIELD(unlock_time) FIELD(use_rct) @@ -930,8 +932,9 @@ namespace tools ptx.tx_key = tx_key; ptx.dests = dsts; ptx.construction_data.sources = sources; - ptx.construction_data.destinations = dsts; ptx.construction_data.change_dts = change_dts; + ptx.construction_data.splitted_dsts = splitted_dsts; + ptx.construction_data.selected_transfers = selected_transfers; ptx.construction_data.extra = tx.extra; ptx.construction_data.unlock_time = unlock_time; ptx.construction_data.use_rct = false;