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.
This commit is contained in:
moneromooo-monero 2016-10-25 21:19:47 +01:00
parent 18e406a0e6
commit 31abac4daf
No known key found for this signature in database
GPG Key ID: 686F07454D6CEFC3
3 changed files with 35 additions and 23 deletions

View File

@ -3128,9 +3128,9 @@ bool simple_wallet::accept_loaded_tx(const tools::wallet2::unsigned_tx_set &txs)
if (mixin < min_mixin) if (mixin < min_mixin)
min_mixin = 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::string address = get_account_address_as_str(m_wallet->testnet(), entry.addr);
std::unordered_map<std::string,uint64_t>::iterator i = dests.find(address); std::unordered_map<std::string,uint64_t>::iterator i = dests.find(address);
if (i == dests.end()) 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) 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)); std::unordered_map<std::string, uint64_t>::iterator it = dests.find(get_account_address_as_str(m_wallet->testnet(), cd.change_dts.addr));
amount_to_dests += cd.change_dts.amount; if (it == dests.end())
change += cd.change_dts.amount; {
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; 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"); dest_string = tr("with no destinations");
uint64_t fee = amount - amount_to_dests; 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); std::string accepted = command_line::input_line(prompt_str);
return is_it_true(accepted); return is_it_true(accepted);
} }

View File

@ -74,8 +74,8 @@ using namespace cryptonote;
// arbitrary, used to generate different hashes from the same input // arbitrary, used to generate different hashes from the same input
#define CHACHA8_KEY_TAIL 0x8c #define CHACHA8_KEY_TAIL 0x8c
#define UNSIGNED_TX_PREFIX "Monero unsigned tx set\001" #define UNSIGNED_TX_PREFIX "Monero unsigned tx set\002"
#define SIGNED_TX_PREFIX "Monero signed tx set\001" #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_RATIO (0.25) // 25% of outputs are from the recent zone
#define RECENT_OUTPUT_ZONE (5 * 86400) // last 5 days are 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()); signed_txes.ptx.push_back(pending_tx());
tools::wallet2::pending_tx &ptx = signed_txes.ptx.back(); tools::wallet2::pending_tx &ptx = signed_txes.ptx.back();
crypto::secret_key tx_key; crypto::secret_key tx_key;
std::vector<cryptonote::tx_destination_entry> dests = sd.destinations; 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);
if (sd.change_dts.amount > 0) THROW_WALLET_EXCEPTION_IF(!r, error::tx_not_constructed, sd.sources, sd.splitted_dsts, sd.unlock_time, m_testnet);
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);
// we don't test tx size, because we don't know the current limit, due to not having a blockchain, // 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 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 // 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.key_images = key_images;
ptx.fee = 0; ptx.fee = 0;
for (const auto &i: sd.sources) ptx.fee += i.amount; 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 = 0;
ptx.dust_added_to_fee = false; ptx.dust_added_to_fee = false;
ptx.change_dts = sd.change_dts; 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.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; ptx.construction_data = sd;
} }
@ -3188,8 +3185,9 @@ void wallet2::transfer_selected(const std::vector<cryptonote::tx_destination_ent
ptx.tx_key = tx_key; ptx.tx_key = tx_key;
ptx.dests = dsts; ptx.dests = dsts;
ptx.construction_data.sources = sources; ptx.construction_data.sources = sources;
ptx.construction_data.destinations = dsts;
ptx.construction_data.change_dts = change_dts; 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.extra = tx.extra;
ptx.construction_data.unlock_time = unlock_time; ptx.construction_data.unlock_time = unlock_time;
ptx.construction_data.use_rct = false; ptx.construction_data.use_rct = false;
@ -3307,8 +3305,9 @@ void wallet2::transfer_selected_rct(std::vector<cryptonote::tx_destination_entry
ptx.tx_key = tx_key; ptx.tx_key = tx_key;
ptx.dests = dsts; ptx.dests = dsts;
ptx.construction_data.sources = sources; ptx.construction_data.sources = sources;
ptx.construction_data.destinations = dsts;
ptx.construction_data.change_dts = change_dts; 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.extra = tx.extra;
ptx.construction_data.unlock_time = unlock_time; ptx.construction_data.unlock_time = unlock_time;
ptx.construction_data.use_rct = true; ptx.construction_data.use_rct = true;
@ -3531,7 +3530,7 @@ std::vector<wallet2::pending_tx> wallet2::create_transactions_2(std::vector<cryp
if (!prefered_inputs.empty()) if (!prefered_inputs.empty())
{ {
string s; string s;
for (auto i: prefered_inputs) s += print_money(m_transfers[i].amount()) + " "; for (auto i: prefered_inputs) s += boost::lexical_cast<std::string>(i) + "(" + print_money(m_transfers[i].amount()) + ") ";
LOG_PRINT_L1("Found prefered rct inputs for rct tx: " << s); LOG_PRINT_L1("Found prefered rct inputs for rct tx: " << s);
} }
} }
@ -3551,7 +3550,7 @@ std::vector<wallet2::pending_tx> wallet2::create_transactions_2(std::vector<cryp
size_t idx = !prefered_inputs.empty() ? pop_back(prefered_inputs) : !unused_transfers_indices.empty() ? pop_best_value(unused_transfers_indices, tx.selected_transfers) : pop_best_value(unused_dust_indices, tx.selected_transfers); size_t idx = !prefered_inputs.empty() ? pop_back(prefered_inputs) : !unused_transfers_indices.empty() ? pop_best_value(unused_transfers_indices, tx.selected_transfers) : pop_best_value(unused_dust_indices, tx.selected_transfers);
const transfer_details &td = m_transfers[idx]; const transfer_details &td = m_transfers[idx];
LOG_PRINT_L2("Picking output " << idx << ", amount " << print_money(td.amount())); LOG_PRINT_L2("Picking output " << idx << ", amount " << print_money(td.amount()) << ", ki " << td.m_key_image);
// add this output to the list to spend // add this output to the list to spend
tx.selected_transfers.push_back(idx); tx.selected_transfers.push_back(idx);

View File

@ -154,16 +154,18 @@ namespace tools
struct tx_construction_data struct tx_construction_data
{ {
std::vector<cryptonote::tx_source_entry> sources; std::vector<cryptonote::tx_source_entry> sources;
std::vector<cryptonote::tx_destination_entry> destinations;
cryptonote::tx_destination_entry change_dts; cryptonote::tx_destination_entry change_dts;
std::vector<cryptonote::tx_destination_entry> splitted_dsts;
std::list<size_t> selected_transfers;
std::vector<uint8_t> extra; std::vector<uint8_t> extra;
uint64_t unlock_time; uint64_t unlock_time;
bool use_rct; bool use_rct;
BEGIN_SERIALIZE_OBJECT() BEGIN_SERIALIZE_OBJECT()
FIELD(sources) FIELD(sources)
FIELD(destinations)
FIELD(change_dts) FIELD(change_dts)
FIELD(splitted_dsts)
FIELD(selected_transfers)
FIELD(extra) FIELD(extra)
VARINT_FIELD(unlock_time) VARINT_FIELD(unlock_time)
FIELD(use_rct) FIELD(use_rct)
@ -930,8 +932,9 @@ namespace tools
ptx.tx_key = tx_key; ptx.tx_key = tx_key;
ptx.dests = dsts; ptx.dests = dsts;
ptx.construction_data.sources = sources; ptx.construction_data.sources = sources;
ptx.construction_data.destinations = dsts;
ptx.construction_data.change_dts = change_dts; 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.extra = tx.extra;
ptx.construction_data.unlock_time = unlock_time; ptx.construction_data.unlock_time = unlock_time;
ptx.construction_data.use_rct = false; ptx.construction_data.use_rct = false;