Net processing: follow ups to #20799 (removing support for v1 compact blocks)#25147
Merged
Conversation
Contributor
Author
maflcko
approved these changes
May 17, 2022
maflcko
reviewed
May 17, 2022
Contributor
|
Concept ACK. |
All uses of CBlockHeaderAndShortTxIDs in the product code are constructed with fUseWTXID=true, so remove the parameter. There is one use of the CBlockHeaderAndShortTxIDs constructor with fUseWTXID=false in the unit tests. This is used to construct a CBlockHeaderAndShortTxIDs for a block with only the coinbase transaction, so setting fUseWTXID to true or false makes no difference. Suggested in bitcoin#20799 (review)
3ab9f69 to
97e4a5d
Compare
Contributor
Author
|
Thanks for the review @MarcoFalke and @naumenkogs. I've addressed all your comments. |
maflcko
approved these changes
May 17, 2022
97e4a5d to
8184dcb
Compare
Contributor
|
ACK 8184dcbdd18d8b732dc285a05884ecda115062e1 |
dergoegge
reviewed
May 18, 2022
The only place that segwit=True is for a block that contains only the coinbase transaction. Since the witness commitment is optional if none of the transactions have a witness, we can leave it out. This doesn't change the test coverage, which is testing p2p compact block logic. Suggested in bitcoin#20799 (comment)
8184dcb to
bf6526f
Compare
Member
|
Code review ACK bf6526f |
Contributor
|
ACK bf6526f |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 28, 2022
…oving support …
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 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.
This implements two of the suggestions from code reviews of PR 20799: