From e8468c5dcff72e1552c5c020af5f176259a0012c Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Sun, 15 Nov 2020 00:08:10 -0500 Subject: Fix timeout checks for forwarded and Dandelion++ stem txes --- src/cryptonote_core/cryptonote_core.cpp | 2 +- src/cryptonote_core/cryptonote_core.h | 1 - src/cryptonote_core/tx_pool.cpp | 41 +++++++++++++++++++++++++++++---- src/cryptonote_core/tx_pool.h | 11 +++++++-- 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index fef411a0c..bd138038d 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -1725,7 +1725,7 @@ namespace cryptonote m_starter_message_showed = true; } - m_txpool_auto_relayer.do_call(boost::bind(&core::relay_txpool_transactions, this)); + relay_txpool_transactions(); // txpool handles periodic DB checking m_check_updates_interval.do_call(boost::bind(&core::check_updates, this)); m_check_disk_space_interval.do_call(boost::bind(&core::check_disk_space, this)); m_block_rate_interval.do_call(boost::bind(&core::check_block_rate, this)); diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h index 7c578ac51..0e4318256 100644 --- a/src/cryptonote_core/cryptonote_core.h +++ b/src/cryptonote_core/cryptonote_core.h @@ -1065,7 +1065,6 @@ namespace cryptonote epee::math_helper::once_a_time_seconds<60*60*12, false> m_store_blockchain_interval; //!< interval for manual storing of Blockchain, if enabled epee::math_helper::once_a_time_seconds<60*60*2, true> m_fork_moaner; //!< interval for checking HardFork status - epee::math_helper::once_a_time_seconds<60*2, false> m_txpool_auto_relayer; //!< interval for checking re-relaying txpool transactions epee::math_helper::once_a_time_seconds<60*60*12, true> m_check_updates_interval; //!< interval for checking for new versions epee::math_helper::once_a_time_seconds<60*10, true> m_check_disk_space_interval; //!< interval for checking for disk space epee::math_helper::once_a_time_seconds<90, false> m_block_rate_interval; //!< interval for checking block rate diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index 28721ee36..0db133b62 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -91,6 +91,9 @@ namespace cryptonote time_t const MAX_RELAY_TIME = (60 * 60 * 4); // at most that many seconds between resends float const ACCEPT_THRESHOLD = 1.0f; + //! Max DB check interval for relayable txes + constexpr const std::chrono::minutes max_relayable_check{2}; + constexpr const std::chrono::seconds forward_delay_average{CRYPTONOTE_FORWARD_DELAY_AVERAGE}; // a kind of increasing backoff within min/max bounds @@ -115,12 +118,21 @@ namespace cryptonote else return get_min_block_weight(version) - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE; } + + // external lock must be held for the comparison+set to work properly + void set_if_less(std::atomic& next_check, const time_t candidate) noexcept + { + if (candidate < next_check.load(std::memory_order_relaxed)) + next_check = candidate; + } } //--------------------------------------------------------------------------------- //--------------------------------------------------------------------------------- - tx_memory_pool::tx_memory_pool(Blockchain& bchs): m_blockchain(bchs), m_cookie(0), m_txpool_max_weight(DEFAULT_TXPOOL_MAX_WEIGHT), m_txpool_weight(0), m_mine_stem_txes(false) + tx_memory_pool::tx_memory_pool(Blockchain& bchs): m_blockchain(bchs), m_cookie(0), m_txpool_max_weight(DEFAULT_TXPOOL_MAX_WEIGHT), m_txpool_weight(0), m_mine_stem_txes(false), m_next_check(std::time(nullptr)) { - + // class code expects unsigned values throughout + if (m_next_check < time_t(0)) + throw std::runtime_error{"Unexpected time_t (system clock) value"}; } //--------------------------------------------------------------------------------- bool tx_memory_pool::add_tx(transaction &tx, /*const crypto::hash& tx_prefix_hash,*/ const crypto::hash &id, const cryptonote::blobdata &blob, size_t tx_weight, tx_verification_context& tvc, relay_method tx_relay, bool relayed, uint8_t version) @@ -314,7 +326,10 @@ namespace cryptonote using clock = std::chrono::system_clock; auto last_relayed_time = std::numeric_limits::max(); if (tx_relay == relay_method::forward) + { last_relayed_time = clock::to_time_t(clock::now() + crypto::random_poisson_seconds{forward_delay_average}()); + set_if_less(m_next_check, time_t(last_relayed_time)); + } // else the `set_relayed` function will adjust the time accordingly later //update transactions container @@ -728,16 +743,22 @@ namespace cryptonote } //--------------------------------------------------------------------------------- //TODO: investigate whether boolean return is appropriate - bool tx_memory_pool::get_relayable_transactions(std::vector> &txs) const + bool tx_memory_pool::get_relayable_transactions(std::vector> &txs) { - std::vector> change_timestamps; + using clock = std::chrono::system_clock; + const uint64_t now = time(NULL); + if (uint64_t{std::numeric_limits::max()} < now || time_t(now) < m_next_check) + return false; + + uint64_t next_check = clock::to_time_t(clock::from_time_t(time_t(now)) + max_relayable_check); + std::vector> change_timestamps; CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL1(m_blockchain); LockedTXN lock(m_blockchain.get_db()); txs.reserve(m_blockchain.get_txpool_tx_count()); - m_blockchain.for_all_txpool_txes([this, now, &txs, &change_timestamps](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref *){ + m_blockchain.for_all_txpool_txes([this, now, &txs, &change_timestamps, &next_check](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref *){ // 0 fee transactions are never relayed if(!meta.pruned && meta.fee > 0 && !meta.do_not_relay) { @@ -747,7 +768,10 @@ namespace cryptonote case relay_method::stem: case relay_method::forward: if (meta.last_relayed_time > now) + { + next_check = std::min(next_check, meta.last_relayed_time); return true; // continue to next tx + } change_timestamps.emplace_back(txid, meta); break; default: @@ -792,6 +816,8 @@ namespace cryptonote elem.second.last_relayed_time = now + get_relay_delay(now, elem.second.receive_time); m_blockchain.update_txpool_tx(elem.first, elem.second); } + + m_next_check = time_t(next_check); return true; } //--------------------------------------------------------------------------------- @@ -799,6 +825,7 @@ namespace cryptonote { crypto::random_poisson_seconds embargo_duration{dandelionpp_embargo_average}; const auto now = std::chrono::system_clock::now(); + uint64_t next_relay = uint64_t{std::numeric_limits::max()}; CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL1(m_blockchain); @@ -815,7 +842,10 @@ namespace cryptonote meta.relayed = true; if (meta.dandelionpp_stem) + { meta.last_relayed_time = std::chrono::system_clock::to_time_t(now + embargo_duration()); + next_relay = std::min(next_relay, meta.last_relayed_time); + } else meta.last_relayed_time = std::chrono::system_clock::to_time_t(now); @@ -829,6 +859,7 @@ namespace cryptonote } } lock.commit(); + set_if_less(m_next_check, time_t(next_relay)); } //--------------------------------------------------------------------------------- size_t tx_memory_pool::get_transactions_count(bool include_sensitive) const diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h index 8955d7551..ab2a57ea2 100644 --- a/src/cryptonote_core/tx_pool.h +++ b/src/cryptonote_core/tx_pool.h @@ -31,6 +31,7 @@ #pragma once #include "include_base_utils.h" +#include #include #include #include @@ -329,11 +330,14 @@ namespace cryptonote * isn't old enough that relaying it is considered harmful * Note a transaction can be "relayable" even if do_not_relay is true * + * This function will skip all DB checks if an insufficient amount of + * time since the last call. + * * @param txs return-by-reference the transactions and their hashes * - * @return true + * @return True if DB was checked, false if DB checks skipped. */ - bool get_relayable_transactions(std::vector>& txs) const; + bool get_relayable_transactions(std::vector>& txs); /** * @brief tell the pool that certain transactions were just relayed @@ -609,6 +613,9 @@ private: mutable std::unordered_map> m_input_cache; std::unordered_map m_parsed_tx_cache; + + //! Next timestamp that a DB check for relayable txes is allowed + std::atomic m_next_check; }; } -- cgit v1.2.3