txdb: Cursor warmup accepts malformed first coin key
- Dominant language
- C++
- Stars
- 90.2k
- Forks
- 39.4k
- Avg merge
- 3d 8h
- Merged PRs (30d)
- 88
Description
### Is there an existing issue for this?
- [x] 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:
```cpp
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`):
```cpp
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:
```cpp
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 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:
```bash
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
```text
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:
```text
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_
Contributor guide
Research direction
Start in src/txdb.cpp at CCoinsViewDB::Cursor() and compare its warmup logic with CCoinsViewDBCursor::Next(). Add the malformed-first-key and positive-control cases in src/test/txdb_cursor_tests.cpp, register the test in src/test/CMakeLists.txt, then run the txdb_cursor_tests target. Done means the malformed cursor is invalid while the valid control remains valid.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100