Merge pull request #1518 from evoskuil/master

Use std::optional for cache, and default tx copy/assign.

include/bitcoin/system/chain/header.hpp

@@ -21,6 +21,7 @@
 
 #include <istream>
 #include <memory>
+#include <optional>
 #include <vector>
 #include <bitcoin/system/chain/chain_state.hpp>
 #include <bitcoin/system/data/data.hpp>
@@ -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<const hash_digest> hash_{};
+    mutable std::optional<hash_digest> hash_{};
 };
 
 typedef std_vector<header> headers;

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 };

include/bitcoin/system/chain/transaction.hpp

@@ -21,6 +21,7 @@
 
 #include <istream>
 #include <memory>
+#include <optional>
 #include <vector>
 #include <bitcoin/system/chain/context.hpp>
 #include <bitcoin/system/chain/input.hpp>
@@ -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<const transaction> 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<const hash_digest> nominal_hash_{};
-    mutable std::unique_ptr<const hash_digest> witness_hash_{};
-    mutable std::unique_ptr<const sighash_cache> sighash_cache_{};
+    mutable std::optional<hash_digest> nominal_hash_{};
+    mutable std::optional<hash_digest> witness_hash_{};
+    mutable std::optional<sighash_cache> sighash_cache_{};
 };
 
 typedef std_vector<transaction> transactions;

include/bitcoin/system/data/memory.hpp

@@ -122,6 +122,7 @@ inline std::unique_ptr<const Type> to_unique(const Type& value) NOEXCEPT
 template <typename Type, typename ...Args>
 inline std::unique_ptr<const Type> to_unique(Args&&... values) NOEXCEPT
 {
+    // Type{} required due to CLang bug.
     return std::make_unique<const Type>(Type{ std::forward<Args>(values)... });
 }
 

include/bitcoin/system/impl/stream/streamers/byte_reader.ipp

@@ -615,8 +615,7 @@ byte_reader<IStream>::get_arena() const NOEXCEPT
 }
 
 template <typename IStream>
-typename byte_reader<IStream>::memory_allocator&
-byte_reader<IStream>::get_allocator() const NOEXCEPT
+byte_allocator& byte_reader<IStream>::get_allocator() const NOEXCEPT
 {
     return allocator_;
 }

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

include/bitcoin/system/stream/streamers/interfaces/bytereader.hpp

@@ -34,7 +34,6 @@ class bytereader
 {
 public:
     using memory_arena = arena*;
-    using memory_allocator = allocator<uint8_t>;
 
     /// 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;

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

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<T>::or_else.
     if (!segregated_)
         return;
 
-    sighash_cache_ = to_unique<sighash_cache>
-    (
+    // This overconstructs the cache (anyone or !all), however it is simple.
+    sighash_cache_ =
+    {
         outputs_hash(),
         points_hash(),
         sequences_hash()
-    );
+    };
 }
 
 // private