[Backport] Serialization updates #2256
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We don't normally use ReadVarInt from untrusted inputs, but we might see this in the case of corruption. This is exposed in test_bitcoin_fuzzy.
Missing back port from btc@8c1dbc5e9ddbafb77e60e8c4e6eb275a3a76ac12
Currently, the READWRITE macro cannot be passed any non-const temporaries, as the SerReadWrite function only accepts lvalue references. Deserializing into a temporary is very common, however. See for example things like 's >> VARINT(n)'. The VARINT macro produces a temporary wrapper that holds a reference to n. Fix this by accepting non-const rvalue references instead of lvalue references. We don't propagate the rvalue-ness down, as there are no useful optimizations that only apply to temporaries. Then use this new functionality to get rid of many (but not all) uses of the 'REF' macro (which casts away constness).
Using VARINT with signed types is dangerous because negative values will appear to serialize correctly, but then deserialize as positive values mod 128. This commit changes the VARINT macro to trigger an error by default if called with an signed value, and updates broken uses of VARINT to pass a special flag that lets them keep working with no change in behavior.
Switch to unsigned encoding, which is backwards compatible and avoids MSVC error reported bitcoin#12732
This adds a READWRITEAS(type, obj) macro which serializes obj as if it were casted to (const type&) when const, and to (type&) when non-const. This makes it usable in serialization code that uses a single implementation for both serialization and deserializing, which doesn't know the constness of the object involved.
furszy
force-pushed
the
2021_serialization_updates
branch
from
ebdcef7 to
172fe15
Compare
Fuzzbawls
approved these changes
Apr 8, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Back ported some pretty concise PRs from upstream over the data serialization area.
ReadVarInt.READWRITEASmacro.