Net processing: follow ups to #20799 (removing support for v1 compact blocks) #25147
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
maflcko
reviewed
May 17, 2022
|
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)
jnewbery
added a commit
to jnewbery/bitcoin
that referenced
this pull request
May 17, 2022
Suggested in bitcoin#25147 (comment) May make IBD minimally faster by avoiding the construction of CBlockHeaderAndShortTxIDs objects where unnecessary.
3ab9f69 to
97e4a5d
Compare
|
Thanks for the review @MarcoFalke and @naumenkogs. I've addressed all your comments. |
jnewbery
added a commit
to jnewbery/bitcoin
that referenced
this pull request
May 17, 2022
Suggested in bitcoin#25147 (comment) May make IBD minimally faster by avoiding the construction of CBlockHeaderAndShortTxIDs objects where unnecessary.
97e4a5d to
8184dcb
Compare
|
ACK 8184dcb |
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
|
Code review ACK bf6526f |
|
ACK bf6526f |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 28, 2022
…oving support …
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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: