net processing: #26140 follow-ups #27379

pull dergoegge wants to merge 2 commits into bitcoin:master from dergoegge:2023-03-26140-followup changing 1 files +13 −20

dergoegge commented at 11:21 AM on March 31, 2023: member
Addresses left over feedback from #26140.
- #26140 (review)
- #26140 (review)
mapRelay is only accessed from the message processing thread and does not need to be kept in sync with anything validation specific, it is therfore perfectly fine to have it guarded by g_msgproc_mutex.

[net processin] Don't take cs_main in FindTxForGetData

Taking cs_main is no longer necessary since we moved
`m_recently_announced_invs` to `Peer` and `mapRelay` is actually only
accessed from the message processing thread.

c85ee76a36

[net processing] Pass TxRelay to FindTxForGetData instead of Peer 3fa4c54ac5
fanquake requested review from jnewbery on Mar 31, 2023
fanquake renamed this:
~~#26140 follow-ups~~
net: #26140 follow-ups
on Mar 31, 2023
fanquake added the label P2P on Mar 31, 2023
dergoegge renamed this:
~~net: #26140 follow-ups~~
net processing: #26140 follow-ups
on Mar 31, 2023

in src/net_processing.cpp:919 in 3fa4c54ac5

 915 | @@ -920,9 +916,9 @@ class PeerManagerImpl final : public PeerManager
 916 |  
 917 |      /** Relay map (txid or wtxid -> CTransactionRef) */
 918 |      typedef std::map<uint256, CTransactionRef> MapRelay;
 919 | -    MapRelay mapRelay GUARDED_BY(cs_main);
 920 | +    MapRelay mapRelay GUARDED_BY(NetEventsInterface::g_msgproc_mutex);

hebasto commented at 12:22 PM on March 31, 2023:

Mention this in the PR description?

dergoegge commented at 12:27 PM on March 31, 2023:

Done

DrahtBot commented at 12:22 PM on March 31, 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 jnewbery, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
maflcko removed the label P2P on Mar 31, 2023
DrahtBot added the label P2P on Mar 31, 2023
dergoegge commented at 12:45 PM on March 31, 2023: member

CI failure is intermittent, see #27380
jnewbery commented at 4:05 PM on March 31, 2023: contributor

utACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3
DrahtBot removed review request from jnewbery on Mar 31, 2023
hebasto commented at 6:27 PM on March 31, 2023: member

Concept ACK.

Type	Reviewers
ACK	jnewbery, hebasto

in src/net_processing.cpp:323 in 3fa4c54ac5

 316 | @@ -317,10 +317,6 @@ struct Peer {
 317 |      {
 318 |          return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get());
 319 |      };
 320 | -    const TxRelay* GetTxRelay() const EXCLUSIVE_LOCKS_REQUIRED(!m_tx_relay_mutex)
 321 | -    {
 322 | -        return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get());
 323 | -    };

hebasto commented at 6:32 PM on March 31, 2023:

Nice!

hebasto approved
hebasto commented at 6:44 PM on March 31, 2023: member

ACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3, I have reviewed the code and it looks OK, I agree it can be merged.
fanquake merged this on Apr 2, 2023
fanquake closed this on Apr 2, 2023
sidhujag referenced this in commit ba4a379d50 on Apr 2, 2023
bitcoin locked this on Apr 1, 2024

Contributors

Labels

Linked (view graph)

#26140 refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer

net processing: #26140 follow-ups #27379

Reviews