refactoring: [Net Processing] Follow-ups to #21160#24692
refactoring: [Net Processing] Follow-ups to #21160#24692fanquake merged 3 commits intobitcoin:masterfrom
Conversation
CNode::m_relays_tx and CNode::m_bloom_filter_loaded access don't require the Peer::TxRelay::m_bloom_filter_mutex lock, so move them out of the lock scope. See bitcoin#21160 (comment) and bitcoin#21160 (comment).
|
cc @dergoegge |
dergoegge
left a comment
dergoegge
left a comment
There was a problem hiding this comment.
The tests are failing because the wrong value is being passed to PushNodeVersion for the block_relay_only param.
jnewbery
force-pushed
the
202202_21160_followups
branch
from
2432ec4 to
0807116
Compare
Oops. Now fixed. Thank you! |
|
See also suggestion in #21160 (comment) |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
The peer object is not mutated by PushNodeVersion, so pass a const reference
Added. I've removed the net_processing: MaybeSendFeeFilter() takes a Peer::TxFilter* and net_processing: PushNodeVersion() takes a bool instead of a Peer& commits. There doesn't seem to be agreement on what should be passed down to these functions, and in the absence of agreement, continuing to pass just a reference to the |
|
I think you forgot to force push your local branch? |
jnewbery
force-pushed
the
202202_21160_followups
branch
from
0807116 to
a40978d
Compare
maflcko
changed the title
|
review ACK a40978d |
|
ACK a40978d |
|
ACK a40978d |
…oin#21160 a40978d [fuzz] Assert that Peer.m_tx_relay.m_relay_txs has been set correctly (John Newbery) 0bca5f2 [net processing] PushNodeVersion() takes a const Peer& (John Newbery) 21154ff net_processing: move CNode data access out of lock (John Newbery) Pull request description: bitcoin#21160 ([net/net processing]: Move tx inventory into net_processing) had some unaddressed review comments when it was merged. This branch addresses those comments. ACKs for top commit: MarcoFalke: review ACK a40978d dergoegge: ACK a40978d ajtowns: ACK a40978d Tree-SHA512: 46624e275f918c5f32d0adab0766e9b3ef8ebdbc74a3c8886d8a2e2ff1079029dcc371b40ef0d787609e9c05219b7456f3e2dfe4fb0cb7bf23ef966769aef1a1
6 participants
#21160 ([net/net processing]: Move tx inventory into net_processing) had some unaddressed review comments when it was merged. This branch addresses those comments.