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

2 files changed, 17 insertions, 39 deletions

diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 6ac8340a31..fa4a505982 100644
--- a/src/wallet/bdb.cpp
+++ b/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;
 }
diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
index 64f573047c..599319482b 100644
--- a/src/wallet/bdb.h
+++ b/src/wallet/bdb.h
@@ -195,10 +195,10 @@ class BerkeleyBatch
     };
 
 private:
-    bool ReadKey(CDataStream& key, CDataStream& value);
-    bool WriteKey(CDataStream& key, CDataStream& value, bool overwrite=true);
-    bool EraseKey(CDataStream& key);
-    bool HasKey(CDataStream& key);
+    bool ReadKey(CDataStream&& key, CDataStream& value);
+    bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true);
+    bool EraseKey(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;
-        bool ret = ReadKey(ssKey, ssValue);
-        if (ret) {
-            // Unserialize value
-            try {
-                ssValue >> value;
-                success = true;
-            } catch (const std::exception&) {
-                // In this case success remains 'false'
-            }
+        if (!ReadKey(std::move(ssKey), ssValue)) return false;
+        try {
+            ssValue >> value;
+            return true;
+        } catch (const std::exception&) {
+            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(ssKey, ssValue, fOverwrite);
+        return WriteKey(std::move(ssKey), std::move(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(ssKey);
+        return EraseKey(std::move(ssKey));
     }
 
     template <typename K>
     bool Exists(const K& key)
     {
-        // Key
         CDataStream ssKey(SER_DISK, CLIENT_VERSION);
         ssKey.reserve(1000);
         ssKey << key;
 
-        // Exists
-        return HasKey(ssKey);
+        return HasKey(std::move(ssKey));
     }
 
     bool StartCursor();