From 88c85c18e0173f479470842fbe49626cdc86e7d1 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 23 Nov 2018 13:05:48 +0000 Subject: [PATCH] cryptonote: avoid double parsing blocks when syncing --- src/cryptonote_core/blockchain.cpp | 13 +++--------- src/cryptonote_core/cryptonote_core.cpp | 13 ++++++++++-- .../cryptonote_protocol_handler.inl | 21 ++++++++++++++++--- tests/core_proxy/core_proxy.cpp | 2 +- tests/core_proxy/core_proxy.h | 4 ++-- tests/core_tests/chaingen.h | 4 ++-- tests/unit_tests/ban.cpp | 4 ++-- 7 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 2890af628..2c1b9d447 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -3889,11 +3889,9 @@ bool Blockchain::update_next_cumulative_weight_limit(uint64_t *long_term_effecti return true; } //------------------------------------------------------------------ -bool Blockchain::add_new_block(const block& bl_, block_verification_context& bvc) +bool Blockchain::add_new_block(const block& bl, block_verification_context& bvc) { LOG_PRINT_L3("Blockchain::" << __func__); - //copy block here to let modify block.target - block bl = bl_; crypto::hash id = get_block_hash(bl); CRITICAL_REGION_LOCAL(m_tx_pool);//to avoid deadlock lets lock tx_pool for whole add/reorganize process CRITICAL_REGION_LOCAL1(m_blockchain_lock); @@ -4296,14 +4294,12 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vector pblocks; - prepare_handle_incoming_blocks(blocks, pblocks); + if (!prepare_handle_incoming_blocks(blocks, pblocks)) + { + MERROR("Block found, but failed to prepare to add"); + m_miner.resume(); + return false; + } m_blockchain_storage.add_new_block(b, bvc); cleanup_handle_incoming_blocks(true); //anyway - update miner template @@ -1385,7 +1390,11 @@ namespace cryptonote bool core::prepare_handle_incoming_blocks(const std::vector &blocks_entry, std::vector &blocks) { m_incoming_tx_lock.lock(); - m_blockchain_storage.prepare_handle_incoming_blocks(blocks_entry, blocks); + if (!m_blockchain_storage.prepare_handle_incoming_blocks(blocks_entry, blocks)) + { + cleanup_handle_incoming_blocks(false); + return false; + } return true; } diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index 9c2e1df88..c8b43fb91 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -419,7 +419,13 @@ namespace cryptonote std::vector blocks; blocks.push_back(arg.b); std::vector pblocks; - m_core.prepare_handle_incoming_blocks(blocks, pblocks); + if (!m_core.prepare_handle_incoming_blocks(blocks, pblocks)) + { + LOG_PRINT_CCONTEXT_L1("Block verification failed: prepare_handle_incoming_blocks failed, dropping connection"); + drop_connection(context, false, false); + m_core.resume_mine(); + return 1; + } for(auto tx_blob_it = arg.b.txs.begin(); tx_blob_it!=arg.b.txs.end();tx_blob_it++) { cryptonote::tx_verification_context tvc = AUTO_VAL_INIT(tvc); @@ -699,7 +705,12 @@ namespace cryptonote std::vector blocks; blocks.push_back(b); std::vector pblocks; - m_core.prepare_handle_incoming_blocks(blocks, pblocks); + if (!m_core.prepare_handle_incoming_blocks(blocks, pblocks)) + { + LOG_PRINT_CCONTEXT_L0("Failure in prepare_handle_incoming_blocks"); + m_core.resume_mine(); + return 1; + } block_verification_context bvc = boost::value_initialized(); m_core.handle_incoming_block(arg.b.block, pblocks.empty() ? NULL : &pblocks[0], bvc); // got block from handle_notify_new_block @@ -1177,7 +1188,11 @@ namespace cryptonote } std::vector pblocks; - m_core.prepare_handle_incoming_blocks(blocks, pblocks); + if (!m_core.prepare_handle_incoming_blocks(blocks, pblocks)) + { + LOG_ERROR_CCONTEXT("Failure in prepare_handle_incoming_blocks"); + return 1; + } if (!pblocks.empty() && pblocks.size() != blocks.size()) { m_core.cleanup_handle_incoming_blocks(); diff --git a/tests/core_proxy/core_proxy.cpp b/tests/core_proxy/core_proxy.cpp index 17e552714..583111517 100644 --- a/tests/core_proxy/core_proxy.cpp +++ b/tests/core_proxy/core_proxy.cpp @@ -197,7 +197,7 @@ bool tests::proxy_core::handle_incoming_txs(const std::vector& tx_blob return true; } -bool tests::proxy_core::handle_incoming_block(const cryptonote::blobdata& block_blob, cryptonote::block_verification_context& bvc, bool update_miner_blocktemplate) { +bool tests::proxy_core::handle_incoming_block(const cryptonote::blobdata& block_blob, const cryptonote::block *block_, cryptonote::block_verification_context& bvc, bool update_miner_blocktemplate) { block b = AUTO_VAL_INIT(b); if(!parse_and_validate_block_from_blob(block_blob, b)) { diff --git a/tests/core_proxy/core_proxy.h b/tests/core_proxy/core_proxy.h index 7888540cc..33a5b4d13 100644 --- a/tests/core_proxy/core_proxy.h +++ b/tests/core_proxy/core_proxy.h @@ -77,7 +77,7 @@ namespace tests void get_blockchain_top(uint64_t& height, crypto::hash& top_id); bool handle_incoming_tx(const cryptonote::blobdata& tx_blob, cryptonote::tx_verification_context& tvc, bool keeped_by_block, bool relayed, bool do_not_relay); bool handle_incoming_txs(const std::vector& tx_blobs, std::vector& tvc, bool keeped_by_block, bool relayed, bool do_not_relay); - bool handle_incoming_block(const cryptonote::blobdata& block_blob, cryptonote::block_verification_context& bvc, bool update_miner_blocktemplate = true); + bool handle_incoming_block(const cryptonote::blobdata& block_blob, const cryptonote::block *block, cryptonote::block_verification_context& bvc, bool update_miner_blocktemplate = true); void pause_mine(){} void resume_mine(){} bool on_idle(){return true;} @@ -86,7 +86,7 @@ namespace tests cryptonote::Blockchain &get_blockchain_storage() { throw std::runtime_error("Called invalid member function: please never call get_blockchain_storage on the TESTING class proxy_core."); } bool get_test_drop_download() {return true;} bool get_test_drop_download_height() {return true;} - bool prepare_handle_incoming_blocks(const std::vector &blocks) { return true; } + bool prepare_handle_incoming_blocks(const std::vector &blocks_entry, std::vector &blocks) { return true; } bool cleanup_handle_incoming_blocks(bool force_sync = false) { return true; } uint64_t get_target_blockchain_height() const { return 1; } size_t get_block_sync_size(uint64_t height) const { return BLOCKS_SYNCHRONIZING_DEFAULT_COUNT; } diff --git a/tests/core_tests/chaingen.h b/tests/core_tests/chaingen.h index 907b32bcd..82c480163 100644 --- a/tests/core_tests/chaingen.h +++ b/tests/core_tests/chaingen.h @@ -388,7 +388,7 @@ public: log_event("cryptonote::block"); cryptonote::block_verification_context bvc = AUTO_VAL_INIT(bvc); - m_c.handle_incoming_block(t_serializable_object_to_blob(b), bvc); + m_c.handle_incoming_block(t_serializable_object_to_blob(b), &b, bvc); bool r = check_block_verification_context(bvc, m_ev_index, b, m_validator); CHECK_AND_NO_ASSERT_MES(r, false, "block verification context check failed"); return r; @@ -411,7 +411,7 @@ public: log_event("serialized_block"); cryptonote::block_verification_context bvc = AUTO_VAL_INIT(bvc); - m_c.handle_incoming_block(sr_block.data, bvc); + m_c.handle_incoming_block(sr_block.data, NULL, bvc); cryptonote::block blk; std::stringstream ss; diff --git a/tests/unit_tests/ban.cpp b/tests/unit_tests/ban.cpp index 1e764c83e..ccfcfc15a 100644 --- a/tests/unit_tests/ban.cpp +++ b/tests/unit_tests/ban.cpp @@ -56,7 +56,7 @@ public: void get_blockchain_top(uint64_t& height, crypto::hash& top_id)const{height=0;top_id=crypto::null_hash;} bool handle_incoming_tx(const cryptonote::blobdata& tx_blob, cryptonote::tx_verification_context& tvc, bool keeped_by_block, bool relayed, bool do_not_relay) { return true; } bool handle_incoming_txs(const std::vector& tx_blob, std::vector& tvc, bool keeped_by_block, bool relayed, bool do_not_relay) { return true; } - bool handle_incoming_block(const cryptonote::blobdata& block_blob, cryptonote::block_verification_context& bvc, bool update_miner_blocktemplate = true) { return true; } + bool handle_incoming_block(const cryptonote::blobdata& block_blob, const cryptonote::block *block, cryptonote::block_verification_context& bvc, bool update_miner_blocktemplate = true) { return true; } void pause_mine(){} void resume_mine(){} bool on_idle(){return true;} @@ -65,7 +65,7 @@ public: cryptonote::blockchain_storage &get_blockchain_storage() { throw std::runtime_error("Called invalid member function: please never call get_blockchain_storage on the TESTING class test_core."); } bool get_test_drop_download() const {return true;} bool get_test_drop_download_height() const {return true;} - bool prepare_handle_incoming_blocks(const std::vector &blocks) { return true; } + bool prepare_handle_incoming_blocks(const std::vector &blocks_entry, std::vector &blocks) { return true; } bool cleanup_handle_incoming_blocks(bool force_sync = false) { return true; } uint64_t get_target_blockchain_height() const { return 1; } size_t get_block_sync_size(uint64_t height) const { return BLOCKS_SYNCHRONIZING_DEFAULT_COUNT; }