From 769d5ef0e6213e55bf8940a03e2de8c8070a8f54 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 15 Aug 2015 12:37:23 +0100 Subject: [PATCH 1/5] blockchain: fix off by 1 in timestamp median calculations The height function apparently used to return the index of the last block, rather than the height of the chain. This now seems to be incorrect, judging the the code, so we remove the now wrong comment, as well as a couple +/- 1 adjustments which now cause the median calculation to differ from the original blockchain_storage version. --- src/cryptonote_core/blockchain.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index efac2d3d..acbd9f9a 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2218,7 +2218,7 @@ bool Blockchain::check_block_timestamp(const block& b) const } // if not enough blocks, no proper median yet, return true - if(m_db->height() < BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW + 1) + if(m_db->height() < BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW) { return true; } @@ -2227,9 +2227,7 @@ bool Blockchain::check_block_timestamp(const block& b) const auto h = m_db->height(); // need most recent 60 blocks, get index of first of those - // using +1 because BlockchainDB::height() returns the index of the top block, - // not the size of the blockchain (0-indexed) - size_t offset = h - BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW - 1; + size_t offset = h - BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW; for(;offset < h; ++offset) { timestamps.push_back(m_db->get_block_timestamp(offset)); From 3f9089a76741b40683e5243d1295b56bed48aac5 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 15 Aug 2015 18:42:29 +0100 Subject: [PATCH 2/5] blockchain: do not try to add a tx the pool when it was nor taken out This is an unintended difference from the old code. Though I don't think it can actually happen in practice with the current take_tx implementation. --- src/cryptonote_core/blockchain.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index acbd9f9a..37e9053c 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2365,6 +2365,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& uint64_t t_pool = 0; uint64_t t_dblspnd = 0; uint64_t t_cc; + bool add_tx_to_pool = false; TIME_MEASURE_FINISH(t3); int tx_index = 0; @@ -2430,6 +2431,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& add_block_as_invalid(bl, id); LOG_PRINT_L1("Block with id " << id << " added as invalid because of wrong inputs in transactions"); bvc.m_verifivation_failed = true; + add_tx_to_pool = true; break; } } @@ -2445,6 +2447,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& add_block_as_invalid(bl, id); LOG_PRINT_L1("Block with id " << id << " added as invalid because of wrong inputs in transactions"); bvc.m_verifivation_failed = true; + add_tx_to_pool = true; break; } } @@ -2503,7 +2506,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& // if we failed for any reason to verify the block, return taken // transactions to the tx_pool. - if (bvc.m_verifivation_failed || !add_success) + if ((bvc.m_verifivation_failed && add_tx_to_pool) || !add_success) { // return taken transactions to transaction pool for (auto& tx : txs) From 4a443775e87d7447c12d313575f18f9c86bb4c4b Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 15 Aug 2015 18:44:31 +0100 Subject: [PATCH 3/5] blockchain: remove dead code --- src/cryptonote_core/blockchain.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 37e9053c..4d1eecfc 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2470,7 +2470,6 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& } TIME_MEASURE_FINISH(vmt); - block_extended_info bei = boost::value_initialized(); size_t block_size; difficulty_type cumulative_difficulty; From 73d42a75d4e441ff0a3d536beedfa3adb82479d8 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 15 Aug 2015 18:44:56 +0100 Subject: [PATCH 4/5] blockchain: update cumulative size after block addition Block addition can fail, and the old code would not update the cumulative size in that case. --- src/cryptonote_core/blockchain.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 4d1eecfc..695be9f4 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2480,8 +2480,6 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& if(m_db->height()) cumulative_difficulty += m_db->get_block_cumulative_difficulty(m_db->height() - 1); - update_next_cumulative_size_limit(); - TIME_MEASURE_FINISH(block_processing_time); if(precomputed) block_processing_time += m_fake_pow_calc_time; @@ -2521,6 +2519,8 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& TIME_MEASURE_FINISH(addblock); + update_next_cumulative_size_limit(); + LOG_PRINT_L1("+++++ BLOCK SUCCESSFULLY ADDED" << std::endl << "id:\t" << id << std::endl << "PoW:\t" << proof_of_work << std::endl << "HEIGHT " << new_height << ", difficulty:\t" << current_diffic << std::endl << "block reward: " << print_money(fee_summary + base_reward) << "(" << print_money(base_reward) << " + " << print_money(fee_summary) << "), coinbase_blob_size: " << coinbase_blob_size << ", cumulative size: " << cumulative_block_size << ", " << block_processing_time << "(" << target_calculating_time << "/" << longhash_calculating_time << ")ms"); if(m_show_time_stats) { From 378d004b375ec6f6be594f15d8bccd54821df4ed Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 15 Aug 2015 18:46:19 +0100 Subject: [PATCH 5/5] blockchain: mark two places where the new code differs from the old And I'd like a comment from tewinget or someone else --- src/cryptonote_core/blockchain.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 695be9f4..be0a4a6d 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2368,6 +2368,8 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& bool add_tx_to_pool = false; TIME_MEASURE_FINISH(t3); +// XXX old code adds miner tx here + int tx_index = 0; // Iterate over the block's transaction hashes, grabbing each // from the tx_pool and validating them. Each is then added @@ -2379,6 +2381,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& uint64_t fee = 0; TIME_MEASURE_START(aa); +// XXX old code does not check whether tx exists if (m_db->tx_exists(tx_id)) { LOG_PRINT_L1("Block with id: " << id << " attempting to add transaction already in blockchain with id: " << tx_id);