-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create CompactSize64 for non-message-length fields #3008
Merged
Merged
+461
−59
Conversation
This file contains 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
3d1cf88
to
f398d4f
Compare
jvff
reviewed
Nov 3, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! I added a few nit-picks, but they are all optional.
But don't remove read_compactsize and write_compactsize yet.
```sh fastmod compactSize CompactSize zebra* book fastmod compactsize CompactSize zebra* book ```
f398d4f
to
1fb3d2d
Compare
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2 tasks
jvff
approved these changes
Nov 4, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-consensus
Area: Consensus rule updates
A-network
Area: Network protocol updates or fixes
A-rust
Area: Updates to Rust code
C-bug
Category: This is a bug
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.
Motivation
To implement addrv2 in #2681, we need a
CompactSize
type that can be larger thanMAX_PROTOCOL_MESSAGE_LEN
.This is the first part of that change, which blocks ticket #2681.
Specifications
https://developer.bitcoin.org/reference/transactions.html#compactsize-unsigned-integers
Designs
We're switching from serialization methods to serialized types.
This makes sure we don't confuse
compactSize
fields with different limits.Solution
CompactSize64
type for non-message-length fieldsCompactSizeMessage
type for message length fieldsCompactSizeMessage
instead ofread_compactsize
andwrite_compactsize
inzebra_chain::serialization
First part of #2173.
Review
@jvff can review the rest of this PR.
This PR is blocking #2681, so I'd like to get it merged this week.
Reviewer Checklist
Follow Up Work
CompactSizeMessage
instead ofread_compactsize
andwrite_compactsize
addrv2
in Zebra #2681