p2p: standardize outbound full/block relay connection type naming #20729

pull jonatack wants to merge 1 commits into bitcoin:master from jonatack:outbound-connection-type-naming changing 19 files +173 −173
  1. jonatack commented at 12:46 pm on December 20, 2020: member

    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:

     0-BEGIN VERIFY SCRIPT-
 1s() { git grep -l "$1" src test/functional doc/files.md doc/reduce-*.md | xargs sed -i "s/$1/$2/g"; }
 2
 3s 'ConnectionType::OUTBOUND '            'ConnectionType::OUTBOUND_FULL_RELAY '
 4s 'IsFullOutboundConn'                   'IsOutboundFullRelayConn'
 5s 'GetExtraFullOutboundCount'            'GetExtraOutboundFullRelayCount'
 6s 'full_outbound_peers'                  'outbound_full_relay_peers'
 7s 'GetTryNewOutboundPeer'                'GetTryNewOutboundFullRelayPeer'
 8s 'SetTryNewOutboundPeer'                'SetTryNewOutboundFullRelayPeer'
 9s 'm_try_another_outbound_peer'          'm_try_another_outbound_full_relay_peer'
10s 'outbound (full-relay)'                'outbound-full-relay'
11s 'outbound full-relay'                  'outbound-full-relay'
12s 'outbound, full-relay'                 'outbound-full-relay'
13s 'outbounds'                            'outbound-full-relay connections'
14s 'f"outbound: '                         'f"outbound-full-relay: '
15s ' full-relay'                          ' outbound-full-relay'
16s 'to an extra outbound peer'            'to an extra outbound-full-relay peer'
17s 'try another outbound peer'            'try another outbound-full-relay peer'
18s '(tx, block, addr) outbound'           '(tx, block, addr)'
19s 'we deal with extra outbound peers'    'we deal with extra outbound-full-relay peers'
20s 'outbound peers we have in excess'     'outbound-full-relay peers we have in excess'
21s 'some outbound connections are not'    'some outbound-full-relay connections are not'
22s 'or this is a'                         'or if this is an'
23
24s 'ConnectionType::BLOCK_RELAY'          'ConnectionType::OUTBOUND_BLOCK_RELAY'
25s ' BLOCK_RELAY'                         ' OUTBOUND_BLOCK_RELAY'
26s 'IsBlockOnlyConn'                      'IsOutboundBlockRelayConn'
27s 'GetExtraBlockRelayCount'              'GetExtraOutboundBlockRelayCount'
28s 'block_relay_peers'                    'outbound_block_relay_peers'
29s 'MAX_BLOCK_RELAY_ONLY_ANCHORS'         'MAX_OUTBOUND_BLOCK_RELAY_ANCHORS'
30s 'MAX_BLOCK_RELAY_ONLY_CONNECTIONS'     'MAX_OUTBOUND_BLOCK_RELAY_CONNECTIONS'
31s 'EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL' 'EXTRA_OUTBOUND_BLOCK_RELAY_PEER_INTERVAL'
32s 'm_start_extra_block_relay_peers'      'm_start_extra_outbound_block_relay_peers'
33s 'outgoing block-relay-only'            'outbound-block-relay'
34s 'outbound block-relay-only'            'outbound-block-relay'
35s 'outbound block-relay'                 'outbound-block-relay'
36s 'block-relay only outbound'            'outbound-block-relay'
37s 'block-relay-only outgoing'            'outbound-block-relay'
38s 'block-relay only peers'               'outbound-block-relay peers'
39s 'block-relay-only'                     'outbound-block-relay'
40
41s ' a outbound'                          ' an outbound'
42-END VERIFY SCRIPT-
  2. fanquake added the label P2P on Dec 20, 2020
  3. practicalswift commented at 6:25 pm on December 20, 2020: contributor
    Concept ACK: consistent is better than inconsistent
  4. DrahtBot commented at 8:04 pm on December 20, 2020: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

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

  5. DrahtBot added the label Needs rebase on Dec 22, 2020
  6. jonatack force-pushed on Dec 22, 2020
  7. jonatack commented at 12:38 pm on December 22, 2020: member
    rebased
  8. DrahtBot removed the label Needs rebase on Dec 22, 2020
  9. DrahtBot added the label Needs rebase on Jan 2, 2021
  10. in src/rpc/net.cpp:35 in 0d4926c94a outdated 
    30@@ -31,7 +31,7 @@
31 
32 const std::vector<std::string> CONNECTION_TYPE_DOC{
33         "outbound-full-relay (default automatic connections)",
34-        "block-relay-only (does not relay transactions or addresses)",
35+        "outbound-block-relay (does not relay transactions or addresses)",
    luke-jr commented at 1:05 am on January 3, 2021:
    This might be easier to confuse with peers that request blocks only (#20726)
    jonatack commented at 10:16 pm on January 3, 2021:
    Yes, the script can be updated depending on what is decided there.
  11. luke-jr commented at 1:06 am on January 3, 2021: member
    While doing this, I wonder if it would make sense to also standardise the prefix (eg, “outbound-feeler”)
  12. jonatack force-pushed on Jan 3, 2021
  13. jonatack commented at 10:29 pm on January 3, 2021: member

    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).

  14. DrahtBot removed the label Needs rebase on Jan 3, 2021
  15. DrahtBot added the label Needs rebase on Jan 7, 2021
  16. jonatack force-pushed on Jan 7, 2021
  17. jonatack commented at 6:53 pm on January 7, 2021: member
    Rebased.
  18. DrahtBot removed the label Needs rebase on Jan 7, 2021
  19. ariard commented at 0:34 am on January 12, 2021: member

    ACK 0a6dae5

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

  20. DrahtBot added the label Needs rebase on Jan 13, 2021
  21. jonatack force-pushed on Jan 13, 2021
  22. jonatack commented at 10:27 am on January 13, 2021: member

    Rebased and updated the script per git range-diff e7eb3712 0a6dae5 de0f38d due to a non-existing connection type added in net.h::AddConnection(), script update:

    0+ s 'ConnectionType::OUTBOUND '            'ConnectionType::OUTBOUND_FULL_RELAY '

    The script may be verified locally after this rebase with:

    0test/lint/commit-script-check.sh e7eb3712..de0f38d5f
  23. DrahtBot removed the label Needs rebase on Jan 13, 2021
  24. DrahtBot commented at 11:46 am on January 13, 2021: member

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

  25. jonatack force-pushed on Jan 13, 2021
  26. jonatack commented at 12:06 pm on January 13, 2021: member

    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

  27. DrahtBot added the label Needs rebase on Jan 14, 2021
  28. jonatack force-pushed on Jan 14, 2021
  29. jonatack commented at 12:37 pm on January 14, 2021: member

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

    0test/lint/commit-script-check.sh 29d2aeb..2645217
  30. DrahtBot removed the label Needs rebase on Jan 14, 2021
  31. jonatack renamed this:
    p2p: standardize on outbound-{full, block}-relay connection type naming
    p2p: standardize outbound full/block relay connection type naming
    on Feb 9, 2021
  32. DrahtBot added the label Needs rebase on Feb 12, 2021
  33. scripted-diff: clean up outbound connection type naming 
    -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-
    c3e3a16d1d
  34. jonatack force-pushed on Feb 12, 2021
  35. jonatack commented at 5:29 pm on February 12, 2021: member

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

    0test/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 :dog2:) to economize rebasing it into the void of the abyss :milky_way:

  36. DrahtBot removed the label Needs rebase on Feb 12, 2021
  37. DrahtBot added the label Needs rebase on Feb 15, 2021
  38. DrahtBot commented at 1:01 pm on February 15, 2021: member

    🐙 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”.

  39. ariard commented at 12:25 pm on February 16, 2021: member
    Code Review ACK c3e3a16d
  40. practicalswift commented at 5:11 pm on March 20, 2021: contributor
    Lack of rebase is blocking my re-review of this PR :)
  41. jonatack commented at 10:12 am on May 11, 2021: member
    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.
  42. jonatack closed this on May 11, 2021
  43. laanwj added the label Up for grabs on May 11, 2021
  44. DrahtBot locked this on Aug 18, 2022


jonatack practicalswift DrahtBot luke-jr ariard

Labels
P2P Up for grabs Needs rebase

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me