Merge bitcoin/bitcoin#26690: wallet: Refactor database cursor into its own object with proper return codes

4aebd832a405090c2608e4b60bb4f34501bcea61 db: Change DatabaseCursor::Next to return status enum (Andrew Chow) d79e8dcf2981ef1964a2fde8c472b5de1ca1c963 wallet: Have cursor users use DatabaseCursor directly (Andrew Chow) 7a198bba0a1d0a0f0fd4ca947955cb52b84bdd4b wallet: Introduce DatabaseCursor RAII class for managing cursor (Andrew Chow) 69efbc011bb74fcd8dd9ed2a8a5d31bc9e323c10 Move SafeDbt out of BerkeleyBatch (Andrew Chow) Pull request description: Instead of having database cursors be tied to a particular `DatabaseBatch` object and requiring its setup and teardown be separate functions in that batch, we can have cursors be separate RAII classes. This makes it easier to create and destroy cursors as well as having cursors that have slightly different behaviors. Additionally, since reading data from a cursor is a tri-state, this PR changes the return value of the `Next` function (formerly `ReadAtCursor`) to return an Enum rather than the current system of 2 booleans. This greatly simplifies and unifies the code that deals with cursors as now there is no confusion as to what the function returns when there are no records left to be read. Extracted from #24914 ACKs for top commit: furszy: diff ACK 4aebd83 theStack: Code-review ACK 4aebd832a405090c2608e4b60bb4f34501bcea61 Tree-SHA512: 5d0be56a18de5b08c777dd5a73ba5a6ef1e696fdb07d1dca952a88ded07887b7c5c04342f9a76feb2f6fe24a45dc31f094f1f5d9500e6bdf4a44f4edb66dcaa1
author: fanquake <fanquake@gmail.com> 2023-01-23 17:37:44 +0000
committer: fanquake <fanquake@gmail.com> 2023-01-23 17:56:16 +0000
commit: a62231bca629e945349255a1d331dd5c7a86ddd1 (patch)
tree: b0ae6baecdf2bff996ff7b0459a011414381e344
parent: 5271c77f8356dc6de95fd6d08a415df3568eb7a1 (diff)
parent: 4aebd832a405090c2608e4b60bb4f34501bcea61 (diff)
11 files changed, 178 insertions, 134 deletions
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 4ec3ac2189..dc14a60595 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -8,6 +8,7 @@
 #include <wallet/bdb.h>
 #include <wallet/db.h>
 
+#include <util/check.h>
 #include <util/strencodings.h>
 #include <util/translation.h>
 
@@ -220,17 +221,17 @@ BerkeleyEnvironment::BerkeleyEnvironment() : m_use_shared_memory(false)
     fMockDb = true;
 }
 
-BerkeleyBatch::SafeDbt::SafeDbt()
+SafeDbt::SafeDbt()
 {
     m_dbt.set_flags(DB_DBT_MALLOC);
 }
 
-BerkeleyBatch::SafeDbt::SafeDbt(void* data, size_t size)
+SafeDbt::SafeDbt(void* data, size_t size)
     : m_dbt(data, size)
 {
 }
 
-BerkeleyBatch::SafeDbt::~SafeDbt()
+SafeDbt::~SafeDbt()
 {
     if (m_dbt.get_data() != nullptr) {
         // Clear memory, e.g. in case it was a private key
@@ -244,17 +245,17 @@ BerkeleyBatch::SafeDbt::~SafeDbt()
     }
 }
 
-const void* BerkeleyBatch::SafeDbt::get_data() const
+const void* SafeDbt::get_data() const
 {
     return m_dbt.get_data();
 }
 
-uint32_t BerkeleyBatch::SafeDbt::get_size() const
+uint32_t SafeDbt::get_size() const
 {
     return m_dbt.get_size();
 }
 
-BerkeleyBatch::SafeDbt::operator Dbt*()
+SafeDbt::operator Dbt*()
 {
     return &m_dbt;
 }
@@ -307,7 +308,7 @@ BerkeleyDatabase::~BerkeleyDatabase()
     }
 }
 
-BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_cursor(nullptr), m_database(database)
+BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_database(database)
 {
     database.AddRef();
     database.Open();
@@ -398,7 +399,6 @@ void BerkeleyBatch::Close()
         activeTxn->abort();
     activeTxn = nullptr;
     pdb = nullptr;
-    CloseCursor();
 
     if (fFlushOnClose)
         Flush();
@@ -476,15 +476,15 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip)
                         fSuccess = false;
                     }
 
-                    if (db.StartCursor()) {
+                    std::unique_ptr<DatabaseCursor> cursor = db.GetNewCursor();
+                    if (cursor) {
                         while (fSuccess) {
                             CDataStream ssKey(SER_DISK, CLIENT_VERSION);
                             CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-                            bool complete;
-                            bool ret1 = db.ReadAtCursor(ssKey, ssValue, complete);
-                            if (complete) {
+                            DatabaseCursor::Status ret1 = cursor->Next(ssKey, ssValue);
+                            if (ret1 == DatabaseCursor::Status::DONE) {
                                 break;
-                            } else if (!ret1) {
+                            } else if (ret1 == DatabaseCursor::Status::FAIL) {
                                 fSuccess = false;
                                 break;
                             }
@@ -502,7 +502,7 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip)
                             if (ret2 > 0)
                                 fSuccess = false;
                         }
-                        db.CloseCursor();
+                        cursor.reset();
                     }
                     if (fSuccess) {
                         db.Close();
@@ -656,30 +656,30 @@ void BerkeleyDatabase::ReloadDbEnv()
     env->ReloadDbEnv();
 }
 
-bool BerkeleyBatch::StartCursor()
+BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database)
 {
-    assert(!m_cursor);
-    if (!pdb)
-        return false;
-    int ret = pdb->cursor(nullptr, &m_cursor, 0);
-    return ret == 0;
+    if (!database.m_db.get()) {
+        throw std::runtime_error(STR_INTERNAL_BUG("BerkeleyDatabase does not exist"));
+    }
+    int ret = database.m_db->cursor(nullptr, &m_cursor, 0);
+    if (ret != 0) {
+        throw std::runtime_error(STR_INTERNAL_BUG(strprintf("BDB Cursor could not be created. Returned %d", ret).c_str()));
+    }
 }
 
-bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete)
+DatabaseCursor::Status BerkeleyCursor::Next(CDataStream& ssKey, CDataStream& ssValue)
 {
-    complete = false;
-    if (m_cursor == nullptr) return false;
+    if (m_cursor == nullptr) return Status::FAIL;
     // 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);
@@ -688,16 +688,22 @@ bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool&
     ssValue.SetType(SER_DISK);
     ssValue.clear();
     ssValue.write({AsBytePtr(datValue.get_data()), datValue.get_size()});
-    return true;
+    return Status::MORE;
 }
 
-void BerkeleyBatch::CloseCursor()
+BerkeleyCursor::~BerkeleyCursor()
 {
     if (!m_cursor) return;
     m_cursor->close();
     m_cursor = nullptr;
 }
 
+std::unique_ptr<DatabaseCursor> BerkeleyBatch::GetNewCursor()
+{
+    if (!pdb) return nullptr;
+    return std::make_unique<BerkeleyCursor>(m_database);
+}
+
 bool BerkeleyBatch::TxnBegin()
 {
     if (!pdb || activeTxn)
diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
index 40a1031c8e..3881419d45 100644
--- a/src/wallet/bdb.h
+++ b/src/wallet/bdb.h
@@ -165,29 +165,41 @@ public:
     std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override;
 };
 
-/** RAII class that provides access to a Berkeley database */
-class BerkeleyBatch : public DatabaseBatch
+/** RAII class that automatically cleanses its data on destruction */
+class SafeDbt final
 {
-    /** RAII class that automatically cleanses its data on destruction */
-    class SafeDbt final
-    {
-        Dbt m_dbt;
+    Dbt m_dbt;
 
-    public:
-        // construct Dbt with internally-managed data
-        SafeDbt();
-        // construct Dbt with provided data
-        SafeDbt(void* data, size_t size);
-        ~SafeDbt();
+public:
+    // construct Dbt with internally-managed data
+    SafeDbt();
+    // construct Dbt with provided data
+    SafeDbt(void* data, size_t size);
+    ~SafeDbt();
+
+    // delegate to Dbt
+    const void* get_data() const;
+    uint32_t get_size() const;
+
+    // conversion operator to access the underlying Dbt
+    operator Dbt*();
+};
 
-        // delegate to Dbt
-        const void* get_data() const;
-        uint32_t get_size() const;
+class BerkeleyCursor : public DatabaseCursor
+{
+private:
+    Dbc* m_cursor;
 
-        // conversion operator to access the underlying Dbt
-        operator Dbt*();
-    };
+public:
+    explicit BerkeleyCursor(BerkeleyDatabase& database);
+    ~BerkeleyCursor() override;
+
+    Status Next(CDataStream& key, CDataStream& value) override;
+};
 
+/** RAII class that provides access to a Berkeley database */
+class BerkeleyBatch : public DatabaseBatch
+{
 private:
     bool ReadKey(CDataStream&& key, CDataStream& value) override;
     bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true) override;
@@ -198,7 +210,6 @@ protected:
     Db* pdb;
     std::string strFile;
     DbTxn* activeTxn;
-    Dbc* m_cursor;
     bool fReadOnly;
     bool fFlushOnClose;
     BerkeleyEnvironment *env;
@@ -214,9 +225,7 @@ public:
     void Flush() override;
     void Close() override;
 
-    bool StartCursor() override;
-    bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override;
-    void CloseCursor() override;
+    std::unique_ptr<DatabaseCursor> GetNewCursor() override;
     bool TxnBegin() override;
     bool TxnCommit() override;
     bool TxnAbort() override;
diff --git a/src/wallet/db.h b/src/wallet/db.h
index f09844c37e..d040af0d14 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -22,6 +22,25 @@ struct bilingual_str;
 namespace wallet {
 void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename);
 
+class DatabaseCursor
+{
+public:
+    explicit DatabaseCursor() {}
+    virtual ~DatabaseCursor() {}
+
+    DatabaseCursor(const DatabaseCursor&) = delete;
+    DatabaseCursor& operator=(const DatabaseCursor&) = delete;
+
+    enum class Status
+    {
+        FAIL,
+        MORE,
+        DONE,
+    };
+
+    virtual Status Next(CDataStream& key, CDataStream& value) { return Status::FAIL; }
+};
+
 /** RAII class that provides access to a WalletDatabase */
 class DatabaseBatch
 {
@@ -92,9 +111,7 @@ public:
         return HasKey(std::move(ssKey));
     }
 
-    virtual bool StartCursor() = 0;
-    virtual bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) = 0;
-    virtual void CloseCursor() = 0;
+    virtual std::unique_ptr<DatabaseCursor> GetNewCursor() = 0;
     virtual bool TxnBegin() = 0;
     virtual bool TxnCommit() = 0;
     virtual bool TxnAbort() = 0;
@@ -156,6 +173,11 @@ public:
     virtual std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) = 0;
 };
 
+class DummyCursor : public DatabaseCursor
+{
+    Status Next(CDataStream& key, CDataStream& value) override { return Status::FAIL; }
+};
+
 /** RAII class that provides access to a DummyDatabase. Never fails. */
 class DummyBatch : public DatabaseBatch
 {
@@ -169,9 +191,7 @@ public:
     void Flush() override {}
     void Close() override {}
 
-    bool StartCursor() override { return true; }
-    bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return true; }
-    void CloseCursor() override {}
+    std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<DummyCursor>(); }
     bool TxnBegin() override { return true; }
     bool TxnCommit() override { return true; }
     bool TxnAbort() override { return true; }
diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp
index efa548ad91..901e23b90a 100644
--- a/src/wallet/dump.cpp
+++ b/src/wallet/dump.cpp
@@ -47,7 +47,8 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error)
     std::unique_ptr<DatabaseBatch> batch = db.MakeBatch();
 
     bool ret = true;
-    if (!batch->StartCursor()) {
+    std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor();
+    if (!cursor) {
         error = _("Error: Couldn't create cursor into database");
         ret = false;
     }
@@ -68,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;
-            ret = batch->ReadAtCursor(ss_key, ss_value, complete);
-            if (complete) {
+            DatabaseCursor::Status status = cursor->Next(ss_key, ss_value);
+            if (status == DatabaseCursor::Status::DONE) {
                 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);
@@ -85,7 +86,7 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error)
         }
     }
 
-    batch->CloseCursor();
+    cursor.reset();
     batch.reset();
 
     // Close the wallet after we're done with it. The caller won't be doing this
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index f2b9909851..7a0c4b1806 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -125,7 +125,6 @@ void SQLiteBatch::SetupSQLStatements()
         {&m_insert_stmt, "INSERT INTO main VALUES(?, ?)"},
         {&m_overwrite_stmt, "INSERT or REPLACE into main values(?, ?)"},
         {&m_delete_stmt, "DELETE FROM main WHERE key = ?"},
-        {&m_cursor_stmt, "SELECT key, value FROM main"},
     };
 
     for (const auto& [stmt_prepared, stmt_text] : statements) {
@@ -374,7 +373,6 @@ void SQLiteBatch::Close()
         {&m_insert_stmt, "insert"},
         {&m_overwrite_stmt, "overwrite"},
         {&m_delete_stmt, "delete"},
-        {&m_cursor_stmt, "cursor"},
     };
 
     for (const auto& [stmt_prepared, stmt_description] : statements) {
@@ -472,28 +470,15 @@ bool SQLiteBatch::HasKey(CDataStream&& key)
     return res == SQLITE_ROW;
 }
 
-bool SQLiteBatch::StartCursor()
+DatabaseCursor::Status SQLiteCursor::Next(CDataStream& key, CDataStream& value)
 {
-    assert(!m_cursor_init);
-    if (!m_database.m_db) return false;
-    m_cursor_init = true;
-    return true;
-}
-
-bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& complete)
-{
-    complete = false;
-
-    if (!m_cursor_init) return false;
-
     int res = sqlite3_step(m_cursor_stmt);
     if (res == SQLITE_DONE) {
-        complete = true;
-        return true;
+        return Status::DONE;
     }
     if (res != SQLITE_ROW) {
-        LogPrintf("SQLiteBatch::ReadAtCursor: Unable to execute cursor step: %s\n", sqlite3_errstr(res));
-        return false;
+        LogPrintf("%s: Unable to execute cursor step: %s\n", __func__, sqlite3_errstr(res));
+        return Status::FAIL;
     }
 
     // Leftmost column in result is index 0
@@ -503,13 +488,32 @@ bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& compl
     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;
 }
 
-void SQLiteBatch::CloseCursor()
+SQLiteCursor::~SQLiteCursor()
 {
     sqlite3_reset(m_cursor_stmt);
-    m_cursor_init = false;
+    int res = sqlite3_finalize(m_cursor_stmt);
+    if (res != SQLITE_OK) {
+        LogPrintf("%s: cursor closed but could not finalize cursor statement: %s\n",
+                  __func__, sqlite3_errstr(res));
+    }
+}
+
+std::unique_ptr<DatabaseCursor> SQLiteBatch::GetNewCursor()
+{
+    if (!m_database.m_db) return nullptr;
+    auto cursor = std::make_unique<SQLiteCursor>();
+
+    const char* stmt_text = "SELECT key, value FROM main";
+    int res = sqlite3_prepare_v2(m_database.m_db, stmt_text, -1, &cursor->m_cursor_stmt, nullptr);
+    if (res != SQLITE_OK) {
+        throw std::runtime_error(strprintf(
+            "%s: Failed to setup cursor SQL statement: %s\n", __func__, sqlite3_errstr(res)));
+    }
+
+    return cursor;
 }
 
 bool SQLiteBatch::TxnBegin()
diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h
index 7680bdd07b..b35d24fe46 100644
--- a/src/wallet/sqlite.h
+++ b/src/wallet/sqlite.h
@@ -14,19 +14,27 @@ struct bilingual_str;
 namespace wallet {
 class SQLiteDatabase;
 
+class SQLiteCursor : public DatabaseCursor
+{
+public:
+    sqlite3_stmt* m_cursor_stmt{nullptr};
+
+    explicit SQLiteCursor() {}
+    ~SQLiteCursor() override;
+
+    Status Next(CDataStream& key, CDataStream& value) override;
+};
+
 /** RAII class that provides access to a WalletDatabase */
 class SQLiteBatch : public DatabaseBatch
 {
 private:
     SQLiteDatabase& m_database;
 
-    bool m_cursor_init = false;
-
     sqlite3_stmt* m_read_stmt{nullptr};
     sqlite3_stmt* m_insert_stmt{nullptr};
     sqlite3_stmt* m_overwrite_stmt{nullptr};
     sqlite3_stmt* m_delete_stmt{nullptr};
-    sqlite3_stmt* m_cursor_stmt{nullptr};
 
     void SetupSQLStatements();
 
@@ -44,9 +52,7 @@ public:
 
     void Close() override;
 
-    bool StartCursor() override;
-    bool ReadAtCursor(CDataStream& key, CDataStream& value, bool& complete) override;
-    void CloseCursor() override;
+    std::unique_ptr<DatabaseCursor> GetNewCursor() override;
     bool TxnBegin() override;
     bool TxnCommit() override;
     bool TxnAbort() override;
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index 88597bd320..f9fb99e11e 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -50,7 +50,7 @@ std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database,
 
     // Get a cursor to the original database
     auto batch = database.MakeBatch();
-    batch->StartCursor();
+    std::unique_ptr<wallet::DatabaseCursor> cursor = batch->GetNewCursor();
 
     // Get a batch for the new database
     auto new_batch = new_database->MakeBatch();
@@ -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;
-        batch->ReadAtCursor(key, value, complete);
-        if (complete) break;
+        DatabaseCursor::Status status = cursor->Next(key, value);
+        assert(status != DatabaseCursor::Status::FAIL);
+        if (status == DatabaseCursor::Status::DONE) break;
         new_batch->Write(key, value);
     }
 
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index b6e50e961a..31fc3344cc 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -907,6 +907,12 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup)
     TestUnloadWallet(std::move(wallet));
 }
 
+class FailCursor : public DatabaseCursor
+{
+public:
+    Status Next(CDataStream& key, CDataStream& value) override { return Status::FAIL; }
+};
+
 /** RAII class that provides access to a FailDatabase. Which fails if needed. */
 class FailBatch : public DatabaseBatch
 {
@@ -922,9 +928,7 @@ public:
     void Flush() override {}
     void Close() override {}
 
-    bool StartCursor() override { return true; }
-    bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return false; }
-    void CloseCursor() override {}
+    std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<FailCursor>(); }
     bool TxnBegin() override { return false; }
     bool TxnCommit() override { return false; }
     bool TxnAbort() override { return false; }
diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
index 24d21c2f22..f74bf54d9e 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -54,13 +54,15 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup)
 bool HasAnyRecordOfType(WalletDatabase& db, const std::string& key)
 {
     std::unique_ptr<DatabaseBatch> batch = db.MakeBatch(false);
-    BOOST_CHECK(batch->StartCursor());
+    BOOST_CHECK(batch);
+    std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor();
+    BOOST_CHECK(cursor);
     while (true) {
         CDataStream ssKey(SER_DISK, CLIENT_VERSION);
         CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-        bool complete;
-        BOOST_CHECK(batch->ReadAtCursor(ssKey, ssValue, complete));
-        if (complete) break;
+        DatabaseCursor::Status status = cursor->Next(ssKey, ssValue);
+        assert(status != DatabaseCursor::Status::FAIL);
+        if (status == DatabaseCursor::Status::DONE) break;
         std::string type;
         ssKey >> type;
         if (type == key) return true;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 6158ff033c..d62c239453 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3775,26 +3775,27 @@ bool CWallet::MigrateToSQLite(bilingual_str& error)
 
     // Get all of the records for DB type migration
     std::unique_ptr<DatabaseBatch> batch = m_database->MakeBatch();
+    std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor();
     std::vector<std::pair<SerializeData, SerializeData>> records;
-    if (!batch->StartCursor()) {
+    if (!cursor) {
         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 = batch->ReadAtCursor(ss_key, ss_value, complete);
-        if (!ret) {
+        status = cursor->Next(ss_key, ss_value);
+        if (status != DatabaseCursor::Status::MORE) {
             break;
         }
         SerializeData key(ss_key.begin(), ss_key.end());
         SerializeData value(ss_value.begin(), ss_value.end());
         records.emplace_back(key, value);
     }
-    batch->CloseCursor();
+    cursor.reset();
     batch.reset();
-    if (!complete) {
+    if (status != DatabaseCursor::Status::DONE) {
         error = _("Error: Unable to read all records in the database");
         return false;
     }
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index b393c35112..7c59b6aac0 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -812,7 +812,8 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
 #endif
 
         // Get cursor
-        if (!m_batch->StartCursor())
+        std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor();
+        if (!cursor)
         {
             pwallet->WalletLogPrintf("Error getting wallet database cursor\n");
             return DBErrors::CORRUPT;
@@ -823,14 +824,11 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
             // Read next record
             CDataStream ssKey(SER_DISK, CLIENT_VERSION);
             CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-            bool complete;
-            bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete);
-            if (complete) {
+            DatabaseCursor::Status status = cursor->Next(ssKey, ssValue);
+            if (status == DatabaseCursor::Status::DONE) {
                 break;
-            }
-            else if (!ret)
-            {
-                m_batch->CloseCursor();
+            } else if (status == DatabaseCursor::Status::FAIL) {
+                cursor.reset();
                 pwallet->WalletLogPrintf("Error reading next record from wallet database\n");
                 return DBErrors::CORRUPT;
             }
@@ -878,7 +876,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
     } catch (...) {
         result = DBErrors::CORRUPT;
     }
-    m_batch->CloseCursor();
 
     // Set the active ScriptPubKeyMans
     for (auto spk_man_pair : wss.m_active_external_spks) {
@@ -986,7 +983,8 @@ DBErrors WalletBatch::FindWalletTxHashes(std::vector<uint256>& tx_hashes)
         }
 
         // Get cursor
-        if (!m_batch->StartCursor())
+        std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor();
+        if (!cursor)
         {
             LogPrintf("Error getting wallet database cursor\n");
             return DBErrors::CORRUPT;
@@ -997,12 +995,10 @@ DBErrors WalletBatch::FindWalletTxHashes(std::vector<uint256>& tx_hashes)
             // Read next record
             CDataStream ssKey(SER_DISK, CLIENT_VERSION);
             CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-            bool complete;
-            bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete);
-            if (complete) {
+            DatabaseCursor::Status status = cursor->Next(ssKey, ssValue);
+            if (status == DatabaseCursor::Status::DONE) {
                 break;
-            } else if (!ret) {
-                m_batch->CloseCursor();
+            } else if (status == DatabaseCursor::Status::FAIL) {
                 LogPrintf("Error reading next record from wallet database\n");
                 return DBErrors::CORRUPT;
             }
@@ -1018,7 +1014,6 @@ DBErrors WalletBatch::FindWalletTxHashes(std::vector<uint256>& tx_hashes)
     } catch (...) {
         result = DBErrors::CORRUPT;
     }
-    m_batch->CloseCursor();
 
     return result;
 }
@@ -1111,7 +1106,8 @@ bool WalletBatch::WriteWalletFlags(const uint64_t flags)
 bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types)
 {
     // Get cursor
-    if (!m_batch->StartCursor())
+    std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor();
+    if (!cursor)
     {
         return false;
     }
@@ -1122,14 +1118,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;
-        bool ret = m_batch->ReadAtCursor(key, value, complete);
-        if (complete) {
+        DatabaseCursor::Status status = cursor->Next(key, value);
+        if (status == DatabaseCursor::Status::DONE) {
             break;
-        }
-        else if (!ret)
-        {
-            m_batch->CloseCursor();
+        } else if (status == DatabaseCursor::Status::FAIL) {
             return false;
         }
 
@@ -1143,7 +1135,6 @@ bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types)
             m_batch->Erase(key_data);
         }
     }
-    m_batch->CloseCursor();
     return true;
 }
author	fanquake <fanquake@gmail.com>	2023-01-23 17:37:44 +0000
committer	fanquake <fanquake@gmail.com>	2023-01-23 17:56:16 +0000
commit	a62231bca629e945349255a1d331dd5c7a86ddd1 (patch)
tree	b0ae6baecdf2bff996ff7b0459a011414381e344
parent	5271c77f8356dc6de95fd6d08a415df3568eb7a1 (diff)
parent	4aebd832a405090c2608e4b60bb4f34501bcea61 (diff)