[rpc] AsyncRPCOperation: std::move instead of assignment

src/asyncrpcoperation.cpp

@@ -27,35 +27,17 @@ std::map<OperationStatus, std::string> OperationStatusMap = {
 /**
  * Every operation instance should have a globally unique id
  */
-AsyncRPCOperation::AsyncRPCOperation() : error_code_(0), error_message_() {
+AsyncRPCOperation::AsyncRPCOperation(std::shared_ptr<CWallet> const wallet) :
+        m_pwallet(wallet), error_code_(0), error_message_()
+{
     // Set a unique reference for each operation
     boost::uuids::uuid uuid = uuidgen();
     id_ = "opid-" + boost::uuids::to_string(uuid);
     creation_time_ = (int64_t)time(NULL);
-    set_state(OperationStatus::READY);
-}
-
-AsyncRPCOperation::AsyncRPCOperation(const AsyncRPCOperation& o) :
-        id_(o.id_), creation_time_(o.creation_time_), state_(o.state_.load()),
-        start_time_(o.start_time_), end_time_(o.end_time_),
-        error_code_(o.error_code_), error_message_(o.error_message_),
-        result_(o.result_)
-{
-}
 
-AsyncRPCOperation& AsyncRPCOperation::operator=( const AsyncRPCOperation& other ) {
-    this->id_ = other.id_;
-    this->creation_time_ = other.creation_time_;
-    this->state_.store(other.state_.load());
-    this->start_time_ = other.start_time_;
-    this->end_time_ = other.end_time_;
-    this->error_code_ = other.error_code_;
-    this->error_message_ = other.error_message_;
-    this->result_ = other.result_;
-    return *this;
+    set_state(OperationStatus::READY);
 }
 
-
 AsyncRPCOperation::~AsyncRPCOperation() {
 }
 

src/asyncrpcoperation.h

@@ -17,11 +17,13 @@
 
 #include <univalue.h>
 
+#include <wallet/wallet.h>
+
 using namespace std;
 
 /**
  * AsyncRPCOperation objects are submitted to the AsyncRPCQueue for processing.
- * 
+ *
  * To subclass AsyncRPCOperation, implement the main() method.
  * Update the operation status as work is underway and completes.
  * If main() can be interrupted, implement the cancel() method.
@@ -39,7 +41,7 @@ typedef enum class operationStateEnum {
 
 class AsyncRPCOperation {
 public:
-    AsyncRPCOperation();
+    AsyncRPCOperation(std::shared_ptr<CWallet> const wallet = nullptr);
     virtual ~AsyncRPCOperation();
 
     // You must implement this method in your subclass.
@@ -114,7 +116,10 @@ class AsyncRPCOperation {
     int error_code_;
     std::string error_message_;
     std::atomic<OperationStatus> state_;
-    std::chrono::time_point<std::chrono::system_clock> start_time_, end_time_;  
+    std::chrono::time_point<std::chrono::system_clock> start_time_, end_time_;
+
+    // Wallet of interest for this operation
+    std::shared_ptr<CWallet> m_pwallet;
 
     void start_execution_clock();
     void stop_execution_clock();
@@ -141,8 +146,8 @@ class AsyncRPCOperation {
 private:
 
     // Derived classes should write their own copy constructor and assignment operators
-    AsyncRPCOperation(const AsyncRPCOperation& orig);
-    AsyncRPCOperation& operator=( const AsyncRPCOperation& other );
+    AsyncRPCOperation(const AsyncRPCOperation& orig) = delete;
+    AsyncRPCOperation& operator=(const AsyncRPCOperation& other) = delete;
 
     // Initialized in the operation constructor, never to be modified again.
     AsyncRPCOperationId id_;

src/asyncrpcqueue.cpp

@@ -10,7 +10,7 @@ static std::atomic<size_t> workerCounter(0);
  * Static method to return the shared/default queue.
  */
 shared_ptr<AsyncRPCQueue> AsyncRPCQueue::sharedInstance() {
-    // Thread-safe in C+11 and gcc 4.3
+    // Thread-safe in C++11 and gcc 4.3
     static shared_ptr<AsyncRPCQueue> q = std::make_shared<AsyncRPCQueue>();
     return q;
 }
@@ -56,7 +56,7 @@ void AsyncRPCQueue::run(size_t workerId) {
             // Search operation map
             AsyncRPCOperationMap::const_iterator iter = operation_map_.find(key);
             if (iter != operation_map_.end()) {
-                operation = iter->second;
+                operation = std::move(iter->second);
             }
         }
 
@@ -104,7 +104,7 @@ std::shared_ptr<AsyncRPCOperation> AsyncRPCQueue::getOperationForId(AsyncRPCOper
     std::lock_guard<std::mutex> guard(lock_);
     AsyncRPCOperationMap::const_iterator iter = operation_map_.find(id);
     if (iter != operation_map_.end()) {
-        ptr = iter->second;
+        ptr = std::move(iter->second);
     }
     return ptr;
 }
@@ -176,7 +176,7 @@ size_t AsyncRPCQueue::getOperationCount() const {
  */
 void AsyncRPCQueue::addWorker() {
     std::lock_guard<std::mutex> guard(lock_);
-    workers_.emplace_back( std::thread(&AsyncRPCQueue::run, this, ++workerCounter) );
+    workers_.emplace_back(std::thread(&AsyncRPCQueue::run, this, ++workerCounter));
 }
 
 /**
@@ -193,7 +193,7 @@ size_t AsyncRPCQueue::getNumberOfWorkers() const {
 std::vector<AsyncRPCOperationId> AsyncRPCQueue::getAllOperationIds() const {
     std::lock_guard<std::mutex> guard(lock_);
     std::vector<AsyncRPCOperationId> v;
-    for(auto & entry: operation_map_) {
+    for (auto & entry : operation_map_) {
         v.push_back(entry.first);
     }
     return v;

src/asyncrpcqueue.h

@@ -19,7 +19,7 @@
 #include <memory>
 
 
-typedef std::unordered_map<AsyncRPCOperationId, std::shared_ptr<AsyncRPCOperation> > AsyncRPCOperationMap; 
+typedef std::unordered_map<AsyncRPCOperationId, std::shared_ptr<AsyncRPCOperation>> AsyncRPCOperationMap;
 
 
 class AsyncRPCQueue {

src/wallet/asyncrpcoperation_mergetoaddress.cpp

@@ -334,13 +334,13 @@ bool AsyncRPCOperation_mergetoaddress::main_impl()
     // change upon arrival of new blocks which contain joinsplit transactions.  This is likely
     // to happen as creating a chained joinsplit transaction can take longer than the block interval.
     {
-        LOCK2(cs_main, pwalletMain->cs_wallet);
+        LOCK2(cs_main, m_pwallet->cs_wallet);
         for (auto t : noteInputs_) {
             JSOutPoint jso = std::get<0>(t);
             std::vector<JSOutPoint> vOutPoints = {jso};
             uint256 inputAnchor;
             std::vector<boost::optional<ZCIncrementalWitness>> vInputWitnesses;
-            pwalletMain->GetNoteWitnesses(vOutPoints, vInputWitnesses, inputAnchor);
+            m_pwallet->GetNoteWitnesses(vOutPoints, vInputWitnesses, inputAnchor);
             jsopWitnessAnchorMap[jso.ToString()] = MergeToAddressWitnessAnchorData{vInputWitnesses[0], inputAnchor};
         }
     }
@@ -422,7 +422,7 @@ bool AsyncRPCOperation_mergetoaddress::main_impl()
         // Consume change as the first input of the JoinSplit.
         //
         if (jsChange > 0) {
-            LOCK2(cs_main, pwalletMain->cs_wallet);
+            LOCK2(cs_main, m_pwallet->cs_wallet);
 
             // Update tree state with previous joinsplit
             ZCIncrementalMerkleTree tree;
@@ -512,8 +512,8 @@ bool AsyncRPCOperation_mergetoaddress::main_impl()
             int wtxHeight = -1;
             int wtxDepth = -1;
             {
-                LOCK2(cs_main, pwalletMain->cs_wallet);
-                const CWalletTx& wtx = pwalletMain->mapWallet[jso.hash];
+                LOCK2(cs_main, m_pwallet->cs_wallet);
+                const CWalletTx& wtx = m_pwallet->mapWallet[jso.hash];
                 // Zero confirmation notes belong to transactions which have not yet been mined
                 if (mapBlockIndex.find(wtx.hashBlock) == mapBlockIndex.end()) {
                     throw JSONRPCError(RPC_WALLET_ERROR, strprintf("mapBlockIndex does not contain block hash %s", wtx.hashBlock.ToString()));
@@ -705,7 +705,7 @@ UniValue AsyncRPCOperation_mergetoaddress::perform_joinsplit(MergeToAddressJSInf
     uint256 anchor;
     {
         LOCK(cs_main);
-        pwalletMain->GetNoteWitnesses(outPoints, witnesses, anchor);
+        m_pwallet->GetNoteWitnesses(outPoints, witnesses, anchor);
     }
     return perform_joinsplit(info, witnesses, anchor);
 }
@@ -915,19 +915,19 @@ UniValue AsyncRPCOperation_mergetoaddress::getStatus() const
  * Lock input utxos
  */
  void AsyncRPCOperation_mergetoaddress::lock_utxos() {
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, m_pwallet->cs_wallet);
     for (auto utxo : utxoInputs_) {
-        pwalletMain->LockCoin(std::get<0>(utxo));
+        m_pwallet->LockCoin(std::get<0>(utxo));
     }
 }
 
 /**
  * Unlock input utxos
  */
 void AsyncRPCOperation_mergetoaddress::unlock_utxos() {
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, m_pwallet->cs_wallet);
     for (auto utxo : utxoInputs_) {
-        pwalletMain->UnlockCoin(std::get<0>(utxo));
+        m_pwallet->UnlockCoin(std::get<0>(utxo));
     }
 }
 
@@ -936,18 +936,18 @@ void AsyncRPCOperation_mergetoaddress::unlock_utxos() {
  * Lock input notes
  */
  void AsyncRPCOperation_mergetoaddress::lock_notes() {
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, m_pwallet->cs_wallet);
     for (auto note : noteInputs_) {
-        pwalletMain->LockNote(std::get<0>(note));
+        m_pwallet->LockNote(std::get<0>(note));
     }
 }
 
 /**
  * Unlock input notes
  */
 void AsyncRPCOperation_mergetoaddress::unlock_notes() {
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, m_pwallet->cs_wallet);
     for (auto note : noteInputs_) {
-        pwalletMain->UnlockNote(std::get<0>(note));
+        m_pwallet->UnlockNote(std::get<0>(note));
     }
 }

src/wallet/asyncrpcoperation_sendmany.cpp

@@ -85,7 +85,7 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
 
             // We don't need to lock on the wallet as spending key related methods are thread-safe
             SpendingKey key;
-            if (!pwalletMain->GetSpendingKey(addr, key)) {
+            if (!m_pwallet->GetSpendingKey(addr, key)) {
                 throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, no spending key found for zaddr");
             }
 
@@ -407,13 +407,13 @@ bool AsyncRPCOperation_sendmany::main_impl() {
     // change upon arrival of new blocks which contain joinsplit transactions.  This is likely
     // to happen as creating a chained joinsplit transaction can take longer than the block interval.
     if (z_inputs_.size() > 0) {
-        LOCK2(cs_main, pwalletMain->cs_wallet);
+        LOCK2(cs_main, m_pwallet->cs_wallet);
         for (auto t : z_inputs_) {
             JSOutPoint jso = std::get<0>(t);
             std::vector<JSOutPoint> vOutPoints = { jso };
             uint256 inputAnchor;
             std::vector<boost::optional<ZCIncrementalWitness>> vInputWitnesses;
-            pwalletMain->GetNoteWitnesses(vOutPoints, vInputWitnesses, inputAnchor);
+            m_pwallet->GetNoteWitnesses(vOutPoints, vInputWitnesses, inputAnchor);
             jsopWitnessAnchorMap[ jso.ToString() ] = WitnessAnchorData{ vInputWitnesses[0], inputAnchor };
         }
     }
@@ -538,7 +538,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         // Consume change as the first input of the JoinSplit.
         //
         if (jsChange > 0) {
-            LOCK2(cs_main, pwalletMain->cs_wallet);
+            LOCK2(cs_main, m_pwallet->cs_wallet);
 
             // Update tree state with previous joinsplit
             ZCIncrementalMerkleTree tree;
@@ -625,8 +625,8 @@ bool AsyncRPCOperation_sendmany::main_impl() {
             int wtxHeight = -1;
             int wtxDepth = -1;
             {
-                LOCK2(cs_main, pwalletMain->cs_wallet);
-                const CWalletTx& wtx = pwalletMain->mapWallet[jso.hash];
+                LOCK2(cs_main, m_pwallet->cs_wallet);
+                const CWalletTx& wtx = m_pwallet->mapWallet[jso.hash];
                 // Zero-confirmation notes belong to transactions which have not yet been mined
                 if (mapBlockIndex.find(wtx.hashBlock) == mapBlockIndex.end()) {
                     throw JSONRPCError(RPC_WALLET_ERROR, strprintf("mapBlockIndex does not contain block hash %s", wtx.hashBlock.ToString()));
@@ -832,9 +832,9 @@ bool AsyncRPCOperation_sendmany::find_utxos(bool fAcceptCoinbase=false) {
     set<CBitcoinAddress> setAddress = {fromtaddr_};
     vector<COutput> vecOutputs;
 
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, m_pwallet->cs_wallet);
 
-    pwalletMain->AvailableCoins(vecOutputs, false, NULL, true, fAcceptCoinbase);
+    m_pwallet->AvailableCoins(vecOutputs, false, NULL, true, fAcceptCoinbase);
 
     BOOST_FOREACH(const COutput& out, vecOutputs) {
         if (!out.fSpendable) {
@@ -879,8 +879,8 @@ bool AsyncRPCOperation_sendmany::find_utxos(bool fAcceptCoinbase=false) {
 bool AsyncRPCOperation_sendmany::find_unspent_notes() {
     std::vector<CNotePlaintextEntry> entries;
     {
-        LOCK2(cs_main, pwalletMain->cs_wallet);
-        pwalletMain->GetFilteredNotes(entries, fromaddress_, mindepth_);
+        LOCK2(cs_main, m_pwallet->cs_wallet);
+        m_pwallet->GetFilteredNotes(entries, fromaddress_, mindepth_);
     }
 
     for (CNotePlaintextEntry & entry : entries) {
@@ -924,7 +924,7 @@ UniValue AsyncRPCOperation_sendmany::perform_joinsplit(AsyncJoinSplitInfo & info
     uint256 anchor;
     {
         LOCK(cs_main);
-        pwalletMain->GetNoteWitnesses(outPoints, witnesses, anchor);
+        m_pwallet->GetNoteWitnesses(outPoints, witnesses, anchor);
     }
     return perform_joinsplit(info, witnesses, anchor);
 }
@@ -1117,10 +1117,10 @@ void AsyncRPCOperation_sendmany::add_taddr_outputs_to_tx() {
 
 void AsyncRPCOperation_sendmany::add_taddr_change_output_to_tx(CAmount amount) {
 
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, m_pwallet->cs_wallet);
 
     EnsureWalletIsUnlocked();
-    CReserveKey keyChange(pwalletMain);
+    CReserveKey keyChange(m_pwallet);
     CPubKey vchPubKey;
     bool ret = keyChange.GetReservedKey(vchPubKey);
     if (!ret) {

src/wallet/asyncrpcoperation_shieldcoinbase.cpp

@@ -484,20 +484,20 @@ UniValue AsyncRPCOperation_shieldcoinbase::getStatus() const {
  * Lock input utxos
  */
  void AsyncRPCOperation_shieldcoinbase::lock_utxos() {
-    LOCK2(cs_main, pwalletMain->cs_wallet);
-    for (auto utxo : inputs_) {
-        COutPoint outpt(utxo.txid, utxo.vout);
-        pwalletMain->LockCoin(outpt);
-    }
+     LOCK2(cs_main, m_pwallet->cs_wallet);
+     for (auto utxo : inputs_) {
+         COutPoint outpt(utxo.txid, utxo.vout);
+         m_pwallet->LockCoin(outpt);
+     }
 }
 
 /**
  * Unlock input utxos
  */
 void AsyncRPCOperation_shieldcoinbase::unlock_utxos() {
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, m_pwallet->cs_wallet);
     for (auto utxo : inputs_) {
         COutPoint outpt(utxo.txid, utxo.vout);
-        pwalletMain->UnlockCoin(outpt);
+        m_pwallet->UnlockCoin(outpt);
     }
 }