From 5500e943dd4d56010c7365c85d3df8653d8cec1b Mon Sep 17 00:00:00 2001 From: sowle Date: Sat, 4 May 2024 02:33:05 +0200 Subject: [PATCH] did some refactoring around validate_miner_transaction() code simplified fixed an issue with already_generated_coins not taking block size penalty into consideration --- src/currency_core/blockchain_storage.cpp | 65 ++++++++++++------------ src/currency_core/blockchain_storage.h | 3 +- tests/core_tests/block_reward.cpp | 7 ++- 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/src/currency_core/blockchain_storage.cpp b/src/currency_core/blockchain_storage.cpp index d10918d6..5941c0fe 100644 --- a/src/currency_core/blockchain_storage.cpp +++ b/src/currency_core/blockchain_storage.cpp @@ -1375,50 +1375,44 @@ bool blockchain_storage::prevalidate_miner_transaction(const block& b, uint64_t return true; } //------------------------------------------------------------------ -bool blockchain_storage::validate_miner_transaction(const block& b, - size_t cumulative_block_size, - uint64_t fee, - uint64_t& base_reward, - const boost::multiprecision::uint128_t& already_generated_coins) const +bool blockchain_storage::calculate_block_reward_for_next_top_block(size_t next_block_cumulative_size, uint64_t& block_reward_without_fee) const { CRITICAL_REGION_LOCAL(m_read_lock); std::vector last_blocks_sizes; get_last_n_blocks_sizes(last_blocks_sizes, CURRENCY_REWARD_BLOCKS_WINDOW); size_t blocks_size_median = misc_utils::median(last_blocks_sizes); - if (!get_block_reward(is_pos_block(b), blocks_size_median, cumulative_block_size, already_generated_coins, base_reward, get_block_height(b))) - { - LOG_PRINT_L0("block size " << cumulative_block_size << " is bigger than allowed for this blockchain"); - return false; - } - - uint64_t block_reward = base_reward; - // before HF4: add tx fee to the block reward; after HF4: burn it - if (b.miner_tx.version < TRANSACTION_VERSION_POST_HF4) + LOG_PRINT_MAGENTA("blocks_size_median = " << blocks_size_median, LOG_LEVEL_2); + block_reward_without_fee = get_block_reward(get_top_block_height() + 1, blocks_size_median, next_block_cumulative_size); + CHECK_AND_ASSERT_MES(block_reward_without_fee != 0, false, "block size " << next_block_cumulative_size << " is bigger than allowed for this blockchain, blocks_size_median: " << blocks_size_median); + return true; +} +//------------------------------------------------------------------ +bool blockchain_storage::validate_miner_transaction(const transaction& miner_tx, + uint64_t fee, + uint64_t block_reward_without_fee) const +{ + uint64_t block_reward = block_reward_without_fee; + // before HF4: add tx fee to the block reward; after HF4: burn fees, so they don't count in block_reward + if (miner_tx.version < TRANSACTION_VERSION_POST_HF4) { block_reward += fee; } - crypto::hash tx_id_for_post_hf4_era = b.miner_tx.version > TRANSACTION_VERSION_PRE_HF4 ? get_transaction_hash(b.miner_tx) : null_hash; - if (!check_tx_balance(b.miner_tx, tx_id_for_post_hf4_era, block_reward)) + crypto::hash tx_id_for_post_hf4_era = miner_tx.version > TRANSACTION_VERSION_PRE_HF4 ? get_transaction_hash(miner_tx) : null_hash; // used in the input context for the proofs for txs ver >= 2 + if (!check_tx_balance(miner_tx, tx_id_for_post_hf4_era, block_reward)) { - LOG_ERROR("coinbase transaction balance check failed. Block reward is " << print_money_brief(block_reward) << "(" << print_money(base_reward) << "+" << print_money(fee) - << ", blocks_size_median = " << blocks_size_median - << ", cumulative_block_size = " << cumulative_block_size - << ", fee = " << fee - << ", already_generated_coins = " << already_generated_coins - << "), tx:"); - LOG_PRINT_L0(currency::obj_to_json_str(b.miner_tx)); + LOG_ERROR("coinbase transaction balance check failed. Block reward is " << print_money_brief(block_reward) << " (" << print_money(block_reward_without_fee) << "+" << print_money(fee) << "), tx:"); + LOG_PRINT_L0(currency::obj_to_json_str(miner_tx)); return false; } - if (!verify_asset_surjection_proof(b.miner_tx, tx_id_for_post_hf4_era)) + if (!verify_asset_surjection_proof(miner_tx, tx_id_for_post_hf4_era)) { LOG_ERROR("asset surjection proof verification failed for miner tx"); return false; } - LOG_PRINT_MAGENTA("Mining tx verification ok, blocks_size_median = " << blocks_size_median, LOG_LEVEL_2); return true; } //------------------------------------------------------------------ @@ -6579,15 +6573,19 @@ bool blockchain_storage::handle_block_to_main_chain(const block& bl, const crypt return false; } - boost::multiprecision::uint128_t already_generated_coins = m_db_blocks.size() ? m_db_blocks.back()->already_generated_coins:0; - uint64_t base_reward = get_base_block_reward(height); + uint64_t block_reward_without_fee = 0; + if (!calculate_block_reward_for_next_top_block(cumulative_block_size, block_reward_without_fee)) + { + LOG_ERROR("calculate_block_reward_for_next_top_block filed"); + purge_block_data_from_blockchain(bl, tx_processed_count); + bvc.m_verification_failed = true; + return false; + } if (!m_is_in_checkpoint_zone) { - // validate_miner_transaction will check balance proof and asset surjection proof - // and, as a side effect, it MAY recalculate base_reward, consider redisign, TODO -- sowle TIME_MEASURE_START_PD(validate_miner_transaction_time); - if (!validate_miner_transaction(bl, cumulative_block_size, fee_summary, base_reward, already_generated_coins)) // TODO @#@# base_reward will be calculated once again, consider refactoring + if (!validate_miner_transaction(bl.miner_tx, fee_summary, block_reward_without_fee)) { LOG_PRINT_L0("Block with id: " << id << " have wrong miner transaction"); @@ -6678,6 +6676,7 @@ bool blockchain_storage::handle_block_to_main_chain(const block& bl, const crypt ////////////////////////////////////////////////////////////////////////// //etc + boost::multiprecision::uint128_t already_generated_coins = m_db_blocks.size() ? m_db_blocks.back()->already_generated_coins : 0; if (already_generated_coins < burned_coins) { LOG_ERROR("Condition failed: already_generated_coins(" << already_generated_coins << ") >= burned_coins(" << burned_coins << ")"); @@ -6685,7 +6684,7 @@ bool blockchain_storage::handle_block_to_main_chain(const block& bl, const crypt bvc.m_verification_failed = true; return false; } - bei.already_generated_coins = already_generated_coins - burned_coins + base_reward; + bei.already_generated_coins = already_generated_coins - burned_coins + block_reward_without_fee; if (bei.bl.miner_tx.version >= TRANSACTION_VERSION_POST_HF4) { bei.already_generated_coins -= fee_summary; @@ -6763,9 +6762,9 @@ bool blockchain_storage::handle_block_to_main_chain(const block& bl, const crypt timestamp_str_entry << ", block ts: " << bei.bl.timestamp << " (diff: " << std::showpos << ts_diff << "s)"; } if(bei.bl.miner_tx.version >= TRANSACTION_VERSION_POST_HF4) - block_reward_entry << "block reward: " << print_money_brief(base_reward) << ", fee burnt: " << print_money_brief(fee_summary); + block_reward_entry << "block reward: " << print_money_brief(block_reward_without_fee) << ", fee burnt: " << print_money_brief(fee_summary); else - block_reward_entry << "block reward: " << print_money_brief(base_reward + fee_summary) << " (" << print_money_brief(base_reward) << " + " << print_money_brief(fee_summary) << ")"; + block_reward_entry << "block reward: " << print_money_brief(block_reward_without_fee + fee_summary) << " (" << print_money_brief(block_reward_without_fee) << " + " << print_money_brief(fee_summary) << ")"; //explanation of this code will be provided later with public announce set_lost_tx_unmixable_for_height(bei.height); diff --git a/src/currency_core/blockchain_storage.h b/src/currency_core/blockchain_storage.h index babebf89..19bd0205 100644 --- a/src/currency_core/blockchain_storage.h +++ b/src/currency_core/blockchain_storage.h @@ -340,7 +340,8 @@ namespace currency uint64_t get_last_timestamps_check_window_median() const; uint64_t get_last_n_blocks_timestamps_median(size_t n) const; bool prevalidate_alias_info(const transaction& tx, const extra_alias_entry& eae); - bool validate_miner_transaction(const block& b, size_t cumulative_block_size, uint64_t fee, uint64_t& base_reward, const boost::multiprecision::uint128_t& already_generated_coins) const; + bool calculate_block_reward_for_next_top_block(size_t next_block_cumulative_size, uint64_t& block_reward_without_fee) const; + bool validate_miner_transaction(const transaction& miner_tx, uint64_t fee, uint64_t block_reward_without_fee) const; performnce_data& get_performnce_data()const; bool validate_instance(const std::string& path); bool is_tx_expired(const transaction& tx) const; diff --git a/tests/core_tests/block_reward.cpp b/tests/core_tests/block_reward.cpp index f4672ff1..0aff096f 100644 --- a/tests/core_tests/block_reward.cpp +++ b/tests/core_tests/block_reward.cpp @@ -91,13 +91,16 @@ bool block_template_against_txs_size::c1(currency::core& c, size_t ev_index, con CHECK_AND_ASSERT_MES(r, false, "create_block_template failed, txs_total_size = " << txs_total_size); CHECK_AND_ASSERT_MES(height == top_block_height + 1, false, "Incorrect height: " << height << ", expected: " << top_block_height + 1 << ", txs_total_size = " << txs_total_size); - uint64_t base_reward = 0; + uint64_t block_reward_without_fee = 0; size_t cumulative_block_size = txs_total_size; size_t coinbase_blob_size = get_object_blobsize(b.miner_tx); if (coinbase_blob_size > CURRENCY_COINBASE_BLOB_RESERVED_SIZE) cumulative_block_size += coinbase_blob_size; - r = bcs.validate_miner_transaction(b, cumulative_block_size, g_block_txs_fee, base_reward, bcs.total_coins()); + + r = bcs.calculate_block_reward_for_next_top_block(cumulative_block_size, block_reward_without_fee); + CHECK_AND_ASSERT_MES(r, false, "calculate_block_reward_for_next_top_block failed"); + r = bcs.validate_miner_transaction(b.miner_tx, g_block_txs_fee, block_reward_without_fee); CHECK_AND_ASSERT_MES(r, false, "validate_miner_transaction failed, txs_total_size = " << txs_total_size); uint64_t generated_coins = get_outs_money_amount(b.miner_tx) - (is_pos != 0 ? boost::get(b.miner_tx.vout.back()).amount : 0) - g_block_txs_fee / 2;