[net processing] Annotate m_recently_announced_{invs|proofs} as guarded by g_msgproc_mutex and move to Peer
ClosedPublic
Actions

Authored by PiRK on Jan 26 2024, 10:22.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

Summary

Test Plan

With Debug

ninja all check-all

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 26670
Build 52908: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 52907: arc lint + arc unit

PiRK created this revision.Jan 26 2024, 10:22

PiRK requested review of this revision.Jan 26 2024, 10:22

Fabien requested changes to this revision.Jan 26 2024, 15:32

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/net_processing.cpp
514	We could probably use the m_recently_announced_invs for proofs rather than duplicating
3235	Do you still need cs_main ?
4662	Do you still need cs_main ?
7871	Are you holding NetEventsInterface::g_msgproc_mutex here ?
7980	Same question

This revision now requires changes to proceed.Jan 26 2024, 15:32

remove unneeded cs_main lock

src/net_processing.cpp
514	There are some differences, I think the TxRelay is not necessarily initialized if a node does not relay transactions whereas the ProofRelay is not necessarily initialized if avalanche is not enabled. Those are just memory optimizations, maybe we could just always initialize those structures are little cost. I will try to guesstimate the footprint.
7871	Yes, see line 7510. bool PeerManagerImpl::SendMessages(const Config &config, CNode *pto) { AssertLockHeld(g_msgproc_mutex); ...
7980	yes, we are still in `PeerManagerImpl::SendMessages`