Merge pull request #9851

70fe289d8 Improve move/copy/default constructor semantics for tx and block (Lee *!* Clagett)

src/cryptonote_basic/cryptonote_basic.h

@@ -225,8 +225,10 @@ namespace cryptonote
 
     transaction();
     transaction(const transaction &t);
+    transaction(transaction &&t);
     transaction &operator=(const transaction &t);
-    virtual ~transaction();
+    transaction &operator=(transaction &&t);
+    ~transaction() = default;
     void set_null();
     void invalidate_hashes();
     bool is_hash_valid() const { return hash_valid.load(std::memory_order_acquire); }
@@ -346,71 +348,96 @@ namespace cryptonote
 
   inline transaction::transaction(const transaction &t):
     transaction_prefix(t),
-    hash_valid(false),
+    hash_valid(t.is_hash_valid()),
-    prunable_hash_valid(false),
+    prunable_hash_valid(t.is_prunable_hash_valid()),
-    blob_size_valid(false),
+    blob_size_valid(t.is_blob_size_valid()),
     signatures(t.signatures),
     rct_signatures(t.rct_signatures),
+    hash(t.hash),
+    prunable_hash(t.prunable_hash),
+    blob_size(t.blob_size),
+    pruned(t.pruned),
+    unprunable_size(t.unprunable_size.load()),
+    prefix_size(t.prefix_size.load())
+  {}
+
+  inline transaction::transaction(transaction &&t):
+    transaction_prefix(std::move(t)),
+    hash_valid(t.is_hash_valid()),
+    prunable_hash_valid(t.is_prunable_hash_valid()),
+    blob_size_valid(t.is_blob_size_valid()),
+    signatures(std::move(t.signatures)),
+    rct_signatures(std::move(t.rct_signatures)),
+    hash(std::move(t.hash)),
+    prunable_hash(std::move(t.prunable_hash)),
+    blob_size(std::move(t.blob_size)),
     pruned(t.pruned),
     unprunable_size(t.unprunable_size.load()),
     prefix_size(t.prefix_size.load())
   {
-    if (t.is_hash_valid())
+    t.set_null();
-    {
-      hash = t.hash;
-      set_hash_valid(true);
-    }
-    if (t.is_blob_size_valid())
-    {
-      blob_size = t.blob_size;
-      set_blob_size_valid(true);
-    }
-    if (t.is_prunable_hash_valid())
-    {
-      prunable_hash = t.prunable_hash;
-      set_prunable_hash_valid(true);
-    }
   }
 
   inline transaction &transaction::operator=(const transaction &t)
   {
+    if (this == std::addressof(t))
+      return *this;
+
     transaction_prefix::operator=(t);
 
-    set_hash_valid(false);
+    set_hash_valid(t.is_hash_valid());
-    set_prunable_hash_valid(false);
+    set_prunable_hash_valid(t.is_prunable_hash_valid());
-    set_blob_size_valid(false);
+    set_blob_size_valid(t.is_blob_size_valid());
     signatures = t.signatures;
     rct_signatures = t.rct_signatures;
-    if (t.is_hash_valid())
+    hash = t.hash;
-    {
+    prunable_hash = t.prunable_hash;
-      hash = t.hash;
+    blob_size = t.blob_size;
-      set_hash_valid(true);
-    }
-    if (t.is_prunable_hash_valid())
-    {
-      prunable_hash = t.prunable_hash;
-      set_prunable_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;
   }
 
-  inline
+  inline transaction &transaction::operator=(transaction &&t)
-  transaction::transaction()
   {
-    set_null();
+    if (this == std::addressof(t))
+      return *this;
+
+    transaction_prefix::operator=(std::move(t));
+
+    set_hash_valid(t.is_hash_valid());
+    set_prunable_hash_valid(t.is_prunable_hash_valid());
+    set_blob_size_valid(t.is_blob_size_valid());
+    signatures = std::move(t.signatures);
+    rct_signatures = std::move(t.rct_signatures);
+    hash = std::move(t.hash);
+    prunable_hash = std::move(t.prunable_hash);
+    blob_size = std::move(t.blob_size);
+    pruned = std::move(t.pruned);
+    unprunable_size = t.unprunable_size.load();
+    prefix_size = t.prefix_size.load();
+
+    t.set_null();
+    return *this;
   }
 
   inline
-  transaction::~transaction()
+  transaction::transaction():
+    transaction_prefix(),
+    hash_valid(false),
+    prunable_hash_valid(false),
+    blob_size_valid(false),
+    signatures(),
+    rct_signatures{},
+    hash{},
+    prunable_hash{},
+    blob_size(0),
+    pruned(false),
+    unprunable_size(0),
+    prefix_size(0)
   {
+    // set_null() incurs atomic fence penalties
   }
 
   inline
@@ -418,7 +445,7 @@ namespace cryptonote
   {
     transaction_prefix::set_null();
     signatures.clear();
-    rct_signatures.type = rct::RCTTypeNull;
+    rct_signatures = rct::rctSig{};
     set_hash_valid(false);
     set_prunable_hash_valid(false);
     set_blob_size_valid(false);
@@ -479,8 +506,50 @@ namespace cryptonote
 
   public:
     block(): block_header(), 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; }
+    block(const block &b):
+      block_header(b),
+      hash_valid(b.is_hash_valid()),
+      miner_tx(b.miner_tx),
+      tx_hashes(b.tx_hashes),
+      hash(b.hash)
+    {}
+    block(block &&b):
+      block_header(std::move(b)),
+      hash_valid(b.is_hash_valid()),
+      miner_tx(std::move(b.miner_tx)),
+      tx_hashes(std::move(b.tx_hashes)),
+      hash(std::move(b.hash))
+    {
+      b.miner_tx.set_null();
+      b.tx_hashes.clear();
+    }
+    block &operator=(const block &b)
+    {
+      if(this != std::addressof(b))
+      {
+        block_header::operator=(b);
+        hash_valid = b.is_hash_valid();
+        miner_tx = b.miner_tx;
+        tx_hashes = b.tx_hashes;
+        hash = b.hash;
+      }
+      return *this;
+    }
+    block &operator=(block &&b)
+    {
+      if (this != std::addressof(b))
+      {
+        block_header::operator=(std::move(b));
+        hash_valid = b.is_hash_valid();
+        miner_tx = std::move(b.miner_tx);
+        tx_hashes = std::move(b.tx_hashes);
+        hash = std::move(b.hash);
+        b.miner_tx.set_null();
+        b.tx_hashes.clear();
+      }
+      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); }

src/cryptonote_core/blockchain.cpp

@@ -2656,8 +2656,8 @@ bool Blockchain::get_transactions(const t_ids_container& txs_ids, t_tx_container
       bool res = pruned ? m_db->get_pruned_tx_blob(tx_hash, tx) : m_db->get_tx_blob(tx_hash, tx);
       if (res)
       {
-        txs.push_back(transaction());
+        auto& added_tx = txs.emplace_back();
-        res = pruned ? parse_and_validate_tx_base_from_blob(tx, txs.back()) : parse_and_validate_tx_from_blob(tx, txs.back());
+        res = pruned ? parse_and_validate_tx_base_from_blob(tx, added_tx) : parse_and_validate_tx_from_blob(tx, added_tx);
         if (!res)
         {
           LOG_ERROR("Invalid transaction");

src/rpc/rpc_payment.cpp

@@ -153,7 +153,7 @@ namespace cryptonote
       extra_nonce = cryptonote::blobdata((const char*)&hash, 4);
       if(!add_extra_nonce_to_tx_extra(new_block.miner_tx.extra, extra_nonce))
         return false;
-      info.previous_block = std::move(info.block);
+      info.previous_block = info.block;
       info.block = std::move(new_block);
       hashing_blob = get_block_hashing_blob(info.block);
       info.previous_hashing_blob = info.hashing_blob;

src/wallet/wallet2.cpp

@@ -3062,7 +3062,7 @@ void read_pool_txs(const cryptonote::COMMAND_RPC_GET_TRANSACTIONS::request &req,
                 [tx_hash](const crypto::hash &e) { return e == tx_hash; });
             if (i != txids.end())
             {
-              txs.push_back(std::make_tuple(tx, tx_hash, tx_entry.double_spend_seen));
+              txs.emplace_back(std::move(tx), tx_hash, tx_entry.double_spend_seen);
             }
             else
             {
@@ -3097,7 +3097,7 @@ void wallet2::process_pool_info_extent(const cryptonote::COMMAND_RPC_GET_BLOCKS_
     cryptonote::transaction tx;
     THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_base_from_blob(pool_tx.tx_blob, tx),
         error::wallet_internal_error, "Failed to validate transaction base from daemon");
-    added_pool_txs.push_back(std::make_tuple(tx, pool_tx.tx_hash, pool_tx.double_spend_seen));
+    added_pool_txs.emplace_back(std::move(tx), pool_tx.tx_hash, pool_tx.double_spend_seen);
   }
 
   // getblocks.bin may return more added pool transactions than we're allowed to request in restricted mode