backport: merge bitcoin#22778, #25156, #26497, #27213, #28189, #28155, #28895, partial bitcoin#26396 (networking backports: part 9)

[kwvg]

Additional Information

• Dependency for backport: merge bitcoin#23496, #26272, #23443, #26381, #26396, #26448, #26359, #26686, #23670, partial bitcoin#19953 (Erlay support) #6333

• When backporting bitcoin#22778, the m_tx_inventory_known_filter.insert() call in queueAndMaybePushInv() had to be moved out as EXCLUSIVE_LOCKS_REQUIRED does not seem to work with lambda parameters list (though it does work with capture list, source), see error below

  Compiler error:

  net_processing.cpp:5895:21: note: found near match 'tx_relay->m_tx_inventory_mutex'
  net_processing.cpp:5955:21: error: calling function 'operator()' requires holding mutex 'queueAndMaybePushInv.m_tx_inventory_mutex' exclusively [-Werror,-Wthread-safety-precise]
                      queueAndMaybePushInv(tx_relay, CInv(nInvType, hash));
                      ^
  net_processing.cpp:5955:21: note: found near match 'tx_relay->m_tx_inventory_mutex'
  net_processing.cpp:5977:17: error: calling function 'operator()' requires holding mutex 'queueAndMaybePushInv.m_tx_inventory_mutex' exclusively [-Werror,-Wthread-safety-precise]
                  queueAndMaybePushInv(inv_relay, inv);
                  ^
  

  • Attempting to remove the EXCLUSIVE_LOCKS_REQUIRED or the AssertLockHeld are not options due to stricter thread sanitization checks being applied since dash#6319 (and in general, removing annotations being inadvisable regardless)

  • We cannot simply lock peer->GetInvRelay()->m_tx_inventory_mutex as a) the caller already has a copy of the relay pointer (which means that m_tx_relay_mutex was already held) that b) they used to lock m_tx_inventory_mutex (which we were already asserting) so c) we cannot simply re-lock m_tx_inventory_mutex as it's already already held, just through a less circuitous path.

    • The reason locking is mentioned instead of asserting is that the compiler treats peer->GetInvRelay()->m_tx_inventory_mutex and tx_relay->m_tx_inventory_mutex as separate locks (despite being different ways of accessing the same thing) and would complain similarly to the error above if attempting to assert the former while holding the latter.

• As m_tx_relay is always initialized for Dash, to mimic the behaviour expected upstream, in bitcoin#22778, SetTxRelay() will allow GetTxRelay() to return m_can_tx_relay (while Dash code that demands unconditional access can use GetInvRelay() instead) with the lack of SetTxRelay() resulting in GetTxRelay() feigning the absence of m_can_tx_relay.

  This allows us to retain the same style of checks used upstream instead of using proxies like !CNode::IsBlockOnlyConn() to determined if we should use m_tx_relay.

• Speaking of proxies, being a block-only connection is now no longer the only reason not to relay transactions

  In preparation for bitcoin#26396, proxy usage of !CNode::IsBlockOnlyConn() and !Peer::m_block_relay_only was replaced with the more explicit Peer::m_can_tx_relay before being used to gate the result of GetTxRelay(), to help it mimic upstream semantics.

Breaking Changes

None expected

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas (note: N/A)
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[UdjinM6]

utACK 09504bd