Merge #2089: [Refactor] Fix some -Wdeprecated-copy warnings

ae41ebe Remove redundant explicitly defined copy ctors (Dan Raviv)
454aa37 Trivial: explicit (default) copy-constructor for CTransaction (random-zebra)
2af2306 Cleanup: Remove redundant copy-constructor for CSignedMessage (random-zebra)
e28259d Masternode: proper copy-assignment for CMasternode/CmasternodePing (random-zebra)

Pull request description:

  Fix the `-Wdeprecated-copy` warnings listed in #2076

  **Background**
  While perfectly valid in C++98, implicit generation of the copy constructor is declared as deprecated in C++11, when there is already a user-defined copy assignment operator, or vice-versa.
  The rationale behind this is the "rule of three" (https://en.cppreference.com/w/cpp/language/rule_of_three)
  > If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three.

  **Issue - Fix**
  We have several places that define redundant copy assignment operators (and then use the implicit copy constructor), or that do need non-default ctors/dtors but don't define copy assignment.
  (See also bitcoin#11161, which is also cherry-picked here)
  Specifically:
  - `CSignedMessage`/`CMasternodePing`/`CMasternode` use a weird copy-assignment that *swaps* with the argument (which, of course, cannot be "const" then). Here we remove all ::swap implementations, and provide actual copy assignment operators.
  - `CTransaction` has an explicit copy-assignment operator, but uses the implicit copy-ctor in `CMerkleTx` constructor.
  - `CFeeRate` and `CTxMemPoolEntry` have explicitly defined copy ctor, which can (and should, for the rule-of-three) be replaced by the default one (bitcoin#11161).

  **Note**
  Reason why these warnings pop up just now.
  Since Clang 10.0 and GCC 9.3, `-Wextra` enables `-Wdeprecated-copy` by default, and that is very spammy, especially when compiling with the QT GUI (bitcoin#15822 / bitcoin#18419).
  It is so spammy (also with `boost-1.72.0`) that Bitcoin decided to temporarily suppress this warning (bitcoin#18738) until QT fixes it upstream, and boost is definitely removed in 0.22 (bitcoin#18967)

ACKs for top commit:
  furszy:
    utACK ae41ebe

Tree-SHA512: 1f59f9f7426abbf4b7d4bb0d1ac50491eb52f776e2a018de3acc8ab381af3b69db6b9a5b60c04b48f0d495e560eaa1e3d19be03d133f230daca66ba66e766289

src/masternode.h

@@ -71,25 +71,7 @@ class CMasternodePing : public CSignedMessage
     bool CheckAndUpdate(int& nDos, bool fRequireAvailable = true, bool fCheckSigTimeOnly = false);
     void Relay();
 
-    void swap(CMasternodePing& first, CMasternodePing& second) // nothrow
-    {
-        CSignedMessage::swap(first, second);
-
-        // enable ADL (not necessary in our case, but good practice)
-        using std::swap;
-
-        // by swapping the members of two classes,
-        // the two classes are effectively swapped
-        swap(first.vin, second.vin);
-        swap(first.blockHash, second.blockHash);
-        swap(first.sigTime, second.sigTime);
-    }
-
-    CMasternodePing& operator=(CMasternodePing from)
-    {
-        swap(*this, from);
-        return *this;
-    }
+    CMasternodePing& operator=(const CMasternodePing& other) = default;
 
     friend bool operator==(const CMasternodePing& a, const CMasternodePing& b)
     {
@@ -142,31 +124,22 @@ class CMasternode : public CSignedMessage
 
     void SetLastPing(const CMasternodePing& _lastPing) { WITH_LOCK(cs, lastPing = _lastPing;); }
 
-    void swap(CMasternode& first, CMasternode& second) // nothrow
-    {
-        CSignedMessage::swap(first, second);
-
-        // enable ADL (not necessary in our case, but good practice)
-        using std::swap;
-
-        // by swapping the members of two classes,
-        // the two classes are effectively swapped
-        swap(first.vin, second.vin);
-        swap(first.addr, second.addr);
-        swap(first.pubKeyCollateralAddress, second.pubKeyCollateralAddress);
-        swap(first.pubKeyMasternode, second.pubKeyMasternode);
-        swap(first.sigTime, second.sigTime);
-        swap(first.lastPing, second.lastPing);
-        swap(first.protocolVersion, second.protocolVersion);
-        swap(first.nScanningErrorCount, second.nScanningErrorCount);
-        swap(first.nLastScanningErrorBlockHeight, second.nLastScanningErrorBlockHeight);
-    }
-
-    CMasternode& operator=(CMasternode from)
+    CMasternode& operator=(const CMasternode& other)
     {
-        swap(*this, from);
+        nMessVersion = other.nMessVersion;
+        vchSig = other.vchSig;
+        vin = other.vin;
+        addr = other.addr;
+        pubKeyCollateralAddress = other.pubKeyCollateralAddress;
+        pubKeyMasternode = other.pubKeyMasternode;
+        sigTime = other.sigTime;
+        lastPing = other.lastPing;
+        protocolVersion = other.protocolVersion;
+        nScanningErrorCount = other.nScanningErrorCount;
+        nLastScanningErrorBlockHeight = other.nLastScanningErrorBlockHeight;
         return *this;
     }
+
     friend bool operator==(const CMasternode& a, const CMasternode& b)
     {
         return a.vin == b.vin;

src/messagesigner.cpp

@@ -144,14 +144,3 @@ std::string CSignedMessage::GetSignatureBase64() const
     return EncodeBase64(&vchSig[0], vchSig.size());
 }
 
-void CSignedMessage::swap(CSignedMessage& first, CSignedMessage& second) // nothrow
-{
-    // enable ADL (not necessary in our case, but good practice)
-    using std::swap;
-
-    // by swapping the members of two classes,
-    // the two classes are effectively swapped
-    swap(first.vchSig, second.vchSig);
-    swap(first.nMessVersion, second.nMessVersion);
-}
-

src/messagesigner.h

@@ -50,7 +50,6 @@ class CSignedMessage
 {
 protected:
     std::vector<unsigned char> vchSig;
-    void swap(CSignedMessage& first, CSignedMessage& second); // Swap two messages
 
 public:
     int nMessVersion;
@@ -59,11 +58,6 @@ class CSignedMessage
         vchSig(),
         nMessVersion(MessageVersion::MESS_VER_HASH)
     {}
-    CSignedMessage(const CSignedMessage& other)
-    {
-        vchSig = other.GetVchSig();
-        nMessVersion = other.nMessVersion;
-    }
     virtual ~CSignedMessage() {};
 
     // Sign-Verify message

src/policy/feerate.h

@@ -24,7 +24,6 @@ class CFeeRate
     CFeeRate() : nSatoshisPerK(0) {}
     explicit CFeeRate(const CAmount& _nSatoshisPerK) : nSatoshisPerK(_nSatoshisPerK) {}
     CFeeRate(const CAmount& nFeePaid, size_t nSize);
-    CFeeRate(const CFeeRate& other) { nSatoshisPerK = other.nSatoshisPerK; }
 
     CAmount GetFee(size_t size) const;                  // unit returned is satoshis
     CAmount GetFeePerK() const { return GetFee(1000); } // satoshis-per-1000-bytes

src/primitives/transaction.h

@@ -266,6 +266,7 @@ class CTransaction
     CTransaction(const CMutableTransaction &tx);
     CTransaction(CMutableTransaction &&tx);
 
+    CTransaction(const CTransaction& tx) = default;
     CTransaction& operator=(const CTransaction& tx);
 
     ADD_SERIALIZE_METHODS;

src/txmempool.cpp

@@ -40,13 +40,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFe
     feeDelta = 0;
 }
 
-CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other)
-{
-    *this = other;
-}
-
-double
-CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const
+double CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const
 {
     double deltaPriority = ((double)(currentHeight - entryHeight) * inChainInputValue) / nModSize;
     double dResult = entryPriority + deltaPriority;

src/txmempool.h

@@ -92,7 +92,6 @@ class CTxMemPoolEntry
             int64_t _nTime, double _entryPriority, unsigned int _entryHeight,
             bool poolHasNoInputsOf, CAmount _inChainInputValue, bool _spendsCoinbaseOrCoinstake,
             unsigned int nSigOps);
-    CTxMemPoolEntry(const CTxMemPoolEntry& other);
 
     const CTransaction& GetTx() const { return *this->tx; }
     std::shared_ptr<const CTransaction> GetSharedTx() const { return this->tx; }