Merge #19320: wallet: Replace CDataStream& with CDataStream&& where appropriate

fa8a341b88cabfd7f8d702db7cb9972b0804bf2a wallet: Replace CDataStream& with CDataStream&& where appropriate (MarcoFalke)
fa021e9a5b7e930a3db0febb416942dea3a90a8f wallet: Remove confusing double return value ret+success (MarcoFalke)

Pull request description:

  The keys and values are only to be used once because their memory is set
  to zero. Make that explicit by moving the bytes into the lower level
  methods.

ACKs for top commit:
  sipa:
    utACK fa8a341b88cabfd7f8d702db7cb9972b0804bf2a
  ryanofsky:
    Code review ACK fa8a341b88cabfd7f8d702db7cb9972b0804bf2a. Nice changes.

Tree-SHA512: 5c0218bae0f3cd2a07346f1bbf4ad232e5dde7ef2f807d82cc6cfd208d11fe60c8b0f37e7986087b52fbfc79cdfd33c3c8a5822b3d4d9a44d1c6b09e354fc424

src/wallet/bdb.cpp

@@ -796,15 +796,13 @@ std::string BerkeleyDatabaseVersion()
     return DbEnv::version(nullptr, nullptr, nullptr);
 }
 
-bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value)
+bool BerkeleyBatch::ReadKey(CDataStream&& key, CDataStream& value)
 {
     if (!pdb)
         return false;
 
-    // Key
     SafeDbt datKey(key.data(), key.size());
 
-    // Read
     SafeDbt datValue;
     int ret = pdb->get(activeTxn, datKey, datValue, 0);
     if (ret == 0 && datValue.get_data() != nullptr) {
@@ -814,48 +812,41 @@ bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value)
     return false;
 }
 
-bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrite)
+bool BerkeleyBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite)
 {
     if (!pdb)
         return true;
     if (fReadOnly)
         assert(!"Write called on database in read-only mode");
 
-    // Key
     SafeDbt datKey(key.data(), key.size());
 
-    // Value
     SafeDbt datValue(value.data(), value.size());
 
-    // Write
     int ret = pdb->put(activeTxn, datKey, datValue, (overwrite ? 0 : DB_NOOVERWRITE));
     return (ret == 0);
 }
 
-bool BerkeleyBatch::EraseKey(CDataStream& key)
+bool BerkeleyBatch::EraseKey(CDataStream&& key)
 {
     if (!pdb)
         return false;
     if (fReadOnly)
         assert(!"Erase called on database in read-only mode");
 
-    // Key
     SafeDbt datKey(key.data(), key.size());
 
-    // Erase
     int ret = pdb->del(activeTxn, datKey, 0);
     return (ret == 0 || ret == DB_NOTFOUND);
 }
 
-bool BerkeleyBatch::HasKey(CDataStream& key)
+bool BerkeleyBatch::HasKey(CDataStream&& key)
 {
     if (!pdb)
         return false;
 
-    // Key
     SafeDbt datKey(key.data(), key.size());
 
-    // Exists
     int ret = pdb->exists(activeTxn, datKey, 0);
     return ret == 0;
 }

src/wallet/bdb.h

@@ -195,10 +195,10 @@ class BerkeleyBatch
     };
 
 private:
-    bool ReadKey(CDataStream& key, CDataStream& value);
+    bool ReadKey(CDataStream&& key, CDataStream& value);
-    bool WriteKey(CDataStream& key, CDataStream& value, bool overwrite=true);
+    bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true);
-    bool EraseKey(CDataStream& key);
+    bool EraseKey(CDataStream&& key);
-    bool HasKey(CDataStream& key);
+    bool HasKey(CDataStream&& key);
 
 protected:
     Db* pdb;
@@ -222,65 +222,52 @@ public:
     template <typename K, typename T>
     bool Read(const K& key, T& value)
     {
-        // Key
         CDataStream ssKey(SER_DISK, CLIENT_VERSION);
         ssKey.reserve(1000);
         ssKey << key;
 
         CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-        bool success = false;
+        if (!ReadKey(std::move(ssKey), ssValue)) return false;
-        bool ret = ReadKey(ssKey, ssValue);
-        if (ret) {
-            // Unserialize value
         try {
             ssValue >> value;
-                success = true;
+            return true;
         } catch (const std::exception&) {
-                // In this case success remains 'false'
+            return false;
         }
     }
-        return ret && success;
-    }
 
     template <typename K, typename T>
     bool Write(const K& key, const T& value, bool fOverwrite = true)
     {
-        // Key
         CDataStream ssKey(SER_DISK, CLIENT_VERSION);
         ssKey.reserve(1000);
         ssKey << key;
 
-        // Value
         CDataStream ssValue(SER_DISK, CLIENT_VERSION);
         ssValue.reserve(10000);
         ssValue << value;
 
-        // Write
+        return WriteKey(std::move(ssKey), std::move(ssValue), fOverwrite);
-        return WriteKey(ssKey, ssValue, fOverwrite);
     }
 
     template <typename K>
     bool Erase(const K& key)
     {
-        // Key
         CDataStream ssKey(SER_DISK, CLIENT_VERSION);
         ssKey.reserve(1000);
         ssKey << key;
 
-        // Erase
+        return EraseKey(std::move(ssKey));
-        return EraseKey(ssKey);
     }
 
     template <typename K>
     bool Exists(const K& key)
     {
-        // Key
         CDataStream ssKey(SER_DISK, CLIENT_VERSION);
         ssKey.reserve(1000);
         ssKey << key;
 
-        // Exists
+        return HasKey(std::move(ssKey));
-        return HasKey(ssKey);
     }
 
     bool StartCursor();