p2p: Remove m_is_manual_connection from CNodeState #20617

pull ariard wants to merge 1 commits into bitcoin:master from ariard:2020-12-remove-is-manual-connection changing 1 files +5 −8

ariard commented at 5:57 PM on December 10, 2020: member

Currently, this member is only used to exclude MANUAL peers from discouragement in MaybePunishNodeForBlock(). Manual connections are already protected in MaybeDiscourageAndDisconnect(), independently from their network processing behaviors.

Remove m_is_manual_connection from CNodeState

Currently, this member is only used to exclude MANUAL peers from discouragement
in MaybePunishNodeForBlock(). Manual connections are already protected in
MaybeDiscourageAndDisconnect(), independently from their network
processing behaviors.

a33442fdc7

MarcoFalke commented at 6:12 PM on December 10, 2020: member

cr ACK a33442fdc73eabd1c5596ab92954344edc9517e6
DrahtBot added the label P2P on Dec 10, 2020
fanquake requested review from jnewbery on Dec 10, 2020
promag commented at 10:37 PM on December 10, 2020: member

m_misbehavior_score goes on forever for manual conns?
MarcoFalke commented at 8:54 AM on December 11, 2020: member

@promag Yes, but that is an unrelated issue. (This pull doesn't change that)
promag commented at 9:12 AM on December 11, 2020: member

@MarcoFalke I think it does? Before Misbehaving was not called for manual nodes?
MarcoFalke commented at 9:18 AM on December 11, 2020: member

Misbehaving is called in 18 places of which only one is guarded by this check. What am I missing?
promag commented at 9:25 AM on December 11, 2020: member

Code review ACK a33442fdc73eabd1c5596ab92954344edc9517e6. @MarcoFalke right, I was just considering Misbehaving() from MaybePunishNodeForBlock. Probably it's fine to leave m_misbehavior_score going on, or it could have a max value.

in src/net_processing.cpp:1054 in a33442fdc7

1050 | @@ -1054,8 +1051,8 @@ bool PeerManager::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationSt
1051 |              }
1052 |  
1053 |              // Discourage outbound (but not inbound) peers if on an invalid chain.
1054 | -            // Exempt HB compact block peers and manual connections.
1055 | -            if (!via_compact_block && !node_state->m_is_inbound && !node_state->m_is_manual_connection) {
1056 | +            // Exempt HB compact block peers. Manual connections are always protected from discouragement.

jnewbery commented at 9:26 AM on December 11, 2020:

I don't think the comment "Manual connections are always protected from discouragement." should be here since it's not specific to this site.

ariard commented at 3:59 PM on December 11, 2020:

Bookmarked, I'll take it if further changes are required.

jnewbery approved
jnewbery commented at 9:27 AM on December 11, 2020: member

utACK a33442fdc73eabd1c5596ab92954344edc9517e6
fanquake requested review from amitiuttarwar on Dec 13, 2020
amitiuttarwar approved
amitiuttarwar commented at 1:33 AM on December 14, 2020: contributor

code review ACK a33442fdc73eabd1c5596ab92954344edc9517e6
fanquake merged this on Dec 14, 2020
fanquake closed this on Dec 14, 2020
sidhujag referenced this in commit 28b54fa294 on Dec 14, 2020
Fabcien referenced this in commit 43a7ffb79d on Feb 1, 2022
DrahtBot locked this on Feb 15, 2022

Contributors

Labels