Repository: bitcoin/bitcoin
File: src/dbwrapper.cpp (C++)

Code before:
    } else {
        if (params.wipe_data) {
            LogPrintf("Wiping LevelDB in %s\n", fs::PathToString(params.path));
            leveldb::Status result = leveldb::DestroyDB(fs::PathToString(params.path), DBContext().options);
            HandleError(result);
        }
        TryCreateDirectories(params.path);
        LogPrintf("Opening LevelDB in %s\n", fs::PathToString(params.path));
    }
    // PathToString() return value is safe to pass to leveldb open function,
    // because on POSIX leveldb passes the byte string directly to ::open(), and
    // on Windows it converts from UTF-8 to UTF-16 before calling ::CreateFileW
    // (see env_posix.cc and env_windows.cc).
    leveldb::Status status = leveldb::DB::Open(DBContext().options, fs::PathToString(params.path), &DBContext().pdb);
    HandleError(status);
    LogPrintf("Opened LevelDB successfully\n");

    if (params.options.force_compact) {
        LogPrintf("Starting database compaction of %s\n", fs::PathToString(params.path));
        DBContext().pdb->CompactRange(nullptr, nullptr);
        LogPrintf("Finished database compaction of %s\n", fs::PathToString(params.path));
    }

    assert(!m_obfuscation); // Needed for unobfuscated Read()/Write() below
    if (!Read(OBFUSCATION_KEY_KEY, m_obfuscation) && params.obfuscate && IsEmpty()) {
        // Generate, write and read back the new obfuscation key, making sure we don't obfuscate the key itself
        Write(OBFUSCATION_KEY_KEY, FastRandomContext{}.randbytes(Obfuscation::KEY_SIZE));
        Read(OBFUSCATION_KEY_KEY, m_obfuscation);
        LogInfo("Wrote new obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey());
    }
    LogInfo("Using obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey());
}

CDBWrapper::~CDBWrapper()
{
    delete DBContext().pdb;
    DBContext().pdb = nullptr;
    delete DBContext().options.filter_policy;
    DBContext().options.filter_policy = nullptr;
    delete DBContext().options.info_log;
    DBContext().options.info_log = nullptr;
    delete DBContext().options.block_cache;
    DBContext().options.block_cache = nullptr;
    delete DBContext().penv;
    DBContext().options.env = nullptr;
}

bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
{
    const bool log_memory = LogAcceptCategory(BCLog::LEVELDB, BCLog::Level::Debug);
    double mem_before = 0;

Review comment (bug):
In commit "refactor: standardize obfuscation memory alignment" (e7b00d69b6e05b6b4fd08ace785727c56626cbc2)

Looks good to declare an obfuscation object and let it serialize itself, and not have an unnecessary read call. I still think the "making sure we don't obfuscate the key" comment is vague, because it's not clear what it refers to. I'd suggest moving that comment next to the assert and moving the assert where it's more relevant:

<details><summary>diff</summary>
<p>

```diff
--- a/src/dbwrapper.cpp
+++ b/src/dbwrapper.cpp
@@ -249,10 +249,10 @@ CDBWrapper::CDBWrapper(const DBParams& params)
         LogPrintf("Finished database compaction of %s\n", fs::PathToString(params.path));
     }
 
-    assert(!m_obfuscation); // Needed for unobfuscated Read()/Write() below
     if (!Read(OBFUSCATION_KEY, m_obfuscation) && params.obfuscate && IsEmpty()) {
-        // Generate and write the new obfuscation key - making sure we don't obfuscate the key itself
+        // Generate and write the new obfuscation key.
         const Obfuscation obfuscation{FastRandomContext{}.randbytes<Obfuscation::KEY_SIZE>()};
+        assert(!m_obfuscation); // Make sure the key is written without obfuscation.
         Write(OBFUSCATION_KEY, obfuscation);
         m_obfuscation = obfuscation;
         LogInfo("Wrote new obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey());
```
</p>
</details>

This also strengthens the assert if the Read method fails in a way leaving m_obfuscation non-zero.

Code after:
    } else {
        if (params.wipe_data) {
            LogPrintf("Wiping LevelDB in %s\n", fs::PathToString(params.path));
            leveldb::Status result = leveldb::DestroyDB(fs::PathToString(params.path), DBContext().options);
            HandleError(result);
        }
        TryCreateDirectories(params.path);
        LogPrintf("Opening LevelDB in %s\n", fs::PathToString(params.path));
    }
    // PathToString() return value is safe to pass to leveldb open function,
    // because on POSIX leveldb passes the byte string directly to ::open(), and
    // on Windows it converts from UTF-8 to UTF-16 before calling ::CreateFileW
    // (see env_posix.cc and env_windows.cc).
    leveldb::Status status = leveldb::DB::Open(DBContext().options, fs::PathToString(params.path), &DBContext().pdb);
    HandleError(status);
    LogPrintf("Opened LevelDB successfully\n");

    if (params.options.force_compact) {
        LogPrintf("Starting database compaction of %s\n", fs::PathToString(params.path));
        DBContext().pdb->CompactRange(nullptr, nullptr);
        LogPrintf("Finished database compaction of %s\n", fs::PathToString(params.path));
    }

    if (!Read(OBFUSCATION_KEY, m_obfuscation) && params.obfuscate && IsEmpty()) {
        // Generate and write the new obfuscation key.
        const Obfuscation obfuscation{FastRandomContext{}.randbytes<Obfuscation::KEY_SIZE>()};
        assert(!m_obfuscation); // Make sure the key is written without obfuscation.
        Write(OBFUSCATION_KEY, obfuscation);
        m_obfuscation = obfuscation;
        LogInfo("Wrote new obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey());
    }
    LogInfo("Using obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey());
}

CDBWrapper::~CDBWrapper()
{
    delete DBContext().pdb;
    DBContext().pdb = nullptr;
    delete DBContext().options.filter_policy;
    DBContext().options.filter_policy = nullptr;
    delete DBContext().options.info_log;
    DBContext().options.info_log = nullptr;
    delete DBContext().options.block_cache;
    DBContext().options.block_cache = nullptr;
    delete DBContext().penv;
    DBContext().options.env = nullptr;
}

bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
{
    const bool log_memory = LogAcceptCategory(BCLog::LEVELDB, BCLog::Level::Debug);