From 043fcb0b053eee6021cc75e3d3f41097f52698c0 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Mon, 1 May 2023 10:21:36 -0300
Subject: [PATCH] wallet: bugfix, GetNewCursor() misses to provide batch ptr to
 BerkeleyCursor

If the batch ptr is not passed, the cursor will not use the db active
txn context which could lead to a deadlock if the code tries to modify
the db while it is traversing it.

E.g. the 'EraseRecords()' function.
---
 src/wallet/bdb.cpp | 8 ++++----
 src/wallet/bdb.h   | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index fa6e56c6e4a..6dce51fc124 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -668,14 +668,14 @@ void BerkeleyDatabase::ReloadDbEnv()
     env->ReloadDbEnv();
 }
 
-BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database, BerkeleyBatch* batch)
+BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch)
 {
     if (!database.m_db.get()) {
         throw std::runtime_error(STR_INTERNAL_BUG("BerkeleyDatabase does not exist"));
     }
     // Transaction argument to cursor is only needed when using the cursor to
     // write to the database. Read-only cursors do not need a txn pointer.
-    int ret = database.m_db->cursor(batch ? batch->txn() : nullptr, &m_cursor, 0);
+    int ret = database.m_db->cursor(batch.txn(), &m_cursor, 0);
     if (ret != 0) {
         throw std::runtime_error(STR_INTERNAL_BUG(strprintf("BDB Cursor could not be created. Returned %d", ret)));
     }
@@ -713,7 +713,7 @@ BerkeleyCursor::~BerkeleyCursor()
 std::unique_ptr<DatabaseCursor> BerkeleyBatch::GetNewCursor()
 {
     if (!pdb) return nullptr;
-    return std::make_unique<BerkeleyCursor>(m_database);
+    return std::make_unique<BerkeleyCursor>(m_database, *this);
 }
 
 bool BerkeleyBatch::TxnBegin()
@@ -825,7 +825,7 @@ bool BerkeleyBatch::HasKey(DataStream&& key)
 bool BerkeleyBatch::ErasePrefix(Span<const std::byte> prefix)
 {
     if (!TxnBegin()) return false;
-    auto cursor{std::make_unique<BerkeleyCursor>(m_database, this)};
+    auto cursor{std::make_unique<BerkeleyCursor>(m_database, *this)};
     // const_cast is safe below even though prefix_key is an in/out parameter,
     // because we are not using the DB_DBT_USERMEM flag, so BDB will allocate
     // and return a different output data pointer
diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
index 4fac128bf16..9134643b238 100644
--- a/src/wallet/bdb.h
+++ b/src/wallet/bdb.h
@@ -192,7 +192,7 @@ private:
     Dbc* m_cursor;
 
 public:
-    explicit BerkeleyCursor(BerkeleyDatabase& database, BerkeleyBatch* batch=nullptr);
+    explicit BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch);
     ~BerkeleyCursor() override;
 
     Status Next(DataStream& key, DataStream& value) override;