Eliminate duplicate P2P connections #1308
Conversation
plugins/net_plugin/net_plugin.cpp
Outdated
| else | ||
| peer_dlog(this, "Invalid handshake p2p_address ${p}", ("p", msg.p2p_address)); | ||
| } else { | ||
| // check if peer requests no trx or no blocks |
There was a problem hiding this comment.
This comment is a little confusing.
| fc_dlog( logger, "not duplicate, my_impl->node_id '${lhs}' < msg.node_id '${rhs}'", | ||
| ("lhs", my_impl->node_id)("rhs", msg.node_id) ); | ||
| // only the connection from lower node_id to higher node_id will be considered as a duplicate, | ||
| // so there is no chance for both connections to be closed |
There was a problem hiding this comment.
The is simpler than the complicated prior version.
|
I wonder if it would not be preferable to do the duplicate connection check when |
plugins/net_plugin/net_plugin.cpp
Outdated
| }; | ||
| if (my_impl->connections.any_of_connections(is_duplicate)) { | ||
| peer_dlog( this, "sending go_away duplicate, msg.p2p_address: ${add}", ("add", msg.p2p_address) ); | ||
| go_away_message gam(duplicate); |
There was a problem hiding this comment.
I don't think the constructor of go_away_message serves any useful purpose, and makes the code (here at least) more awkward. Without it we could do:
enqueue(go_away_message{duplicate, conn_node_id});
which I think is clearer.
There was a problem hiding this comment.
bonus point if making go_away_reason an enum class :-)
I think the cost is probaly negligeable, but OK. |
|
Note:start |
Fix for an issue where
net_pluginwould allow some duplicate connections to remain open.Simplified to just checking both incoming and outgoing connections for duplicate.
Resolves #1255