Update to latest libp2p #1386

tomaka · 2019-01-10T16:19:09Z

Another networking rewrite.
Switches to libp2p 0.2 and moves a lot of code from Substrate to libp2p.

This fixes quite a lot of issues:

Substreams were not closed gracefully when disconnecting.
Kademlia substreams are now unidirectional, similar to what go-libp2p and js-libp2p do (fixes Kademlia is in fact unidirectional #901)
Kademlia in general should be more robust.
At initialization, we start by performing lots of random Kademlia queries, then the delay increases over time.
Fixes the fact that the dialing address of nodes (ie. the TCP dialing port) was added in the topology as well.
If nodes are full, they will refuse Substrate connections ("custom protocols") but will still accept Kademlia requests. This means that nodes will now be able to join the network even if bootnodes are full.
Fixes a race between the two nodes for the one which will open the Substrate substream the first. Now only the dialing side of the connection opens the substream.

twittner

LGTM

(custom_proto::upgrade::RegisteredProtocols::find_protocol is never used)

core/network-libp2p/src/topology.rs

twittner · 2019-01-11T10:48:02Z

core/network-libp2p/src/custom_proto/behaviour.rs

+		self.banned_peers.shrink_to_fit();
+
+		if !self.events.is_empty() {
+			return Async::Ready(self.events.remove(0))


Should self.events be a VecDeque to allow a more efficient pop? Removing the head of a vector will always shift the remaining elements. Now I guess there will not be too many of those, but still.

Right now it's a SmallVec, and since the elements are quite small I think it shouldn't be very expensive to remove the first element.

I think we should eventually remove this events system, but that requires some big modifications of the NetworkBehaviour trait.

Co-Authored-By: tomaka <[email protected]>

bkchr

Overall it looks good, just some nitpicks.

bkchr · 2019-01-14T07:42:49Z

core/network-libp2p/src/behaviour.rs

@@ -0,0 +1,262 @@
+// Copyright 2018 Parity Technologies (UK) Ltd.


bkchr · 2019-01-14T07:43:36Z

core/network-libp2p/src/behaviour.rs

+
+impl<TSubstream> Behaviour<TSubstream> {
+	/// Builds a new `Behaviour`.
+	// TODO: redundancy between config and local_peer_id


Please create a github issue and put a link into the comment.

bkchr · 2019-01-14T08:03:33Z

core/network-libp2p/src/custom_proto/behaviour.rs

@@ -0,0 +1,471 @@
+// Copyright 2018 Parity Technologies (UK) Ltd.


bkchr · 2019-01-14T08:09:18Z

core/network-libp2p/src/custom_proto/behaviour.rs

+		}
+	}
+
+	/// Try to add a reserved peer.


Why try? The function does seem to try anything.

bkchr · 2019-01-14T08:09:24Z

core/network-libp2p/src/custom_proto/behaviour.rs

+		self.next_connect_to_nodes = Delay::new(Instant::now());
+	}
+
+	/// Try to remove a reserved peer.


bkchr · 2019-01-14T08:20:30Z

core/network-libp2p/src/custom_proto/behaviour.rs

+		// Check whether peer is banned.
+		if !is_reserved {
+			if let Some((_, expire)) = self.banned_peers.iter().find(|(p, _)| p == &peer_id) {
+				if *expire < Instant::now() {


Shouldn't it be *expire > Instant::now(). Because when expire < Instant::now() the ban is over?

bkchr · 2019-01-14T08:27:47Z

core/network-libp2p/src/custom_proto/handler.rs

@@ -0,0 +1,327 @@
+// Copyright 2018 Parity Technologies (UK) Ltd.


bkchr · 2019-01-14T08:37:59Z

core/network-libp2p/src/custom_proto/mod.rs

@@ -0,0 +1,22 @@
+// Copyright 2018 Parity Technologies (UK) Ltd.


bkchr · 2019-01-14T08:40:32Z

core/network-libp2p/src/transport.rs

-		.map(|(id, muxer), _| (id, StreamMuxerBox::new(muxer)));
+			let peer_id2 = peer_id.clone();
+			let upgrade = core::upgrade::SelectUpgrade::new(yamux::Config::default(), mplex_config)
+				// TODO: use a single `.map` instead of two maps


Please create and link a github issue.

…90107

* Update to latest libp2p * Fix indentations * Add basic test * Apply suggestions from code review Co-Authored-By: tomaka <[email protected]> * Remove Mutex from topology * Remove unused method * Fix concerns

…raud-proof-window-is-closed Finalize domains when the fraud proof period is over

Update to latest libp2p

b1cd24b

tomaka added A0-please_review Pull request needs code review. M4-core labels Jan 10, 2019

This was referenced Jan 10, 2019

Random peer disconnecting #1335

Closed

Substrate on Windows #1343

Closed

Tracking: non-published external dependencies #1037

Closed

tomaka requested a review from twittner January 10, 2019 16:40

tomaka added 2 commits January 11, 2019 10:28

Fix indentations

df99928

Add basic test

00c325a

gnunicorn requested a review from bkchr January 11, 2019 10:48

twittner reviewed Jan 11, 2019

View reviewed changes

Apply suggestions from code review

7a8346f

Co-Authored-By: tomaka <[email protected]>

tomaka mentioned this pull request Jan 11, 2019

Rewrite network protocol/service to use channels #1340

Merged

tomaka added 2 commits January 11, 2019 15:23

Remove Mutex from topology

0fd2f21

Remove unused method

e516da8

bkchr approved these changes Jan 14, 2019

View reviewed changes

tomaka added 2 commits January 14, 2019 10:42

Merge remote-tracking branch 'upstream/master' into update-libp2p-201…

96670f3

…90107

Fix concerns

f727a3e

bkchr added A8-looksgood and removed A0-please_review Pull request needs code review. labels Jan 14, 2019

gavofyork merged commit 75c3b51 into paritytech:master Jan 14, 2019

tomaka deleted the update-libp2p-20190107 branch January 14, 2019 12:40

xlc mentioned this pull request Jan 14, 2019

dns4 libp2p address no longer works #1432

Closed

liuchengxu added a commit to liuchengxu/substrate that referenced this pull request May 31, 2023

Merge pull request paritytech#1386 from subspace/finalize-domain-on-f…

108271c

…raud-proof-window-is-closed Finalize domains when the fraud proof period is over

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to latest libp2p #1386

Update to latest libp2p #1386

tomaka commented Jan 10, 2019 •

edited

Loading

twittner left a comment

twittner Jan 11, 2019

tomaka Jan 11, 2019

bkchr left a comment

bkchr Jan 14, 2019

bkchr Jan 14, 2019

bkchr Jan 14, 2019

bkchr Jan 14, 2019

bkchr Jan 14, 2019

bkchr Jan 14, 2019

bkchr Jan 14, 2019

bkchr Jan 14, 2019

bkchr Jan 14, 2019

		@@ -0,0 +1,262 @@
		// Copyright 2018 Parity Technologies (UK) Ltd.

		@@ -0,0 +1,471 @@
		// Copyright 2018 Parity Technologies (UK) Ltd.

		@@ -0,0 +1,327 @@
		// Copyright 2018 Parity Technologies (UK) Ltd.

		@@ -0,0 +1,22 @@
		// Copyright 2018 Parity Technologies (UK) Ltd.

Update to latest libp2p #1386

Update to latest libp2p #1386

Conversation

tomaka commented Jan 10, 2019 • edited Loading

twittner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkchr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomaka commented Jan 10, 2019 •

edited

Loading