From a883467a8bc32c967e68c08fe2fd5f6923a8d8db Mon Sep 17 00:00:00 2001 From: evoskuil Date: Mon, 12 Aug 2024 21:07:03 -0400 Subject: [PATCH 1/3] Use byte_allocator. --- include/bitcoin/system/impl/stream/streamers/byte_reader.ipp | 3 +-- include/bitcoin/system/stream/streamers/byte_reader.hpp | 4 ++-- .../bitcoin/system/stream/streamers/interfaces/bytereader.hpp | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/bitcoin/system/impl/stream/streamers/byte_reader.ipp b/include/bitcoin/system/impl/stream/streamers/byte_reader.ipp index df21a31350..4b72b17578 100644 --- a/include/bitcoin/system/impl/stream/streamers/byte_reader.ipp +++ b/include/bitcoin/system/impl/stream/streamers/byte_reader.ipp @@ -615,8 +615,7 @@ byte_reader::get_arena() const NOEXCEPT } template -typename byte_reader::memory_allocator& -byte_reader::get_allocator() const NOEXCEPT +byte_allocator& byte_reader::get_allocator() const NOEXCEPT { return allocator_; } diff --git a/include/bitcoin/system/stream/streamers/byte_reader.hpp b/include/bitcoin/system/stream/streamers/byte_reader.hpp index be2a456d35..649971d327 100644 --- a/include/bitcoin/system/stream/streamers/byte_reader.hpp +++ b/include/bitcoin/system/stream/streamers/byte_reader.hpp @@ -184,7 +184,7 @@ class byte_reader memory_arena get_arena() const NOEXCEPT override; /// Memory allocator used to construct objects. - memory_allocator& get_allocator() const NOEXCEPT override; + byte_allocator& get_allocator() const NOEXCEPT override; /// The stream is valid. operator bool() const NOEXCEPT override; @@ -219,7 +219,7 @@ class byte_reader IStream& stream_; size_t remaining_; - mutable memory_allocator allocator_; + mutable byte_allocator allocator_; }; } // namespace system diff --git a/include/bitcoin/system/stream/streamers/interfaces/bytereader.hpp b/include/bitcoin/system/stream/streamers/interfaces/bytereader.hpp index a1d9dc529c..5d18904df3 100644 --- a/include/bitcoin/system/stream/streamers/interfaces/bytereader.hpp +++ b/include/bitcoin/system/stream/streamers/interfaces/bytereader.hpp @@ -34,7 +34,6 @@ class bytereader { public: using memory_arena = arena*; - using memory_allocator = allocator; /// Integrals. /// ----------------------------------------------------------------------- @@ -152,7 +151,7 @@ class bytereader virtual memory_arena get_arena() const NOEXCEPT = 0; /// Memory allocator used to construct objects. - virtual memory_allocator& get_allocator() const NOEXCEPT = 0; + virtual byte_allocator& get_allocator() const NOEXCEPT = 0; /// The stream is valid. virtual operator bool() const NOEXCEPT = 0; From a01087e4b7ac500316274798e5dd119485fcf0d1 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Mon, 12 Aug 2024 21:13:16 -0400 Subject: [PATCH 2/3] Comments. --- include/bitcoin/system/data/memory.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/bitcoin/system/data/memory.hpp b/include/bitcoin/system/data/memory.hpp index eee15263f3..3ffb4d7fa4 100644 --- a/include/bitcoin/system/data/memory.hpp +++ b/include/bitcoin/system/data/memory.hpp @@ -122,6 +122,7 @@ inline std::unique_ptr to_unique(const Type& value) NOEXCEPT template inline std::unique_ptr to_unique(Args&&... values) NOEXCEPT { + // Type{} required due to CLang bug. return std::make_unique(Type{ std::forward(values)... }); } From ffd7d5613393d125ce09aeebba88c5c60772771b Mon Sep 17 00:00:00 2001 From: evoskuil Date: Tue, 13 Aug 2024 20:09:06 -0400 Subject: [PATCH 3/3] Use std::optional for cache, and default tx copy/assign. --- include/bitcoin/system/chain/header.hpp | 5 +- include/bitcoin/system/chain/input.hpp | 1 + include/bitcoin/system/chain/transaction.hpp | 23 +++--- src/chain/header.cpp | 4 +- src/chain/transaction.cpp | 75 +++----------------- 5 files changed, 24 insertions(+), 84 deletions(-) diff --git a/include/bitcoin/system/chain/header.hpp b/include/bitcoin/system/chain/header.hpp index cc5a791d2e..2415cdd23c 100644 --- a/include/bitcoin/system/chain/header.hpp +++ b/include/bitcoin/system/chain/header.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -103,7 +104,7 @@ class BC_API header uint256_t proof() const NOEXCEPT; /// Cache (this overrides hash() computation). - void set_hash(hash_digest&& hash) const NOEXCEPT; + void set_hash(const hash_digest& hash) const NOEXCEPT; /// Reference used to avoid copy, sets cache if not set (not thread safe). const hash_digest& get_hash() const NOEXCEPT; @@ -152,7 +153,7 @@ class BC_API header bool valid_; // Identity hash caching. - mutable std::shared_ptr hash_{}; + mutable std::optional hash_{}; }; typedef std_vector
headers; diff --git a/include/bitcoin/system/chain/input.hpp b/include/bitcoin/system/chain/input.hpp index 74396c96d1..2a0834c078 100644 --- a/include/bitcoin/system/chain/input.hpp +++ b/include/bitcoin/system/chain/input.hpp @@ -156,6 +156,7 @@ class BC_API input sizes size_; public: + /// TODO: prevout destruct requires input destruct. /// Public mutable metadata access, copied but not compared for equality. mutable chain::output::cptr prevout{}; mutable chain::prevout metadata{ zero, max_uint32, false, false }; diff --git a/include/bitcoin/system/chain/transaction.hpp b/include/bitcoin/system/chain/transaction.hpp index a2c4bfe465..9d9f6f5fb3 100644 --- a/include/bitcoin/system/chain/transaction.hpp +++ b/include/bitcoin/system/chain/transaction.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -38,6 +39,9 @@ namespace chain { class BC_API transaction { public: + /// Cache is copied/moved on copy/assign. + DEFAULT_COPY_MOVE_DESTRUCT(transaction); + typedef std::shared_ptr cptr; typedef input_cptrs::const_iterator input_iterator; @@ -49,11 +53,6 @@ class BC_API transaction /// Default transaction is an invalid object. transaction() NOEXCEPT; - virtual ~transaction() NOEXCEPT; - - /// Cache is defaulted on copy/assign. - transaction(transaction&& other) NOEXCEPT; - transaction(const transaction& other) NOEXCEPT; transaction(uint32_t version, chain::inputs&& inputs, chain::outputs&& outputs, uint32_t locktime) NOEXCEPT; @@ -73,10 +72,6 @@ class BC_API transaction /// Operators. /// ----------------------------------------------------------------------- - /// Cache is defaulted on copy/assign. - transaction& operator=(transaction&& other) NOEXCEPT; - transaction& operator=(const transaction& other) NOEXCEPT; - bool operator==(const transaction& other) const NOEXCEPT; bool operator!=(const transaction& other) const NOEXCEPT; @@ -113,11 +108,11 @@ class BC_API transaction /// ----------------------------------------------------------------------- /// Initialize with externally-produced nominal hash value, as from store. - void set_nominal_hash(hash_digest&& hash) const NOEXCEPT; + void set_nominal_hash(const hash_digest& hash) const NOEXCEPT; /// Initialize with externally-produced witness hash value, as from store. /// This need not be set if the transaction is not segmented. - void set_witness_hash(hash_digest&& hash) const NOEXCEPT; + void set_witness_hash(const hash_digest& hash) const NOEXCEPT; /// Reference used to avoid copy, sets cache if not set (not thread safe). const hash_digest& get_hash(bool witness) const NOEXCEPT; @@ -275,9 +270,9 @@ class BC_API transaction sizes size_; // Signature and identity hash caching (witness hash if witnessed). - mutable std::unique_ptr nominal_hash_{}; - mutable std::unique_ptr witness_hash_{}; - mutable std::unique_ptr sighash_cache_{}; + mutable std::optional nominal_hash_{}; + mutable std::optional witness_hash_{}; + mutable std::optional sighash_cache_{}; }; typedef std_vector transactions; diff --git a/src/chain/header.cpp b/src/chain/header.cpp index f6f404be55..536a975eef 100644 --- a/src/chain/header.cpp +++ b/src/chain/header.cpp @@ -228,9 +228,9 @@ uint32_t header::nonce() const NOEXCEPT return nonce_; } -void header::set_hash(hash_digest&& hash) const NOEXCEPT +void header::set_hash(const hash_digest& hash) const NOEXCEPT { - hash_ = to_shared(std::move(hash)); + hash_ = hash; } // computed diff --git a/src/chain/transaction.cpp b/src/chain/transaction.cpp index c1ce944b62..5b0051cc22 100644 --- a/src/chain/transaction.cpp +++ b/src/chain/transaction.cpp @@ -79,33 +79,6 @@ transaction::transaction() NOEXCEPT { } -transaction::~transaction() NOEXCEPT -{ -} - -transaction::transaction(transaction&& other) NOEXCEPT - : transaction(other) -{ -} - -transaction::transaction(const transaction& other) NOEXCEPT - : transaction( - other.version_, - other.inputs_, - other.outputs_, - other.locktime_, - other.segregated_, - other.valid_) -{ - // Optimized for faster optional, not for copy. - if (other.nominal_hash_) - nominal_hash_ = to_unique(*other.nominal_hash_); - if (other.witness_hash_) - witness_hash_ = to_unique(*other.witness_hash_); - if (other.sighash_cache_) - sighash_cache_ = to_unique(*other.sighash_cache_); -} - transaction::transaction(uint32_t version, chain::inputs&& inputs, chain::outputs&& outputs, uint32_t locktime) NOEXCEPT : transaction(version, to_shareds(std::move(inputs)), @@ -183,37 +156,6 @@ transaction::transaction(uint32_t version, // Operators. // ---------------------------------------------------------------------------- -transaction& transaction::operator=(transaction&& other) NOEXCEPT -{ - *this = other; - return *this; -} - -transaction& transaction::operator=(const transaction& other) NOEXCEPT -{ - version_ = other.version_; - inputs_ = other.inputs_; - outputs_ = other.outputs_; - locktime_ = other.locktime_; - segregated_ = other.segregated_; - valid_ = other.valid_; - size_ = other.size_; - - nominal_hash_.reset(); - witness_hash_.reset(); - sighash_cache_.reset(); - - // Optimized for faster optional, not for copy. - if (other.nominal_hash_) - nominal_hash_ = to_unique(*other.nominal_hash_); - if (other.witness_hash_) - witness_hash_ = to_unique(*other.witness_hash_); - if (other.sighash_cache_) - sighash_cache_ = to_unique(*other.sighash_cache_); - - return *this; -} - bool transaction::operator==(const transaction& other) const NOEXCEPT { // Compares input/output elements, not pointers, cache not compared. @@ -452,14 +394,14 @@ uint64_t transaction::fee() const NOEXCEPT return floored_subtract(value(), claim()); } -void transaction::set_nominal_hash(hash_digest&& hash) const NOEXCEPT +void transaction::set_nominal_hash(const hash_digest& hash) const NOEXCEPT { - nominal_hash_ = to_unique(std::move(hash)); + nominal_hash_ = hash; } -void transaction::set_witness_hash(hash_digest&& hash) const NOEXCEPT +void transaction::set_witness_hash(const hash_digest& hash) const NOEXCEPT { - witness_hash_ = to_unique(std::move(hash)); + witness_hash_ = hash; } const hash_digest& transaction::get_hash(bool witness) const NOEXCEPT @@ -844,16 +786,17 @@ hash_digest transaction::unversioned_signature_hash( // TODO: taproot requires both single and double hash of each. void transaction::initialize_sighash_cache() const NOEXCEPT { - // This overconstructs the cache (anyone or !all), however it is simple. + // C++23: std::optional::or_else. if (!segregated_) return; - sighash_cache_ = to_unique - ( + // This overconstructs the cache (anyone or !all), however it is simple. + sighash_cache_ = + { outputs_hash(), points_hash(), sequences_hash() - ); + }; } // private