modified the interface to create transactions to remove requirement to construct and send dsts with either change or fake dst, by exposing change amount as param and uncommenting kept change addr / dsts construction - which actually solves a tx not created bug when sending to an addr (who is not oneself) with a short pid

pull/29/head
Paul Shapiro 6 years ago
parent c12e98f4ba
commit 08edc0bd58

@ -304,10 +304,11 @@ void monero_transfer_utils::create_transaction(
const account_keys& sender_account_keys, // this will reference a particular hw::device
const uint32_t subaddr_account_idx,
const std::unordered_map<crypto::public_key, cryptonote::subaddress_index> &subaddresses,
const vector<tx_destination_entry> &dsts,
uint64_t sending_amount,
uint64_t change_amount,
uint64_t fee_amount,
vector<SpendableOutput> &outputs,
vector<RandomAmountOutputs> &mix_outs,
uint64_t fee_amount,
std::vector<uint8_t> &extra,
use_fork_rules_fn_type use_fork_rules_fn,
uint64_t unlock_time, // or 0
@ -321,10 +322,6 @@ void monero_transfer_utils::create_transaction(
uint32_t fake_outputs_count = fixed_mixinsize();
bool bulletproof = use_fork_rules_fn(get_bulletproof_fork(), 0);
//
if (dsts.size() == 0) {
retVals.errCode = noDestinations;
return;
}
if (mix_outs.size() != outputs.size() && fake_outputs_count != 0) {
retVals.errCode = wrongNumberOfMixOutsProvided;
return;
@ -340,14 +337,12 @@ void monero_transfer_utils::create_transaction(
retVals.errCode = invalidSecretKeys;
return;
}
uint64_t needed_money = 0;
for (size_t i = 0; i < dsts.size(); i++) {
needed_money += dsts[i].amount;
if (needed_money > UINT64_MAX) {
retVals.errCode = outputAmountOverflow;
return;
}
uint64_t needed_money = sending_amount + change_amount + fee_amount; // TODO: is this correct?
if (needed_money > UINT64_MAX) {
retVals.errCode = outputAmountOverflow;
return;
}
//
uint64_t found_money = 0;
std::vector<tx_source_entry> sources;
// TODO: log: "Selected transfers: " << outputs
@ -481,33 +476,31 @@ void monero_transfer_utils::create_transaction(
}
//
// TODO: if this is a multisig wallet, create a list of multisig signers we can use
std::vector<cryptonote::tx_destination_entry> splitted_dsts = dsts;
cryptonote::tx_destination_entry change_dts = AUTO_VAL_INIT(change_dts);
change_dts.amount = found_money - needed_money;
std::vector<cryptonote::tx_destination_entry> splitted_dsts;
cryptonote::tx_destination_entry change_dst = AUTO_VAL_INIT(change_dst);
change_dst.amount = change_amount;
//
/* This is commented because it's presently supplied by whoever is calling this function.... But there's a good argument for bringing it in, here, especially after MyMonero clients integrate with this code and soon, share an implementation of SendFunds() (the analog of create_transactions_2 + transfer_selected*)
if (change_dts.amount == 0) {
if (splitted_dsts.size() == 1) {
// If the change is 0, send it to a random address, to avoid confusing
// the sender with a 0 amount output. We send a 0 amount in order to avoid
// letting the destination be able to work out which of the inputs is the
// real one in our rings
LOG_PRINT_L2("generating dummy address for 0 change");
cryptonote::account_base dummy;
dummy.generate();
change_dts.addr = dummy.get_keys().m_account_address;
LOG_PRINT_L2("generated dummy address for 0 change");
splitted_dsts.push_back(change_dts);
}
} else {
change_dts.addr = sender_account_keys.m_account_address;
splitted_dsts.push_back(change_dts);
}
*/
if (change_dst.amount == 0) {
if (splitted_dsts.size() == 1) {
// If the change is 0, send it to a random address, to avoid confusing
// the sender with a 0 amount output. We send a 0 amount in order to avoid
// letting the destination be able to work out which of the inputs is the
// real one in our rings
LOG_PRINT_L2("generating dummy address for 0 change");
cryptonote::account_base dummy;
dummy.generate();
change_dst.addr = dummy.get_keys().m_account_address;
LOG_PRINT_L2("generated dummy address for 0 change");
splitted_dsts.push_back(change_dst);
}
} else {
change_dst.addr = sender_account_keys.m_account_address;
splitted_dsts.push_back(change_dst);
}
//
// TODO: log: "sources: " << sources
if (found_money > needed_money) {
if (change_dts.amount != fee_amount) {
if (change_dst.amount != fee_amount) {
retVals.errCode = resultFeeNotEqualToGiven; // aka "early fee calculation != later"
return; // early
}
@ -523,7 +516,7 @@ void monero_transfer_utils::create_transaction(
std::vector<crypto::secret_key> additional_tx_keys;
bool r = cryptonote::construct_tx_and_get_tx_key(
sender_account_keys, subaddresses,
sources, splitted_dsts, change_dts.addr, extra,
sources, splitted_dsts, change_dst.addr, extra,
tx, unlock_time, tx_key, additional_tx_keys,
true, bulletproof,
/*m_multisig ? &msout : */NULL
@ -555,9 +548,9 @@ void monero_transfer_utils::convenience__create_transaction(
const string &sec_spendKey_string,
const string &to_address_string,
optional<string> payment_id_string,
uint64_t amount, // to send
uint64_t sending_amount,
uint64_t change_amount,
uint64_t fee_amount,
const std::vector<cryptonote::tx_destination_entry> &dsts, // this must include change or dummy address
vector<SpendableOutput> &outputs,
vector<RandomAmountOutputs> &mix_outs,
use_fork_rules_fn_type use_fork_rules_fn,
@ -641,10 +634,11 @@ void monero_transfer_utils::convenience__create_transaction(
account_keys,
subaddr_account_idx,
subaddresses,
dsts,
sending_amount,
change_amount,
fee_amount,
outputs,
mix_outs,
fee_amount,
extra,
use_fork_rules_fn,
unlock_time,

@ -185,10 +185,11 @@ namespace monero_transfer_utils
const account_keys& sender_account_keys, // this will reference a particular hw::device
const uint32_t subaddr_account_idx, // pass 0 for no subaddrs
const std::unordered_map<crypto::public_key, cryptonote::subaddress_index> &subaddresses,
const vector<tx_destination_entry> &dsts, // presently, this must include change as well as, if necessary, dummy output
uint64_t sending_amount,
uint64_t change_amount,
uint64_t fee_amount,
vector<SpendableOutput> &outputs,
vector<RandomAmountOutputs> &mix_outs,
uint64_t fee_amount,
std::vector<uint8_t> &extra, // this is not declared const b/c it may have the output tx pub key appended to it
use_fork_rules_fn_type use_fork_rules_fn,
uint64_t unlock_time = 0, // or 0
@ -211,9 +212,9 @@ namespace monero_transfer_utils
const string &sec_spendKey_string,
const string &to_address_string,
optional<string> payment_id_string,
uint64_t amount, // to send
uint64_t sending_amount,
uint64_t change_amount,
uint64_t fee_amount,
const std::vector<cryptonote::tx_destination_entry> &dsts, // this must include change or else dummy address
vector<SpendableOutput> &outputs,
vector<RandomAmountOutputs> &mix_outs,
use_fork_rules_fn_type use_fork_rules_fn,

@ -474,20 +474,6 @@ string serial_bridge::create_transaction(const string &args_string)
}
network_type nettype = nettype_from_string(json_root.get<string>("nettype_string"));
//
std::vector<cryptonote::tx_destination_entry> dsts;
BOOST_FOREACH(boost::property_tree::ptree::value_type &dst_desc, json_root.get_child("dsts"))
{
assert(dst_desc.first.empty()); // array elements have no names
cryptonote::tx_destination_entry de;
cryptonote::address_parse_info de_addr_info;
THROW_WALLET_EXCEPTION_IF(!cryptonote::get_account_address_from_str(de_addr_info, nettype, dst_desc.second.get<string>("addr")), error::wallet_internal_error, "Invalid dsts.addr");
de.addr = de_addr_info.address;
de.is_subaddress = de_addr_info.is_subaddress;
de.amount = stoull(dst_desc.second.get<string>("amount"));
//
dsts.push_back(de);
}
//
vector<SpendableOutput> outputs;
BOOST_FOREACH(boost::property_tree::ptree::value_type &output_desc, json_root.get_child("outputs"))
{
@ -529,9 +515,9 @@ string serial_bridge::create_transaction(const string &args_string)
json_root.get<string>("sec_spendKey_string"),
json_root.get<string>("to_address_string"),
json_root.get_optional<string>("payment_id_string"),
stoull(json_root.get<string>("amount")), // to send
stoull(json_root.get<string>("sending_amount")),
stoull(json_root.get<string>("change_amount")),
stoull(json_root.get<string>("fee_amount")),
dsts,
outputs,
mix_outs,
[] (uint8_t version, int64_t early_blocks) -> bool

@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(transfers__create)
cout << "transfers__create: spend pub key: " << string_tools::pod_to_hex(from_addr_info.address.m_spend_public_key) << endl;
//
optional<string> payment_id_string = string("b79f8efc81f58f67");
uint64_t amount = 10000000000;
uint64_t sending_amount = 10000000000;
uint64_t fee_amount = 2167750000;
string to_address_string("43zxvpcj5Xv9SEkNXbMCG7LPQStHMpFCQCmkmR4u5nzjWwq5Xkv5VmGgYEsHXg4ja2FGRD5wMWbBVMijDTqmmVqm93wHGkg");
cryptonote::address_parse_info to_addr_info;
@ -173,12 +173,7 @@ BOOST_AUTO_TEST_CASE(transfers__create)
}
}
//
std::vector<cryptonote::tx_destination_entry> dsts; // without change this would normally require a dummy addr with 0 amount pushed as a fake 'change' output ... that should probably be moved into the function
{ // 0. actual destination address
cryptonote::tx_destination_entry de;
de.addr = to_addr_info.address;
de.amount = amount;
de.is_subaddress = to_addr_info.is_subaddress;
{ // payment id
if (to_addr_info.is_subaddress && payment_id_seen) {
BOOST_REQUIRE_MESSAGE(false, "Illegal: Never supply a pid with a subaddress."); // TODO: is this true?
return;
@ -201,14 +196,6 @@ BOOST_AUTO_TEST_CASE(transfers__create)
}
payment_id_seen = true;
}
dsts.push_back(de);
}
{ // 1. change
cryptonote::tx_destination_entry de;
de.addr = from_addr_info.address;
de.amount = 112832250000;
de.is_subaddress = from_addr_info.is_subaddress; // not
dsts.push_back(de);
}
//
vector<SpendableOutput> outputs;
@ -289,10 +276,11 @@ BOOST_AUTO_TEST_CASE(transfers__create)
account_keys,
subaddr_account_idx,
subaddresses,
dsts,
sending_amount,
112832250000, // change amount
fee_amount,
outputs,
mix_outs,
fee_amount,
extra,
use_fork_rules_fn,
0, // unlock_time
@ -347,24 +335,10 @@ BOOST_AUTO_TEST_CASE(bridged__transfers__create)
root.put("sec_spendKey_string", "4e6d43cd03812b803c6f3206689f5fcc910005fc7e91d50d79b0776dbefcd803");
root.put("to_address_string", to_address_string);
root.put("payment_id_string", "b79f8efc81f58f67");
root.put("amount", amount_string);
root.put("sending_amount", amount_string);
root.put("change_amount", "112832250000");
root.put("fee_amount", "2167750000");
//
boost::property_tree::ptree dsts;
{ // 0. actual destination address
boost::property_tree::ptree dst;
dst.put("addr", to_address_string);
dst.put("amount", amount_string);
dsts.push_back(std::make_pair("", dst));
}
{ // 1. change (otherwise we'd have to supply a dummy addr)
boost::property_tree::ptree dst;
dst.put("addr", from_address_string);
dst.put("amount", "112832250000");
dsts.push_back(std::make_pair("", dst));
}
root.add_child("dsts", dsts);
//
boost::property_tree::ptree outputs;
{
boost::property_tree::ptree out;

Loading…
Cancel
Save