From 7ce49e4e82d63b1e855ae3e5ef6348ca249732f0 Mon Sep 17 00:00:00 2001 From: sowle Date: Mon, 3 Oct 2022 20:34:09 +0200 Subject: [PATCH] tx_pool: correct handling of key images regardless of input type + get_key_image_from_txin_v refactoring --- src/currency_core/blockchain_storage.cpp | 4 +- .../currency_format_utils_abstract.h | 51 +++++-- .../currency_format_utils_transactions.cpp | 2 +- src/currency_core/tx_pool.cpp | 134 ++++++++---------- 4 files changed, 97 insertions(+), 94 deletions(-) diff --git a/src/currency_core/blockchain_storage.cpp b/src/currency_core/blockchain_storage.cpp index 3640c11e..d1a3397a 100644 --- a/src/currency_core/blockchain_storage.cpp +++ b/src/currency_core/blockchain_storage.cpp @@ -1642,7 +1642,7 @@ bool blockchain_storage::purge_altblock_keyimages_from_big_heap(const block& b, { if (tx.vin[n].type() == typeid(txin_to_key) || tx.vin[n].type() == typeid(txin_htlc)) { - purge_keyimage_from_big_heap(get_key_image_txin_v(tx.vin[n]), block_id); + purge_keyimage_from_big_heap(get_key_image_from_txin_v(tx.vin[n]), block_id); } else if (tx.vin[n].type() == typeid(txin_zc_input)) { @@ -5308,7 +5308,7 @@ bool blockchain_storage::validate_pos_block(const block& b, CHECK_AND_ASSERT_MES(b.miner_tx.vin.size() == 2, false, "incorrect: miner_tx.vin.size() = " << b.miner_tx.vin.size()); CHECK_AND_ASSERT_MES(b.miner_tx.vin[0].type() == typeid(txin_gen), false, "incorrect input 0 type: " << b.miner_tx.vin[0].type().name()); CHECK_AND_ASSERT_MES(b.miner_tx.vin[1].type() == typeid(txin_to_key) || b.miner_tx.vin[1].type() == typeid(txin_zc_input), false, "incorrect input 1 type: " << b.miner_tx.vin[1].type().name()); - const crypto::key_image& stake_key_image = get_key_image_txin_v(b.miner_tx.vin[1]); + const crypto::key_image& stake_key_image = get_key_image_from_txin_v(b.miner_tx.vin[1]); //check keyimage if it's main chain candidate if (!for_altchain) { diff --git a/src/currency_core/currency_format_utils_abstract.h b/src/currency_core/currency_format_utils_abstract.h index 1c9f0fff..7c0e8013 100644 --- a/src/currency_core/currency_format_utils_abstract.h +++ b/src/currency_core/currency_format_utils_abstract.h @@ -168,24 +168,49 @@ namespace currency } //--------------------------------------------------------------- inline - const crypto::key_image & get_key_image_txin_v(const txin_v& in_v) + const bool get_key_image_from_txin_v(const txin_v& in_v, crypto::key_image& result) noexcept + { + try + { + if (in_v.type() == typeid(txin_to_key)) + { + result = boost::get(in_v).k_image; + return true; + } + + if (in_v.type() == typeid(txin_htlc)) + { + result = boost::get(in_v).k_image; + return true; + } + + if (in_v.type() == typeid(txin_zc_input)) + { + result = boost::get(in_v).k_image; + return true; + } + } + catch(...) + { + // should never go here, just precaution + } + + return false; + } + //--------------------------------------------------------------- + inline + const crypto::key_image& get_key_image_from_txin_v(const txin_v& in_v) { if (in_v.type() == typeid(txin_to_key)) - { return boost::get(in_v).k_image; - } - else if (in_v.type() == typeid(txin_htlc)) - { + + if (in_v.type() == typeid(txin_htlc)) return boost::get(in_v).k_image; - } - else if (in_v.type() == typeid(txin_zc_input)) - { + + if (in_v.type() == typeid(txin_zc_input)) return boost::get(in_v).k_image; - } - else - { - ASSERT_MES_AND_THROW("[get_key_image_txin_v] Wrong type: " << in_v.type().name()); - } + + ASSERT_MES_AND_THROW("[get_key_image_from_txin_v] Wrong type: " << in_v.type().name()); } //--------------------------------------------------------------- //, txin_htlc, txin_zc_input diff --git a/src/currency_core/currency_format_utils_transactions.cpp b/src/currency_core/currency_format_utils_transactions.cpp index 5792295e..ffb6d813 100644 --- a/src/currency_core/currency_format_utils_transactions.cpp +++ b/src/currency_core/currency_format_utils_transactions.cpp @@ -274,7 +274,7 @@ namespace currency if (in.type() == typeid(txin_to_key) || in.type() == typeid(txin_htlc) || in.type() == typeid(txin_zc_input)) { - if (!ki.insert(get_key_image_txin_v(in)).second) + if (!ki.insert(get_key_image_from_txin_v(in)).second) return false; } } diff --git a/src/currency_core/tx_pool.cpp b/src/currency_core/tx_pool.cpp index 2193e89e..61e5a449 100644 --- a/src/currency_core/tx_pool.cpp +++ b/src/currency_core/tx_pool.cpp @@ -93,7 +93,13 @@ namespace currency } //--------------------------------------------------------------------------------- bool tx_memory_pool::add_tx(const transaction &tx, const crypto::hash &id, uint64_t blob_size, tx_verification_context& tvc, bool kept_by_block, bool from_core) - { + { + bool r = false; + + // defaults + tvc.m_added_to_pool = false; + tvc.m_verification_failed = true; + if (!kept_by_block && !from_core && m_blockchain.is_in_checkpoint_zone()) { // BCS is in CP zone, tx verification is impossible until it gets synchronized @@ -104,21 +110,13 @@ namespace currency return false; } - if (!m_blockchain.validate_tx_for_hardfork_specific_terms(tx, id)) - { - // - LOG_ERROR("Transaction " << id <<" doesn't fit current hardfork"); - tvc.m_verification_failed = true; - return false; - } + r = m_blockchain.validate_tx_for_hardfork_specific_terms(tx, id); + CHECK_AND_ASSERT_MES(r, false, "Transaction " << id <<" doesn't fit current hardfork"); TIME_MEASURE_START_PD(tx_processing_time); TIME_MEASURE_START_PD(check_inputs_types_supported_time); - if(!check_inputs_types_supported(tx)) - { - tvc.m_verification_failed = true; - return false; - } + r = check_inputs_types_supported(tx); + CHECK_AND_ASSERT_MES(r, false, "tx " << id << " has wrong inputs types"); TIME_MEASURE_FINISH_PD(check_inputs_types_supported_time); TIME_MEASURE_START_PD(expiration_validate_time); @@ -134,33 +132,26 @@ namespace currency } TIME_MEASURE_FINISH_PD(expiration_validate_time); + TIME_MEASURE_START_PD(validate_amount_time); - uint64_t inputs_amount = 0; - if(!get_inputs_money_amount(tx, inputs_amount)) - { - tvc.m_verification_failed = true; - return false; - } + CHECK_AND_ASSERT_MES(tx.vout.size() <= CURRENCY_TX_MAX_ALLOWED_OUTS, false, "transaction has too many outs = " << tx.vout.size()); - CHECK_AND_ASSERT_MES_CUSTOM(tx.vout.size() <= CURRENCY_TX_MAX_ALLOWED_OUTS, false, tvc.m_verification_failed = true, "transaction has too many outs = " << tx.vout.size()); + uint64_t tx_fee = 0; + r = get_tx_fee(tx, tx_fee); + CHECK_AND_ASSERT_MES(r, false, "get_tx_fee failed"); - uint64_t outputs_amount = get_outs_money_amount(tx); - - if(outputs_amount > inputs_amount) - { - LOG_PRINT_L0("transaction use more money then it has: use " << outputs_amount << ", have " << inputs_amount); - tvc.m_verification_failed = true; - return false; - } + // @#@# consider removing the following + //if (!check_tx_balance(tx)) // TODO (performance): check_tx_balance calls get_tx_fee as well, consider refactoring -- sowle + //{ + // LOG_PRINT_L0("balance check failed for tx " << id); + // tvc.m_verification_failed = true; + // return false; + //} TIME_MEASURE_FINISH_PD(validate_amount_time); TIME_MEASURE_START_PD(validate_alias_time); - if (!from_core && !validate_alias_info(tx, kept_by_block)) - { - LOG_PRINT_RED_L0("validate_alias_info failed"); - tvc.m_verification_failed = true; - return false; - } + r = from_core || validate_alias_info(tx, kept_by_block); + CHECK_AND_ASSERT_MES(r, false, "validate_alias_info failed"); TIME_MEASURE_FINISH_PD(validate_alias_time); TIME_MEASURE_START_PD(check_keyimages_ws_ms_time); @@ -168,15 +159,10 @@ namespace currency if(!from_core && !kept_by_block) { crypto::key_image spent_ki = AUTO_VAL_INIT(spent_ki); - if(have_tx_keyimges_as_spent(tx, &spent_ki)) - { - LOG_ERROR("Transaction " << id << " uses already spent key image " << spent_ki); - tvc.m_verification_failed = true; - return false; - } + r = !have_tx_keyimges_as_spent(tx, &spent_ki); + CHECK_AND_ASSERT_MES(r, false, "Transaction " << id << " uses already spent key image " << spent_ki); //transaction spam protection, soft rule - uint64_t tx_fee = inputs_amount - outputs_amount; if (tx_fee < m_blockchain.get_core_runtime_config().tx_pool_min_fee) { if (is_valid_contract_finalization_tx(tx)) @@ -188,7 +174,7 @@ namespace currency else { // this tx has no fee - LOG_PRINT_RED_L0("Transaction with id= " << id << " has too small fee: " << tx_fee << ", expected fee: " << m_blockchain.get_core_runtime_config().tx_pool_min_fee); + LOG_PRINT_RED_L0("Transaction " << id << " has too small fee: " << print_money_brief(tx_fee) << ", minimum fee: " << print_money_brief(m_blockchain.get_core_runtime_config().tx_pool_min_fee)); tvc.m_verification_failed = false; tvc.m_should_be_relayed = false; tvc.m_added_to_pool = false; @@ -211,13 +197,13 @@ namespace currency bool ch_inp_res = m_blockchain.check_tx_inputs(tx, id, max_used_block_height, max_used_block_id); if (!ch_inp_res && !kept_by_block && !from_core) { - LOG_PRINT_L0("tx used wrong inputs, rejected"); + LOG_PRINT_L0("check_tx_inputs failed, tx rejected"); tvc.m_verification_failed = true; return false; } TIME_MEASURE_FINISH_PD(check_inputs_time); - do_insert_transaction(tx, id, blob_size, kept_by_block, inputs_amount - outputs_amount, ch_inp_res ? max_used_block_id : null_hash, ch_inp_res ? max_used_block_height : 0); + do_insert_transaction(tx, id, blob_size, kept_by_block, tx_fee, ch_inp_res ? max_used_block_id : null_hash, ch_inp_res ? max_used_block_height : 0); TIME_MEASURE_FINISH_PD(tx_processing_time); tvc.m_added_to_pool = true; @@ -479,10 +465,10 @@ namespace currency should_be_spent_before_height -= CONFLICT_KEY_IMAGE_SPENT_DEPTH_TO_REMOVE_TX_FROM_POOL; for (auto& in : tx_entry.tx.vin) { - if (in.type() == typeid(txin_to_key)) + crypto::key_image ki = AUTO_VAL_INIT(ki); + if (get_key_image_from_txin_v(in, ki)) { // if at least one key image is spent deep enought -- remove such tx - const crypto::key_image& ki = boost::get(in).k_image; if (m_blockchain.have_tx_keyimg_as_spent(ki, should_be_spent_before_height)) { LOG_PRINT_L0("tx " << h << " is about to be removed from tx pool, reason: ki was spent in the blockchain before height " << should_be_spent_before_height << ", tx age: " << misc_utils::get_time_interval_string(tx_age)); @@ -709,16 +695,15 @@ namespace currency { for(const auto& in : tx.vin) { - if (in.type() == typeid(txin_to_key)) + crypto::key_image k_image = AUTO_VAL_INIT(k_image); + if (get_key_image_from_txin_v(in, k_image)) { - CHECKED_GET_SPECIFIC_VARIANT(in, const txin_to_key, tokey_in, true);//should never fail - if (have_tx_keyimg_as_spent(tokey_in.k_image)) + if (have_tx_keyimg_as_spent(k_image)) { if (p_spent_ki) - *p_spent_ki = tokey_in.k_image; + *p_spent_ki = k_image; return true; } - } } return false; @@ -729,13 +714,13 @@ namespace currency CRITICAL_REGION_LOCAL(m_key_images_lock); for(const auto& in : tx.vin) { - if (in.type() == typeid(txin_to_key)) + crypto::key_image k_image = AUTO_VAL_INIT(k_image); + if (get_key_image_from_txin_v(in, k_image)) { - const txin_to_key& tokey_in = boost::get(in); - auto& id_set = m_key_images[tokey_in.k_image]; + auto& id_set = m_key_images[k_image]; size_t sz_before = id_set.size(); id_set.insert(tx_id); - LOG_PRINT_L2("tx pool: key image added: " << tokey_in.k_image << ", from tx " << tx_id << ", counter: " << sz_before << " -> " << id_set.size()); + LOG_PRINT_L2("tx pool: key image added: " << k_image << ", from tx " << tx_id << ", counter: " << sz_before << " -> " << id_set.size()); } } return false; @@ -789,11 +774,10 @@ namespace currency CRITICAL_REGION_LOCAL(m_key_images_lock); for(const auto& in : tx.vin) { - if (in.type() == typeid(txin_to_key)) + crypto::key_image k_image = AUTO_VAL_INIT(k_image); + if (get_key_image_from_txin_v(in, k_image)) { - const txin_to_key& tokey_in = boost::get(in); - - auto it_map = epee::misc_utils::it_get_or_insert_value_initialized(m_key_images, tokey_in.k_image); + auto it_map = epee::misc_utils::it_get_or_insert_value_initialized(m_key_images, k_image); auto& id_set = it_map->second; size_t count_before = id_set.size(); auto it_set = id_set.find(tx_id); @@ -804,22 +788,22 @@ namespace currency if (id_set.size() == 0) m_key_images.erase(it_map); - LOG_PRINT_L2("tx pool: key image removed: " << tokey_in.k_image << ", from tx " << tx_id << ", counter: " << count_before << " -> " << count_after); + LOG_PRINT_L2("tx pool: key image removed: " << k_image << ", from tx " << tx_id << ", counter: " << count_before << " -> " << count_after); } } return false; } //--------------------------------------------------------------------------------- bool tx_memory_pool::get_key_images_from_tx_pool(key_image_cache& key_images) const - { - + { m_db_transactions.enumerate_items([&](uint64_t i, const crypto::hash& h, const tx_details &tx_entry) { for (auto& in : tx_entry.tx.vin) { - if (in.type() == typeid(txin_to_key)) + crypto::key_image k_image = AUTO_VAL_INIT(k_image); + if (get_key_image_from_txin_v(in, k_image)) { - key_images[boost::get(in).k_image].insert(h); + key_images[k_image].insert(h); } } return true; @@ -923,10 +907,10 @@ namespace currency LOCAL_READONLY_TRANSACTION(); for(size_t i = 0; i!= tx.vin.size(); i++) { - if (tx.vin[i].type() == typeid(txin_to_key)) + crypto::key_image k_image = AUTO_VAL_INIT(k_image); + if (get_key_image_from_txin_v(tx.vin[i], k_image)) { - CHECKED_GET_SPECIFIC_VARIANT(tx.vin[i], const txin_to_key, itk, false); - if (k_images.count(itk.k_image)) + if (k_images.count(k_image)) return true; } } @@ -935,13 +919,13 @@ namespace currency //--------------------------------------------------------------------------------- bool tx_memory_pool::append_key_images(std::unordered_set& k_images, const transaction& tx) { - for(size_t i = 0; i!= tx.vin.size(); i++) + for(size_t i = 0; i != tx.vin.size(); i++) { - if (tx.vin[i].type() == typeid(txin_to_key)) + crypto::key_image k_image = AUTO_VAL_INIT(k_image); + if (get_key_image_from_txin_v(tx.vin[i], k_image)) { - CHECKED_GET_SPECIFIC_VARIANT(tx.vin[i], const txin_to_key, itk, false); - auto i_res = k_images.insert(itk.k_image); - CHECK_AND_ASSERT_MES(i_res.second, false, "internal error: key images pool cache - inserted duplicate image in set: " << itk.k_image); + auto i_res = k_images.insert(k_image); + CHECK_AND_ASSERT_MES(i_res.second, false, "internal error: key images pool cache - inserted duplicate image in set: " << k_image); } } return true; @@ -1334,10 +1318,6 @@ namespace currency VARIANT_CASE_CONST(tx_out_bare, o) if (o.target.type() == typeid(txout_multisig)) result.push_back(ms_out_info({ get_multisig_out_id(tx, idx), idx, false })); - - VARIANT_CASE_CONST(tx_out_zarcanum, o) - //@#@ - VARIANT_CASE_THROW_ON_OTHER(); VARIANT_SWITCH_END(); ++idx; } @@ -1358,8 +1338,6 @@ namespace currency VARIANT_CASE_CONST(tx_out_bare, o) if (o.target.type() == typeid(txout_multisig) && get_multisig_out_id(tx, idx) == multisig_id) return true; - VARIANT_CASE_CONST(tx_out_zarcanum, o) - //@#@ VARIANT_SWITCH_END(); ++idx; }