txdb: Cursor warmup accepts malformed first coin key #35172

Is there an existing issue for this?

• I have searched the existing issues

Current behaviour

CCoinsViewDB::Cursor() primes the first cached key without checking whether pcursor->GetKey(entry) succeeded. When the first record in the DB_COIN keyspace cannot be decoded as CoinEntry, entry.key stays at its default DB_COIN value from CoinEntry { uint8_t key{DB_COIN}; ... }, so the returned cursor reports Valid()==true and GetKey()==true.

At 859215218667ca9f35d5adae0289e4a125798087, src/txdb.cpp does this in the constructor warmup:

if (i->pcursor->Valid()) {
    CoinEntry entry(&i->keyTmp.second);
    i->pcursor->GetKey(entry);
    i->keyTmp.first = entry.key;
}

The sibling path in CCoinsViewDBCursor::Next() already handles the same failure correctly, as fixed in #7890 (a3310b4d48):

if (!pcursor->Valid() || !pcursor->GetKey(entry)) {
    keyTmp.first = 0;
} else {
    keyTmp.first = entry.key;
}

This requires an already malformed chainstate keyspace, such as manual DB modification or database corruption that leaves a readable LevelDB record. Cursor() is used by UTXO stats, snapshot export, scantxoutset, and a rollback-copy path, not by consensus or wallet code. With a malformed first key whose value still deserializes as Coin, those paths can silently process a bogus first entry instead of rejecting it.

Expected behaviour

Cursor() should mirror Next(): after Seek(DB_COIN), if !i->pcursor->Valid() or !i->pcursor->GetKey(entry), it should set i->keyTmp.first = 0; otherwise it should cache entry.key.

A malformed first DB_COIN record should leave the returned cursor invalid, so cursor->Valid() and cursor->GetKey(outpoint) both return false.

Steps to reproduce

Add a unit test that writes a malformed first DB_COIN key into a real CDBWrapper, reopens the same path via CCoinsViewDB, and checks the returned cursor. The key uint8_t{'C'} is enough to enter the coin keyspace but is too short to decode as CoinEntry.

Minimal test case:

BOOST_AUTO_TEST_CASE(malformed_first_coin_key_cursor_invalid)
{
    const fs::path path{m_args.GetDataDirBase() / "malformed_first_coin_key_cursor_invalid"};

    {
        CDBWrapper dbw({.path = path, .cache_bytes = 1_MiB, .wipe_data = true, .obfuscate = false});
        dbw.Write(uint8_t{'C'}, Coin{CTxOut{1, CScript{}}, 1, false});
    }

    CCoinsViewDB view({.path = path, .cache_bytes = 1_MiB, .wipe_data = false, .obfuscate = false}, {});
    std::unique_ptr<CCoinsViewCursor> cursor{view.Cursor()};
    BOOST_REQUIRE(cursor);

    COutPoint outpoint;
    BOOST_CHECK(!cursor->Valid());
    BOOST_CHECK(!cursor->GetKey(outpoint));
}

After registering the test in src/test/CMakeLists.txt, build and run:

cmake -B build -GNinja -DBUILD_TESTS=ON -DENABLE_WALLET=OFF -DWITH_MINIUPNPC=OFF -DWITH_ZMQ=OFF -DCMAKE_BUILD_TYPE=Debug
cmake --build build --target test_bitcoin -j$(sysctl -n hw.ncpu)
./build/bin/test_bitcoin --run_test=txdb_cursor_tests

The positive control valid_first_coin_key_cursor_valid, using a normal CCoinsViewCache::AddCoin() entry as the first key, passes cleanly.

Relevant log output

Running 2 test cases...
test/txdb_cursor_tests.cpp:42: error: in "txdb_cursor_tests/malformed_first_coin_key_cursor_invalid": check !cursor->Valid() has failed
test/txdb_cursor_tests.cpp:43: error: in "txdb_cursor_tests/malformed_first_coin_key_cursor_invalid": check !cursor->GetKey(outpoint) has failed

*** 2 failures are detected in the test module "Bitcoin Core Test Suite"

Positive control:

Running 1 test case...

*** No errors detected

How did you obtain Bitcoin Core

Compiled from source

What version of Bitcoin Core are you using?

master@859215218667ca9f35d5adae0289e4a125798087

Operating system and version

macOS 26.4.1

Machine specifications

No response

Not sure if this is an actual issue. If the chainstate is corrupted to the point that the condition described here is reached, I'm not sure if the check introduced here actually improves the situation for the running node. Commit 822755a424d0abfa408dc34313f4aca4b816f54f also explicitly works around the "no valid first entry" scenario.

Agreed that this is limited to an already malformed chainstate keyspace, and that it does not repair a corrupted DB or affect normal operation.

The case here is slightly different from 822755a424: that commit handles Seek(DB_COIN) leaving the iterator invalid. Here the iterator is valid, but GetKey(entry) fails while decoding the first CoinEntry. Since CoinEntry::key is initialized to DB_COIN, the cursor is left valid. Next() already invalidates the cursor on the same GetKey failure for later entries.

I tested the mirror-Next() change locally. It makes the malformed-first-key reproducer pass, and dbwrapper_tests and coins_tests still pass. So I agree the impact is low, but I still think this is a small consistency issue in corrupt-db handling rather than the same no-valid-first-entry case fixed by 822755a424.