From 6d315459b6d001ecee3a6048ba70255b1ecf381a Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 25 Mar 2017 09:18:53 +0000 Subject: [PATCH] core: avoid possible reordering bugs wth tx/bloch hash cache --- src/cryptonote_basic/cryptonote_basic.h | 40 ++++++++++++++----- .../cryptonote_format_utils.cpp | 26 +++++------- src/cryptonote_basic/miner.cpp | 4 +- src/cryptonote_core/cryptonote_tx_utils.cpp | 6 +-- 4 files changed, 45 insertions(+), 31 deletions(-) diff --git a/src/cryptonote_basic/cryptonote_basic.h b/src/cryptonote_basic/cryptonote_basic.h index 3492ace2b..dab0d9f26 100644 --- a/src/cryptonote_basic/cryptonote_basic.h +++ b/src/cryptonote_basic/cryptonote_basic.h @@ -35,6 +35,7 @@ #include #include // memcmp #include +#include #include "serialization/serialization.h" #include "serialization/variant.h" #include "serialization/vector.h" @@ -186,6 +187,11 @@ namespace cryptonote class transaction: public transaction_prefix { + private: + // hash cash + mutable std::atomic hash_valid; + mutable std::atomic blob_size_valid; + public: std::vector > signatures; //count signatures always the same as inputs count rct::rctSig rct_signatures; @@ -193,19 +199,23 @@ namespace cryptonote // hash cash mutable crypto::hash hash; mutable size_t blob_size; - mutable bool hash_valid; - mutable bool blob_size_valid; transaction(); + transaction(const transaction &t): transaction_prefix(t), hash_valid(false), 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); } } + 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); } return *this; } virtual ~transaction(); void set_null(); void invalidate_hashes(); + bool is_hash_valid() const { return hash_valid.load(std::memory_order_acquire); } + void set_hash_valid(bool v) const { hash_valid.store(v,std::memory_order_release); } + bool is_blob_size_valid() const { return hash_valid.load(std::memory_order_acquire); } + void set_blob_size_valid(bool v) const { blob_size_valid.store(v,std::memory_order_release); } BEGIN_SERIALIZE_OBJECT() if (!typename Archive::is_saving()) { - hash_valid = false; - blob_size_valid = false; + set_hash_valid(false); + set_blob_size_valid(false); } FIELDS(*static_cast(this)) @@ -312,15 +322,15 @@ namespace cryptonote extra.clear(); signatures.clear(); rct_signatures.type = rct::RCTTypeNull; - hash_valid = false; - blob_size_valid = false; + set_hash_valid(false); + set_blob_size_valid(false); } inline void transaction::invalidate_hashes() { - hash_valid = false; - blob_size_valid = false; + set_hash_valid(false); + set_blob_size_valid(false); } inline @@ -361,19 +371,27 @@ namespace cryptonote struct block: public block_header { + private: + // hash cash + mutable std::atomic hash_valid; + + public: block(): block_header(), hash_valid(false) {} - void invalidate_hashes() { hash_valid = false; } + block(const block &b): block_header(b), hash_valid(false), miner_tx(b.miner_tx), tx_hashes(b.tx_hashes) { if (b.is_hash_valid()) { hash = b.hash; set_hash_valid(true); } } + block &operator=(const block &b) { block_header::operator=(b); hash_valid = false; miner_tx = b.miner_tx; tx_hashes = b.tx_hashes; if (b.is_hash_valid()) { hash = b.hash; set_hash_valid(true); } return *this; } + void invalidate_hashes() { set_hash_valid(false); } + bool is_hash_valid() const { return hash_valid.load(std::memory_order_acquire); } + void set_hash_valid(bool v) const { hash_valid.store(v,std::memory_order_release); } transaction miner_tx; std::vector tx_hashes; // hash cash mutable crypto::hash hash; - mutable bool hash_valid; BEGIN_SERIALIZE_OBJECT() if (!typename Archive::is_saving()) - hash_valid = false; + set_hash_valid(false); FIELDS(*static_cast(this)) FIELD(miner_tx) diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index 2f23194c8..745dfb72e 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -100,8 +100,7 @@ namespace cryptonote binary_archive ba(ss); bool r = ::serialization::serialize(ba, tx); CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob"); - tx.hash_valid = false; - tx.blob_size_valid = false; + tx.invalidate_hashes(); return true; } //--------------------------------------------------------------- @@ -122,8 +121,7 @@ namespace cryptonote binary_archive ba(ss); bool r = ::serialization::serialize(ba, tx); CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob"); - tx.hash_valid = false; - tx.blob_size_valid = false; + tx.invalidate_hashes(); //TODO: validate tx get_transaction_hash(tx, tx_hash); @@ -660,7 +658,7 @@ namespace cryptonote //--------------------------------------------------------------- bool get_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size) { - if (t.hash_valid) + if (t.is_hash_valid()) { #ifdef ENABLE_HASH_CASH_INTEGRITY_CHECK CHECK_AND_ASSERT_THROW_MES(!calculate_transaction_hash(t, res, blob_size) || t.hash == res, "tx hash cash integrity failure"); @@ -668,10 +666,10 @@ namespace cryptonote res = t.hash; if (blob_size) { - if (!t.blob_size_valid) + if (!t.is_blob_size_valid()) { t.blob_size = get_object_blobsize(t); - t.blob_size_valid = true; + t.set_blob_size_valid(true); } *blob_size = t.blob_size; } @@ -683,11 +681,11 @@ namespace cryptonote if (!ret) return false; t.hash = res; - t.hash_valid = true; + t.set_hash_valid(true); if (blob_size) { t.blob_size = *blob_size; - t.blob_size_valid = true; + t.set_blob_size_valid(true); } return true; } @@ -735,7 +733,7 @@ namespace cryptonote //--------------------------------------------------------------- bool get_block_hash(const block& b, crypto::hash& res) { - if (b.hash_valid) + if (b.is_hash_valid()) { #ifdef ENABLE_HASH_CASH_INTEGRITY_CHECK CHECK_AND_ASSERT_THROW_MES(!calculate_block_hash(b, res) || b.hash == res, "block hash cash integrity failure"); @@ -749,7 +747,7 @@ namespace cryptonote if (!ret) return false; b.hash = res; - b.hash_valid = true; + b.set_hash_valid(true); return true; } //--------------------------------------------------------------- @@ -769,7 +767,6 @@ namespace cryptonote string_tools::hex_to_pod(longhash_202612, res); return true; } - block b_local = b; //workaround to avoid const errors with do_serialize blobdata bd = get_block_hashing_blob(b); crypto::cn_slow_hash(bd.data(), bd.size(), res); return true; @@ -809,9 +806,8 @@ namespace cryptonote binary_archive ba(ss); bool r = ::serialization::serialize(ba, b); CHECK_AND_ASSERT_MES(r, false, "Failed to parse block from blob"); - b.hash_valid = false; - b.miner_tx.hash_valid = false; - b.miner_tx.blob_size_valid = false; + b.invalidate_hashes(); + b.miner_tx.invalidate_hashes(); return true; } //--------------------------------------------------------------- diff --git a/src/cryptonote_basic/miner.cpp b/src/cryptonote_basic/miner.cpp index 5a2a0d009..4c84e7eee 100644 --- a/src/cryptonote_basic/miner.cpp +++ b/src/cryptonote_basic/miner.cpp @@ -355,11 +355,11 @@ namespace cryptonote if(check_hash(h, diffic)) { - bl.hash_valid = false; + bl.invalidate_hashes(); return true; } } - bl.hash_valid = false; + bl.invalidate_hashes(); return false; } //----------------------------------------------------------------------------------------------------- diff --git a/src/cryptonote_core/cryptonote_tx_utils.cpp b/src/cryptonote_core/cryptonote_tx_utils.cpp index 47211ff95..26d5fb767 100644 --- a/src/cryptonote_core/cryptonote_tx_utils.cpp +++ b/src/cryptonote_core/cryptonote_tx_utils.cpp @@ -133,7 +133,7 @@ namespace cryptonote tx.unlock_time = height + CRYPTONOTE_MINED_MONEY_UNLOCK_WINDOW; tx.vin.push_back(in); - tx.hash_valid = tx.blob_size_valid = false; + tx.invalidate_hashes(); //LOG_PRINT("MINER_TX generated ok, block_reward=" << print_money(block_reward) << "(" << print_money(block_reward - fee) << "+" << print_money(fee) // << "), current_block_size=" << current_block_size << ", already_generated_coins=" << already_generated_coins << ", tx_id=" << get_transaction_hash(tx), LOG_LEVEL_2); @@ -454,7 +454,7 @@ namespace cryptonote MCINFO("construct_tx", "transaction_created: " << get_transaction_hash(tx) << ENDL << obj_to_json_str(tx) << ENDL); } - tx.hash_valid = tx.blob_size_valid = false; + tx.invalidate_hashes(); return true; } @@ -492,7 +492,7 @@ namespace cryptonote bl.timestamp = 0; bl.nonce = nonce; miner::find_nonce_for_given_block(bl, 1, 0); - bl.hash_valid = false; + bl.invalidate_hashes(); return true; } //---------------------------------------------------------------