dbwrapper: reuse iterator scratch stream
What the developer wrote
Authored by Lőrinc
dbwrapper: reuse iterator scratch stream
`CDBIterator::Seek()` and `CDBIterator::GetValue()` only need a temporary owning buffer for immediate use.
Keep one preallocated `DataStream` scratch member on the non-copyable iterator and guard each use with `ScopedDataStreamUsage`.
`SeekImpl()` consumes the serialized key during the immediate LevelDB seek, and `GetValue()` still copies LevelDB value bytes into the owning scratch stream before in-place deobfuscation.
A single stream is enough because these methods cannot overlap on the same iterator without re-entering or concurrently using it, which the guard asserts against.
The preceding test covers repeated seeks and failed value deserialization followed by a successful read from the same entry.
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
What changed, and why it matters
This is a small internal performance cleanup in Bitcoin Core's database iterator code. It replaces repeatedly created temporary data buffers with a single reusable scratch buffer stored on the iterator object. There is no indication this fixes a security bug; it appears aimed at reducing memory allocations and object churn.
No security action required. Treat as a routine refactor/optimization. Standard code review and regression testing are sufficient.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit modifies CDBIterator so that Seek() and GetValue() use a persistent DataStream member (m_scratch) instead of constructing a fresh DataStream each call. A ScopedDataStreamUsage guard resets the stream between uses and asserts against overlapping use. SeekImpl consumes the serialized key immediately, and GetValue still copies LevelDB bytes into the scratch stream before deobfuscation and deserialization. The change is non-copy-safe but CDBIterator is already non-copyable. The preceding test exercises repeated seeks and failed deserialization followed by successful reads.
Changed components
src/dbwrapper.cppsrc/dbwrapper.hCDBIteratorInspect captured patch +12 / −8
diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp
index 5bfdc75e..3743d434 100644
--- a/src/dbwrapper.cpp
+++ b/src/dbwrapper.cpp
@@ -360,7 +360,10 @@ struct CDBIterator::IteratorImpl {
};
CDBIterator::CDBIterator(const CDBWrapper& _parent, std::unique_ptr<IteratorImpl> _piter) : parent(_parent),
- m_impl_iter(std::move(_piter)) {}
+ m_impl_iter(std::move(_piter))
+{
+ m_scratch.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
+}
CDBIterator* CDBWrapper::NewIterator()
{
diff --git a/src/dbwrapper.h b/src/dbwrapper.h
index 6957f327..51775ee6 100644
--- a/src/dbwrapper.h
+++ b/src/dbwrapper.h
@@ -121,6 +121,7 @@ public:
private:
const CDBWrapper &parent;
const std::unique_ptr<IteratorImpl> m_impl_iter;
+ DataStream m_scratch{};
void SeekImpl(std::span<const std::byte> key);
std::span<const std::byte> GetKeyImpl() const;
@@ -140,10 +141,9 @@ public:
void SeekToFirst();
template<typename K> void Seek(const K& key) {
- DataStream ssKey{};
- ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
- ssKey << key;
- SeekImpl(ssKey);
+ ScopedDataStreamUsage scoped_scratch{m_scratch};
+ m_scratch << key;
+ SeekImpl(m_scratch);
}
void Next();
@@ -160,9 +160,10 @@ public:
template<typename V> bool GetValue(V& value) {
try {
- DataStream ssValue{GetValueImpl()};
- dbwrapper_private::GetObfuscation(parent)(ssValue);
- ssValue >> value;
+ ScopedDataStreamUsage scoped_scratch{m_scratch};
+ m_scratch.write(GetValueImpl());
+ dbwrapper_private::GetObfuscation(parent)(m_scratch);
+ m_scratch >> value;
} catch (const std::exception&) {
return false;
}
Why this scored 13/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.