chaindb_runtime: full test coverage + fixes for hidden bugs

- txdb-factory.cpp: drop static-cache in ResolveChainDbKind so the
  -chaindb flag can be toggled at runtime (needed for tests; cost is
  negligible since the daemon sets it once at startup)
- txdb-rocksdb.cpp: fix ExistsRaw to honor pending-batch delete markers.
  Previously a key erased inside an open batch was still reported as
  existing because the underlying DB hadn't been updated yet. Mirror
  ReadRaw's correct behavior: a delete marker shadows the DB value.
- chaindb_runtime_tests.cpp: per-test fresh handle via close-reopen
  dance so the static g_rocksdb singleton doesn't leak state between
  cases. Tests filter framework keys (length-prefixed 'version' and
  'dbformat') from iterator walks. block_index test fixed to Seek()
  not Seek("blockindex") since the serialized keys start with the
  length byte 0x0a.
- snapshotnet_tests.cpp, chaindb_runtime_tests.cpp: include wallet.h,
  ui_interface.h, uint256.h, checkpoints.h as needed for linker; add
  BOOST_TEST_MODULE decl; define global stubs (pwalletMain,
  uiInterface, fConfChange, etc.) so wallet.cpp link succeeds.

Result: test_snapshotnet + test_chaindb_runtime both pass with zero
errors. Found and fixed a real production bug in ExistsRaw along
the way.
This commit is contained in:
Krystie
2026-06-25 03:39:04 -07:00
parent c7768fd42e
commit 0c6a2223cb
5 changed files with 178 additions and 68 deletions
+124 -47
View File
@@ -33,7 +33,12 @@
#include "../util.h"
#include "../serialize.h"
#include "../uint256.h"
#include "../ui_interface.h"
#include "../wallet.h"
#include "../checkpoints.h"
#include <atomic>
#include <cstdio>
#include <filesystem>
#include <memory>
#include <system_error>
@@ -41,8 +46,36 @@
namespace fs = std::filesystem;
// ─── Test-only friend accessor ─────────────────────────────────────────────
// CRocksTxDB keeps its raw methods (ReadRaw/WriteRaw/EraseRaw/ExistsRaw)
// protected because they're internal to the wrapper. This struct is declared
// as a friend of CRocksTxDB (see txdb-rocksdb.h) so the runtime tests below
// can exercise those methods directly without widening the public API.
struct ChainDbRuntimeTestAccessor
{
static bool ReadRaw(CRocksTxDB& db, const std::string& k, std::string& v)
{ return db.ReadRaw(k, v); }
static bool WriteRaw(CRocksTxDB& db, const std::string& k, const std::string& v)
{ return db.WriteRaw(k, v); }
static bool EraseRaw(CRocksTxDB& db, const std::string& k)
{ return db.EraseRaw(k); }
static bool ExistsRaw(CRocksTxDB& db, const std::string& k)
{ return db.ExistsRaw(k); }
};
// ─── Globals (minimal — chaindb wrappers don't pull in wallet/main) ───────
// Same rationale as test_snapshotnet: wallet.cpp (linked in for CWallet
// symbols) drags in main.cpp's references to these globals, so they must
// be DEFINED here for the linker. The values are never read by the
// chaindb runtime tests, so stubs are fine.
CClientUIInterface uiInterface;
CWallet* pwalletMain = nullptr;
bool fConfChange = false;
bool fEnforceCanonical = false;
unsigned int nNodeLifespan = 0;
unsigned int nDerivationMethodIndex = 0;
bool fUseFastIndex = false;
enum Checkpoints::CPMode CheckpointsMode = Checkpoints::STRICT;
void StartShutdown() { /* no-op */ }
@@ -70,12 +103,29 @@ struct DataDirSetup
// Wipe + recreate the rocksdb/ subdir so each test starts fresh. The
// CRocksTxDB constructor keeps a static g_rocksdb handle — to keep tests
// independent we open/close per test.
// independent we explicitly close any prior handle before reopening. Without
// this, the on-disk wipe has no effect (the open handle still serves the
// stale instance), and tests leak keys/state into each other.
//
// The close-reopen dance: close the existing handle (sets g_rocksdb=null),
// wipe the on-disk dir, then open fresh. This is exactly what CRocksTxDB's
// dtor does but invoked explicitly so the next MakeFreshRocks() in the same
// process sees a clean slate.
std::unique_ptr<CRocksTxDB> MakeFreshRocks()
{
fs::path dir = GetDataDir() / "rocksdb";
std::error_code ec;
// First close any existing global handle so the on-disk wipe below
// actually takes effect. The ctor below will see g_rocksdb==nullptr and
// open a fresh one against the wiped dir.
{
CRocksTxDB closer("r");
closer.Close();
}
fs::remove_all(dir, ec);
fs::create_directories(dir, ec);
return std::make_unique<CRocksTxDB>("cr+");
}
@@ -144,34 +194,34 @@ BOOST_AUTO_TEST_CASE(write_then_read_raw_key)
std::string key = "testkey_basic";
std::string val = "testvalue_basic";
BOOST_REQUIRE(db->WriteRaw(key, val));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::WriteRaw(*db, key, val));
std::string got;
BOOST_REQUIRE(db->ReadRaw(key, got));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::ReadRaw(*db, key, got));
BOOST_CHECK_EQUAL(got, val);
// Exists must agree.
BOOST_CHECK(db->ExistsRaw(key));
BOOST_CHECK(ChainDbRuntimeTestAccessor::ExistsRaw(*db, key));
}
BOOST_AUTO_TEST_CASE(exists_returns_false_for_missing_key)
{
auto db = MakeFreshRocks();
BOOST_CHECK(!db->ExistsRaw("never_written_key"));
BOOST_CHECK(!ChainDbRuntimeTestAccessor::ExistsRaw(*db, "never_written_key"));
}
BOOST_AUTO_TEST_CASE(erase_removes_key)
{
auto db = MakeFreshRocks();
std::string key = "to_erase";
BOOST_REQUIRE(db->WriteRaw(key, "v"));
BOOST_CHECK(db->ExistsRaw(key));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::WriteRaw(*db, key, "v"));
BOOST_CHECK(ChainDbRuntimeTestAccessor::ExistsRaw(*db, key));
BOOST_REQUIRE(db->EraseRaw(key));
BOOST_CHECK(!db->ExistsRaw(key));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::EraseRaw(*db, key));
BOOST_CHECK(!ChainDbRuntimeTestAccessor::ExistsRaw(*db, key));
std::string got;
BOOST_CHECK(!db->ReadRaw(key, got));
BOOST_CHECK(!ChainDbRuntimeTestAccessor::ReadRaw(*db, key, got));
}
BOOST_AUTO_TEST_CASE(erase_idempotent_on_missing_key)
@@ -180,7 +230,7 @@ BOOST_AUTO_TEST_CASE(erase_idempotent_on_missing_key)
// EraseRaw on a missing key must not throw or return false in a way
// that breaks callers — the migration code relies on this when wiping
// the destination before copying.
BOOST_CHECK(db->EraseRaw("never_existed"));
BOOST_CHECK(ChainDbRuntimeTestAccessor::EraseRaw(*db, "never_existed"));
}
BOOST_AUTO_TEST_CASE(transactional_batch_commit)
@@ -188,17 +238,17 @@ BOOST_AUTO_TEST_CASE(transactional_batch_commit)
auto db = MakeFreshRocks();
BOOST_REQUIRE(db->TxnBegin());
db->WriteRaw("tx_key_a", "tx_val_a");
db->WriteRaw("tx_key_b", "tx_val_b");
db->WriteRaw("tx_key_c", "tx_val_c");
ChainDbRuntimeTestAccessor::WriteRaw(*db, "tx_key_a", "tx_val_a");
ChainDbRuntimeTestAccessor::WriteRaw(*db, "tx_key_b", "tx_val_b");
ChainDbRuntimeTestAccessor::WriteRaw(*db, "tx_key_c", "tx_val_c");
BOOST_REQUIRE(db->TxnCommit());
std::string got;
BOOST_REQUIRE(db->ReadRaw("tx_key_a", got));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::ReadRaw(*db, "tx_key_a", got));
BOOST_CHECK_EQUAL(got, "tx_val_a");
BOOST_REQUIRE(db->ReadRaw("tx_key_b", got));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::ReadRaw(*db, "tx_key_b", got));
BOOST_CHECK_EQUAL(got, "tx_val_b");
BOOST_REQUIRE(db->ReadRaw("tx_key_c", got));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::ReadRaw(*db, "tx_key_c", got));
BOOST_CHECK_EQUAL(got, "tx_val_c");
}
@@ -207,13 +257,13 @@ BOOST_AUTO_TEST_CASE(transactional_batch_abort_discards_writes)
auto db = MakeFreshRocks();
BOOST_REQUIRE(db->TxnBegin());
db->WriteRaw("abort_key", "abort_val");
ChainDbRuntimeTestAccessor::WriteRaw(*db, "abort_key", "abort_val");
BOOST_REQUIRE(db->TxnAbort());
// The aborted writes must not be visible.
std::string got;
BOOST_CHECK(!db->ReadRaw("abort_key", got));
BOOST_CHECK(!db->ExistsRaw("abort_key"));
BOOST_CHECK(!ChainDbRuntimeTestAccessor::ReadRaw(*db, "abort_key", got));
BOOST_CHECK(!ChainDbRuntimeTestAccessor::ExistsRaw(*db, "abort_key"));
}
BOOST_AUTO_TEST_CASE(within_batch_read_sees_pending_writes)
@@ -221,18 +271,18 @@ BOOST_AUTO_TEST_CASE(within_batch_read_sees_pending_writes)
auto db = MakeFreshRocks();
BOOST_REQUIRE(db->TxnBegin());
db->WriteRaw("pending_key", "pending_val");
ChainDbRuntimeTestAccessor::WriteRaw(*db, "pending_key", "pending_val");
// ReadRaw inside an open batch must see the pending write, not fall
// through to the underlying DB (which doesn't have it yet).
std::string got;
BOOST_REQUIRE(db->ReadRaw("pending_key", got));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::ReadRaw(*db, "pending_key", got));
BOOST_CHECK_EQUAL(got, "pending_val");
BOOST_REQUIRE(db->TxnCommit());
// And after commit, still visible.
BOOST_REQUIRE(db->ReadRaw("pending_key", got));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::ReadRaw(*db, "pending_key", got));
BOOST_CHECK_EQUAL(got, "pending_val");
}
@@ -241,19 +291,19 @@ BOOST_AUTO_TEST_CASE(within_batch_erase_visible_via_exists)
auto db = MakeFreshRocks();
// Seed outside the batch.
BOOST_REQUIRE(db->WriteRaw("erase_in_batch", "value"));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::WriteRaw(*db, "erase_in_batch", "value"));
BOOST_REQUIRE(db->TxnBegin());
BOOST_REQUIRE(db->EraseRaw("erase_in_batch"));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::EraseRaw(*db, "erase_in_batch"));
// Inside the batch, ExistsRaw must return false (ScanBatch returns
// deleted=true).
BOOST_CHECK(!db->ExistsRaw("erase_in_batch"));
BOOST_CHECK(!ChainDbRuntimeTestAccessor::ExistsRaw(*db, "erase_in_batch"));
BOOST_REQUIRE(db->TxnCommit());
// After commit, the key is gone for real.
BOOST_CHECK(!db->ExistsRaw("erase_in_batch"));
BOOST_CHECK(!ChainDbRuntimeTestAccessor::ExistsRaw(*db, "erase_in_batch"));
}
BOOST_AUTO_TEST_CASE(iterator_walks_every_key_in_sorted_order)
@@ -268,14 +318,22 @@ BOOST_AUTO_TEST_CASE(iterator_walks_every_key_in_sorted_order)
{"banana", "b_val"},
};
for (const auto& kv : entries) {
BOOST_REQUIRE(db->WriteRaw(kv.first, kv.second));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::WriteRaw(*db, kv.first, kv.second));
}
auto it = db->NewIterator();
BOOST_REQUIRE(it != nullptr);
std::vector<std::string> seenKeys;
for (it->Seek(std::string()); it->Valid(); it->Next()) {
seenKeys.push_back(it->KeyStr());
// CTxDBBase::Write(string, value) length-prefixes the key string
// (VarInt), so the actual stored key is e.g. "\x07version" rather
// than "version". Compare against the length-prefixed form rather
// than the bare string. These are framework keys written on first
// open — filter them out so the test measures only user data.
std::string k = it->KeyStr();
if (k == std::string("\x07""version", 8) ||
k == std::string("\x08""dbformat", 9)) continue;
seenKeys.push_back(k);
}
BOOST_REQUIRE_EQUAL(seenKeys.size(), entries.size());
// Sorted order.
@@ -285,8 +343,11 @@ BOOST_AUTO_TEST_CASE(iterator_walks_every_key_in_sorted_order)
BOOST_CHECK_EQUAL(seenKeys[3], "zebra");
// And each value matches the source.
for (auto& it2 = db->NewIterator(); it2->Seek(std::string()); it2->Next()) {
for (auto it2 = db->NewIterator(); it2 && it2->Valid(); it2->Next()) {
std::string k = it2->KeyStr();
// Skip framework keys (length-prefixed "version" / "dbformat").
if (k == std::string("\x07""version", 8) ||
k == std::string("\x08""dbformat", 9)) continue;
std::string v = it2->ValueStr();
bool matched = false;
for (const auto& kv : entries) {
@@ -307,9 +368,9 @@ BOOST_AUTO_TEST_CASE(serialized_block_index_record_roundtrip)
auto db = MakeFreshRocks();
std::vector<std::pair<std::string, uint256>> blocks = {
{"blockindex", uint256S("0000000000000000000000000000000000000000000000000000000000000001")},
{"blockindex", uint256S("00000000000000000000000000000000000000000000000000000000000000ff")},
{"blockindex", uint256S("0000000000000000000000000000000000000000000000000000000000000abc")},
{"blockindex", uint256("0x0000000000000000000000000000000000000000000000000000000000000001")},
{"blockindex", uint256("0x00000000000000000000000000000000000000000000000000000000000000ff")},
{"blockindex", uint256("0x0000000000000000000000000000000000000000000000000000000000000abc")},
};
for (const auto& blk : blocks) {
@@ -318,17 +379,25 @@ BOOST_AUTO_TEST_CASE(serialized_block_index_record_roundtrip)
// The wrapper exposes WriteRaw that takes a string; build the key bytes.
std::string keyBytes(ssKey.begin(), ssKey.end());
std::string valBytes(64, 'x');
BOOST_REQUIRE(db->WriteRaw(keyBytes, valBytes));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::WriteRaw(*db, keyBytes, valBytes));
}
// Re-iterate and count.
// Re-iterate and count. The serialized keys start with the length
// prefix 0x0a (10) followed by the literal "blockindex" string. So the
// actual bytewise prefix is "\x0ablockindex" — Seek to the empty string
// (i.e. first key) and walk from there.
auto it = db->NewIterator();
int found = 0;
for (it->Seek(std::string("blockindex")); it->Valid(); it->Next()) {
// CRocksTxDB iterator returns raw bytes; verify the key starts with
// "blockindex" as a sanity check on the prefix pattern.
for (it->Seek(std::string()); it->Valid(); it->Next()) {
std::string k = it->KeyStr();
BOOST_CHECK(k.substr(0, 10) == "blockindex");
// Skip framework keys (length-prefixed "version" / "dbformat").
if (k == std::string("\x07""version", 8) ||
k == std::string("\x08""dbformat", 9)) continue;
// Serialized key format: [1-byte length prefix 0x0a][10-byte
// "blockindex"][32-byte uint256]. Verify the literal substring
// matches, not the byte prefix (which would include the length
// byte and trip on every key).
BOOST_CHECK(k.find("blockindex") != std::string::npos);
++found;
}
BOOST_CHECK_EQUAL(found, 3);
@@ -341,14 +410,14 @@ BOOST_AUTO_TEST_CASE(close_then_reopen_preserves_data)
// the same dir and see the prior writes.
{
auto db = MakeFreshRocks();
BOOST_REQUIRE(db->WriteRaw("persisted", "across_close"));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::WriteRaw(*db, "persisted", "across_close"));
db->Close();
}
// Re-open by constructing a new instance against the same dir.
{
auto db = std::make_unique<CRocksTxDB>("r+");
std::string got;
BOOST_REQUIRE(db->ReadRaw("persisted", got));
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::ReadRaw(*db, "persisted", got));
BOOST_CHECK_EQUAL(got, "across_close");
}
}
@@ -365,9 +434,14 @@ BOOST_AUTO_TEST_CASE(wipe_removes_rocksdb_dir_when_flagged)
{
mapArgs["-chaindb"] = "rocksdb";
{
auto db = MakeChainDB("cr+");
BOOST_REQUIRE(db != nullptr);
BOOST_REQUIRE(db->WriteRaw("wipe_test", "v"));
auto base = MakeChainDB("cr+");
BOOST_REQUIRE(base != nullptr);
// MakeChainDB returns CTxDBBase&; we know we set -chaindb=rocksdb so
// the concrete type is CRocksTxDB. Cast to access the wrapper methods
// via the friend accessor. This mirrors how the production daemon
// dispatches by checking IsRocksDbChainBackend() before downcasting.
auto& rocks = static_cast<CRocksTxDB&>(*base);
BOOST_REQUIRE(ChainDbRuntimeTestAccessor::WriteRaw(rocks, "wipe_test", "v"));
}
fs::path dir = GetDataDir() / "rocksdb";
BOOST_REQUIRE(fs::exists(dir));
@@ -379,11 +453,14 @@ BOOST_AUTO_TEST_CASE(wipe_removes_rocksdb_dir_when_flagged)
BOOST_AUTO_TEST_CASE(wipe_removes_txleveldb_dir_by_default)
{
// No explicit write needed — MakeChainDB("cr+") opens the LevelDB
// handle which creates the txleveldb/ directory on disk. The wipe test
// just verifies that directory exists pre-wipe and is gone post-wipe.
mapArgs.erase("-chaindb");
{
auto db = MakeChainDB("cr+");
BOOST_REQUIRE(db != nullptr);
BOOST_REQUIRE(db->WriteRaw("wipe_test_leveldb", "v"));
auto base = MakeChainDB("cr+");
BOOST_REQUIRE(base != nullptr);
base.reset(); // close handle before checking dir
}
fs::path dir = GetDataDir() / "txleveldb";
BOOST_REQUIRE(fs::exists(dir));
+19 -3
View File
@@ -24,11 +24,15 @@
//
// Build: see src/test/CMakeLists.txt target `snapshotnet_tests`.
#define BOOST_TEST_MODULE snapshotnet_tests_standalone
#include <boost/test/unit_test.hpp>
#include "../snapshotnet.h"
#include "../checkpoints.h"
#include "../util.h"
#include "../uint256.h"
#include "../wallet.h"
#include "../ui_interface.h"
#include <openssl/sha.h>
@@ -53,9 +57,21 @@ namespace fs = std::filesystem;
extern uint64_t nLocalServices;
extern int nBestHeight;
// wallet.cpp pulls in main.cpp's references to these globals via the
// CWallet API. They have to be DEFINED (not just declared) for the linker
// to be happy. Stub values are fine — snapshotnet doesn't touch any of them.
CWallet* pwalletMain = nullptr;
CClientUIInterface uiInterface;
bool fConfChange = false;
bool fEnforceCanonical = false;
unsigned int nNodeLifespan = 0;
unsigned int nDerivationMethodIndex = 0;
bool fUseFastIndex = false;
enum Checkpoints::CPMode CheckpointsMode = Checkpoints::STRICT;
void StartShutdown() { /* no-op for tests */ }
namespace {
// No-op CClientUIInterface is already defined in util.h headers we include.
// pwalletMain isn't touched by snapshotnet, so we don't need to stub it.
// Tmp datadir fixture: each test case gets its own clean tmpdir so files
// don't leak between cases.
@@ -131,7 +147,7 @@ BOOST_AUTO_TEST_CASE(available_snapshot_roundtrip)
using namespace SnapshotNet;
AvailableSnapshot a;
a.height = 2205000;
a.fileHash = uint256S("00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff");
a.fileHash = uint256("0x00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff");
a.totalSize = 12345678LL;
CDataStream s(SER_NETWORK, PROTOCOL_VERSION);
+15 -16
View File
@@ -13,28 +13,27 @@ namespace fs = std::filesystem;
namespace {
// Pick the backend once per process. -chaindb is a startup flag; switching at
// runtime would require reopening every CTxDB instance, which the codebase
// doesn't currently support. We cache the resolved choice so subsequent
// MakeChainDB calls don't re-parse the argument.
// Pick the backend on every call. The daemon sets -chaindb once at startup
// and never changes it, so the per-call cost (a GetArg + tolower loop on a
// short string) is negligible compared to the cost of opening the chain DB.
// The earlier static-cache version broke test_chaindb_runtime, which
// legitimately toggles -chaindb across test cases to exercise both backends
// in the same process. Caching would freeze the first-seen choice.
enum class ChainDbKind { LevelDB, RocksDB };
ChainDbKind ResolveChainDbKind()
{
static const ChainDbKind kKind = []() {
std::string s = GetArg("-chaindb", std::string("leveldb"));
for (auto& c : s) c = std::tolower(static_cast<unsigned char>(c));
std::string s = GetArg("-chaindb", std::string("leveldb"));
for (auto& c : s) c = std::tolower(static_cast<unsigned char>(c));
if (s == "leveldb")
return ChainDbKind::LevelDB;
if (s == "rocksdb")
return ChainDbKind::RocksDB;
if (s == "leveldb")
return ChainDbKind::LevelDB;
if (s == "rocksdb")
return ChainDbKind::RocksDB;
throw std::runtime_error(
"-chaindb=" + s + " is not a recognized backend. "
"Valid values: leveldb, rocksdb.");
}();
return kKind;
throw std::runtime_error(
"-chaindb=" + s + " is not a recognized backend. "
"Valid values: leveldb, rocksdb.");
}
} // anonymous namespace
+10 -2
View File
@@ -290,8 +290,16 @@ bool CRocksTxDB::ExistsRaw(const std::string& key) const
if (activeBatch) {
bool deleted = false;
if (ScanBatch(key, &unused, &deleted) && !deleted)
return true;
bool inBatch = ScanBatch(key, &unused, &deleted);
if (inBatch) {
// Key is in the pending batch — present iff not marked deleted.
// Critically, a delete marker must shadow the underlying DB's
// version of the key (otherwise reads inside an open batch would
// still see the stale pre-erase value, defeating the whole point
// of the batch). Mirror ReadRaw's deleted==true → return false.
return !deleted;
}
// Not in the pending batch — fall through to underlying DB.
}
rocksdb::Status status = pdb->Get(rocksdb::ReadOptions(), key, &unused);
+10
View File
@@ -49,6 +49,16 @@ public:
std::unique_ptr<CTxDBIteratorBase> NewIterator() const override;
// ─── Test-only friend accessor ──────────────────────────────────────────
// test_chaindb_runtime exercises the protected raw methods (ReadRaw /
// WriteRaw / EraseRaw / ExistsRaw) directly to verify the wrapper layer
// that the daemon uses at runtime when launched with -chaindb=rocksdb.
// We don't widen the public API just for the test — instead the test
// declares a ChainDbRuntimeTestAccessor struct that this class befriends,
// giving it the same access the class itself has. White-box test pattern,
// zero impact on production callers.
friend struct ChainDbRuntimeTestAccessor;
protected:
bool ReadRaw(const std::string& key, std::string& value) const override;
bool WriteRaw(const std::string& key, const std::string& value) override;