doc: clarify processing of mempool-msgs when NODE_BLOOM #27559

pull 0xB10C wants to merge 1 commits into bitcoin:master from 0xB10C:2023-05-clarify-mempool-bloom changing 1 files +4 −1

0xB10C commented at 10:48 PM on May 2, 2023: contributor

Under which circumstances we process received 'mempool' P2P messages caused confusion in #27426. Rather than bike-shedding the formulation of the IF-statement, this adds a comment clarifying when we process the message. Also, correcting the m_send_mempool description.

clarify processing of mempool-msgs when NODE_BLOOM

Under which circumstances we process received 'mempool' P2P messages
caused confusion in #27426. Rather than bikeshedding the formulation
of the IF-statement, this adds a comment clarifing when we process
the message. Also, correcting the comment of `m_send_mempool`.

Co-authored-by: willcl-ark <will8clark@gmail.com>

4581a682d2

DrahtBot commented at 10:48 PM on May 2, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK dergoegge, willcl-ark, glozow

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Docs on May 2, 2023
dergoegge commented at 9:38 AM on May 3, 2023: member

ACK 4581a682d2d1fdd0e56fb4a56e6228be878a04a3
willcl-ark commented at 10:13 AM on May 3, 2023: member

ACK 4581a682d2
fanquake commented at 10:33 AM on May 3, 2023: member

cc also @jnewbery @mzumsande @sdaftuar
glozow commented at 11:54 AM on May 3, 2023: member

ACK 4581a682d2d1fdd0e56fb4a56e6228be878a04a3
glozow merged this on May 3, 2023
glozow closed this on May 3, 2023
mzumsande commented at 12:21 PM on May 3, 2023: contributor

Post-merge ACK 4581a682d2d1fdd0e56fb4a56e6228be878a04a3
0xB10C deleted the branch on May 3, 2023
jnewbery commented at 12:46 PM on May 3, 2023: contributor

ACK 4581a682d2d1fdd0e56fb4a56e6228be878a04a3
sidhujag referenced this in commit 79447ce778 on May 4, 2023

Type	Reviewers
ACK	dergoegge, willcl-ark, glozow

in src/net_processing.cpp:296 in 4581a682d2

 291 | @@ -292,7 +292,8 @@ struct Peer {
 292 |           *  this does not have to be sorted. */
 293 |          std::set<uint256> m_tx_inventory_to_send GUARDED_BY(m_tx_inventory_mutex);
 294 |          /** Whether the peer has requested us to send our complete mempool. Only
 295 | -         *  permitted if the peer has NetPermissionFlags::Mempool. See BIP35. */
 296 | +         *  permitted if the peer has NetPermissionFlags::Mempool or we advertise
 297 | +         *  NODE_BLOOM. See BIP35. */

maflcko commented at 9:36 AM on May 10, 2023:

I think it makes sense to also document this in src/protocol.h, which generally refers to the p2p version and the BIP number. I read BIP 35, 37, and 111, but I couldn't find a mention that the message type is guarded by NODE_BLOOM. So maybe this can also be mentioned in src/protocol.h?