From e69477bf25630c94a5aff666433f3da9ab29298c Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 11 Nov 2018 13:39:00 +0000 Subject: [PATCH 01/12] db: speedup block addition by avoiding repeated (de)serialization --- src/blockchain_db/lmdb/db_lmdb.cpp | 27 ++++---- src/cryptonote_basic/cryptonote_basic.h | 25 +++++++- .../cryptonote_format_utils.cpp | 62 ++++++++++++------- .../cryptonote_format_utils.h | 6 +- 4 files changed, 83 insertions(+), 37 deletions(-) diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 9d2f7821e..376170a6b 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -865,21 +865,26 @@ uint64_t BlockchainLMDB::add_transaction_data(const crypto::hash& blk_hash, cons cryptonote::blobdata blob = tx_to_blob(tx); MDB_val_sized(blobval, blob); - std::stringstream ss; - binary_archive ba(ss); - bool r = const_cast(tx).serialize_base(ba); - if (!r) - throw0(DB_ERROR("Failed to serialize pruned tx")); - std::string pruned = ss.str(); - MDB_val_sized(pruned_blob, pruned); + unsigned int unprunable_size = tx.unprunable_size; + if (unprunable_size == 0) + { + std::stringstream ss; + binary_archive ba(ss); + bool r = const_cast(tx).serialize_base(ba); + if (!r) + throw0(DB_ERROR("Failed to serialize pruned tx")); + unprunable_size = ss.str().size(); + } + + if (unprunable_size > blob.size()) + throw0(DB_ERROR("pruned tx size is larger than tx size")); + + MDB_val pruned_blob = {unprunable_size, (void*)blob.data()}; result = mdb_cursor_put(m_cur_txs_pruned, &val_tx_id, &pruned_blob, MDB_APPEND); if (result) throw0(DB_ERROR(lmdb_error("Failed to add pruned tx blob to db transaction: ", result).c_str())); - if (pruned.size() > blob.size()) - throw0(DB_ERROR("pruned tx size is larger than tx size")); - cryptonote::blobdata prunable(blob.data() + pruned.size(), blob.size() - pruned.size()); - MDB_val_sized(prunable_blob, prunable); + MDB_val prunable_blob = {blob.size() - unprunable_size, (void*)(blob.data() + unprunable_size)}; result = mdb_cursor_put(m_cur_txs_prunable, &val_tx_id, &prunable_blob, MDB_APPEND); if (result) throw0(DB_ERROR(lmdb_error("Failed to add prunable tx blob to db transaction: ", result).c_str())); diff --git a/src/cryptonote_basic/cryptonote_basic.h b/src/cryptonote_basic/cryptonote_basic.h index c9c783a56..172a99e84 100644 --- a/src/cryptonote_basic/cryptonote_basic.h +++ b/src/cryptonote_basic/cryptonote_basic.h @@ -152,6 +152,10 @@ namespace cryptonote }; + template static inline unsigned int getpos(T &ar) { return 0; } + template<> inline unsigned int getpos(binary_archive &ar) { return ar.stream().tellp(); } + template<> inline unsigned int getpos(binary_archive &ar) { return ar.stream().tellg(); } + class transaction_prefix { @@ -203,9 +207,12 @@ namespace cryptonote bool pruned; + std::atomic unprunable_size; + std::atomic prefix_size; + transaction(); - transaction(const transaction &t): transaction_prefix(t), hash_valid(false), blob_size_valid(false), signatures(t.signatures), rct_signatures(t.rct_signatures), pruned(t.pruned) { if (t.is_hash_valid()) { hash = t.hash; set_hash_valid(true); } if (t.is_blob_size_valid()) { blob_size = t.blob_size; set_blob_size_valid(true); } } - transaction &operator=(const transaction &t) { transaction_prefix::operator=(t); set_hash_valid(false); set_blob_size_valid(false); signatures = t.signatures; rct_signatures = t.rct_signatures; if (t.is_hash_valid()) { hash = t.hash; set_hash_valid(true); } if (t.is_blob_size_valid()) { blob_size = t.blob_size; set_blob_size_valid(true); } pruned = t.pruned; return *this; } + transaction(const transaction &t): transaction_prefix(t), hash_valid(false), blob_size_valid(false), signatures(t.signatures), rct_signatures(t.rct_signatures), pruned(t.pruned), unprunable_size(t.unprunable_size.load()), prefix_size(t.prefix_size.load()) { if (t.is_hash_valid()) { hash = t.hash; set_hash_valid(true); } if (t.is_blob_size_valid()) { blob_size = t.blob_size; set_blob_size_valid(true); } } + transaction &operator=(const transaction &t) { transaction_prefix::operator=(t); set_hash_valid(false); set_blob_size_valid(false); signatures = t.signatures; rct_signatures = t.rct_signatures; if (t.is_hash_valid()) { hash = t.hash; set_hash_valid(true); } if (t.is_blob_size_valid()) { blob_size = t.blob_size; set_blob_size_valid(true); } pruned = t.pruned; unprunable_size = t.unprunable_size.load(); prefix_size = t.prefix_size.load(); return *this; } virtual ~transaction(); void set_null(); void invalidate_hashes(); @@ -223,10 +230,18 @@ namespace cryptonote set_blob_size_valid(false); } + const unsigned int start_pos = getpos(ar); + FIELDS(*static_cast(this)) + if (std::is_same, binary_archive>()) + prefix_size = getpos(ar) - start_pos; + if (version == 1) { + if (std::is_same, binary_archive>()) + unprunable_size = getpos(ar) - start_pos; + ar.tag("signatures"); ar.begin_array(); PREPARE_CUSTOM_VECTOR_SERIALIZATION(vin.size(), signatures); @@ -265,6 +280,10 @@ namespace cryptonote bool r = rct_signatures.serialize_rctsig_base(ar, vin.size(), vout.size()); if (!r || !ar.stream().good()) return false; ar.end_object(); + + if (std::is_same, binary_archive>()) + unprunable_size = getpos(ar) - start_pos; + if (!pruned && rct_signatures.type != rct::RCTTypeNull) { ar.tag("rctsig_prunable"); @@ -329,6 +348,8 @@ namespace cryptonote set_hash_valid(false); set_blob_size_valid(false); pruned = false; + unprunable_size = 0; + prefix_size = 0; } inline diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index 10fb5444c..85c5131f6 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -878,6 +878,11 @@ namespace cryptonote return true; } //--------------------------------------------------------------- + void get_blob_hash(const epee::span& blob, crypto::hash& res) + { + cn_fast_hash(blob.data(), blob.size(), res); + } + //--------------------------------------------------------------- void get_blob_hash(const blobdata& blob, crypto::hash& res) { cn_fast_hash(blob.data(), blob.size(), res); @@ -946,6 +951,13 @@ namespace cryptonote return h; } //--------------------------------------------------------------- + crypto::hash get_blob_hash(const epee::span& blob) + { + crypto::hash h = null_hash; + get_blob_hash(blob, h); + return h; + } + //--------------------------------------------------------------- crypto::hash get_transaction_hash(const transaction& t) { crypto::hash h = null_hash; @@ -958,26 +970,35 @@ namespace cryptonote return get_transaction_hash(t, res, NULL); } //--------------------------------------------------------------- - bool calculate_transaction_prunable_hash(const transaction& t, crypto::hash& res) + bool calculate_transaction_prunable_hash(const transaction& t, const cryptonote::blobdata *blob, crypto::hash& res) { if (t.version == 1) return false; - transaction &tt = const_cast(t); - std::stringstream ss; - binary_archive ba(ss); - const size_t inputs = t.vin.size(); - const size_t outputs = t.vout.size(); - const size_t mixin = t.vin.empty() ? 0 : t.vin[0].type() == typeid(txin_to_key) ? boost::get(t.vin[0]).key_offsets.size() - 1 : 0; - bool r = tt.rct_signatures.p.serialize_rctsig_prunable(ba, t.rct_signatures.type, inputs, outputs, mixin); - CHECK_AND_ASSERT_MES(r, false, "Failed to serialize rct signatures prunable"); - cryptonote::get_blob_hash(ss.str(), res); + const unsigned int unprunable_size = t.unprunable_size; + if (blob && unprunable_size) + { + CHECK_AND_ASSERT_MES(unprunable_size <= blob->size(), false, "Inconsistent transaction unprunable and blob sizes"); + cryptonote::get_blob_hash(epee::span(blob->data() + unprunable_size, blob->size() - unprunable_size), res); + } + else + { + transaction &tt = const_cast(t); + std::stringstream ss; + binary_archive ba(ss); + const size_t inputs = t.vin.size(); + const size_t outputs = t.vout.size(); + const size_t mixin = t.vin.empty() ? 0 : t.vin[0].type() == typeid(txin_to_key) ? boost::get(t.vin[0]).key_offsets.size() - 1 : 0; + bool r = tt.rct_signatures.p.serialize_rctsig_prunable(ba, t.rct_signatures.type, inputs, outputs, mixin); + CHECK_AND_ASSERT_MES(r, false, "Failed to serialize rct signatures prunable"); + cryptonote::get_blob_hash(ss.str(), res); + } return true; } //--------------------------------------------------------------- - crypto::hash get_transaction_prunable_hash(const transaction& t) + crypto::hash get_transaction_prunable_hash(const transaction& t, const cryptonote::blobdata *blobdata) { crypto::hash res; - CHECK_AND_ASSERT_THROW_MES(calculate_transaction_prunable_hash(t, res), "Failed to calculate tx prunable hash"); + CHECK_AND_ASSERT_THROW_MES(calculate_transaction_prunable_hash(t, blobdata, res), "Failed to calculate tx prunable hash"); return res; } //--------------------------------------------------------------- @@ -1030,16 +1051,13 @@ namespace cryptonote transaction &tt = const_cast(t); + const blobdata blob = tx_to_blob(t); + const unsigned int unprunable_size = t.unprunable_size; + const unsigned int prefix_size = t.prefix_size; + // base rct - { - std::stringstream ss; - binary_archive ba(ss); - const size_t inputs = t.vin.size(); - const size_t outputs = t.vout.size(); - bool r = tt.rct_signatures.serialize_rctsig_base(ba, inputs, outputs); - CHECK_AND_ASSERT_MES(r, false, "Failed to serialize rct signatures base"); - cryptonote::get_blob_hash(ss.str(), hashes[1]); - } + CHECK_AND_ASSERT_MES(prefix_size <= unprunable_size && unprunable_size <= blob.size(), false, "Inconsistent transaction prefix, unprunable and blob sizes"); + cryptonote::get_blob_hash(epee::span(blob.data() + prefix_size, unprunable_size - prefix_size), hashes[1]); // prunable rct if (t.rct_signatures.type == rct::RCTTypeNull) @@ -1048,7 +1066,7 @@ namespace cryptonote } else { - CHECK_AND_ASSERT_MES(calculate_transaction_prunable_hash(t, hashes[2]), false, "Failed to get tx prunable hash"); + CHECK_AND_ASSERT_MES(calculate_transaction_prunable_hash(t, &blob, hashes[2]), false, "Failed to get tx prunable hash"); } // the tx hash is the hash of the 3 hashes diff --git a/src/cryptonote_basic/cryptonote_format_utils.h b/src/cryptonote_basic/cryptonote_format_utils.h index 994978c10..4247e13da 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.h +++ b/src/cryptonote_basic/cryptonote_format_utils.h @@ -98,15 +98,17 @@ namespace cryptonote bool generate_key_image_helper(const account_keys& ack, const std::unordered_map& subaddresses, const crypto::public_key& out_key, const crypto::public_key& tx_public_key, const std::vector& additional_tx_public_keys, size_t real_output_index, keypair& in_ephemeral, crypto::key_image& ki, hw::device &hwdev); bool generate_key_image_helper_precomp(const account_keys& ack, const crypto::public_key& out_key, const crypto::key_derivation& recv_derivation, size_t real_output_index, const subaddress_index& received_index, keypair& in_ephemeral, crypto::key_image& ki, hw::device &hwdev); void get_blob_hash(const blobdata& blob, crypto::hash& res); + void get_blob_hash(const epee::span& blob, crypto::hash& res); crypto::hash get_blob_hash(const blobdata& blob); + crypto::hash get_blob_hash(const epee::span& blob); std::string short_hash_str(const crypto::hash& h); crypto::hash get_transaction_hash(const transaction& t); bool get_transaction_hash(const transaction& t, crypto::hash& res); bool get_transaction_hash(const transaction& t, crypto::hash& res, size_t& blob_size); bool get_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size); - bool calculate_transaction_prunable_hash(const transaction& t, crypto::hash& res); - crypto::hash get_transaction_prunable_hash(const transaction& t); + bool calculate_transaction_prunable_hash(const transaction& t, const cryptonote::blobdata *blob, crypto::hash& res); + crypto::hash get_transaction_prunable_hash(const transaction& t, const cryptonote::blobdata *blob = NULL); bool calculate_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size); crypto::hash get_pruned_transaction_hash(const transaction& t, const crypto::hash &pruned_data_hash); From b747e836c884ac037825804636aac1c7786a3a13 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 11 Nov 2018 13:48:07 +0000 Subject: [PATCH 02/12] wallet2: don't calculate prefix hash when we don't need it --- src/wallet/wallet2.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index f2c0a7ba9..a476ecdc5 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -10175,11 +10175,11 @@ void wallet2::check_tx_key_helper(const crypto::hash &txid, const crypto::key_de else { cryptonote::blobdata tx_data; - crypto::hash tx_prefix_hash; ok = string_tools::parse_hexstr_to_binbuff(res.txs_as_hex.front(), tx_data); THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon"); - THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx, tx_hash, tx_prefix_hash), + THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx), error::wallet_internal_error, "Failed to validate transaction from daemon"); + tx_hash = cryptonote::get_transaction_hash(tx); } THROW_WALLET_EXCEPTION_IF(tx_hash != txid, error::wallet_internal_error, @@ -10321,11 +10321,11 @@ std::string wallet2::get_tx_proof(const crypto::hash &txid, const cryptonote::ac else { cryptonote::blobdata tx_data; - crypto::hash tx_prefix_hash; ok = string_tools::parse_hexstr_to_binbuff(res.txs_as_hex.front(), tx_data); THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon"); - THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx, tx_hash, tx_prefix_hash), + THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx), error::wallet_internal_error, "Failed to validate transaction from daemon"); + tx_hash = cryptonote::get_transaction_hash(tx); } THROW_WALLET_EXCEPTION_IF(tx_hash != txid, error::wallet_internal_error, "Failed to get the right transaction from daemon"); @@ -10439,11 +10439,11 @@ bool wallet2::check_tx_proof(const crypto::hash &txid, const cryptonote::account else { cryptonote::blobdata tx_data; - crypto::hash tx_prefix_hash; ok = string_tools::parse_hexstr_to_binbuff(res.txs_as_hex.front(), tx_data); THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon"); - THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx, tx_hash, tx_prefix_hash), + THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx), error::wallet_internal_error, "Failed to validate transaction from daemon"); + tx_hash = cryptonote::get_transaction_hash(tx); } THROW_WALLET_EXCEPTION_IF(tx_hash != txid, error::wallet_internal_error, "Failed to get the right transaction from daemon"); From b044d03a51e1dc64bebe2461813e0cfc50f71b0d Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 11 Nov 2018 14:51:03 +0000 Subject: [PATCH 03/12] Avoid repeated (de)serialization when syncing --- src/blockchain_db/blockchain_db.cpp | 20 ++++++++----- src/blockchain_db/blockchain_db.h | 8 ++--- src/blockchain_db/lmdb/db_lmdb.cpp | 13 ++++---- src/blockchain_db/lmdb/db_lmdb.h | 6 ++-- src/blockchain_db/testdb.h | 2 +- .../blockchain_import.cpp | 6 ++-- src/cryptonote_core/blockchain.cpp | 23 ++++++++------ src/cryptonote_core/blockchain.h | 2 +- src/cryptonote_core/tx_pool.cpp | 4 +-- src/cryptonote_core/tx_pool.h | 3 +- tests/block_weight/block_weight.cpp | 4 +-- tests/unit_tests/blockchain_db.cpp | 30 +++++++++---------- tests/unit_tests/long_term_block_weight.cpp | 28 ++++++++--------- 13 files changed, 80 insertions(+), 69 deletions(-) diff --git a/src/blockchain_db/blockchain_db.cpp b/src/blockchain_db/blockchain_db.cpp index 041759593..754c3c2da 100644 --- a/src/blockchain_db/blockchain_db.cpp +++ b/src/blockchain_db/blockchain_db.cpp @@ -121,8 +121,10 @@ void BlockchainDB::pop_block() pop_block(blk, txs); } -void BlockchainDB::add_transaction(const crypto::hash& blk_hash, const transaction& tx, const crypto::hash* tx_hash_ptr, const crypto::hash* tx_prunable_hash_ptr) +void BlockchainDB::add_transaction(const crypto::hash& blk_hash, const std::pair& txp, const crypto::hash* tx_hash_ptr, const crypto::hash* tx_prunable_hash_ptr) { + const transaction &tx = txp.first; + bool miner_tx = false; crypto::hash tx_hash, tx_prunable_hash; if (!tx_hash_ptr) @@ -138,7 +140,7 @@ void BlockchainDB::add_transaction(const crypto::hash& blk_hash, const transacti if (tx.version >= 2) { if (!tx_prunable_hash_ptr) - tx_prunable_hash = get_transaction_prunable_hash(tx); + tx_prunable_hash = get_transaction_prunable_hash(tx, &txp.second); else tx_prunable_hash = *tx_prunable_hash_ptr; } @@ -168,7 +170,7 @@ void BlockchainDB::add_transaction(const crypto::hash& blk_hash, const transacti } } - uint64_t tx_id = add_transaction_data(blk_hash, tx, tx_hash, tx_prunable_hash); + uint64_t tx_id = add_transaction_data(blk_hash, txp, tx_hash, tx_prunable_hash); std::vector amount_output_indices(tx.vout.size()); @@ -195,14 +197,16 @@ void BlockchainDB::add_transaction(const crypto::hash& blk_hash, const transacti add_tx_amount_output_indices(tx_id, amount_output_indices); } -uint64_t BlockchainDB::add_block( const block& blk +uint64_t BlockchainDB::add_block( const std::pair& blck , size_t block_weight , uint64_t long_term_block_weight , const difficulty_type& cumulative_difficulty , const uint64_t& coins_generated - , const std::vector& txs + , const std::vector>& txs ) { + const block &blk = blck.first; + // sanity if (blk.tx_hashes.size() != txs.size()) throw std::runtime_error("Inconsistent tx/hashes sizes"); @@ -221,16 +225,16 @@ uint64_t BlockchainDB::add_block( const block& blk time1 = epee::misc_utils::get_tick_count(); uint64_t num_rct_outs = 0; - add_transaction(blk_hash, blk.miner_tx); + add_transaction(blk_hash, std::make_pair(blk.miner_tx, tx_to_blob(blk.miner_tx))); if (blk.miner_tx.version == 2) num_rct_outs += blk.miner_tx.vout.size(); int tx_i = 0; crypto::hash tx_hash = crypto::null_hash; - for (const transaction& tx : txs) + for (const std::pair& tx : txs) { tx_hash = blk.tx_hashes[tx_i]; add_transaction(blk_hash, tx, &tx_hash); - for (const auto &vout: tx.vout) + for (const auto &vout: tx.first.vout) { if (vout.amount == 0) ++num_rct_outs; diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h index c3f11ba28..4d0b8935f 100644 --- a/src/blockchain_db/blockchain_db.h +++ b/src/blockchain_db/blockchain_db.h @@ -404,7 +404,7 @@ private: * @param tx_prunable_hash the hash of the prunable part of the transaction * @return the transaction ID */ - virtual uint64_t add_transaction_data(const crypto::hash& blk_hash, const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prunable_hash) = 0; + virtual uint64_t add_transaction_data(const crypto::hash& blk_hash, const std::pair& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prunable_hash) = 0; /** * @brief remove data about a transaction @@ -532,7 +532,7 @@ protected: * @param tx_hash_ptr the hash of the transaction, if already calculated * @param tx_prunable_hash_ptr the hash of the prunable part of the transaction, if already calculated */ - void add_transaction(const crypto::hash& blk_hash, const transaction& tx, const crypto::hash* tx_hash_ptr = NULL, const crypto::hash* tx_prunable_hash_ptr = NULL); + void add_transaction(const crypto::hash& blk_hash, const std::pair& tx, const crypto::hash* tx_hash_ptr = NULL, const crypto::hash* tx_prunable_hash_ptr = NULL); mutable uint64_t time_tx_exists = 0; //!< a performance metric uint64_t time_commit1 = 0; //!< a performance metric @@ -798,12 +798,12 @@ public: * * @return the height of the chain post-addition */ - virtual uint64_t add_block( const block& blk + virtual uint64_t add_block( const std::pair& blk , size_t block_weight , uint64_t long_term_block_weight , const difficulty_type& cumulative_difficulty , const uint64_t& coins_generated - , const std::vector& txs + , const std::vector>& txs ); /** diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 376170a6b..e7ed75c83 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -823,7 +823,7 @@ void BlockchainLMDB::remove_block() throw1(DB_ERROR(lmdb_error("Failed to add removal of block info to db transaction: ", result).c_str())); } -uint64_t BlockchainLMDB::add_transaction_data(const crypto::hash& blk_hash, const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prunable_hash) +uint64_t BlockchainLMDB::add_transaction_data(const crypto::hash& blk_hash, const std::pair& txp, const crypto::hash& tx_hash, const crypto::hash& tx_prunable_hash) { LOG_PRINT_L3("BlockchainLMDB::" << __func__); check_open(); @@ -849,6 +849,7 @@ uint64_t BlockchainLMDB::add_transaction_data(const crypto::hash& blk_hash, cons throw1(DB_ERROR(lmdb_error(std::string("Error checking if tx index exists for tx hash ") + epee::string_tools::pod_to_hex(tx_hash) + ": ", result).c_str())); } + const cryptonote::transaction &tx = txp.first; txindex ti; ti.key = tx_hash; ti.data.tx_id = tx_id; @@ -862,7 +863,7 @@ uint64_t BlockchainLMDB::add_transaction_data(const crypto::hash& blk_hash, cons if (result) throw0(DB_ERROR(lmdb_error("Failed to add tx data to db transaction: ", result).c_str())); - cryptonote::blobdata blob = tx_to_blob(tx); + const cryptonote::blobdata &blob = txp.second; MDB_val_sized(blobval, blob); unsigned int unprunable_size = tx.unprunable_size; @@ -3568,8 +3569,8 @@ void BlockchainLMDB::block_txn_abort() } } -uint64_t BlockchainLMDB::add_block(const block& blk, size_t block_weight, uint64_t long_term_block_weight, const difficulty_type& cumulative_difficulty, const uint64_t& coins_generated, - const std::vector& txs) +uint64_t BlockchainLMDB::add_block(const std::pair& blk, size_t block_weight, uint64_t long_term_block_weight, const difficulty_type& cumulative_difficulty, const uint64_t& coins_generated, + const std::vector>& txs) { LOG_PRINT_L3("BlockchainLMDB::" << __func__); check_open(); @@ -4488,7 +4489,7 @@ void BlockchainLMDB::migrate_0_1() if (!parse_and_validate_block_from_blob(bd, b)) throw0(DB_ERROR("Failed to parse block from blob retrieved from the db")); - add_transaction(null_hash, b.miner_tx); + add_transaction(null_hash, std::make_pair(b.miner_tx, tx_to_blob(b.miner_tx))); for (unsigned int j = 0; j(v.mv_data), v.mv_size); if (!parse_and_validate_tx_from_blob(bd, tx)) throw0(DB_ERROR("Failed to parse tx from blob retrieved from the db")); - add_transaction(null_hash, tx, &b.tx_hashes[j]); + add_transaction(null_hash, std::make_pair(std::move(tx), bd), &b.tx_hashes[j]); result = mdb_cursor_del(c_txs, 0); if (result) throw0(DB_ERROR(lmdb_error("Failed to get record from txs: ", result).c_str())); diff --git a/src/blockchain_db/lmdb/db_lmdb.h b/src/blockchain_db/lmdb/db_lmdb.h index 5764f9ae4..2f28c48e2 100644 --- a/src/blockchain_db/lmdb/db_lmdb.h +++ b/src/blockchain_db/lmdb/db_lmdb.h @@ -292,12 +292,12 @@ public: virtual bool for_all_outputs(std::function f) const; virtual bool for_all_outputs(uint64_t amount, const std::function &f) const; - virtual uint64_t add_block( const block& blk + virtual uint64_t add_block( const std::pair& blk , size_t block_weight , uint64_t long_term_block_weight , const difficulty_type& cumulative_difficulty , const uint64_t& coins_generated - , const std::vector& txs + , const std::vector>& txs ); virtual void set_batch_transactions(bool batch_transactions); @@ -353,7 +353,7 @@ private: virtual void remove_block(); - virtual uint64_t add_transaction_data(const crypto::hash& blk_hash, const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prunable_hash); + virtual uint64_t add_transaction_data(const crypto::hash& blk_hash, const std::pair& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prunable_hash); virtual void remove_transaction_data(const crypto::hash& tx_hash, const transaction& tx); diff --git a/src/blockchain_db/testdb.h b/src/blockchain_db/testdb.h index 35dfbe673..dcaee79b7 100644 --- a/src/blockchain_db/testdb.h +++ b/src/blockchain_db/testdb.h @@ -102,7 +102,7 @@ public: virtual std::vector> get_tx_amount_output_indices(const uint64_t tx_index, size_t n_txes) const { return std::vector>(); } virtual bool has_key_image(const crypto::key_image& img) const { return false; } virtual void remove_block() { } - virtual uint64_t add_transaction_data(const crypto::hash& blk_hash, const cryptonote::transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prunable_hash) {return 0;} + virtual uint64_t add_transaction_data(const crypto::hash& blk_hash, const std::pair& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prunable_hash) {return 0;} virtual void remove_transaction_data(const crypto::hash& tx_hash, const cryptonote::transaction& tx) {} virtual uint64_t add_output(const crypto::hash& tx_hash, const cryptonote::tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time, const rct::key *commitment) {return 0;} virtual void add_tx_amount_output_indices(const uint64_t tx_index, const std::vector& amount_output_indices) {} diff --git a/src/blockchain_utilities/blockchain_import.cpp b/src/blockchain_utilities/blockchain_import.cpp index e6ec20c3b..d2c2b1ea9 100644 --- a/src/blockchain_utilities/blockchain_import.cpp +++ b/src/blockchain_utilities/blockchain_import.cpp @@ -455,7 +455,7 @@ int import_from_file(cryptonote::core& core, const std::string& import_file_path } else { - std::vector txs; + std::vector> txs; std::vector archived_txs; archived_txs = bp.txs; @@ -472,7 +472,7 @@ int import_from_file(cryptonote::core& core, const std::string& import_file_path // because add_block() calls // add_transaction(blk_hash, blk.miner_tx) first, and // then a for loop for the transactions in txs. - txs.push_back(tx); + txs.push_back(std::make_pair(tx, tx_to_blob(tx))); } size_t block_weight; @@ -486,7 +486,7 @@ int import_from_file(cryptonote::core& core, const std::string& import_file_path try { uint64_t long_term_block_weight = core.get_blockchain_storage().get_next_long_term_block_weight(block_weight); - core.get_blockchain_storage().get_db().add_block(b, block_weight, long_term_block_weight, cumulative_difficulty, coins_generated, txs); + core.get_blockchain_storage().get_db().add_block(std::make_pair(b, block_to_blob(b)), block_weight, long_term_block_weight, cumulative_difficulty, coins_generated, txs); } catch (const std::exception& e) { 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); } } } diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index 92aef1278..6e966781d 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -1396,7 +1396,7 @@ namespace cryptonote * @return true */ bool update_next_cumulative_weight_limit(uint64_t *long_term_effective_median_block_weight = NULL); - void return_tx_to_pool(std::vector &txs); + void return_tx_to_pool(std::vector> &txs); /** * @brief make sure a transaction isn't attempting a double-spend diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index 1c8f1c62c..c2cc93530 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -453,7 +453,7 @@ namespace cryptonote return true; } //--------------------------------------------------------------------------------- - bool tx_memory_pool::take_tx(const crypto::hash &id, transaction &tx, size_t& tx_weight, uint64_t& fee, bool &relayed, bool &do_not_relay, bool &double_spend_seen) + bool tx_memory_pool::take_tx(const crypto::hash &id, transaction &tx, cryptonote::blobdata &txblob, size_t& tx_weight, uint64_t& fee, bool &relayed, bool &do_not_relay, bool &double_spend_seen) { CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL1(m_blockchain); @@ -469,7 +469,7 @@ namespace cryptonote MERROR("Failed to find tx in txpool"); return false; } - cryptonote::blobdata txblob = m_blockchain.get_txpool_tx_blob(id); + txblob = m_blockchain.get_txpool_tx_blob(id); auto ci = m_parsed_tx_cache.find(id); if (ci != m_parsed_tx_cache.end()) { diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h index 8dd0337f0..7a8af8799 100644 --- a/src/cryptonote_core/tx_pool.h +++ b/src/cryptonote_core/tx_pool.h @@ -133,6 +133,7 @@ namespace cryptonote * * @param id the hash of the transaction * @param tx return-by-reference the transaction taken + * @param txblob return-by-reference the transaction as a blob * @param tx_weight return-by-reference the transaction's weight * @param fee the transaction fee * @param relayed return-by-reference was transaction relayed to us by the network? @@ -141,7 +142,7 @@ namespace cryptonote * * @return true unless the transaction cannot be found in the pool */ - bool take_tx(const crypto::hash &id, transaction &tx, size_t& tx_weight, uint64_t& fee, bool &relayed, bool &do_not_relay, bool &double_spend_seen); + bool take_tx(const crypto::hash &id, transaction &tx, cryptonote::blobdata &txblob, size_t& tx_weight, uint64_t& fee, bool &relayed, bool &do_not_relay, bool &double_spend_seen); /** * @brief checks if the pool has a transaction with the given hash diff --git a/tests/block_weight/block_weight.cpp b/tests/block_weight/block_weight.cpp index a67f40f0f..c70c2f950 100644 --- a/tests/block_weight/block_weight.cpp +++ b/tests/block_weight/block_weight.cpp @@ -129,7 +129,7 @@ static void test(test_t t, uint64_t blocks) cryptonote::block b; b.major_version = 1; b.minor_version = 1; - bc->get_db().add_block(b, 300000, 300000, bc->get_db().height(), bc->get_db().height(), {}); + bc->get_db().add_block(std::make_pair(b, ""), 300000, 300000, bc->get_db().height(), bc->get_db().height(), {}); if (!bc->update_next_cumulative_weight_limit()) { fprintf(stderr, "Failed to update cumulative weight limit 1\n"); @@ -163,7 +163,7 @@ static void test(test_t t, uint64_t blocks) cryptonote::block b; b.major_version = 10; b.minor_version = 10; - bc->get_db().add_block(std::move(b), w, ltw, bc->get_db().height(), bc->get_db().height(), {}); + bc->get_db().add_block(std::make_pair(std::move(b), ""), w, ltw, bc->get_db().height(), bc->get_db().height(), {}); if (!bc->update_next_cumulative_weight_limit()) { diff --git a/tests/unit_tests/blockchain_db.cpp b/tests/unit_tests/blockchain_db.cpp index 9cf8c5fbe..f9a6adf03 100644 --- a/tests/unit_tests/blockchain_db.cpp +++ b/tests/unit_tests/blockchain_db.cpp @@ -163,17 +163,17 @@ protected: block bl; blobdata bd = h2b(i); parse_and_validate_block_from_blob(bd, bl); - m_blocks.push_back(bl); + m_blocks.push_back(std::make_pair(bl, bd)); } for (auto& i : t_transactions) { - std::vector txs; + std::vector> txs; for (auto& j : i) { transaction tx; blobdata bd = h2b(j); parse_and_validate_tx_from_blob(bd, tx); - txs.push_back(tx); + txs.push_back(std::make_pair(tx, bd)); } m_txs.push_back(txs); } @@ -187,8 +187,8 @@ protected: BlockchainDB* m_db; HardFork m_hardfork; std::string m_prefix; - std::vector m_blocks; - std::vector > m_txs; + std::vector> m_blocks; + std::vector>> m_txs; std::vector m_filenames; void init_hard_fork() @@ -283,19 +283,19 @@ TYPED_TEST(BlockchainDBTest, AddBlock) ASSERT_NO_THROW(this->m_db->add_block(this->m_blocks[1], t_sizes[1], t_sizes[1], t_diffs[1], t_coins[1], this->m_txs[1])); block b; - ASSERT_TRUE(this->m_db->block_exists(get_block_hash(this->m_blocks[0]))); - ASSERT_NO_THROW(b = this->m_db->get_block(get_block_hash(this->m_blocks[0]))); + ASSERT_TRUE(this->m_db->block_exists(get_block_hash(this->m_blocks[0].first))); + ASSERT_NO_THROW(b = this->m_db->get_block(get_block_hash(this->m_blocks[0].first))); - ASSERT_TRUE(compare_blocks(this->m_blocks[0], b)); + ASSERT_TRUE(compare_blocks(this->m_blocks[0].first, b)); ASSERT_NO_THROW(b = this->m_db->get_block_from_height(0)); - ASSERT_TRUE(compare_blocks(this->m_blocks[0], b)); + ASSERT_TRUE(compare_blocks(this->m_blocks[0].first, b)); // assert that we can't add the same block twice ASSERT_THROW(this->m_db->add_block(this->m_blocks[0], t_sizes[0], t_sizes[0], t_diffs[0], t_coins[0], this->m_txs[0]), TX_EXISTS); - for (auto& h : this->m_blocks[0].tx_hashes) + for (auto& h : this->m_blocks[0].first.tx_hashes) { transaction tx; ASSERT_TRUE(this->m_db->tx_exists(h)); @@ -327,21 +327,21 @@ TYPED_TEST(BlockchainDBTest, RetrieveBlockData) ASSERT_NO_THROW(this->m_db->add_block(this->m_blocks[1], t_sizes[1], t_sizes[1], t_diffs[1], t_coins[1], this->m_txs[1])); ASSERT_EQ(t_diffs[1] - t_diffs[0], this->m_db->get_block_difficulty(1)); - ASSERT_HASH_EQ(get_block_hash(this->m_blocks[0]), this->m_db->get_block_hash_from_height(0)); + ASSERT_HASH_EQ(get_block_hash(this->m_blocks[0].first), this->m_db->get_block_hash_from_height(0)); std::vector blks; ASSERT_NO_THROW(blks = this->m_db->get_blocks_range(0, 1)); ASSERT_EQ(2, blks.size()); - ASSERT_HASH_EQ(get_block_hash(this->m_blocks[0]), get_block_hash(blks[0])); - ASSERT_HASH_EQ(get_block_hash(this->m_blocks[1]), get_block_hash(blks[1])); + ASSERT_HASH_EQ(get_block_hash(this->m_blocks[0].first), get_block_hash(blks[0])); + ASSERT_HASH_EQ(get_block_hash(this->m_blocks[1].first), get_block_hash(blks[1])); std::vector hashes; ASSERT_NO_THROW(hashes = this->m_db->get_hashes_range(0, 1)); ASSERT_EQ(2, hashes.size()); - ASSERT_HASH_EQ(get_block_hash(this->m_blocks[0]), hashes[0]); - ASSERT_HASH_EQ(get_block_hash(this->m_blocks[1]), hashes[1]); + ASSERT_HASH_EQ(get_block_hash(this->m_blocks[0].first), hashes[0]); + ASSERT_HASH_EQ(get_block_hash(this->m_blocks[1].first), hashes[1]); } } // anonymous namespace diff --git a/tests/unit_tests/long_term_block_weight.cpp b/tests/unit_tests/long_term_block_weight.cpp index 0ee6c5c2b..113b01865 100644 --- a/tests/unit_tests/long_term_block_weight.cpp +++ b/tests/unit_tests/long_term_block_weight.cpp @@ -123,7 +123,7 @@ TEST(long_term_block_weight, identical_before_fork) { size_t w = h < CRYPTONOTE_REWARD_BLOCKS_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); } for (uint64_t h = 0; h < 10 * TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW; ++h) @@ -140,7 +140,7 @@ TEST(long_term_block_weight, identical_after_fork_before_long_term_window) { size_t w = h < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); } for (uint64_t h = 0; h < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW; ++h) @@ -157,7 +157,7 @@ TEST(long_term_block_weight, ceiling_at_30000000) { size_t w = h < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); } ASSERT_EQ(bc->get_current_cumulative_block_weight_median(), 15000000); @@ -172,7 +172,7 @@ TEST(long_term_block_weight, multi_pop) { size_t w = h < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); } @@ -183,7 +183,7 @@ TEST(long_term_block_weight, multi_pop) { size_t w = h < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); } @@ -207,7 +207,7 @@ TEST(long_term_block_weight, multiple_updates) { size_t w = h < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); const uint64_t effective_median = bc->get_current_cumulative_block_weight_median(); const uint64_t effective_limit = bc->get_current_cumulative_block_weight_limit(); @@ -231,7 +231,7 @@ TEST(long_term_block_weight, pop_invariant_max) { size_t w = bc->get_db().height() < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); } @@ -259,7 +259,7 @@ TEST(long_term_block_weight, pop_invariant_max) { size_t w = bc->get_db().height() < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, bc->get_db().height(), bc->get_db().height(), {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, bc->get_db().height(), bc->get_db().height(), {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); } @@ -279,7 +279,7 @@ TEST(long_term_block_weight, pop_invariant_random) { size_t w = bc->get_db().height() < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); } @@ -314,7 +314,7 @@ TEST(long_term_block_weight, pop_invariant_random) uint32_t r = lcg(); size_t w = bc->get_db().height() < TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW ? CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5 : (r % bc->get_current_cumulative_block_weight_limit()); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, bc->get_db().height(), bc->get_db().height(), {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, bc->get_db().height(), bc->get_db().height(), {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit()); const uint64_t effective_median = bc->get_current_cumulative_block_weight_median(); const uint64_t effective_limit = bc->get_current_cumulative_block_weight_limit(); @@ -342,7 +342,7 @@ TEST(long_term_block_weight, long_growth_spike_and_drop) { size_t w = CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V5; uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit(&long_term_effective_median_block_weight)); } ASSERT_EQ(long_term_effective_median_block_weight, 300000); @@ -354,7 +354,7 @@ TEST(long_term_block_weight, long_growth_spike_and_drop) float t = h / float(365 * 720 * TEST_LONG_TERM_BLOCK_WEIGHT_WINDOW / 100000); size_t w = 300000 + t * 30000; uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit(&long_term_effective_median_block_weight)); } ASSERT_GT(long_term_effective_median_block_weight, 300000 * 1.07); @@ -365,7 +365,7 @@ TEST(long_term_block_weight, long_growth_spike_and_drop) { size_t w = bc->get_current_cumulative_block_weight_limit(); uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit(&long_term_effective_median_block_weight)); } ASSERT_GT(long_term_effective_median_block_weight, 300000 * 1.07); @@ -376,7 +376,7 @@ TEST(long_term_block_weight, long_growth_spike_and_drop) { size_t w = bc->get_current_cumulative_block_weight_median() * .25; uint64_t ltw = bc->get_next_long_term_block_weight(w); - bc->get_db().add_block(cryptonote::block(), w, ltw, h, h, {}); + bc->get_db().add_block(std::make_pair(cryptonote::block(), ""), w, ltw, h, h, {}); ASSERT_TRUE(bc->update_next_cumulative_weight_limit(&long_term_effective_median_block_weight)); } ASSERT_GT(long_term_effective_median_block_weight, 300000 * 1.07); From f75d51abda4c83ef58236bdc217f5a4c967084d2 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 12 Nov 2018 23:22:20 +0000 Subject: [PATCH 04/12] core: avoid calculating tx prefix hash when we don't need it --- .../cryptonote_format_utils.cpp | 9 +++++- .../cryptonote_format_utils.h | 1 + src/cryptonote_core/cryptonote_core.cpp | 28 +++++++++---------- src/cryptonote_core/cryptonote_core.h | 9 +++--- 4 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index 85c5131f6..e0033b682 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -210,7 +210,7 @@ namespace cryptonote return true; } //--------------------------------------------------------------- - bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash) + bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash) { std::stringstream ss; ss << tx_blob; @@ -222,6 +222,13 @@ namespace cryptonote //TODO: validate tx get_transaction_hash(tx, tx_hash); + return true; + } + //--------------------------------------------------------------- + bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash) + { + if (!parse_and_validate_tx_from_blob(tx_blob, tx, tx_hash)) + return false; get_transaction_prefix_hash(tx, tx_prefix_hash); return true; } diff --git a/src/cryptonote_basic/cryptonote_format_utils.h b/src/cryptonote_basic/cryptonote_format_utils.h index 4247e13da..92dbdbff0 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.h +++ b/src/cryptonote_basic/cryptonote_format_utils.h @@ -51,6 +51,7 @@ namespace cryptonote crypto::hash get_transaction_prefix_hash(const transaction_prefix& tx); bool parse_and_validate_tx_prefix_from_blob(const blobdata& tx_blob, transaction_prefix& tx); bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash); + bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash); bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx); bool parse_and_validate_tx_base_from_blob(const blobdata& tx_blob, transaction& tx); bool is_v1_tx(const blobdata_ref& tx_blob); diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index 599f42774..0d29f996a 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -724,7 +724,7 @@ namespace cryptonote return false; } //----------------------------------------------------------------------------------------------- - bool core::handle_incoming_tx_pre(const blobdata& tx_blob, tx_verification_context& tvc, cryptonote::transaction &tx, crypto::hash &tx_hash, crypto::hash &tx_prefixt_hash, bool keeped_by_block, bool relayed, bool do_not_relay) + bool core::handle_incoming_tx_pre(const blobdata& tx_blob, tx_verification_context& tvc, cryptonote::transaction &tx, crypto::hash &tx_hash, bool keeped_by_block, bool relayed, bool do_not_relay) { tvc = boost::value_initialized(); @@ -737,9 +737,8 @@ namespace cryptonote } tx_hash = crypto::null_hash; - tx_prefixt_hash = crypto::null_hash; - if(!parse_tx_from_blob(tx, tx_hash, tx_prefixt_hash, tx_blob)) + if(!parse_tx_from_blob(tx, tx_hash, tx_blob)) { LOG_PRINT_L1("WRONG TRANSACTION BLOB, Failed to parse, rejected"); tvc.m_verifivation_failed = true; @@ -772,7 +771,7 @@ namespace cryptonote return true; } //----------------------------------------------------------------------------------------------- - bool core::handle_incoming_tx_post(const blobdata& tx_blob, tx_verification_context& tvc, cryptonote::transaction &tx, crypto::hash &tx_hash, crypto::hash &tx_prefixt_hash, bool keeped_by_block, bool relayed, bool do_not_relay) + bool core::handle_incoming_tx_post(const blobdata& tx_blob, tx_verification_context& tvc, cryptonote::transaction &tx, crypto::hash &tx_hash, bool keeped_by_block, bool relayed, bool do_not_relay) { if(!check_tx_syntax(tx)) { @@ -906,7 +905,7 @@ namespace cryptonote TRY_ENTRY(); CRITICAL_REGION_LOCAL(m_incoming_tx_lock); - struct result { bool res; cryptonote::transaction tx; crypto::hash hash; crypto::hash prefix_hash; }; + struct result { bool res; cryptonote::transaction tx; crypto::hash hash; }; std::vector results(tx_blobs.size()); tvc.resize(tx_blobs.size()); @@ -917,7 +916,7 @@ namespace cryptonote tpool.submit(&waiter, [&, i, it] { try { - results[i].res = handle_incoming_tx_pre(*it, tvc[i], results[i].tx, results[i].hash, results[i].prefix_hash, keeped_by_block, relayed, do_not_relay); + results[i].res = handle_incoming_tx_pre(*it, tvc[i], results[i].tx, results[i].hash, keeped_by_block, relayed, do_not_relay); } catch (const std::exception &e) { @@ -947,7 +946,7 @@ namespace cryptonote tpool.submit(&waiter, [&, i, it] { try { - results[i].res = handle_incoming_tx_post(*it, tvc[i], results[i].tx, results[i].hash, results[i].prefix_hash, keeped_by_block, relayed, do_not_relay); + results[i].res = handle_incoming_tx_post(*it, tvc[i], results[i].tx, results[i].hash, keeped_by_block, relayed, do_not_relay); } catch (const std::exception &e) { @@ -983,7 +982,7 @@ namespace cryptonote continue; const size_t weight = get_transaction_weight(results[i].tx, it->size()); - ok &= add_new_tx(results[i].tx, results[i].hash, tx_blobs[i], results[i].prefix_hash, weight, tvc[i], keeped_by_block, relayed, do_not_relay); + ok &= add_new_tx(results[i].tx, results[i].hash, tx_blobs[i], weight, tvc[i], keeped_by_block, relayed, do_not_relay); if(tvc[i].m_verifivation_failed) {MERROR_VER("Transaction verification failed: " << results[i].hash);} else if(tvc[i].m_verifivation_impossible) @@ -1197,11 +1196,10 @@ namespace cryptonote bool core::add_new_tx(transaction& tx, tx_verification_context& tvc, bool keeped_by_block, bool relayed, bool do_not_relay) { crypto::hash tx_hash = get_transaction_hash(tx); - crypto::hash tx_prefix_hash = get_transaction_prefix_hash(tx); blobdata bl; t_serializable_object_to_blob(tx, bl); size_t tx_weight = get_transaction_weight(tx, bl.size()); - return add_new_tx(tx, tx_hash, bl, tx_prefix_hash, tx_weight, tvc, keeped_by_block, relayed, do_not_relay); + return add_new_tx(tx, tx_hash, bl, tx_weight, tvc, keeped_by_block, relayed, do_not_relay); } //----------------------------------------------------------------------------------------------- size_t core::get_blockchain_total_transactions() const @@ -1209,7 +1207,7 @@ namespace cryptonote return m_blockchain_storage.get_total_transactions(); } //----------------------------------------------------------------------------------------------- - bool core::add_new_tx(transaction& tx, const crypto::hash& tx_hash, const cryptonote::blobdata &blob, const crypto::hash& tx_prefix_hash, size_t tx_weight, tx_verification_context& tvc, bool keeped_by_block, bool relayed, bool do_not_relay) + bool core::add_new_tx(transaction& tx, const crypto::hash& tx_hash, const cryptonote::blobdata &blob, size_t tx_weight, tx_verification_context& tvc, bool keeped_by_block, bool relayed, bool do_not_relay) { if(m_mempool.have_tx(tx_hash)) { @@ -1250,8 +1248,8 @@ namespace cryptonote { std::vector> txs; cryptonote::transaction tx; - crypto::hash tx_hash, tx_prefix_hash; - if (!parse_and_validate_tx_from_blob(tx_blob, tx, tx_hash, tx_prefix_hash)) + crypto::hash tx_hash; + if (!parse_and_validate_tx_from_blob(tx_blob, tx, tx_hash)) { LOG_ERROR("Failed to parse relayed transaction"); return; @@ -1466,9 +1464,9 @@ namespace cryptonote return m_blockchain_storage.have_block(id); } //----------------------------------------------------------------------------------------------- - bool core::parse_tx_from_blob(transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash, const blobdata& blob) const + bool core::parse_tx_from_blob(transaction& tx, crypto::hash& tx_hash, const blobdata& blob) const { - return parse_and_validate_tx_from_blob(blob, tx, tx_hash, tx_prefix_hash); + return parse_and_validate_tx_from_blob(blob, tx, tx_hash); } //----------------------------------------------------------------------------------------------- bool core::check_tx_syntax(const transaction& tx) const diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h index 79d06662e..131440511 100644 --- a/src/cryptonote_core/cryptonote_core.h +++ b/src/cryptonote_core/cryptonote_core.h @@ -829,13 +829,12 @@ namespace cryptonote * * @param tx_hash the transaction's hash * @param blob the transaction as a blob - * @param tx_prefix_hash the transaction prefix' hash * @param tx_weight the weight of the transaction * @param relayed whether or not the transaction was relayed to us * @param do_not_relay whether to prevent the transaction from being relayed * */ - bool add_new_tx(transaction& tx, const crypto::hash& tx_hash, const cryptonote::blobdata &blob, const crypto::hash& tx_prefix_hash, size_t tx_weight, tx_verification_context& tvc, bool keeped_by_block, bool relayed, bool do_not_relay); + bool add_new_tx(transaction& tx, const crypto::hash& tx_hash, const cryptonote::blobdata &blob, size_t tx_weight, tx_verification_context& tvc, bool keeped_by_block, bool relayed, bool do_not_relay); /** * @brief add a new transaction to the transaction pool @@ -875,7 +874,7 @@ namespace cryptonote * * @note see parse_tx_from_blob(transaction&, crypto::hash&, crypto::hash&, const blobdata&) const */ - bool parse_tx_from_blob(transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash, const blobdata& blob) const; + bool parse_tx_from_blob(transaction& tx, crypto::hash& tx_hash, const blobdata& blob) const; /** * @brief check a transaction's syntax @@ -908,8 +907,8 @@ namespace cryptonote bool check_tx_semantic(const transaction& tx, bool keeped_by_block) const; void set_semantics_failed(const crypto::hash &tx_hash); - bool handle_incoming_tx_pre(const blobdata& tx_blob, tx_verification_context& tvc, cryptonote::transaction &tx, crypto::hash &tx_hash, crypto::hash &tx_prefixt_hash, bool keeped_by_block, bool relayed, bool do_not_relay); - bool handle_incoming_tx_post(const blobdata& tx_blob, tx_verification_context& tvc, cryptonote::transaction &tx, crypto::hash &tx_hash, crypto::hash &tx_prefixt_hash, bool keeped_by_block, bool relayed, bool do_not_relay); + bool handle_incoming_tx_pre(const blobdata& tx_blob, tx_verification_context& tvc, cryptonote::transaction &tx, crypto::hash &tx_hash, bool keeped_by_block, bool relayed, bool do_not_relay); + bool handle_incoming_tx_post(const blobdata& tx_blob, tx_verification_context& tvc, cryptonote::transaction &tx, crypto::hash &tx_hash, bool keeped_by_block, bool relayed, bool do_not_relay); struct tx_verification_batch_info { const cryptonote::transaction *tx; crypto::hash tx_hash; tx_verification_context &tvc; bool &result; }; bool handle_incoming_tx_accumulated_batch(std::vector &tx_info, bool keeped_by_block); From 07d655e438e1b1e05426a5c75627f5d8c63bbc72 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 20 Nov 2018 10:58:41 +0000 Subject: [PATCH 05/12] blockchain: avoid duplicate block hash computation --- src/cryptonote_core/blockchain.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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; From 9827880877299ff8d30b90302a10aeface7598c4 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 20 Nov 2018 16:24:12 +0000 Subject: [PATCH 06/12] blockchain: avoid pointless transaction copy and temporary --- src/cryptonote_core/blockchain.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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. From 79b4e9f3779796d9233e3e169f5c1086a49717dd Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 20 Nov 2018 20:19:39 +0000 Subject: [PATCH 07/12] 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(-) 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(); } From dc5a76095cc4b05d7a706c3b650e54f2db314926 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 21 Nov 2018 12:57:51 +0000 Subject: [PATCH 08/12] blockchain: avoid unneeded block copy --- src/cryptonote_core/blockchain.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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) { From 238401d4e950dad6a243fb895027dd26d2992ee6 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 23 Nov 2018 11:15:04 +0000 Subject: [PATCH 09/12] core: avoid double parsing blocks after hoh --- .../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(-) 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 From 9feda0eeba34ca6c23e6e7fd819d00932b3ce5aa Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 23 Nov 2018 12:11:51 +0000 Subject: [PATCH 10/12] cryptonote: speed up calculating coinbase tx prunable hash It's a hash of an empty buffer, so we can avoid keccak --- src/cryptonote_basic/cryptonote_format_utils.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index e0033b682..f4e7c9d36 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -981,11 +981,15 @@ namespace cryptonote { if (t.version == 1) return false; + static const crypto::hash empty_hash = { (char)0x70, (char)0xa4, (char)0x85, (char)0x5d, (char)0x04, (char)0xd8, (char)0xfa, (char)0x7b, (char)0x3b, (char)0x27, (char)0x82, (char)0xca, (char)0x53, (char)0xb6, (char)0x00, (char)0xe5, (char)0xc0, (char)0x03, (char)0xc7, (char)0xdc, (char)0xb2, (char)0x7d, (char)0x7e, (char)0x92, (char)0x3c, (char)0x23, (char)0xf7, (char)0x86, (char)0x01, (char)0x46, (char)0xd2, (char)0xc5 }; const unsigned int unprunable_size = t.unprunable_size; if (blob && unprunable_size) { CHECK_AND_ASSERT_MES(unprunable_size <= blob->size(), false, "Inconsistent transaction unprunable and blob sizes"); - cryptonote::get_blob_hash(epee::span(blob->data() + unprunable_size, blob->size() - unprunable_size), res); + if (blob->size() - unprunable_size == 0) + res = empty_hash; + else + cryptonote::get_blob_hash(epee::span(blob->data() + unprunable_size, blob->size() - unprunable_size), res); } else { @@ -997,7 +1001,10 @@ namespace cryptonote const size_t mixin = t.vin.empty() ? 0 : t.vin[0].type() == typeid(txin_to_key) ? boost::get(t.vin[0]).key_offsets.size() - 1 : 0; bool r = tt.rct_signatures.p.serialize_rctsig_prunable(ba, t.rct_signatures.type, inputs, outputs, mixin); CHECK_AND_ASSERT_MES(r, false, "Failed to serialize rct signatures prunable"); - cryptonote::get_blob_hash(ss.str(), res); + if (ss.str().empty()) + res = empty_hash; + else + cryptonote::get_blob_hash(ss.str(), res); } return true; } From 88c85c18e0173f479470842fbe49626cdc86e7d1 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 23 Nov 2018 13:05:48 +0000 Subject: [PATCH 11/12] 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; } From c4851024ced48988c9a61db2a89b0cfd0c56915a Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 26 Nov 2018 00:19:50 +0000 Subject: [PATCH 12/12] wallet_rpc_server: avoid repeated string allocations when parsing --- src/wallet/wallet_rpc_server.cpp | 34 +++++++++++++------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 9adfb06c8..110777c4f 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -1684,23 +1684,14 @@ namespace tools cryptonote::blobdata payment_id_blob; // TODO - should the whole thing fail because of one bad id? - - if(!epee::string_tools::parse_hexstr_to_binbuff(payment_id_str, payment_id_blob)) - { - er.code = WALLET_RPC_ERROR_CODE_WRONG_PAYMENT_ID; - er.message = "Payment ID has invalid format: " + payment_id_str; - return false; - } - - if(sizeof(payment_id) == payment_id_blob.size()) + bool r; + if (payment_id_str.size() == 2 * sizeof(payment_id)) { - payment_id = *reinterpret_cast(payment_id_blob.data()); + r = epee::string_tools::hex_to_pod(payment_id_str, payment_id); } - else if(sizeof(payment_id8) == payment_id_blob.size()) + else if (payment_id_str.size() == 2 * sizeof(payment_id8)) { - payment_id8 = *reinterpret_cast(payment_id_blob.data()); - memcpy(payment_id.data, payment_id8.data, 8); - memset(payment_id.data + 8, 0, 24); + r = epee::string_tools::hex_to_pod(payment_id_str, payment_id8); } else { @@ -1709,6 +1700,13 @@ namespace tools return false; } + if(!r) + { + er.code = WALLET_RPC_ERROR_CODE_WRONG_PAYMENT_ID; + er.message = "Payment ID has invalid format: " + payment_id_str; + return false; + } + std::list payment_list; m_wallet->get_payments(payment_id, payment_list, req.min_block_height); @@ -2588,23 +2586,19 @@ namespace tools ski.resize(req.signed_key_images.size()); for (size_t n = 0; n < ski.size(); ++n) { - cryptonote::blobdata bd; - - if(!epee::string_tools::parse_hexstr_to_binbuff(req.signed_key_images[n].key_image, bd) || bd.size() != sizeof(crypto::key_image)) + if (!epee::string_tools::hex_to_pod(req.signed_key_images[n].key_image, ski[n].first)) { er.code = WALLET_RPC_ERROR_CODE_WRONG_KEY_IMAGE; er.message = "failed to parse key image"; return false; } - ski[n].first = *reinterpret_cast(bd.data()); - if(!epee::string_tools::parse_hexstr_to_binbuff(req.signed_key_images[n].signature, bd) || bd.size() != sizeof(crypto::signature)) + if (!epee::string_tools::hex_to_pod(req.signed_key_images[n].signature, ski[n].second)) { er.code = WALLET_RPC_ERROR_CODE_WRONG_SIGNATURE; er.message = "failed to parse signature"; return false; } - ski[n].second = *reinterpret_cast(bd.data()); } uint64_t spent = 0, unspent = 0; uint64_t height = m_wallet->import_key_images(ski, req.offset, spent, unspent);