-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Conversation
excellent. this will be one more dep not using an old rand version. |
@kpp Any news on this? Now that 0.40 is out this could use an update? |
Yes, I will update it. |
I think that some methods have been added to the Could you make sure that all our implementations of this trait have all the methods when necessary? |
It was introduced in libp2p-swarm in 0.20.0 [2020-07-01]. I made a diff of methods of NetworkBehaviour: $ diff 0.39.txt 0.40.txt
3d2
< inject_addr_reach_failure
13a13
> inject_listen_failure So only NetworkBehaviour::inject_listen_failure should be addressed in this PR. Will fix. |
for k in self.kademlias.values_mut() { | ||
NetworkBehaviour::inject_connection_closed(k, peer_id, conn, endpoint) | ||
} | ||
// NetworkBehaviour::inject_connection_closed on Kademlia<MemoryStore> does nothing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not really future-proof. Maybe in a future version it will do something.
We need a way to deconstruct a MultiHandler
and libp2p doesn't provide any. When this PR is merged, could you open an issue in libp2p and in Substrate and follow up with a fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mxinden can you take a look at this one? ^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a way to deconstruct a MultiHandler and libp2p doesn't provide any.
Any road blockers to contributing this upstream (rust-libp2p)?
@@ -649,6 +689,10 @@ impl NetworkBehaviour for DiscoveryBehaviour { | |||
} | |||
} | |||
|
|||
fn inject_listen_failure(&mut self, _: &Multiaddr, _: &Multiaddr, _: Self::ProtocolsHandler) { | |||
// NetworkBehaviour::inject_listen_failure on Kademlia<MemoryStore> does nothing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same remark as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM provided we fix my two remaining review comments in the near future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just needs master merge
bot merge |
* Bump libp2p to 0.40.0-rc.1 * Fix PingFailure import * Reduce the number of compilation errors (this is a FIXME commit) * Bump libp2p to 0.40.0-rc.2 * Fix sc-network::Behaviour to inject events into fields * Fix some NetworkBehaviourAction types * More fixes * More fixes * More fixes * Fix DiscoveryBehaviour * Fix PeerInfoBehaviour * Fix RequestResponsesBehaviour * Fix RequestResponsesBehaviour * Fix Notifications * Fix NetworkWorker * Fix Behaviour * Please borrowchk * Please borrowchk * Please borrowchk * Fix fmt * Cover all cases in matches * Fix some clippy warnings * Fix into_peer_id -> to_peer_id * Fix some warnings * Fix some inject_dial_failure FIXMEs * Fix DiscoveryBehaviour::inject_dial_failure * Fix RequestResponsesBehaviour::inject_dial_failure * Fix the order of inject_connection_closed PeerInfoBehaviour events * Make KademliaEvent with filtering unreachable * Fix Notifications::inject_dial_failure * Use concurrent_dial_errors in NetworkWorker * Remove commented-out RequestResponsesBehaviour::inject_addr_reach_failure * Fix tests * Dont report new PendingConnectionError and DialError variants to metrics * Bump libp2p to 0.40.0 * Add fn inject_listen_failure and inject_address_change * Review fixes
* Bump libp2p to 0.40.0-rc.1 * Fix PingFailure import * Reduce the number of compilation errors (this is a FIXME commit) * Bump libp2p to 0.40.0-rc.2 * Fix sc-network::Behaviour to inject events into fields * Fix some NetworkBehaviourAction types * More fixes * More fixes * More fixes * Fix DiscoveryBehaviour * Fix PeerInfoBehaviour * Fix RequestResponsesBehaviour * Fix RequestResponsesBehaviour * Fix Notifications * Fix NetworkWorker * Fix Behaviour * Please borrowchk * Please borrowchk * Please borrowchk * Fix fmt * Cover all cases in matches * Fix some clippy warnings * Fix into_peer_id -> to_peer_id * Fix some warnings * Fix some inject_dial_failure FIXMEs * Fix DiscoveryBehaviour::inject_dial_failure * Fix RequestResponsesBehaviour::inject_dial_failure * Fix the order of inject_connection_closed PeerInfoBehaviour events * Make KademliaEvent with filtering unreachable * Fix Notifications::inject_dial_failure * Use concurrent_dial_errors in NetworkWorker * Remove commented-out RequestResponsesBehaviour::inject_addr_reach_failure * Fix tests * Dont report new PendingConnectionError and DialError variants to metrics * Bump libp2p to 0.40.0 * Add fn inject_listen_failure and inject_address_change * Review fixes
Updates libp2p to a 0.40.0 version.