db: Change DatabaseCursor::Next to return status enum

Next()'s result is a tri-state - failed, more to go, complete. Replace
the way that this is returned with an enum with values FAIL, MORE, and
DONE rather than with two booleans.

src/wallet/bdb.cpp

@@ -481,11 +481,10 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip)
                         while (fSuccess) {
                             CDataStream ssKey(SER_DISK, CLIENT_VERSION);
                             CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-                            bool complete;
+                            DatabaseCursor::Status ret1 = cursor->Next(ssKey, ssValue);
-                            bool ret1 = cursor->Next(ssKey, ssValue, complete);
+                            if (ret1 == DatabaseCursor::Status::DONE) {
-                            if (complete) {
                                 break;
-                            } else if (!ret1) {
+                            } else if (ret1 == DatabaseCursor::Status::FAIL) {
                                 fSuccess = false;
                                 break;
                             }
@@ -668,21 +667,19 @@ BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database)
     }
 }
 
-bool BerkeleyCursor::Next(CDataStream& ssKey, CDataStream& ssValue, bool& complete)
+DatabaseCursor::Status BerkeleyCursor::Next(CDataStream& ssKey, CDataStream& ssValue)
 {
-    complete = false;
+    if (m_cursor == nullptr) return Status::FAIL;
-    if (m_cursor == nullptr) return false;
     // Read at cursor
     SafeDbt datKey;
     SafeDbt datValue;
     int ret = m_cursor->get(datKey, datValue, DB_NEXT);
     if (ret == DB_NOTFOUND) {
-        complete = true;
+        return Status::DONE;
+    }
+    if (ret != 0 || datKey.get_data() == nullptr || datValue.get_data() == nullptr) {
+        return Status::FAIL;
     }
-    if (ret != 0)
-        return false;
-    else if (datKey.get_data() == nullptr || datValue.get_data() == nullptr)
-        return false;
 
     // Convert to streams
     ssKey.SetType(SER_DISK);
@@ -691,7 +688,7 @@ bool BerkeleyCursor::Next(CDataStream& ssKey, CDataStream& ssValue, bool& comple
     ssValue.SetType(SER_DISK);
     ssValue.clear();
     ssValue.write({AsBytePtr(datValue.get_data()), datValue.get_size()});
-    return true;
+    return Status::MORE;
 }
 
 BerkeleyCursor::~BerkeleyCursor()

src/wallet/bdb.h

@@ -194,7 +194,7 @@ public:
     explicit BerkeleyCursor(BerkeleyDatabase& database);
     ~BerkeleyCursor() override;
 
-    bool Next(CDataStream& key, CDataStream& value, bool& complete) override;
+    Status Next(CDataStream& key, CDataStream& value) override;
 };
 
 /** RAII class that provides access to a Berkeley database */

src/wallet/db.h

@@ -31,7 +31,14 @@ public:
     DatabaseCursor(const DatabaseCursor&) = delete;
     DatabaseCursor& operator=(const DatabaseCursor&) = delete;
 
-    virtual bool Next(CDataStream& key, CDataStream& value, bool& complete) { return false; }
+    enum class Status
+    {
+        FAIL,
+        MORE,
+        DONE,
+    };
+
+    virtual Status Next(CDataStream& key, CDataStream& value) { return Status::FAIL; }
 };
 
 /** RAII class that provides access to a WalletDatabase */
@@ -168,7 +175,7 @@ public:
 
 class DummyCursor : public DatabaseCursor
 {
-    bool Next(CDataStream& key, CDataStream& value, bool& complete) override { return false; }
+    Status Next(CDataStream& key, CDataStream& value) override { return Status::FAIL; }
 };
 
 /** RAII class that provides access to a DummyDatabase. Never fails. */

src/wallet/dump.cpp

@@ -69,13 +69,13 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error)
         while (true) {
             CDataStream ss_key(SER_DISK, CLIENT_VERSION);
             CDataStream ss_value(SER_DISK, CLIENT_VERSION);
-            bool complete;
+            DatabaseCursor::Status status = cursor->Next(ss_key, ss_value);
-            ret = cursor->Next(ss_key, ss_value, complete);
+            if (status == DatabaseCursor::Status::DONE) {
-            if (complete) {
                 ret = true;
                 break;
-            } else if (!ret) {
+            } else if (status == DatabaseCursor::Status::FAIL) {
                 error = _("Error reading next record from wallet database");
+                ret = false;
                 break;
             }
             std::string key_str = HexStr(ss_key);

src/wallet/sqlite.cpp

@@ -470,18 +470,15 @@ bool SQLiteBatch::HasKey(CDataStream&& key)
     return res == SQLITE_ROW;
 }
 
-bool SQLiteCursor::Next(CDataStream& key, CDataStream& value, bool& complete)
+DatabaseCursor::Status SQLiteCursor::Next(CDataStream& key, CDataStream& value)
 {
-    complete = false;
-
     int res = sqlite3_step(m_cursor_stmt);
     if (res == SQLITE_DONE) {
-        complete = true;
+        return Status::DONE;
-        return true;
     }
     if (res != SQLITE_ROW) {
         LogPrintf("%s: Unable to execute cursor step: %s\n", __func__, sqlite3_errstr(res));
-        return false;
+        return Status::FAIL;
     }
 
     // Leftmost column in result is index 0
@@ -491,7 +488,7 @@ bool SQLiteCursor::Next(CDataStream& key, CDataStream& value, bool& complete)
     const std::byte* value_data{AsBytePtr(sqlite3_column_blob(m_cursor_stmt, 1))};
     size_t value_data_size(sqlite3_column_bytes(m_cursor_stmt, 1));
     value.write({value_data, value_data_size});
-    return true;
+    return Status::MORE;
 }
 
 SQLiteCursor::~SQLiteCursor()

src/wallet/sqlite.h

@@ -22,7 +22,7 @@ public:
     explicit SQLiteCursor() {}
     ~SQLiteCursor() override;
 
-    bool Next(CDataStream& key, CDataStream& value, bool& complete) override;
+    Status Next(CDataStream& key, CDataStream& value) override;
 };
 
 /** RAII class that provides access to a WalletDatabase */

src/wallet/test/util.cpp

@@ -59,9 +59,9 @@ std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database,
     while (true) {
         CDataStream key(SER_DISK, CLIENT_VERSION);
         CDataStream value(SER_DISK, CLIENT_VERSION);
-        bool complete;
+        DatabaseCursor::Status status = cursor->Next(key, value);
-        cursor->Next(key, value, complete);
+        assert(status != DatabaseCursor::Status::FAIL);
-        if (complete) break;
+        if (status == DatabaseCursor::Status::DONE) break;
         new_batch->Write(key, value);
     }
 

src/wallet/test/wallet_tests.cpp

@@ -870,7 +870,7 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup)
 class FailCursor : public DatabaseCursor
 {
 public:
-    bool Next(CDataStream& key, CDataStream& value, bool& complete) override { return false; }
+    Status Next(CDataStream& key, CDataStream& value) override { return Status::FAIL; }
 };
 
 /** RAII class that provides access to a FailDatabase. Which fails if needed. */

src/wallet/test/walletload_tests.cpp

@@ -60,9 +60,9 @@ bool HasAnyRecordOfType(WalletDatabase& db, const std::string& key)
     while (true) {
         CDataStream ssKey(SER_DISK, CLIENT_VERSION);
         CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-        bool complete;
+        DatabaseCursor::Status status = cursor->Next(ssKey, ssValue);
-        BOOST_CHECK(cursor->Next(ssKey, ssValue, complete));
+        assert(status != DatabaseCursor::Status::FAIL);
-        if (complete) break;
+        if (status == DatabaseCursor::Status::DONE) break;
         std::string type;
         ssKey >> type;
         if (type == key) return true;

src/wallet/wallet.cpp

@@ -3776,12 +3776,12 @@ bool CWallet::MigrateToSQLite(bilingual_str& error)
         error = _("Error: Unable to begin reading all records in the database");
         return false;
     }
-    bool complete = false;
+    DatabaseCursor::Status status = DatabaseCursor::Status::FAIL;
     while (true) {
         CDataStream ss_key(SER_DISK, CLIENT_VERSION);
         CDataStream ss_value(SER_DISK, CLIENT_VERSION);
-        bool ret = cursor->Next(ss_key, ss_value, complete);
+        status = cursor->Next(ss_key, ss_value);
-        if (complete || !ret) {
+        if (status != DatabaseCursor::Status::MORE) {
             break;
         }
         SerializeData key(ss_key.begin(), ss_key.end());
@@ -3790,7 +3790,7 @@ bool CWallet::MigrateToSQLite(bilingual_str& error)
     }
     cursor.reset();
     batch.reset();
-    if (!complete) {
+    if (status != DatabaseCursor::Status::DONE) {
         error = _("Error: Unable to read all records in the database");
         return false;
     }

src/wallet/walletdb.cpp

@@ -824,13 +824,10 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
             // Read next record
             CDataStream ssKey(SER_DISK, CLIENT_VERSION);
             CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-            bool complete;
+            DatabaseCursor::Status status = cursor->Next(ssKey, ssValue);
-            bool ret = cursor->Next(ssKey, ssValue, complete);
+            if (status == DatabaseCursor::Status::DONE) {
-            if (complete) {
                 break;
-            }
+            } else if (status == DatabaseCursor::Status::FAIL) {
-            else if (!ret)
-            {
                 cursor.reset();
                 pwallet->WalletLogPrintf("Error reading next record from wallet database\n");
                 return DBErrors::CORRUPT;
@@ -998,11 +995,10 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWal
             // Read next record
             CDataStream ssKey(SER_DISK, CLIENT_VERSION);
             CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-            bool complete;
+            DatabaseCursor::Status status = cursor->Next(ssKey, ssValue);
-            bool ret = cursor->Next(ssKey, ssValue, complete);
+            if (status == DatabaseCursor::Status::DONE) {
-            if (complete) {
                 break;
-            } else if (!ret) {
+            } else if (status == DatabaseCursor::Status::FAIL) {
                 LogPrintf("Error reading next record from wallet database\n");
                 return DBErrors::CORRUPT;
             }
@@ -1125,13 +1121,10 @@ bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types)
         // Read next record
         CDataStream key(SER_DISK, CLIENT_VERSION);
         CDataStream value(SER_DISK, CLIENT_VERSION);
-        bool complete;
+        DatabaseCursor::Status status = cursor->Next(key, value);
-        bool ret = cursor->Next(key, value, complete);
+        if (status == DatabaseCursor::Status::DONE) {
-        if (complete) {
             break;
-        }
+        } else if (status == DatabaseCursor::Status::FAIL) {
-        else if (!ret)
-        {
             return false;
         }