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

fa8a341b88 wallet: Replace CDataStream& with CDataStream&& where appropriate (MarcoFalke)
fa021e9a5b 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 fa8a341b88
  ryanofsky:
    Code review ACK fa8a341b88. 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);
+        try {
-        if (ret) {
+            ssValue >> value;
-            // Unserialize value
+            return true;
-            try {
+        } catch (const std::exception&) {
-                ssValue >> value;
+            return false;
-                success = true;
-            } catch (const std::exception&) {
-                // In this case success remains '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();