The next step of changes from #10785.
This one adds:
VectorUsing logic in the vector/prevector serializers themselves (avoiding code duplication).
24@@ -25,6 +25,9 @@
25
26 static const unsigned int MAX_SIZE = 0x02000000;
27
28+/** Maximum amount of memory to allocate at once when deserializing vectors. */
722@@ -673,6 +723,16 @@ inline void Unserialize(Stream& is, T&& a)
723 a.Unserialize(is);
724 }
725
726+/** Identity formatter. Serializes objects as themselves. */
629+ size_t allocated = 0;
630+ while (allocated < size) {
631+ // For DoS prevention, do not blindly allocate as much as the stream claims to contain.
632+ // Instead, allocate in 5MiB batches, so that an attacker actually needs to provide
633+ // X MiB of data to make us allocate X+5 Mib.
634+ allocated = std::min(size, allocated + MAX_VECTOR_ALLOCATE / sizeof(typename V::value_type));
sizeof(typename V::value_type) <= MAX_VECTOR_ALLOCATE could catch an edge case here.
635+ v.reserve(allocated);
636+ while (v.size() < allocated) {
637+ typename V::value_type val;
638+ auto wrapped = Using<Formatter>(val);
639+ s >> wrapped;
640+ v.push_back(std::move(val));
In commit “Add custom vector-element formatter” (741e27bfe5eb6bdc61f99b300c16f4b2f92e8ec7)
Could avoid creating the temporary:
0v.emplace_back();
1s >> Using<Formatter>(v.back());
98- // TODO: avoid reimplementing vector deserializer
99- uint64_t count = 0;
100- ::Unserialize(s, COMPACTSIZE(count));
101- if (count > MAX_INPUTS_PER_BLOCK) {
102- throw std::ios_base::failure("Too many input undo records");
103- }
In commit “Convert undo.h to new serialization framework” (24235e50e9d532cbe5836dcebbce0c6a04be8652)
Note: With this check going away, there doesn’t appear to be any limit on the total size of the deserialized vprevout vector. The incremental allocation size is now limited to MAX_VECTOR_ALLOCATE, though.
608+ *
609+ * V is not required to be an std::vector type. It works for any class that
610+ * exposes a value_type, size, reserve, push_back, and const iterators.
611+ */
612+template<class Formatter>
613+struct VectorUsing
I don’t think the VectorUsing and UsingSelf names for formatters are the best. Would suggest sticking to the FooFormatter convention and renaming VectorUsing to VectorFormatter and UsingSelf to DefaultFormatter.
Aside from giving more consistent class names, it should also make usages more readable: Using<VectorUsing<UsingSelf>>(v) -> Using<VectorFormatter<DefaultFormatter>>(v)
This allows a very compact notation for serialization of vectors whose
elements are not serialized using their default encoding.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
628+ size_t size = ReadCompactSize(s);
629+ size_t allocated = 0;
630+ while (allocated < size) {
631+ // For DoS prevention, do not blindly allocate as much as the stream claims to contain.
632+ // Instead, allocate in 5MiB batches, so that an attacker actually needs to provide
633+ // X MiB of data to make us allocate X+5 Mib.
MAX_VECTOR_ALLOCATE changes, perhaps replace “5 MiB” and “5 Mib” with “MAX_VECTOR_ALLOCATE” (and s/X Mib/X/ as the constant is defined in bytes).
630+ while (allocated < size) {
631+ // For DoS prevention, do not blindly allocate as much as the stream claims to contain.
632+ // Instead, allocate in 5MiB batches, so that an attacker actually needs to provide
633+ // X MiB of data to make us allocate X+5 Mib.
634+ static_assert(sizeof(typename V::value_type) <= MAX_VECTOR_ALLOCATE, "Vector element size too large");
635+ allocated = std::min(size, allocated + MAX_VECTOR_ALLOCATE / sizeof(typename V::value_type));
sizeof(typename V::value_type) once and cache it.
static_assert requires a bool constexpr, otherwise a compile-time error is issued.
Qualified ACK 3c94b0039d2c
Code review, built/ran tests/ran bitcoind. I am not yet highly competent wrt C++ templates but this looks like a worthwhile refactoring. Would review again. Some thoughts below, feel free to ignore.
