From 5f8c90bac6ace52016a0b1cd2203ddf18f04e25a Mon Sep 17 00:00:00 2001 From: sowle Date: Tue, 8 Nov 2022 00:17:34 +0100 Subject: [PATCH] 1) absolute_output_offsets_to_relative() is now deprecated, new functions introduced to be used instead: prepare_outputs_entries_for_key_offsets() and absolute_sorted_output_offsets_to_relative_in_place() 2) fixed incorrect key_offsets sorting in construct_tx. Coretest zarcanum_txs_with_big_decoy_set now should pass okay. --- src/currency_core/currency_format_utils.cpp | 102 +++++++++++------- src/currency_core/currency_format_utils.h | 5 + .../currency_format_utils_transactions.cpp | 61 +++++++++++ .../currency_format_utils_transactions.h | 4 +- src/wallet/wallet2.cpp | 5 +- tests/core_tests/chaingen.h | 2 +- tests/core_tests/tx_builder.h | 2 +- 7 files changed, 137 insertions(+), 44 deletions(-) diff --git a/src/currency_core/currency_format_utils.cpp b/src/currency_core/currency_format_utils.cpp index aa89a4b5..14bc69f7 100644 --- a/src/currency_core/currency_format_utils.cpp +++ b/src/currency_core/currency_format_utils.cpp @@ -1596,8 +1596,9 @@ namespace currency // prepare inputs struct input_generation_context_data { - keypair in_ephemeral; - //std::vector participants_derived_keys; + keypair in_ephemeral {}; // ephemeral output key (stealth_address and secret_x) + size_t real_out_index = SIZE_MAX; // index of real output in local outputs vector + std::vector outputs{}; // sorted by gindex }; //-------------------------------------------------------------------------------- bool generate_ZC_sig(const crypto::hash& tx_hash_for_signature, size_t input_index, const tx_source_entry& se, const input_generation_context_data& in_context, @@ -1618,7 +1619,7 @@ namespace currency #ifndef NDEBUG { crypto::point_t source_amount_commitment = crypto::c_scalar_1div8 * se.amount * crypto::c_point_H + crypto::c_scalar_1div8 * se.real_out_amount_blinding_mask * crypto::c_point_G; - CHECK_AND_ASSERT_MES(se.outputs[se.real_output].amount_commitment == source_amount_commitment.to_public_key(), false, "real output amount commitment check failed"); + CHECK_AND_ASSERT_MES(in_context.outputs[in_context.real_out_index].amount_commitment == source_amount_commitment.to_public_key(), false, "real output amount commitment check failed"); } #endif @@ -1653,10 +1654,10 @@ namespace currency // se.real_out_amount_blinding_mask - blinding_mask; std::vector ring; - for(size_t j = 0; j < se.outputs.size(); ++j) - ring.emplace_back(se.outputs[j].stealth_address, se.outputs[j].amount_commitment); + for(size_t j = 0; j < in_context.outputs.size(); ++j) + ring.emplace_back(in_context.outputs[j].stealth_address, in_context.outputs[j].amount_commitment); - return crypto::generate_CLSAG_GG(tx_hash_for_signature, ring, pseudo_out_amount_commitment, in.k_image, in_context.in_ephemeral.sec, se.real_out_amount_blinding_mask - blinding_mask, se.real_output, sig.clsags_gg); + return crypto::generate_CLSAG_GG(tx_hash_for_signature, ring, pseudo_out_amount_commitment, in.k_image, in_context.in_ephemeral.sec, se.real_out_amount_blinding_mask - blinding_mask, in_context.real_out_index, sig.clsags_gg); } //-------------------------------------------------------------------------------- bool generate_NLSAG_sig(const crypto::hash& tx_hash_for_signature, const crypto::hash& tx_prefix_hash, size_t input_index, const tx_source_entry& src_entr, @@ -1681,22 +1682,22 @@ namespace currency *pss_ring_s << "input #" << input_index << ", pub_keys:" << ENDL; std::vector keys_ptrs; - for(const tx_source_entry::output_entry& o : src_entr.outputs) + for(const tx_source_entry::output_entry& o : in_context.outputs) { keys_ptrs.push_back(&o.stealth_address); if (pss_ring_s) *pss_ring_s << o.stealth_address << ENDL; } - sigs.resize(src_entr.outputs.size()); + sigs.resize(in_context.outputs.size()); if (!watch_only_mode) - crypto::generate_ring_signature(tx_hash_for_signature, get_key_image_from_txin_v(tx.vin[input_index]), keys_ptrs, in_context.in_ephemeral.sec, src_entr.real_output, sigs.data()); + crypto::generate_ring_signature(tx_hash_for_signature, get_key_image_from_txin_v(tx.vin[input_index]), keys_ptrs, in_context.in_ephemeral.sec, in_context.real_out_index, sigs.data()); if (pss_ring_s) { *pss_ring_s << "signatures:" << ENDL; std::for_each(sigs.begin(), sigs.end(), [&pss_ring_s](const crypto::signature& s) { *pss_ring_s << s << ENDL; }); - *pss_ring_s << "prefix_hash: " << tx_prefix_hash << ENDL << "in_ephemeral_key: " << in_context.in_ephemeral.sec << ENDL << "real_output: " << src_entr.real_output << ENDL; + *pss_ring_s << "prefix_hash: " << tx_prefix_hash << ENDL << "in_ephemeral_key: " << in_context.in_ephemeral.sec << ENDL << "real_output: " << in_context.real_out_index << ENDL; } } @@ -1834,7 +1835,11 @@ namespace currency { inputs_mapping[current_index] = current_index; current_index++; - in_contexts.push_back(input_generation_context_data()); + in_contexts.push_back(input_generation_context_data{}); + input_generation_context_data& in_context = in_contexts.back(); + + in_context.outputs = prepare_outputs_entries_for_key_offsets(src_entr.outputs, src_entr.real_output, in_context.real_out_index); + if(src_entr.is_multisig()) {//multisig input txin_multisig input_multisig = AUTO_VAL_INIT(input_multisig); @@ -1846,7 +1851,7 @@ namespace currency else if (src_entr.htlc_origin.size()) { //htlc redeem - keypair& in_ephemeral = in_contexts.back().in_ephemeral; + keypair& in_ephemeral = in_context.in_ephemeral; //txin_to_key if(src_entr.outputs.size() != 1) { @@ -1861,11 +1866,11 @@ namespace currency return false; //check that derivated key is equal with real output key - if (!(in_ephemeral.pub == src_entr.outputs[src_entr.real_output].stealth_address)) + if (!(in_ephemeral.pub == src_entr.outputs.front().stealth_address)) { LOG_ERROR("derived public key missmatch with output public key! " << ENDL << "derived_key:" << string_tools::pod_to_hex(in_ephemeral.pub) << ENDL << "real output_public_key:" - << string_tools::pod_to_hex(src_entr.outputs[src_entr.real_output].stealth_address)); + << string_tools::pod_to_hex(src_entr.outputs.front().stealth_address)); return false; } @@ -1874,47 +1879,37 @@ namespace currency input_to_key.amount = src_entr.amount; input_to_key.k_image = img; input_to_key.hltc_origin = src_entr.htlc_origin; + input_to_key.key_offsets.push_back(src_entr.outputs.front().out_reference); - //fill outputs array and use relative offsets - for(const tx_source_entry::output_entry& out_entry : src_entr.outputs) - input_to_key.key_offsets.push_back(out_entry.out_reference); - - input_to_key.key_offsets = absolute_output_offsets_to_relative(input_to_key.key_offsets); tx.vin.push_back(input_to_key); } else { - //regular to key out - keypair& in_ephemeral = in_contexts.back().in_ephemeral; - //txin_to_key - if (src_entr.real_output >= src_entr.outputs.size()) - { - LOG_ERROR("real_output index (" << src_entr.real_output << ") greater than or equal to output_keys.size()=" << src_entr.outputs.size()); - return false; - } + // txin_to_key or txin_zc_input + CHECK_AND_ASSERT_MES(in_context.real_out_index < in_context.outputs.size(), false, + "real_output index (" << in_context.real_out_index << ") greater than or equal to in_context.outputs.size()=" << in_context.outputs.size()); + summary_inputs_money += src_entr.amount; //key_derivation recv_derivation; crypto::key_image img; - if (!generate_key_image_helper(sender_account_keys, src_entr.real_out_tx_key, src_entr.real_output_in_tx_index, in_ephemeral, img)) + if (!generate_key_image_helper(sender_account_keys, src_entr.real_out_tx_key, src_entr.real_output_in_tx_index, in_context.in_ephemeral, img)) return false; //check that derivated key is equal with real output key - if (!(in_ephemeral.pub == src_entr.outputs[src_entr.real_output].stealth_address)) + if (!(in_context.in_ephemeral.pub == in_context.outputs[in_context.real_out_index].stealth_address)) { LOG_ERROR("derived public key missmatch with output public key! " << ENDL << "derived_key:" - << string_tools::pod_to_hex(in_ephemeral.pub) << ENDL << "real output_public_key:" - << string_tools::pod_to_hex(src_entr.outputs[src_entr.real_output].stealth_address)); + << string_tools::pod_to_hex(in_context.in_ephemeral.pub) << ENDL << "real output_public_key:" + << string_tools::pod_to_hex(in_context.outputs[in_context.real_out_index].stealth_address)); return false; } //fill key_offsets array with relative offsets std::vector key_offsets; - for(const tx_source_entry::output_entry& out_entry : src_entr.outputs) + for(const tx_source_entry::output_entry& out_entry : in_context.outputs) key_offsets.push_back(out_entry.out_reference); - key_offsets = absolute_output_offsets_to_relative(key_offsets); - //TODO: Might need some refactoring since this scheme is not the clearest one(did it this way for now to keep less changes to not broke anything) //potentially this approach might help to support htlc and multisig without making to complicated code if (src_entr.is_zarcanum()) @@ -1938,15 +1933,10 @@ namespace currency input_to_key.k_image = img; input_to_key.key_offsets = std::move(key_offsets); tx.vin.push_back(input_to_key); - //NLSAG_sources.push_back(&src_entr); } } } - /*if (ins_zc.elements.size()) - { - tx.vin.push_back(ins_zc); - }*/ uint64_t amount_of_assets = 0; std::vector shuffled_dsts(destinations); if (asset_id_for_destinations != currency::null_hash) @@ -3041,6 +3031,7 @@ namespace currency return res; } //--------------------------------------------------------------- + // DEPRECATED: consider using prepare_outputs_entries_for_key_offsets and absolute_sorted_output_offsets_to_relative_in_place instead std::vector absolute_output_offsets_to_relative(const std::vector& off) { std::vector res = off; @@ -3084,6 +3075,34 @@ namespace currency return res; } //--------------------------------------------------------------- + bool absolute_sorted_output_offsets_to_relative_in_place(std::vector& offsets) noexcept + { + if (offsets.size() < 2) + return true; + + size_t i = offsets.size() - 1; + while (i != 0 && offsets[i].type() == typeid(ref_by_id)) + --i; + + try + { + for (; i != 0; i--) + { + uint64_t& offset_i = boost::get(offsets[i]); + uint64_t& offset_im1 = boost::get(offsets[i - 1]); + if (offset_i <= offset_im1) + return false; // input was not properly sorted + offset_i -= offset_im1; + } + } + catch(...) + { + return false; // unexpected type in boost::get (all ref_by_id's must be at the end of 'offsets') + } + + return true; + } + //--------------------------------------------------------------- bool parse_and_validate_block_from_blob(const blobdata& b_blob, block& b) { return parse_and_validate_object_from_blob(b_blob, b); @@ -3956,6 +3975,11 @@ namespace currency return false; } //-------------------------------------------------------------------------------- + bool operator ==(const currency::ref_by_id& a, const currency::ref_by_id& b) + { + return a.n == b.n && a.tx_id == b.tx_id; + } + //-------------------------------------------------------------------------------- bool verify_multiple_zc_outs_range_proofs(const std::vector& range_proofs) { if (range_proofs.empty()) diff --git a/src/currency_core/currency_format_utils.h b/src/currency_core/currency_format_utils.h index d2cd4592..0114b5db 100644 --- a/src/currency_core/currency_format_utils.h +++ b/src/currency_core/currency_format_utils.h @@ -60,6 +60,7 @@ namespace currency bool operator ==(const currency::void_sig& a, const currency::void_sig& b); bool operator ==(const currency::ZC_sig& a, const currency::ZC_sig& b); bool operator ==(const currency::zarcanum_sig& a, const currency::zarcanum_sig& b); + bool operator ==(const currency::ref_by_id& a, const currency::ref_by_id& b); typedef boost::multiprecision::uint128_t uint128_tl; @@ -365,7 +366,11 @@ namespace currency uint64_t get_block_height(const transaction& coinbase); uint64_t get_block_height(const block& b); std::vector relative_output_offsets_to_absolute(const std::vector& off); + // DEPRECATED: consider using prepare_outputs_entries_for_key_offsets and absolute_sorted_output_offsets_to_relative_in_place instead std::vector absolute_output_offsets_to_relative(const std::vector& off); + bool absolute_sorted_output_offsets_to_relative_in_place(std::vector& offsets) noexcept; + + // prints amount in format "3.14", "0.0" std::string print_money_brief(uint64_t amount); diff --git a/src/currency_core/currency_format_utils_transactions.cpp b/src/currency_core/currency_format_utils_transactions.cpp index 0c552fa1..c0e06a3b 100644 --- a/src/currency_core/currency_format_utils_transactions.cpp +++ b/src/currency_core/currency_format_utils_transactions.cpp @@ -323,4 +323,65 @@ namespace currency } return true; } + //--------------------------------------------------------------- + // Prepapres vector of output_entry to be used in key_offsets in a transaction input: + // 1) sort all entries by gindex (while moving all ref_by_id to the end, keeping they relative order) + // 2) convert absolute global indices to relative key_offsets + std::vector prepare_outputs_entries_for_key_offsets(const std::vector& outputs, size_t old_real_index, size_t& new_real_index) noexcept + { + TRY_ENTRY() + + std::vector result = outputs; + if (outputs.size() < 2) + { + new_real_index = old_real_index; + return result; + } + + std::sort(result.begin(), result.end(), [](const tx_source_entry::output_entry& lhs, const tx_source_entry::output_entry& rhs) + { + if (lhs.out_reference.type() == typeid(uint64_t)) + { + if (rhs.out_reference.type() == typeid(uint64_t)) + return boost::get(lhs.out_reference) < boost::get(rhs.out_reference); + if (rhs.out_reference.type() == typeid(ref_by_id)) + return true; + CHECK_AND_ASSERT_THROW_MES(false, "unexpected type in out_reference 1: " << rhs.out_reference.type().name()); + } + else if (lhs.out_reference.type() == typeid(ref_by_id)) + { + if (rhs.out_reference.type() == typeid(uint64_t)) + return false; + if (rhs.out_reference.type() == typeid(ref_by_id)) + return false; // don't change the order of ref_by_id elements + CHECK_AND_ASSERT_THROW_MES(false, "unexpected type in out_reference 2: " << rhs.out_reference.type().name()); + } + return false; + }); + + // restore index of the selected element, if needed + if (old_real_index != SIZE_MAX) + { + CHECK_AND_ASSERT_THROW_MES(old_real_index < outputs.size(), "old_real_index is OOB"); + auto it = std::find(result.begin(), result.end(), outputs[old_real_index]); + CHECK_AND_ASSERT_THROW_MES(it != result.end(), "internal error: cannot find old_real_index"); + new_real_index = it - result.begin(); + } + + // find the last uint64_t entry - skip ref_by_id entries goint from the end to the beginnning + size_t i = result.size() - 1; + while (i != 0 && result[i].out_reference.type() == typeid(ref_by_id)) + --i; + + for (; i != 0; i--) + { + boost::get(result[i].out_reference) -= boost::get(result[i - 1].out_reference); + } + + return result; + + CATCH_ENTRY2(std::vector{}); + } + //--------------------------------------------------------------- + } \ No newline at end of file diff --git a/src/currency_core/currency_format_utils_transactions.h b/src/currency_core/currency_format_utils_transactions.h index c8ce1209..29b70bc4 100644 --- a/src/currency_core/currency_format_utils_transactions.h +++ b/src/currency_core/currency_format_utils_transactions.h @@ -30,6 +30,8 @@ namespace currency crypto::public_key concealing_point; // only for zarcaum outputs crypto::public_key amount_commitment; // only for zarcaum outputs + bool operator==(const output_entry& rhs) const { return out_reference == rhs.out_reference; } // used in prepare_outputs_entries_for_key_offsets, it's okay to do partially comparison + BEGIN_SERIALIZE_OBJECT() FIELD(out_reference) FIELD(stealth_address) @@ -161,5 +163,5 @@ namespace currency bool read_keyimages_from_tx(const transaction& tx, std::list& kil); bool validate_inputs_sorting(const transaction& tx); - + std::vector prepare_outputs_entries_for_key_offsets(const std::vector& outputs, size_t old_real_index, size_t& new_real_index) noexcept; } diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index ee77c90c..f51c6d7e 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -3769,7 +3769,7 @@ bool wallet2::prepare_and_sign_pos_block(const mining_context& cxt, currency::bl } WLT_THROW_IF_FALSE_WALLET_CMN_ERR_EX(decoys.size() == required_decoys_count + 1, "for PoS stake got less good decoys than required: " << decoys.size() << " < " << required_decoys_count); - decoys.sort([](auto& l, auto& r){ return l.global_amount_index < r.global_amount_index; }); // sort them now (absolute_output_offsets_to_relative) + decoys.sort([](auto& l, auto& r){ return l.global_amount_index < r.global_amount_index; }); // sort them now (note absolute_sorted_output_offsets_to_relative_in_place() below) uint64_t i = 0; for(auto& el : decoys) @@ -3781,7 +3781,8 @@ bool wallet2::prepare_and_sign_pos_block(const mining_context& cxt, currency::bl ring.emplace_back(el.stealth_address, el.amount_commitment, el.concealing_point); stake_input.key_offsets.push_back(el.global_amount_index); } - stake_input.key_offsets = absolute_output_offsets_to_relative(stake_input.key_offsets); + r = absolute_sorted_output_offsets_to_relative_in_place(stake_input.key_offsets); + WLT_THROW_IF_FALSE_WALLET_CMN_ERR_EX(r, "absolute_sorted_output_offsets_to_relative_in_place failed"); } else { diff --git a/tests/core_tests/chaingen.h b/tests/core_tests/chaingen.h index 71af6aa8..56db1e4a 100644 --- a/tests/core_tests/chaingen.h +++ b/tests/core_tests/chaingen.h @@ -803,7 +803,7 @@ bool construct_broken_tx(const currency::account_keys& sender_account_keys, cons BOOST_FOREACH(const currency::tx_source_entry::output_entry& out_entry, src_entr.outputs) input_to_key.key_offsets.push_back(out_entry.out_reference); - input_to_key.key_offsets = currency::absolute_output_offsets_to_relative(input_to_key.key_offsets); + input_to_key.key_offsets = currency::absolute_output_offsets_to_relative(input_to_key.key_offsets); // TODO @#@# tx.vin.push_back(input_to_key); } diff --git a/tests/core_tests/tx_builder.h b/tests/core_tests/tx_builder.h index 17828e5b..22ff3f5c 100644 --- a/tests/core_tests/tx_builder.h +++ b/tests/core_tests/tx_builder.h @@ -37,7 +37,7 @@ struct tx_builder for(const currency::tx_source_entry::output_entry& out_entry : src_entr.outputs) input_to_key.key_offsets.push_back(out_entry.out_reference); - input_to_key.key_offsets = currency::absolute_output_offsets_to_relative(input_to_key.key_offsets); + input_to_key.key_offsets = currency::absolute_output_offsets_to_relative(input_to_key.key_offsets); // TODO @#@# m_tx.vin.push_back(input_to_key); } }