dbwrapper: guard `CDBBatch` scratch streams
What the developer wrote
Authored by Lőrinc
dbwrapper: guard `CDBBatch` scratch streams
`CDBBatch` already owns key and value serialization buffers, but manually reserved and cleared them on each call.
Reserve those scratch buffers once in the constructor, then guard `Write()` and `Erase()` use with `ScopedDataStreamUsage`, requiring empty streams on entry and clearing them on every exit path.
`WriteImpl()` and `EraseImpl()` pass the bytes immediately to LevelDB's write batch, which copies them and does not retain pointers.
`Clear()` now asserts the guards left both scratch streams clean.
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
What changed, and why it matters
This change is a small internal cleanup in how Bitcoin Core prepares database writes. It reuses temporary memory buffers instead of repeatedly reserving and clearing them, and adds a helper to make sure those buffers are empty before and after each use. There is no direct evidence this fixes an active security bug; it is best described as defensive hardening or code-quality improvement.
Treat as routine code-quality / defensive-hardening. No urgent action required. If reviewing for security, verify that ScopedDataStreamUsage correctly clears streams on all exception paths and that the assertions are not compiled out in release builds used for consensus-critical paths.
Security signals we found
Defensive invariant enforcement via ScopedDataStreamUsage RAII guard
Assertions added for empty scratch streams on entry/exit and in Clear()
No change to cryptographic obfuscation, serialization format, or LevelDB trust boundary
No mention of vulnerability, CVE, bug, exploit, or security fix in commit message
Evidence from the diff
The patch refactors CDBBatch’s temporary DataStream members (renamed ssKey/ssValue to m_key_scratch/m_value_scratch). It moves buffer reservation into the constructor, introduces a ScopedDataStreamUsage RAII guard that asserts empty-on-entry and clears-on-exit, and adds asserts in Clear() that the scratch streams are empty. The actual key/value bytes are still immediately copied by LevelDB’s write batch, so lifetime concerns are unchanged. The change reduces repeated allocations and makes stream-state invariants explicit.
Changed components
src/dbwrapper.cppsrc/dbwrapper.hCDBBatch classInspect captured patch +17 / −17
diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp
index 3c574f89..5bfdc75e 100644
--- a/src/dbwrapper.cpp
+++ b/src/dbwrapper.cpp
@@ -163,6 +163,8 @@ CDBBatch::CDBBatch(const CDBWrapper& _parent)
: parent{_parent},
m_impl_batch{std::make_unique<CDBBatch::WriteBatchImpl>()}
{
+ m_key_scratch.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
+ m_value_scratch.reserve(DBWRAPPER_PREALLOC_VALUE_SIZE);
Clear();
};
@@ -171,13 +173,15 @@ CDBBatch::~CDBBatch() = default;
void CDBBatch::Clear()
{
m_impl_batch->batch.Clear();
+ assert(m_key_scratch.empty());
+ assert(m_value_scratch.empty());
}
-void CDBBatch::WriteImpl(std::span<const std::byte> key, DataStream& ssValue)
+void CDBBatch::WriteImpl(std::span<const std::byte> key, DataStream& value)
{
leveldb::Slice slKey(CharCast(key.data()), key.size());
- dbwrapper_private::GetObfuscation(parent)(ssValue);
- leveldb::Slice slValue(CharCast(ssValue.data()), ssValue.size());
+ dbwrapper_private::GetObfuscation(parent)(value);
+ leveldb::Slice slValue(CharCast(value.data()), value.size());
m_impl_batch->batch.Put(slKey, slValue);
}
diff --git a/src/dbwrapper.h b/src/dbwrapper.h
index 4710af16..6957f327 100644
--- a/src/dbwrapper.h
+++ b/src/dbwrapper.h
@@ -79,10 +79,10 @@ private:
struct WriteBatchImpl;
const std::unique_ptr<WriteBatchImpl> m_impl_batch;
- DataStream ssKey{};
- DataStream ssValue{};
+ DataStream m_key_scratch{};
+ DataStream m_value_scratch{};
- void WriteImpl(std::span<const std::byte> key, DataStream& ssValue);
+ void WriteImpl(std::span<const std::byte> key, DataStream& value);
void EraseImpl(std::span<const std::byte> key);
public:
@@ -96,22 +96,18 @@ public:
template <typename K, typename V>
void Write(const K& key, const V& value)
{
- ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
- ssValue.reserve(DBWRAPPER_PREALLOC_VALUE_SIZE);
- ssKey << key;
- ssValue << value;
- WriteImpl(ssKey, ssValue);
- ssKey.clear();
- ssValue.clear();
+ ScopedDataStreamUsage scoped_key{m_key_scratch}, scoped_value{m_value_scratch};
+ m_key_scratch << key;
+ m_value_scratch << value;
+ WriteImpl(m_key_scratch, m_value_scratch);
}
template <typename K>
void Erase(const K& key)
{
- ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
- ssKey << key;
- EraseImpl(ssKey);
- ssKey.clear();
+ ScopedDataStreamUsage scoped_key{m_key_scratch};
+ m_key_scratch << key;
+ EraseImpl(m_key_scratch);
}
size_t ApproximateSize() const;
Why this scored 16/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.