From b044d03a51e1dc64bebe2461813e0cfc50f71b0d Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 11 Nov 2018 14:51:03 +0000 Subject: Avoid repeated (de)serialization when syncing --- src/cryptonote_core/blockchain.cpp | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'src/cryptonote_core/blockchain.cpp') diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 9f1f376f3..a15fda093 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -3345,7 +3345,7 @@ bool Blockchain::check_block_timestamp(const block& b, uint64_t& median_ts) cons return check_block_timestamp(timestamps, b, median_ts); } //------------------------------------------------------------------ -void Blockchain::return_tx_to_pool(std::vector &txs) +void Blockchain::return_tx_to_pool(std::vector> &txs) { uint8_t version = get_current_hard_fork_version(); for (auto& tx : txs) @@ -3356,9 +3356,11 @@ void Blockchain::return_tx_to_pool(std::vector &txs) // that might not be always true. Unlikely though, and always relaying // these again might cause a spike of traffic as many nodes re-relay // all the transactions in a popped block when a reorg happens. - if (!m_tx_pool.add_tx(tx, tvc, true, true, false, version)) + const size_t weight = get_transaction_weight(tx.first, tx.second.size()); + const crypto::hash tx_hash = get_transaction_hash(tx.first); + if (!m_tx_pool.add_tx(tx.first, tx_hash, tx.second, weight, tvc, true, true, false, version)) { - MERROR("Failed to return taken transaction with hash: " << get_transaction_hash(tx) << " to tx_pool"); + MERROR("Failed to return taken transaction with hash: " << get_transaction_hash(tx.first) << " to tx_pool"); } } } @@ -3371,11 +3373,12 @@ bool Blockchain::flush_txes_from_pool(const std::vector &txids) for (const auto &txid: txids) { cryptonote::transaction tx; + cryptonote::blobdata txblob; size_t tx_weight; uint64_t fee; bool relayed, do_not_relay, double_spend_seen; MINFO("Removing txid " << txid << " from the pool"); - if(m_tx_pool.have_tx(txid) && !m_tx_pool.take_tx(txid, tx, tx_weight, fee, relayed, do_not_relay, double_spend_seen)) + if(m_tx_pool.have_tx(txid) && !m_tx_pool.take_tx(txid, tx, txblob, tx_weight, fee, relayed, do_not_relay, double_spend_seen)) { MERROR("Failed to remove txid " << txid << " from the pool"); res = false; @@ -3538,7 +3541,7 @@ leave: size_t coinbase_weight = get_transaction_weight(bl.miner_tx); size_t cumulative_block_weight = coinbase_weight; - std::vector txs; + std::vector> txs; key_images_container keys; uint64_t fee_summary = 0; @@ -3558,6 +3561,7 @@ leave: for (const crypto::hash& tx_id : bl.tx_hashes) { transaction tx; + blobdata txblob; size_t tx_weight = 0; uint64_t fee = 0; bool relayed = false, do_not_relay = false, double_spend_seen = false; @@ -3577,7 +3581,7 @@ leave: TIME_MEASURE_START(bb); // get transaction with hash from tx_pool - if(!m_tx_pool.take_tx(tx_id, tx, tx_weight, fee, relayed, do_not_relay, double_spend_seen)) + if(!m_tx_pool.take_tx(tx_id, tx, txblob, tx_weight, fee, relayed, do_not_relay, double_spend_seen)) { MERROR_VER("Block with id: " << id << " has at least one unknown transaction with id: " << tx_id); bvc.m_verifivation_failed = true; @@ -3590,7 +3594,7 @@ leave: // add the transaction to the temp list of transactions, so we can either // store the list of transactions all at once or return the ones we've // taken from the tx_pool back to it if the block fails verification. - txs.push_back(tx); + txs.push_back(std::make_pair(tx, std::move(txblob))); TIME_MEASURE_START(dd); // FIXME: the storage should not be responsible for validation. @@ -3691,7 +3695,7 @@ leave: try { uint64_t long_term_block_weight = get_next_long_term_block_weight(block_weight); - new_height = m_db->add_block(bl, block_weight, long_term_block_weight, cumulative_difficulty, already_generated_coins, txs); + new_height = m_db->add_block(std::make_pair(bl, block_to_blob(bl)), block_weight, long_term_block_weight, cumulative_difficulty, already_generated_coins, txs); } catch (const KEY_IMAGE_EXISTS& e) { @@ -4770,10 +4774,11 @@ void Blockchain::load_compiled_in_block_hashes(const GetCheckpointsCallback& get uint64_t fee; bool relayed, do_not_relay, double_spend_seen; transaction pool_tx; + blobdata txblob; for(const transaction &tx : txs) { crypto::hash tx_hash = get_transaction_hash(tx); - m_tx_pool.take_tx(tx_hash, pool_tx, tx_weight, fee, relayed, do_not_relay, double_spend_seen); + m_tx_pool.take_tx(tx_hash, pool_tx, txblob, tx_weight, fee, relayed, do_not_relay, double_spend_seen); } } } -- cgit v1.2.3 From 07d655e438e1b1e05426a5c75627f5d8c63bbc72 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 20 Nov 2018 10:58:41 +0000 Subject: blockchain: avoid duplicate block hash computation --- src/cryptonote_core/blockchain.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/cryptonote_core/blockchain.cpp') diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index a15fda093..0379ab760 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -3474,11 +3474,10 @@ leave: #if defined(PER_BLOCK_CHECKPOINT) if (m_db->height() < m_blocks_hash_check.size()) { - auto hash = get_block_hash(bl); const auto &expected_hash = m_blocks_hash_check[m_db->height()]; if (expected_hash != crypto::null_hash) { - if (memcmp(&hash, &expected_hash, sizeof(hash)) != 0) + if (memcmp(&id, &expected_hash, sizeof(hash)) != 0) { MERROR_VER("Block with id is INVALID: " << id << ", expected " << expected_hash); bvc.m_verifivation_failed = true; -- cgit v1.2.3 From 9827880877299ff8d30b90302a10aeface7598c4 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 20 Nov 2018 16:24:12 +0000 Subject: blockchain: avoid pointless transaction copy and temporary --- src/cryptonote_core/blockchain.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'src/cryptonote_core/blockchain.cpp') diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 0379ab760..73d0e160d 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -3559,7 +3559,7 @@ leave: txs.reserve(bl.tx_hashes.size()); for (const crypto::hash& tx_id : bl.tx_hashes) { - transaction tx; + transaction tx_tmp; blobdata txblob; size_t tx_weight = 0; uint64_t fee = 0; @@ -3580,7 +3580,7 @@ leave: TIME_MEASURE_START(bb); // get transaction with hash from tx_pool - if(!m_tx_pool.take_tx(tx_id, tx, txblob, tx_weight, fee, relayed, do_not_relay, double_spend_seen)) + if(!m_tx_pool.take_tx(tx_id, tx_tmp, txblob, tx_weight, fee, relayed, do_not_relay, double_spend_seen)) { MERROR_VER("Block with id: " << id << " has at least one unknown transaction with id: " << tx_id); bvc.m_verifivation_failed = true; @@ -3593,7 +3593,8 @@ leave: // add the transaction to the temp list of transactions, so we can either // store the list of transactions all at once or return the ones we've // taken from the tx_pool back to it if the block fails verification. - txs.push_back(std::make_pair(tx, std::move(txblob))); + txs.push_back(std::make_pair(std::move(tx_tmp), std::move(txblob))); + transaction &tx = txs.back().first; TIME_MEASURE_START(dd); // FIXME: the storage should not be responsible for validation. -- cgit v1.2.3 From 79b4e9f3779796d9233e3e169f5c1086a49717dd Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 20 Nov 2018 20:19:39 +0000 Subject: save some database calls when getting top block hash and height --- src/blockchain_db/blockchain_db.h | 4 +- src/blockchain_db/lmdb/db_lmdb.cpp | 4 +- src/blockchain_db/lmdb/db_lmdb.h | 2 +- src/blockchain_db/testdb.h | 2 +- src/cryptonote_core/blockchain.cpp | 60 +++++++++++++++++------------ tests/block_weight/block_weight.cpp | 4 +- tests/unit_tests/long_term_block_weight.cpp | 4 +- 7 files changed, 49 insertions(+), 31 deletions(-) (limited to 'src/cryptonote_core/blockchain.cpp') diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h index 4d0b8935f..ed13de5b5 100644 --- a/src/blockchain_db/blockchain_db.h +++ b/src/blockchain_db/blockchain_db.h @@ -1052,9 +1052,11 @@ public: * * The subclass should return the hash of the most recent block * + * @param block_height if non NULL, returns the height of that block (ie, the blockchain height minus 1) + * * @return the top block's hash */ - virtual crypto::hash top_block_hash() const = 0; + virtual crypto::hash top_block_hash(uint64_t *block_height = NULL) const = 0; /** * @brief fetch the top block diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index e7ed75c83..c6971c613 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -2582,11 +2582,13 @@ std::vector BlockchainLMDB::get_hashes_range(const uint64_t& h1, c return v; } -crypto::hash BlockchainLMDB::top_block_hash() const +crypto::hash BlockchainLMDB::top_block_hash(uint64_t *block_height) const { LOG_PRINT_L3("BlockchainLMDB::" << __func__); check_open(); uint64_t m_height = height(); + if (block_height) + *block_height = m_height - 1; if (m_height != 0) { return get_block_hash_from_height(m_height - 1); diff --git a/src/blockchain_db/lmdb/db_lmdb.h b/src/blockchain_db/lmdb/db_lmdb.h index 2f28c48e2..82016c17a 100644 --- a/src/blockchain_db/lmdb/db_lmdb.h +++ b/src/blockchain_db/lmdb/db_lmdb.h @@ -233,7 +233,7 @@ public: virtual std::vector get_hashes_range(const uint64_t& h1, const uint64_t& h2) const; - virtual crypto::hash top_block_hash() const; + virtual crypto::hash top_block_hash(uint64_t *block_height = NULL) const; virtual block get_top_block() const; diff --git a/src/blockchain_db/testdb.h b/src/blockchain_db/testdb.h index dcaee79b7..ac1849b5f 100644 --- a/src/blockchain_db/testdb.h +++ b/src/blockchain_db/testdb.h @@ -79,7 +79,7 @@ public: virtual crypto::hash get_block_hash_from_height(const uint64_t& height) const { return crypto::hash(); } virtual std::vector get_blocks_range(const uint64_t& h1, const uint64_t& h2) const { return std::vector(); } virtual std::vector get_hashes_range(const uint64_t& h1, const uint64_t& h2) const { return std::vector(); } - virtual crypto::hash top_block_hash() const { return crypto::hash(); } + virtual crypto::hash top_block_hash(uint64_t *block_height = NULL) const { if (block_height) *block_height = 0; return crypto::hash(); } virtual cryptonote::block get_top_block() const { return cryptonote::block(); } virtual uint64_t height() const { return 1; } virtual bool tx_exists(const crypto::hash& h) const { return false; } diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 73d0e160d..941be234f 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -468,8 +468,8 @@ bool Blockchain::init(BlockchainDB* db, const network_type nettype, bool offline uint64_t num_popped_blocks = 0; while (!m_db->is_read_only()) { - const uint64_t top_height = m_db->height() - 1; - const crypto::hash top_id = m_db->top_block_hash(); + uint64_t top_height; + const crypto::hash top_id = m_db->top_block_hash(&top_height); const block top_block = m_db->get_top_block(); const uint8_t ideal_hf_version = get_ideal_hard_fork_version(top_height); if (ideal_hf_version <= 1 || ideal_hf_version == top_block.major_version) @@ -509,7 +509,9 @@ bool Blockchain::init(BlockchainDB* db, const network_type nettype, bool offline { m_timestamps_and_difficulties_height = 0; m_hardfork->reorganize_from_chain_height(get_current_blockchain_height()); - m_tx_pool.on_blockchain_dec(m_db->height()-1, get_tail_id()); + uint64_t top_block_height; + crypto::hash top_block_hash = get_tail_id(top_block_height); + m_tx_pool.on_blockchain_dec(top_block_height, top_block_hash); } if (test_options && test_options->long_term_block_weight_window) @@ -702,7 +704,9 @@ block Blockchain::pop_block_from_blockchain() m_check_txin_table.clear(); CHECK_AND_ASSERT_THROW_MES(update_next_cumulative_weight_limit(), "Error updating next cumulative weight limit"); - m_tx_pool.on_blockchain_dec(m_db->height()-1, get_tail_id()); + uint64_t top_block_height; + crypto::hash top_block_hash = get_tail_id(top_block_height); + m_tx_pool.on_blockchain_dec(top_block_height, top_block_hash); invalidate_block_template_cache(); return popped_block; @@ -729,8 +733,7 @@ crypto::hash Blockchain::get_tail_id(uint64_t& height) const { LOG_PRINT_L3("Blockchain::" << __func__); CRITICAL_REGION_LOCAL(m_blockchain_lock); - height = m_db->height() - 1; - return get_tail_id(); + return m_db->top_block_hash(&height); } //------------------------------------------------------------------ crypto::hash Blockchain::get_tail_id() const @@ -891,8 +894,9 @@ difficulty_type Blockchain::get_difficulty_for_next_block() CRITICAL_REGION_LOCAL(m_blockchain_lock); std::vector timestamps; std::vector difficulties; - auto height = m_db->height(); - top_hash = get_tail_id(); // get it again now that we have the lock + uint64_t height; + top_hash = get_tail_id(height); // get it again now that we have the lock + ++height; // top block height to blockchain height // ND: Speedup // 1. Keep a list of the last 735 (or less) blocks that is used to compute difficulty, // then when the next block difficulty is queried, push the latest height data and @@ -1806,11 +1810,12 @@ uint64_t Blockchain::get_num_mature_outputs(uint64_t amount) const uint64_t num_outs = m_db->get_num_outputs(amount); // ensure we don't include outputs that aren't yet eligible to be used // outpouts are sorted by height + const uint64_t blockchain_height = m_db->height(); while (num_outs > 0) { const tx_out_index toi = m_db->get_output_tx_and_index(amount, num_outs - 1); const uint64_t height = m_db->get_tx_block_height(toi.first); - if (height + CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE <= m_db->height()) + if (height + CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE <= blockchain_height) break; --num_outs; } @@ -3325,14 +3330,15 @@ bool Blockchain::check_block_timestamp(const block& b, uint64_t& median_ts) cons return false; } + const auto h = m_db->height(); + // if not enough blocks, no proper median yet, return true - if(m_db->height() < BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW) + if(h < BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW) { return true; } std::vector timestamps; - auto h = m_db->height(); // need most recent 60 blocks, get index of first of those size_t offset = h - BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW; @@ -3401,9 +3407,12 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& static bool seen_future_version = false; m_db->block_txn_start(true); - if(bl.prev_id != get_tail_id()) + uint64_t blockchain_height; + const crypto::hash top_hash = get_tail_id(blockchain_height); + ++blockchain_height; // block height to chain height + if(bl.prev_id != top_hash) { - MERROR_VER("Block with id: " << id << std::endl << "has wrong prev_id: " << bl.prev_id << std::endl << "expected: " << get_tail_id()); + MERROR_VER("Block with id: " << id << std::endl << "has wrong prev_id: " << bl.prev_id << std::endl << "expected: " << top_hash); bvc.m_verifivation_failed = true; leave: m_db->block_txn_stop(); @@ -3472,9 +3481,9 @@ leave: bool precomputed = false; bool fast_check = false; #if defined(PER_BLOCK_CHECKPOINT) - if (m_db->height() < m_blocks_hash_check.size()) + if (blockchain_height < m_blocks_hash_check.size()) { - const auto &expected_hash = m_blocks_hash_check[m_db->height()]; + const auto &expected_hash = m_blocks_hash_check[blockchain_height]; if (expected_hash != crypto::null_hash) { if (memcmp(&id, &expected_hash, sizeof(hash)) != 0) @@ -3487,7 +3496,7 @@ leave: } else { - MCINFO("verify", "No pre-validated hash at height " << m_db->height() << ", verifying fully"); + MCINFO("verify", "No pre-validated hash at height " << blockchain_height << ", verifying fully"); } } else @@ -3500,7 +3509,7 @@ leave: proof_of_work = it->second; } else - proof_of_work = get_block_longhash(bl, m_db->height()); + proof_of_work = get_block_longhash(bl, blockchain_height); // validate proof_of_work versus difficulty target if(!check_hash(proof_of_work, current_diffic)) @@ -3513,9 +3522,9 @@ leave: // If we're at a checkpoint, ensure that our hardcoded checkpoint hash // is correct. - if(m_checkpoints.is_in_checkpoint_zone(get_current_blockchain_height())) + if(m_checkpoints.is_in_checkpoint_zone(blockchain_height)) { - if(!m_checkpoints.check_block(get_current_blockchain_height(), id)) + if(!m_checkpoints.check_block(blockchain_height, id)) { LOG_ERROR("CHECKPOINT VALIDATION FAILED"); bvc.m_verifivation_failed = true; @@ -3530,7 +3539,7 @@ leave: TIME_MEASURE_START(t3); // sanity check basic miner tx properties; - if(!prevalidate_miner_transaction(bl, m_db->height())) + if(!prevalidate_miner_transaction(bl, blockchain_height)) { MERROR_VER("Block with id: " << id << " failed to pass prevalidation"); bvc.m_verifivation_failed = true; @@ -3659,7 +3668,7 @@ leave: TIME_MEASURE_START(vmt); uint64_t base_reward = 0; - uint64_t already_generated_coins = m_db->height() ? m_db->get_block_already_generated_coins(m_db->height() - 1) : 0; + uint64_t already_generated_coins = blockchain_height ? m_db->get_block_already_generated_coins(blockchain_height - 1) : 0; if(!validate_miner_transaction(bl, cumulative_block_weight, fee_summary, base_reward, already_generated_coins, bvc.m_partial_block_reward, m_hardfork->get_current_version())) { MERROR_VER("Block with id: " << id << " has incorrect miner transaction"); @@ -3680,8 +3689,8 @@ leave: // at MONEY_SUPPLY. already_generated_coins is only used to compute the block subsidy and MONEY_SUPPLY yields a // subsidy of 0 under the base formula and therefore the minimum subsidy >0 in the tail state. already_generated_coins = base_reward < (MONEY_SUPPLY-already_generated_coins) ? already_generated_coins + base_reward : MONEY_SUPPLY; - if(m_db->height()) - cumulative_difficulty += m_db->get_block_cumulative_difficulty(m_db->height() - 1); + if(blockchain_height) + cumulative_difficulty += m_db->get_block_cumulative_difficulty(blockchain_height - 1); TIME_MEASURE_FINISH(block_processing_time); if(precomputed) @@ -3922,10 +3931,11 @@ void Blockchain::check_against_checkpoints(const checkpoints& points, bool enfor CRITICAL_REGION_LOCAL(m_blockchain_lock); stop_batch = m_db->batch_start(); + const uint64_t blockchain_height = m_db->height(); for (const auto& pt : pts) { // if the checkpoint is for a block we don't have yet, move on - if (pt.first >= m_db->height()) + if (pt.first >= blockchain_height) { continue; } @@ -4269,6 +4279,7 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vectortop_block_hash(); for (unsigned i = 0; i < threads; i++) { for (unsigned int j = 0; j < batches; j++, ++blockidx) @@ -4281,7 +4292,6 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vectortop_block_hash(); if (block.prev_id != tophash) { MDEBUG("Skipping prepare blocks. New blocks don't belong to chain."); diff --git a/tests/block_weight/block_weight.cpp b/tests/block_weight/block_weight.cpp index c70c2f950..57fcb497e 100644 --- a/tests/block_weight/block_weight.cpp +++ b/tests/block_weight/block_weight.cpp @@ -72,11 +72,13 @@ public: virtual uint64_t height() const override { return blocks.size(); } virtual size_t get_block_weight(const uint64_t &h) const override { return blocks[h].weight; } virtual uint64_t get_block_long_term_weight(const uint64_t &h) const override { return blocks[h].long_term_weight; } - virtual crypto::hash top_block_hash() const override { + virtual crypto::hash top_block_hash(uint64_t *block_height = NULL) const override { uint64_t h = height(); crypto::hash top = crypto::null_hash; if (h) *(uint64_t*)&top = h - 1; + if (block_height) + *block_height = h - 1; return top; } virtual void pop_block(cryptonote::block &blk, std::vector &txs) override { blocks.pop_back(); } diff --git a/tests/unit_tests/long_term_block_weight.cpp b/tests/unit_tests/long_term_block_weight.cpp index 113b01865..f37b608b6 100644 --- a/tests/unit_tests/long_term_block_weight.cpp +++ b/tests/unit_tests/long_term_block_weight.cpp @@ -64,11 +64,13 @@ public: virtual uint64_t height() const override { return blocks.size(); } virtual size_t get_block_weight(const uint64_t &h) const override { return blocks[h].weight; } virtual uint64_t get_block_long_term_weight(const uint64_t &h) const override { return blocks[h].long_term_weight; } - virtual crypto::hash top_block_hash() const override { + virtual crypto::hash top_block_hash(uint64_t *block_height = NULL) const override { uint64_t h = height(); crypto::hash top = crypto::null_hash; if (h) *(uint64_t*)&top = h - 1; + if (block_height) + *block_height = h - 1; return top; } virtual void pop_block(cryptonote::block &blk, std::vector &txs) override { blocks.pop_back(); } -- cgit v1.2.3 From dc5a76095cc4b05d7a706c3b650e54f2db314926 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 21 Nov 2018 12:57:51 +0000 Subject: blockchain: avoid unneeded block copy --- src/cryptonote_core/blockchain.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/cryptonote_core/blockchain.cpp') diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 941be234f..a1bee09dc 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -3704,7 +3704,8 @@ leave: try { uint64_t long_term_block_weight = get_next_long_term_block_weight(block_weight); - new_height = m_db->add_block(std::make_pair(bl, block_to_blob(bl)), block_weight, long_term_block_weight, cumulative_difficulty, already_generated_coins, txs); + cryptonote::blobdata bd = cryptonote::block_to_blob(bl); + new_height = m_db->add_block(std::make_pair(std::move(bl), std::move(bd)), block_weight, long_term_block_weight, cumulative_difficulty, already_generated_coins, txs); } catch (const KEY_IMAGE_EXISTS& e) { -- cgit v1.2.3 From 238401d4e950dad6a243fb895027dd26d2992ee6 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 23 Nov 2018 11:15:04 +0000 Subject: core: avoid double parsing blocks after hoh --- src/blockchain_utilities/blockchain_import.cpp | 12 +++++++++-- src/cryptonote_core/blockchain.cpp | 4 ++-- src/cryptonote_core/blockchain.h | 5 +++-- src/cryptonote_core/cryptonote_core.cpp | 25 +++++++++++++--------- src/cryptonote_core/cryptonote_core.h | 5 +++-- .../cryptonote_protocol_handler.inl | 24 +++++++++++++++------ 6 files changed, 50 insertions(+), 25 deletions(-) (limited to 'src/cryptonote_core/blockchain.cpp') diff --git a/src/blockchain_utilities/blockchain_import.cpp b/src/blockchain_utilities/blockchain_import.cpp index d2c2b1ea9..d7a88f935 100644 --- a/src/blockchain_utilities/blockchain_import.cpp +++ b/src/blockchain_utilities/blockchain_import.cpp @@ -193,8 +193,16 @@ int check_flush(cryptonote::core &core, std::vector &block } core.prevalidate_block_hashes(core.get_blockchain_storage().get_db().height(), hashes); - core.prepare_handle_incoming_blocks(blocks); + std::vector pblocks; + core.prepare_handle_incoming_blocks(blocks, pblocks); + if (!pblocks.empty() && pblocks.size() != blocks.size()) + { + MERROR("Unexpected parsed blocks size"); + core.cleanup_handle_incoming_blocks(); + return 1; + } + size_t blockidx = 0; for(const block_complete_entry& block_entry: blocks) { // process transactions @@ -215,7 +223,7 @@ int check_flush(cryptonote::core &core, std::vector &block block_verification_context bvc = boost::value_initialized(); - core.handle_incoming_block(block_entry.block, bvc, false); // <--- process block + core.handle_incoming_block(block_entry.block, pblocks.empty() ? NULL : &pblocks[blockidx++], bvc, false); // <--- process block if(bvc.m_verifivation_failed) { diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index a1bee09dc..2890af628 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -4211,13 +4211,14 @@ uint64_t Blockchain::prevalidate_block_hashes(uint64_t height, const std::vector // vs [k_image, output_keys] (m_scan_table). This is faster because it takes advantage of bulk queries // and is threaded if possible. The table (m_scan_table) will be used later when querying output // keys. -bool Blockchain::prepare_handle_incoming_blocks(const std::vector &blocks_entry) +bool Blockchain::prepare_handle_incoming_blocks(const std::vector &blocks_entry, std::vector &blocks) { MTRACE("Blockchain::" << __func__); TIME_MEASURE_START(prepare); bool stop_batch; uint64_t bytes = 0; size_t total_txs = 0; + blocks.clear(); // Order of locking must be: // m_incoming_tx_lock (optional) @@ -4264,7 +4265,6 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vector blocks; blocks.resize(blocks_entry.size()); if (1) diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index 6e966781d..9d928e386 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -230,11 +230,12 @@ namespace cryptonote /** * @brief performs some preprocessing on a group of incoming blocks to speed up verification * - * @param blocks a list of incoming blocks + * @param blocks_entry a list of incoming blocks + * @param blocks the parsed blocks * * @return false on erroneous blocks, else true */ - bool prepare_handle_incoming_blocks(const std::vector &blocks); + bool prepare_handle_incoming_blocks(const std::vector &blocks_entry, std::vector &blocks); /** * @brief incoming blocks post-processing, cleanup, and disk sync diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index 0d29f996a..bab0e2dd9 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -1330,7 +1330,8 @@ namespace cryptonote m_miner.resume(); return false; } - prepare_handle_incoming_blocks(blocks); + std::vector pblocks; + prepare_handle_incoming_blocks(blocks, pblocks); m_blockchain_storage.add_new_block(b, bvc); cleanup_handle_incoming_blocks(true); //anyway - update miner template @@ -1381,10 +1382,10 @@ namespace cryptonote } //----------------------------------------------------------------------------------------------- - bool core::prepare_handle_incoming_blocks(const std::vector &blocks) + 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); + m_blockchain_storage.prepare_handle_incoming_blocks(blocks_entry, blocks); return true; } @@ -1401,7 +1402,7 @@ namespace cryptonote } //----------------------------------------------------------------------------------------------- - bool core::handle_incoming_block(const blobdata& block_blob, block_verification_context& bvc, bool update_miner_blocktemplate) + bool core::handle_incoming_block(const blobdata& block_blob, const block *b, block_verification_context& bvc, bool update_miner_blocktemplate) { TRY_ENTRY(); @@ -1417,14 +1418,18 @@ namespace cryptonote return false; } - block b = AUTO_VAL_INIT(b); - if(!parse_and_validate_block_from_blob(block_blob, b)) + block lb; + if (!b) { - LOG_PRINT_L1("Failed to parse and validate new block"); - bvc.m_verifivation_failed = true; - return false; + if(!parse_and_validate_block_from_blob(block_blob, lb)) + { + LOG_PRINT_L1("Failed to parse and validate new block"); + bvc.m_verifivation_failed = true; + return false; + } + b = &lb; } - add_new_block(b, bvc); + add_new_block(*b, bvc); if(update_miner_blocktemplate && bvc.m_added_to_main_chain) update_miner_block_template(); return true; diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h index 131440511..6d0ff098d 100644 --- a/src/cryptonote_core/cryptonote_core.h +++ b/src/cryptonote_core/cryptonote_core.h @@ -146,20 +146,21 @@ namespace cryptonote * optionally updates the miner's block template. * * @param block_blob the block to be added + * @param block the block to be added, or NULL * @param bvc return-by-reference metadata context about the block's validity * @param update_miner_blocktemplate whether or not to update the miner's block template * * @return false if loading new checkpoints fails, or the block is not * added, otherwise true */ - bool handle_incoming_block(const blobdata& block_blob, block_verification_context& bvc, bool update_miner_blocktemplate = true); + bool handle_incoming_block(const blobdata& block_blob, const block *b, block_verification_context& bvc, bool update_miner_blocktemplate = true); /** * @copydoc Blockchain::prepare_handle_incoming_blocks * * @note see Blockchain::prepare_handle_incoming_blocks */ - bool prepare_handle_incoming_blocks(const std::vector &blocks); + bool prepare_handle_incoming_blocks(const std::vector &blocks_entry, std::vector &blocks); /** * @copydoc Blockchain::cleanup_handle_incoming_blocks diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index 8fada4e3c..9c2e1df88 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -418,7 +418,8 @@ namespace cryptonote m_core.pause_mine(); std::vector blocks; blocks.push_back(arg.b); - m_core.prepare_handle_incoming_blocks(blocks); + std::vector pblocks; + m_core.prepare_handle_incoming_blocks(blocks, pblocks); 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); @@ -434,7 +435,7 @@ namespace cryptonote } block_verification_context bvc = boost::value_initialized(); - m_core.handle_incoming_block(arg.b.block, bvc); // got block from handle_notify_new_block + m_core.handle_incoming_block(arg.b.block, pblocks.empty() ? NULL : &pblocks[0], bvc); // got block from handle_notify_new_block if (!m_core.cleanup_handle_incoming_blocks(true)) { LOG_PRINT_CCONTEXT_L0("Failure in cleanup_handle_incoming_blocks"); @@ -697,10 +698,11 @@ namespace cryptonote std::vector blocks; blocks.push_back(b); - m_core.prepare_handle_incoming_blocks(blocks); + std::vector pblocks; + m_core.prepare_handle_incoming_blocks(blocks, pblocks); block_verification_context bvc = boost::value_initialized(); - m_core.handle_incoming_block(arg.b.block, bvc); // got block from handle_notify_new_block + m_core.handle_incoming_block(arg.b.block, pblocks.empty() ? NULL : &pblocks[0], bvc); // got block from handle_notify_new_block if (!m_core.cleanup_handle_incoming_blocks(true)) { LOG_PRINT_CCONTEXT_L0("Failure in cleanup_handle_incoming_blocks"); @@ -1174,10 +1176,17 @@ namespace cryptonote } } - m_core.prepare_handle_incoming_blocks(blocks); + std::vector pblocks; + m_core.prepare_handle_incoming_blocks(blocks, pblocks); + if (!pblocks.empty() && pblocks.size() != blocks.size()) + { + m_core.cleanup_handle_incoming_blocks(); + LOG_ERROR_CCONTEXT("Internal error: blocks.size() != block_entry.txs.size()"); + return 1; + } uint64_t block_process_time_full = 0, transactions_process_time_full = 0; - size_t num_txs = 0; + size_t num_txs = 0, blockidx = 0; for(const block_complete_entry& block_entry: blocks) { if (m_stopping) @@ -1229,7 +1238,7 @@ namespace cryptonote TIME_MEASURE_START(block_process_time); block_verification_context bvc = boost::value_initialized(); - m_core.handle_incoming_block(block_entry.block, bvc, false); // <--- process block + m_core.handle_incoming_block(block_entry.block, pblocks.empty() ? NULL : &pblocks[blockidx], bvc, false); // <--- process block if(bvc.m_verifivation_failed) { @@ -1272,6 +1281,7 @@ namespace cryptonote TIME_MEASURE_FINISH(block_process_time); block_process_time_full += block_process_time; + ++blockidx; } // each download block -- cgit v1.2.3 From 88c85c18e0173f479470842fbe49626cdc86e7d1 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 23 Nov 2018 13:05:48 +0000 Subject: 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(-) (limited to 'src/cryptonote_core/blockchain.cpp') 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; } -- cgit v1.2.3