refactor: Treat CDataStream bytes as uint8_t #20464

There is already a bool Rewind(size_type n), overloading it here is confusing. We should think of a new name. I had hoped to think of a way to use the existing method in the only place it is used, but it appears there is no way to access nReadPos (or even derive it) from the outside.

I think the name make sense. Rewind(size_t) rewinds only how much is passed. Rewind() rewinds all the way. Though, I am happy to rename this to anything else, if there are suggestions.

It's a pity that there is no defined invalid value for size_t, which could be passed as default value. Would Rewind(std::optional<size_t> n = std::nullopt) be a possibility here to unite the two methods? Like e.g.:

diff --git a/src/streams.h b/src/streams.h
index 2a921c2c5..a6e781b95 100644
--- a/src/streams.h
+++ b/src/streams.h
@@ -14,6 +14,7 @@
 #include <assert.h>
 #include <ios>
 #include <limits>
+#include <optional>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
@@ -238,11 +239,6 @@ public:
         ::SerializeMany(*this, std::forward<Args>(args)...);
     }

-    void Rewind()
-    {
-        nReadPos = 0;
-    }
-
     std::string str() const
     {
         return (std::string(begin(), end()));
@@ -339,12 +335,17 @@ public:
         nReadPos = 0;
     }

-    bool Rewind(size_type n)
+    bool Rewind(std::optional<size_type> n = std::nullopt)
     {
+        // Total rewind if no size is passed
+        if (!n) {
+            nReadPos = 0;
+            return true;
+        }
         // Rewind by n characters if the buffer hasn't been compacted yet
-        if (n > nReadPos)
+        if (*n > nReadPos)
             return false;
-        nReadPos -= n;
+        nReadPos -= *n;
         return true;
     }

Probably overkill for methods that are only ever called in the benchmarks though...

Thanks, fixed

I think the name make sense. Rewind(size_t) rewinds only how much is passed.

Right. My remark was triggered mostly by that the two implementations of rewind() were in different places in the method list and not together, the name makes some sense.

But I like the new combined implementation.