[Core] Bitcoin 0.12-0.14 serialization improvements#1633
Merged
furszy merged 14 commits intoPIVX-Project:masterfrom May 29, 2020
Merged
[Core] Bitcoin 0.12-0.14 serialization improvements#1633furszy merged 14 commits intoPIVX-Project:masterfrom
furszy merged 14 commits intoPIVX-Project:masterfrom
Conversation
These changes decode valid SIGHASH types on signatures in assembly (asm) representations of scriptSig scripts. This squashed commit incorporates substantial helpful feedback from jtimon, laanwj, and sipa. backports bitcoin/bitcoin@af3208b
backports bitcoin/bitcoin@90604f1 PIVX: serialization not needed, only named constants
CDataStream and CAutoFile had a ReadVersion and WriteVersion method that was never used. Remove them. backports bitcoin/bitcoin@50e8a9c
The stream implementations had two cascading layers (the upper one with operator<< and operator>>, and a lower one with read and write). The lower layer's functions are never cascaded (nor should they, as they should only be used from the higher layer), so make them return void instead. backports bitcoin/bitcoin@c2c5d42
Make the various stream implementations' nType and nVersion private and const (except in CDataStream where we really need a setter). backports bitcoin/bitcoin@fad9b66
Given that in default GetSerializeSize implementations created by ADD_SERIALIZE_METHODS we're already using CSizeComputer(), get rid of the specialized GetSerializeSize methods everywhere, and just use CSizeComputer. This removes a lot of code which isn't actually used anywhere. For CCompactSize and CVarInt this actually removes a more efficient size computing algorithm, which is brought back in a later commit. backports bitcoin/bitcoin@657e05a
Remove the nType and nVersion as parameters to all serialization methods and functions. There is only one place where it's read and has an impact (in CAddress), and even there it does not impact any of the recursively invoked serializers. Instead, the few places that need nType or nVersion are changed to read it directly from the stream object, through GetType() and GetVersion() methods which are added to all stream classes. backports bitcoin/bitcoin@5284721
The CSerAction's ForRead() method does not depend on any runtime data, so guarantee that requests to it can be optimized out by making it constexpr. Suggested by Cory Fields. backports bitcoin/bitcoin@a2929a2
To get the advantages of faster GetSerializeSize() implementations back that were removed in "Make GetSerializeSize a wrapper on top of CSizeComputer", reintroduce them in the few places in the form of a specialized Serialize() implementation. This actually gets us in a better state than before, as these even get used when they're invoked indirectly in the serialization of another object. backports bitcoin/bitcoin@25a211a
Dbwrapper used GetSerializeSize() to compute the size of the buffer to preallocate. For some cases (specifically: CCoins) this requires a costly compression call. Avoid this by just using fixed size preallocations instead. backports bitcoin/bitcoin@d59a518
Suggested by Pavel Janik. backports bitcoin/bitcoin@a603925
4aa1293 to
249cc9d
Compare
Author
|
Rebased after #1629 merge. Ready for review/QA. |
ambassador000
approved these changes
May 27, 2020
ambassador000
left a comment
There was a problem hiding this comment.
Functionality tested, working as intended.
Fuzzbawls
approved these changes
May 29, 2020
furszy
approved these changes
May 29, 2020
random-zebra
added a commit
to random-zebra/PIVX
that referenced
this pull request
Jun 24, 2020
Document the changes introduced in: - PIVX-Project#1549: Nuke zPIV from the GUI - PIVX-Project#1586: Minimum value for stake split threshold - PIVX-Project#1633: Bitcoin 0.12-0.14 serialization improvements - PIVX-Project#1645: Implement accurate memory accounting for mempool - PIVX-Project#1647: MemPool package tracking and limits - PIVX-Project#1650: Benchmarking Framework - PIVX-Project#1688: TopBar navigation (sync/peers)
random-zebra
added a commit
that referenced
this pull request
Jun 29, 2020
4819ee7 [Doc] Add/Update some release notes for 4.2 (random-zebra) Pull request description: Document the changes introduced in: - #1549: Nuke zPIV from the GUI - #1586: Minimum value for stake split threshold - #1633: Bitcoin 0.12-0.14 serialization improvements - #1645: Implement accurate memory accounting for mempool - #1647: MemPool package tracking and limits - #1650: Benchmarking Framework - #1688: TopBar navigation (sync/peers) ACKs for top commit: furszy: utACK 4819ee7 Fuzzbawls: ACK 4819ee7 Tree-SHA512: 62ad949ea26a2f877ef0b40ec86616cc8105f81e1fcd380c8162cd93af04a46f1093f878c0668408654f198a0059b240798b83af3bf1d5e6c1c1d8611276a325
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
-Based on top of
Backports the following serialization improvements from upstream and adds the required changes for the 2nd layer network and the legacy zerocoin code.
show scriptSig signature hash types in transaction decodes. fixes #3166 bitcoin/bitcoin#5264
add bip32 pub key serialization bitcoin/bitcoin#6215
Compact Blocks bitcoin/bitcoin#8068 (only commit 5249dac)
This adds COMPACTSIZE wrapper similar to VARINT for serialization
Remove unused statements in serialization bitcoin/bitcoin#8658
Various serialization simplifcations and optimizations bitcoin/bitcoin#9039