p2p: comment out unused MSG_FILTERED_WITNESS_BLOCK #19721

pull adamjonas wants to merge 1 commits into bitcoin:master from adamjonas:2020_8_MSG_FILTERED_WITNESS_BLOCK_comment changing 1 files +3 −1

adamjonas commented at 2:02 PM on August 14, 2020: member

Commenting out and adding a note to unused MSG_FILTERED_WITNESS_BLOCK defined in BIP144.

There was an attempt to make use of this in #10350, but it was closed due to lack of support. (h/t sdaftuar for pointing to the PR and jnewbery for the idea)
doc: comment out and add annotation to unused MSG_FILTERED_WITNESS_BLOCK 4792cad88c
fanquake added the label Docs on Aug 14, 2020
fanquake removed the label Docs on Aug 14, 2020
DrahtBot added the label Docs on Aug 14, 2020
DrahtBot added the label P2P on Aug 14, 2020

practicalswift commented at 3:08 PM on August 14, 2020: contributor

ACK 4792cad88c5c3c93e639a051df779230ee817396

$ git grep MSG_FILTERED_WITNESS_BLOCK
src/protocol.h:    MSG_FILTERED_WITNESS_BLOCK = MSG_FILTERED_BLOCK | MSG_WITNESS_FLAG,

jonatack commented at 3:22 PM on August 14, 2020: member

ACK, this change was suggested in #19610 (review)

in src/protocol.h:402 in 4792cad88c

 396 | @@ -397,7 +397,9 @@ enum GetDataMsg : uint32_t {
 397 |      MSG_CMPCT_BLOCK = 4,                              //!< Defined in BIP152
 398 |      MSG_WITNESS_BLOCK = MSG_BLOCK | MSG_WITNESS_FLAG, //!< Defined in BIP144
 399 |      MSG_WITNESS_TX = MSG_TX | MSG_WITNESS_FLAG,       //!< Defined in BIP144
 400 | -    MSG_FILTERED_WITNESS_BLOCK = MSG_FILTERED_BLOCK | MSG_WITNESS_FLAG,
 401 | +    // MSG_FILTERED_WITNESS_BLOCK is defined in BIP144 as reserved for future
 402 | +    // use and remains unused.
 403 | +    // MSG_FILTERED_WITNESS_BLOCK = MSG_FILTERED_BLOCK | MSG_WITNESS_FLAG,

luke-jr commented at 6:13 PM on August 14, 2020:

It's supported by Bitcoin Knots, so I'm not sure "unused" is clear enough here... Maybe "remains unsupported by Bitcoin Core"?

jonatack commented at 10:07 AM on August 15, 2020:

I'll need to rebase #19610 after this change so can touch it up there if needed.

luke-jr changes_requested
fanquake removed the label Docs on Aug 15, 2020
fanquake requested review from jnewbery on Aug 15, 2020
jnewbery commented at 9:50 AM on August 15, 2020: member

Obvious ACK 4792cad88c5c3c93e639a051df779230ee817396

Thanks @adamjonas!
jnewbery approved
MarcoFalke commented at 10:04 AM on August 15, 2020: member

cr ACK 4792cad88c5c3c93e639a051df779230ee817396 good to keep it around in a comment to avoid accidental future re-assignment
MarcoFalke added the label Docs on Aug 15, 2020
fanquake removed the label Docs on Aug 15, 2020
fanquake commented at 10:08 AM on August 15, 2020: member

This is not a documentation change. It's changing code.
MarcoFalke renamed this:
~~doc: comment out unused MSG_FILTERED_WITNESS_BLOCK~~
p2p: comment out unused MSG_FILTERED_WITNESS_BLOCK
on Aug 15, 2020
MarcoFalke commented at 10:16 AM on August 15, 2020: member

This is not a documentation change. It's changing code.

Ok, adjusted title
MarcoFalke added the label Refactoring on Aug 15, 2020
theStack approved
theStack commented at 11:26 AM on August 15, 2020: member

ACK https://github.com/bitcoin/bitcoin/pull/19721/commits/4792cad88c5c3c93e639a051df779230ee817396 📜
fanquake merged this on Aug 18, 2020
fanquake closed this on Aug 18, 2020
sidhujag referenced this in commit 61201a625c on Aug 18, 2020
adamjonas deleted the branch on Aug 25, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#10350 Added support for MSG_FILTERED_WITNESS_BLOCK messages.