p2p: standardize outbound full/block relay connection type naming

[jonatack]

We've accumulated many different ways of referring to outbound full relay and outbound block relay connection types in the codebase. In places, it is becoming less clear which type of connection is referred to. This PR proposes standardizing the naming to outbound-{full, block}-relay with a scripted diff as follows:

-BEGIN VERIFY SCRIPT-
s() { git grep -l "$1" src test/functional doc/files.md doc/reduce-*.md | xargs sed -i "s/$1/$2/g"; }

s 'ConnectionType::OUTBOUND '            'ConnectionType::OUTBOUND_FULL_RELAY '
s 'IsFullOutboundConn'                   'IsOutboundFullRelayConn'
s 'GetExtraFullOutboundCount'            'GetExtraOutboundFullRelayCount'
s 'full_outbound_peers'                  'outbound_full_relay_peers'
s 'GetTryNewOutboundPeer'                'GetTryNewOutboundFullRelayPeer'
s 'SetTryNewOutboundPeer'                'SetTryNewOutboundFullRelayPeer'
s 'm_try_another_outbound_peer'          'm_try_another_outbound_full_relay_peer'
s 'outbound (full-relay)'                'outbound-full-relay'
s 'outbound full-relay'                  'outbound-full-relay'
s 'outbound, full-relay'                 'outbound-full-relay'
s 'outbounds'                            'outbound-full-relay connections'
s 'f"outbound: '                         'f"outbound-full-relay: '
s ' full-relay'                          ' outbound-full-relay'
s 'to an extra outbound peer'            'to an extra outbound-full-relay peer'
s 'try another outbound peer'            'try another outbound-full-relay peer'
s '(tx, block, addr) outbound'           '(tx, block, addr)'
s 'we deal with extra outbound peers'    'we deal with extra outbound-full-relay peers'
s 'outbound peers we have in excess'     'outbound-full-relay peers we have in excess'
s 'some outbound connections are not'    'some outbound-full-relay connections are not'
s 'or this is a'                         'or if this is an'

s 'ConnectionType::BLOCK_RELAY'          'ConnectionType::OUTBOUND_BLOCK_RELAY'
s ' BLOCK_RELAY'                         ' OUTBOUND_BLOCK_RELAY'
s 'IsBlockOnlyConn'                      'IsOutboundBlockRelayConn'
s 'GetExtraBlockRelayCount'              'GetExtraOutboundBlockRelayCount'
s 'block_relay_peers'                    'outbound_block_relay_peers'
s 'MAX_BLOCK_RELAY_ONLY_ANCHORS'         'MAX_OUTBOUND_BLOCK_RELAY_ANCHORS'
s 'MAX_BLOCK_RELAY_ONLY_CONNECTIONS'     'MAX_OUTBOUND_BLOCK_RELAY_CONNECTIONS'
s 'EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL' 'EXTRA_OUTBOUND_BLOCK_RELAY_PEER_INTERVAL'
s 'm_start_extra_block_relay_peers'      'm_start_extra_outbound_block_relay_peers'
s 'outgoing block-relay-only'            'outbound-block-relay'
s 'outbound block-relay-only'            'outbound-block-relay'
s 'outbound block-relay'                 'outbound-block-relay'
s 'block-relay only outbound'            'outbound-block-relay'
s 'block-relay-only outgoing'            'outbound-block-relay'
s 'block-relay only peers'               'outbound-block-relay peers'
s 'block-relay-only'                     'outbound-block-relay'

s ' a outbound'                          ' an outbound'
-END VERIFY SCRIPT-

[practicalswift]

Concept ACK: consistent is better than inconsistent

[DrahtBot]

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• net: make CNode::m_inbound_onion public, initialize explicitly #21167 (net: make CNode::m_inbound_onion public, initialize explicitly by jonatack)
• net/net processing: Move tx inventory into net_processing #21160 (Net/Net processing: Move tx inventory into net_processing by jnewbery)
• Split orphan handling from net_processing into txorphanage #21148 (Split orphan handling from net_processing into txorphanage by ajtowns)
• Log: Add debug log category tag #21102 (Log: Add debug log category tag by wodry)
• Make all of net_processing (and some of net) use std::chrono types #21015 (Make all of net_processing (and some of net) use std::chrono types by dhruv)
• banman: save the banlist in a JSON format on disk #20966 (banman: save the banlist in a JSON format on disk by vasild)
• [refactor] Move some net_processing globals into PeerManagerImpl #20942 ([refactor] Move some net_processing globals into PeerManagerImpl by ajtowns)
• RFC: Move Peer and PeerManagerImpl declarations to separate header #20925 (RFC: Move Peer and PeerManagerImpl declarations to separate header by jnewbery)
• p2p: Add DISABLETX message for negotiating block-relay-only connections #20726 (p2p: Add DISABLETX message for negotiating block-relay-only connections by sdaftuar)
• addrman: Make addrman a top-level component #20228 (addrman: Make addrman a top-level component by jnewbery)
• p2p: Improve diversification of new connections #19860 (Improve diversification of new connections: privacy and stability by naumenkogs)

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.

[jonatack]

rebased

[luke-jr]

This might be easier to confuse with peers that request blocks only (#20726)

[jonatack]

Yes, the script can be updated depending on what is decided there.

[luke-jr]

While doing this, I wonder if it would make sense to also standardise the prefix (eg, "outbound-feeler")

[jonatack]

Rebased.

While doing this, I wonder if it would make sense to also standardise the prefix (eg, "outbound-feeler")

Yes, on the user-facing side, that may be where it's heading. For instance, in #20764 the -netinfo "dir" (in/out) column is next to the "type" column, and the type is expressed as simply full/block/manual/feeler/addr prefixed by in/out of the dir column (I may propose the same for the GUI peers tab columns). But for now at least, the codebase doesn't have the same variance regarding how a feeler connection is referred to, like it does with outbound-{full, block}-relay ones (at least, I don't recall noticing it).

[DrahtBot]

🕵️ @fanquake @harding @jonasschnelli @sipa @hebasto have been requested to review this pull request as specified in the REVIEWERS file.

[jonatack]

Had to update the script for other changes per git range-diff e7eb3712 de0f38d bb9f28e

The script may be verified locally with test/lint/commit-script-check.sh e7eb371..bb9f28e

[jonatack]

Rebased following the merge of #20828. The script may be verified locally with

test/lint/commit-script-check.sh 29d2aeb..2645217

[jonatack]

6th rebase. The scripted diff may be verified locally with:

test/lint/commit-script-check.sh e9c037b..c3e3a16

ACK 0a6dae5

Thanks for normalizing connection types and +1 for Luke suggestion!

Thanks @ariard for the review! I'd be happy to do @luke-jr's suggestion in a follow-up. This patch requires frequent rebasing, so if anyone is against it or would like it to be modified (e.g. to some other naming), please LMK (I won't bite 🐕) to economize rebasing it into the void of the abyss 🌌

[DrahtBot]

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

[ariard]

Code Review ACK c3e3a16

[practicalswift]

Lack of rebase is blocking my re-review of this PR :)

[jonatack]

Thanks everyone for the reviews and ACKs but this commit continually needs rebasing and the issue and diff have become too large without separating this into smaller changes. Up for grabs or maybe will re-open as smaller changes but I'm not sure there is enough support to fix/clean this up.