Update libp2p to v0.43.0 #499

rand0m-cloud · 2022-03-06T18:44:20Z

This PR updates the dependency on libp2p. This is needed because currently building ipfs-http fails with an ambiguous type error inside of libp2p.

rand0m-cloud · 2022-03-06T21:15:56Z

error[E0053]: method `poll` has an incompatible type for trait
   --> src/p2p/pubsub.rs:328:10
    |
328 |     ) -> Poll<PubsubNetworkBehaviourAction> {
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |          |
    |          expected enum `Void`, found enum `floodsub::layer::InnerMessage`
    |          help: change the output type to match the trait: `Poll<libp2p::libp2p_swarm::NetworkBehaviourAction<Void, OneShotHandler<FloodsubProtocol, FloodsubRpc, floodsub::layer::InnerMessage>, FloodsubRpc>>`
    |
    = note: expected fn pointer `fn(&mut Pubsub, &mut std::task::Context<'_>, &mut impl PollParameters) -> Poll<libp2p::libp2p_swarm::NetworkBehaviourAction<Void, _, _>>`
               found fn pointer `fn(&mut Pubsub, &mut std::task::Context<'_>, &mut impl PollParameters) -> Poll<libp2p::libp2p_swarm::NetworkBehaviourAction<floodsub::layer::InnerMessage, _, _>>`

~~I'm not sure how to approach this error.~~ Resolved.

rand0m-cloud · 2022-03-07T00:46:05Z

@mxinden I believe these are issues stemming from libp2p.

error[E0599]: no method named `into_inner` found for struct `KademliaHandler` in the current scope
  --> src/p2p/behaviour.rs:25:10
   |
25 | #[derive(libp2p::NetworkBehaviour)]
   |          ^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `KademliaHandler<QueryId>`
   |
   = note: this error originates in the derive macro `libp2p::NetworkBehaviour` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0599]: no method named `into_inner` found for struct `KademliaHandlerProto` in the current scope
  --> src/p2p/behaviour.rs:25:10
   |
25 | #[derive(libp2p::NetworkBehaviour)]
   |          ^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `KademliaHandlerProto<QueryId>`
   |
   = note: this error originates in the derive macro `libp2p::NetworkBehaviour` (in Nightly builds, run with -Z macro-backtrace for more info)

For this struct

#[derive(libp2p::NetworkBehaviour)]
#[behaviour(out_event = "BehaviourEvent")]
pub struct Behaviour<Types: IpfsTypes> {
    #[behaviour(ignore)]
    repo: Arc<Repo<Types>>,
    // mdns: Toggle<TokioMdns>,
    kademlia: Kademlia<MemoryStore>,
    #[behaviour(ignore)]
    kad_subscriptions: SubscriptionRegistry<KadResult, String>,
    bitswap: Bitswap,
    ping: Ping,
    identify: Identify,
    pubsub: Pubsub,
    pub swarm: SwarmApi,
}

rand0m-cloud · 2022-03-07T01:16:40Z

https://github.com/libp2p/rust-libp2p/blob/a168410dbed0d0941f2e5a14543206044ccb2260/swarm/Cargo.toml#L6

Isn't libp2p-swarm supposed to be version 0.35.0?

error: failed to select a version for the requirement `libp2p-swarm = "^0.35"`
candidate versions found which didn't match: 0.34.0, 0.33.0, 0.32.0, ...
location searched: crates.io index

mxinden · 2022-03-07T14:00:06Z

https://github.com/libp2p/rust-libp2p/blob/a168410dbed0d0941f2e5a14543206044ccb2260/swarm/Cargo.toml#L6

Isn't libp2p-swarm supposed to be version 0.35.0?
error: failed to select a version for the requirement `libp2p-swarm = "^0.35"`
candidate versions found which didn't match: 0.34.0, 0.33.0, 0.32.0, ...
location searched: crates.io index

v0.35.0 is not yet released, i.e. only in master thus far.

bitswap/src/behaviour.rs

koivunej · 2022-03-14T11:18:26Z

Thanks for doing this @rand0m-cloud. I was on a vacation but back now. Let me know when this is ready, or if you need any help with it.

rand0m-cloud · 2022-03-14T19:18:30Z

I've had a chance to play around more with the code and I can't figure out to how to solve the error. I've compared it to the file-sharing example of libp2p, and I can't make sense of what is wrong. The error is from the derive macro libp2p::NetworkBehaviour expecting some handler to implement into_inner, but it's my understanding that into_inner only exists for the ConnectionHandlerSelect that the macro creates.

I am lost but it feels like a bug in the derive macro.

mxinden · 2022-03-14T19:20:31Z

@rand0m-cloud mind posting the compiler error here?

rand0m-cloud · 2022-03-14T19:25:47Z

Sure, the errors in the PR description are the current ones but here it is again:

error[E0599]: no method named `into_inner` found for struct `KademliaHandler` in the current scope
  --> src/p2p/behaviour.rs:25:10
   |
25 | #[derive(libp2p::NetworkBehaviour)]
   |          ^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `KademliaHandler<QueryId>`
   |
   = note: this error originates in the derive macro `libp2p::NetworkBehaviour` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0599]: no method named `into_inner` found for struct `KademliaHandlerProto` in the current scope
  --> src/p2p/behaviour.rs:25:10
   |
25 | #[derive(libp2p::NetworkBehaviour)]
   |          ^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `KademliaHandlerProto<QueryId>`
   |
   = note: this error originates in the derive macro `libp2p::NetworkBehaviour` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0599`.

mxinden

Would this fix your compile time error?

bitswap/src/behaviour.rs

src/p2p/behaviour.rs

rand0m-cloud · 2022-03-14T22:21:20Z

Thanks to @mxinden's workaround, I was able to continue working on the PR. Now the PR builds, lints, but fails testing:

failures:

---- p2p::swarm::tests::racy_connecting_attempts stdout ----
thread 'p2p::swarm::tests::racy_connecting_attempts' panicked at 'assertion failed: `(left == right)`
  left: `[Err(Some("addresses exhausted")), Err(Some("addresses exhausted"))]`,
 right: `[Ok(()), Err(Some("finished connecting to another address"))]`', src/p2p/swarm.rs:517:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- p2p::swarm::tests::wrong_peerid stdout ----
thread 'p2p::swarm::tests::wrong_peerid' panicked at 'assertion failed: `(left == right)`
  left: `Some("addresses exhausted")`,
 right: `Some("Pending connection: Invalid peer ID.")`', src/p2p/swarm.rs:466:21


failures:
    p2p::swarm::tests::racy_connecting_attempts
    p2p::swarm::tests::wrong_peerid

test result: FAILED. 85 passed; 2 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.66s

rand0m-cloud · 2022-03-17T15:11:37Z

@koivunej I need some guidance on these test failures. I've traced the error message to src/p2p/swarm.rs: 378. Can you verify if the tests need to be updated, or if the underlying behavior has changed?

Some grepping I did,

$ git checkout v0.39.0 && find . -not -path "*/target/*" -name "*.rs" | xargs -n1 grep -n "Pending connection" /dev/null

./core/src/connection/error.rs:91:                write!(f, "Pending connection: I/O error: {}", err),
./core/src/connection/error.rs:93:                write!(f, "Pending connection: Transport error: {}", err),
./core/src/connection/error.rs:95:                write!(f, "Pending connection: Invalid peer ID."),

$ git checkout v0.43.0 && find . -not -path "*/target/*" -name "*.rs" | xargs -n1 grep -n "Pending connection" /dev/null
./swarm/src/connection/error.rs:100:    /// Pending connection attempt has been aborted.
./swarm/src/connection/error.rs:137:            PendingConnectionError::IO(err) => write!(f, "Pending connection: I/O error: {}", err),
./swarm/src/connection/error.rs:138:            PendingConnectionError::Aborted => write!(f, "Pending connection: Aborted."),
./swarm/src/connection/error.rs:142:                    "Pending connection: Transport error on connection: {}",
./swarm/src/connection/error.rs:152:                    "Pending connection: Unexpected peer ID {} at {:?}.",
./swarm/src/lib.rs:1381:    /// Pending connection attempt has been aborted.
./swarm/src/lib.rs:1426:                "Dial error: Pending connection attempt has been aborted."

The "finished connecting to another address" error seems to be from this crate, so I'll take that test failure as broken from this PR.

mxinden · 2022-03-17T15:39:19Z

Thanks to @mxinden's workaround, I was able to continue working on the PR. Now the PR builds, lints, but fails testing:

Sorry, but I don't think I am of much help on the rust-ipfs specific tests.

koivunej · 2022-03-17T17:25:30Z

@rand0m-cloud Yeah ... these connection tests ... They are most likely extra, and become more and more useless now that libp2p must've evolved. Back in the day it was a bit more tricky trying to get the similar behaviour as to go-ipfs. I'll try to check the branch locally tomorrow! Thanks for your work so far.

koivunej · 2022-03-18T12:27:03Z

src/p2p/swarm.rs

+ for failed in self
+ .pending_connections
+ .remove(&peer_id)
+ .unwrap_or_default()
+ {
+ self.connect_registry
+ .finish_subscription(failed.into(), Err("addresses exhausted".into()));
 }


Doing this unconditionally creates the failure for p2p::swarm::tests::racy_connecting_attempts.

I wonder ... why were changes around this necessary? Seems this logic would had been easiest to do handle with the old structure using the entry api?

I think this change also affects the p2p::swarm::tests::wrong_peerid test case.

koivunej · 2022-03-18T12:44:13Z

So in total I see 3 test failures:

swarm_api (really bad name)
racy_connection_attempts
wrong_peerid

For the last two, I think a solution should come out of the discussion thread I already started.

For the first one, I think the near-correct answer is to add biased; to the tokio::select! so the events are observed in the assumed order always. Though, this is prone to break because we are racing with the idle disconnection timer. However, I remember looking at the idle disconnection should be done right away so this might be ok. I did not yet push this fix, let me know if it should be pushed. If this doesn't fail for you, please let me know!

koivunej

Apart from my comment on the inject_dial_failure I think this is looking good!

rand0m-cloud · 2022-03-18T16:02:00Z

Oh, sorry for the noise. I was hoping Github would collapse it all. Just reauthored those commits with my email.

rand0m-cloud · 2022-03-18T16:08:28Z

I've been experiencing swarm_api failing maybe 1 in 10 times. I'll apply that biased; to the select, but I don't really know if it fixes the test.

rand0m-cloud · 2022-03-18T16:38:19Z

@koivunej Okay, I think I've re-added the code fragment I removed and would make inject_dial_failure work correctly.

The problem is the original code was given the Multiaddr it failed to dial from inject_addr_reach_failure and the new inject_dial_failure doesn't provide that. I can't find a clear way to go from the PeerId to MultiaddrWithPeerId.

rand0m-cloud · 2022-03-18T16:43:48Z

Do we need a new ConnectionHandler that can store the Multiaddr?

rand0m-cloud · 2022-03-18T17:03:14Z

src/p2p/swarm.rs

+ // it is possible that these addresses have not been tried yet; they will be asked
+ // for soon.
+ let handler = self.new_handler();
+ self.events.push_back(swarm::NetworkBehaviourAction::Dial {


I feel like this needs to be removed? I'm not sure how to adapt the behavior because inject_dial_failure only tells us when a connection failed to dial.

I think this current block was made for attempting another connection when one fails.

All right ... Looking at the networkbehaviour, I think this is how it used to work:

dial event is given to swarm

swarm collects all of the known addresses with NetworkBehaviour::addresses_of_peer, dials them somehow

for each of the dials, we used to get a signal that this multiaddr failed and we would signal that future as failure

there used to be another trait method for having exhausted the addresses, when we'd know to launch a new dial if we'd still have addresses

While writing this @mxinden replied. Oki yeah it would appear the failures have now moved within the dialerror, which I did not expect, AND there is only one "notification" for all of the attempts.

However I think the idea with the original impl was that since there would be one gathering of addresses for the peer (NetworkBehaviour::addresses_of_peer) per dial events (caused either by "swarm_api" or by any other place) it would be possible to add new addresses during a dial attempt, and those would be noticed at (4) in the above ordered list, and thus get dialed afterwards.

I now realize that this all should had been in the swarm api implementation as comments, but I guess I was expecting the datastructures and network behaviour api to make this "apparent" and did not account for possible future changes in the network behaviour api.

I think for the next steps would be to gather the failed addresses from the error (if found), then continue dialing to the remaining addresses, if any.

this might take care of the node-gyp problem, which might also be fixed by updating it's version.

forgot, you must not use backticks outside apostrophes...

originally created in 8eae8e1 by altering the single topic test, included in this commit as duplicating version. Co-authored-by: Addy Bryant <[email protected]>

simplify away the use of hashset's for messages along with any filtering, instead simply assert that who witnessed what message and include the sent message in the assertion as well. comment as in use less broad technical names and more context specific names. also removes some of the duplicate comments.

needs to be looked at.

koivunej · 2022-04-01T13:22:42Z

I would rather not delay this further and a lot of work has been put into this already. Thanks @rand0m-cloud! I ended up ignoring the problematic windows tests pending a) packet capture b) additional logging in the js test. Also added a FIXME for my concern.

bors r+

bors · 2022-04-01T13:46:00Z

Build succeeded:

ci

rand0m-cloud force-pushed the libp2p_update branch 2 times, most recently from e049b6a to a0ae83d Compare March 6, 2022 18:56

mxinden reviewed Mar 7, 2022

View reviewed changes

bitswap/src/behaviour.rs Outdated Show resolved Hide resolved

driemworks mentioned this pull request Mar 9, 2022

Iris Milestone 2 Delivery w3f/Grant-Milestone-Delivery#371

Merged

5 tasks

mxinden reviewed Mar 14, 2022

View reviewed changes

bitswap/src/behaviour.rs Outdated Show resolved Hide resolved

mxinden reviewed Mar 14, 2022

View reviewed changes

src/p2p/behaviour.rs Outdated Show resolved Hide resolved

mxinden reviewed Mar 14, 2022

View reviewed changes

src/p2p/behaviour.rs Show resolved Hide resolved

rand0m-cloud force-pushed the libp2p_update branch from 413fbd1 to b5df04a Compare March 14, 2022 23:15

mxinden mentioned this pull request Mar 17, 2022

swarm-derive/: Don't fail when ignored fields are first libp2p/rust-libp2p#2569

Closed

koivunej reviewed Mar 18, 2022

View reviewed changes

koivunej approved these changes Mar 18, 2022

View reviewed changes

rand0m-cloud force-pushed the libp2p_update branch from b5df04a to f6a8312 Compare March 18, 2022 15:54

rand0m-cloud commented Mar 18, 2022

View reviewed changes

rand0m-cloud and others added 21 commits April 1, 2022 12:11

some updates to pubsub

c1a5bba

fix the pubsub network behaviour action type

7e9da72

replaced todo placeholders

085be77

re-add connection closed and established

e4002d6

added change to changelog

a996922

enable event_process for BehaviourEvent

bdf977c

chore: clean up type signature

93b31b3

fix: removed unneeded BehaviourEvent struct

25c8d58

temp fix: changed field order to workaround bug in libp2p

3b59193

chore: more updating to libp2p

31262b5

fix: update libp2p and renamed the changed types

6c6fc3d

fix(swarm-test): add biased to tokio::select for non-random behavior

77291ee

wip: re-add code fragment to handle dial failure

888e6f1

fix(swarm): corrected dial failure logic

72ff95d

fix: corrected faulty Vec::retain logic and updated WrongPeerId test

1cee67d

fix: apply review suggestions and fix clippy lints

897c16f

fix(pubsub): tell Floodsub about the peers we want to hear from

d4d3def

ci(win): use windows-2019 image

87a4114

this might take care of the node-gyp problem, which might also be fixed by updating it's version.

fix(build): stop building while writing an error

82453e5

forgot, you must not use backticks outside apostrophes...

test(pubsub): disjoint topics as new test case

277954b

originally created in 8eae8e1 by altering the single topic test, included in this commit as duplicating version. Co-authored-by: Addy Bryant <[email protected]>

koivunej force-pushed the libp2p_update branch from 5ab14e1 to 50ad10f Compare April 1, 2022 10:48

koivunej added 2 commits April 1, 2022 15:09

test(conf): ignore pubsub tests on windows for now

081a598

doc(p2p): add fixme for possible issue

bf7a807

needs to be looked at.

koivunej mentioned this pull request Apr 1, 2022

conformance: ignored windows tests #501

Open

bors bot merged commit e8f6d66 into rs-ipfs:master Apr 1, 2022

This was referenced Apr 1, 2022

Update libp2p #485

Closed

Expose Floodsub target_peer list. #498

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update libp2p to v0.43.0 #499

Update libp2p to v0.43.0 #499

rand0m-cloud commented Mar 6, 2022 •

edited

Loading

rand0m-cloud commented Mar 6, 2022 •

edited

Loading

rand0m-cloud commented Mar 7, 2022 •

edited

Loading

rand0m-cloud commented Mar 7, 2022

mxinden commented Mar 7, 2022

koivunej commented Mar 14, 2022

rand0m-cloud commented Mar 14, 2022

mxinden commented Mar 14, 2022

rand0m-cloud commented Mar 14, 2022

mxinden left a comment

rand0m-cloud commented Mar 14, 2022

rand0m-cloud commented Mar 17, 2022

mxinden commented Mar 17, 2022

koivunej commented Mar 17, 2022

koivunej Mar 18, 2022 •

edited

Loading

koivunej commented Mar 18, 2022

koivunej left a comment

rand0m-cloud commented Mar 18, 2022

rand0m-cloud commented Mar 18, 2022

rand0m-cloud commented Mar 18, 2022

rand0m-cloud commented Mar 18, 2022

rand0m-cloud Mar 18, 2022 •

edited

Loading

koivunej Mar 21, 2022 •

edited

Loading

koivunej Mar 21, 2022

koivunej commented Apr 1, 2022

bors bot commented Apr 1, 2022

Update libp2p to v0.43.0 #499

Update libp2p to v0.43.0 #499

Conversation

rand0m-cloud commented Mar 6, 2022 • edited Loading

rand0m-cloud commented Mar 6, 2022 • edited Loading

rand0m-cloud commented Mar 7, 2022 • edited Loading

rand0m-cloud commented Mar 7, 2022

mxinden commented Mar 7, 2022

koivunej commented Mar 14, 2022

rand0m-cloud commented Mar 14, 2022

mxinden commented Mar 14, 2022

rand0m-cloud commented Mar 14, 2022

mxinden left a comment

Choose a reason for hiding this comment

rand0m-cloud commented Mar 14, 2022

rand0m-cloud commented Mar 17, 2022

mxinden commented Mar 17, 2022

koivunej commented Mar 17, 2022

koivunej Mar 18, 2022 • edited Loading

Choose a reason for hiding this comment

koivunej commented Mar 18, 2022

koivunej left a comment

Choose a reason for hiding this comment

rand0m-cloud commented Mar 18, 2022

rand0m-cloud commented Mar 18, 2022

rand0m-cloud commented Mar 18, 2022

rand0m-cloud commented Mar 18, 2022

rand0m-cloud Mar 18, 2022 • edited Loading

Choose a reason for hiding this comment

koivunej Mar 21, 2022 • edited Loading

Choose a reason for hiding this comment

koivunej Mar 21, 2022

Choose a reason for hiding this comment

koivunej commented Apr 1, 2022

bors bot commented Apr 1, 2022

rand0m-cloud commented Mar 6, 2022 •

edited

Loading

rand0m-cloud commented Mar 6, 2022 •

edited

Loading

rand0m-cloud commented Mar 7, 2022 •

edited

Loading

koivunej Mar 18, 2022 •

edited

Loading

rand0m-cloud Mar 18, 2022 •

edited

Loading

koivunej Mar 21, 2022 •

edited

Loading