Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Upgrade to libp2p 0.51.3 #13587

Conversation

melekes
Copy link
Contributor

@melekes melekes commented Mar 13, 2023

Polkadot companion: paritytech/polkadot#7000
Cumulus companion: paritytech/cumulus#2429

Closes #13537

@melekes melekes self-assigned this Mar 13, 2023
)
}
FromSwarm::DialFailure(e) =>
for (p, _) in self.protocols.values_mut() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this assumes that for every connection we register all protocols. not sure why handler.into_iter() was used before since the logic seems equal in both cases to me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this assumes that for every connection we register all protocols.

Sounds correct

not sure why handler.into_iter() was used before since the logic seems equal in both cases to me

I don't know either but it looks right what has changed here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least we don't have ConnectionHandler passed as an argument of DialFailure in libp2p-0.51.2, so I would presume this means the connection failed completely.

}

{
let mut list_to_filter = self.kademlia.addresses_of_peer(peer_id);
let mut list_to_filter = self.kademlia.handle_pending_outbound_connection(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we attempt to get more addresses from kademlia and mdns for a peer if there's no ID (maybe_peer is None)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there easy way to do it? I'd probably not bother with it and just let the next random walk find new peers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there easy way to do it?

the code already does this. no need to do anything.

I was asking if it even makes sense to probe kademlia / mdns if peerID is empty. I guess it won't hurt. Otherwise we can just return empty Vec if maybe_peer.is_none() in the beginning of this function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would err on the side of caution and keep the old behavior for now at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in a02f891

@melekes melekes added A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Mar 16, 2023
@melekes

This comment was marked as outdated.

@melekes melekes added the S4-blocked Issue is blocked, see comments for further information. label Mar 17, 2023
@melekes melekes changed the title client/network: upgrade to libp2p 0.51.0 client/network: upgrade to libp2p 0.51.1 Mar 23, 2023
@melekes melekes changed the title client/network: upgrade to libp2p 0.51.1 Upgrade to libp2p 0.51.1 Mar 23, 2023
@melekes melekes marked this pull request as ready for review March 23, 2023 07:48
@melekes melekes added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Mar 23, 2023
paritytech#13587 (comment)

fyi: I don't really know what the old behaviour was.
@melekes

This comment was marked as outdated.

@altonen

This comment was marked as outdated.

@melekes melekes changed the title Upgrade to libp2p 0.51.3 Upgrade to libp2p 0.51.2 May 4, 2023
@melekes melekes changed the title Upgrade to libp2p 0.51.2 Upgrade to libp2p 0.51.3 May 11, 2023
@melekes
Copy link
Contributor Author

melekes commented May 11, 2023

Don't understand why libp2p-identity fails to compile:

[2023-05-11 11:17:53]     Checking libp2p-identity v0.1.2
[2023-05-11 11:17:53] error[E0599]: no method named `encode_protobuf` found for reference `&PublicKey` in the current scope
[2023-05-11 11:17:53]   --> /cargo_home/registry/src/github.com-1ecc6299db9ec823/libp2p-identity-0.1.2/src/peer_id.rs:69:27
[2023-05-11 11:17:53]    |
[2023-05-11 11:17:53] 69 |         let key_enc = key.encode_protobuf();
[2023-05-11 11:17:53]    |                           ^^^^^^^^^^^^^^^ method not found in `&PublicKey`
[2023-05-11 11:17:53] 

encode_protobuf is clearly present...

};

let (transport, bandwidth) = bandwidth::BandwidthLogging::new(transport);

let authentication_config =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the biggest change in 0.51.3 along with deprecating mplex

@altonen
Copy link
Contributor

altonen commented May 11, 2023

Don't understand why libp2p-identity fails to compile:

[2023-05-11 11:17:53]     Checking libp2p-identity v0.1.2
[2023-05-11 11:17:53] error[E0599]: no method named `encode_protobuf` found for reference `&PublicKey` in the current scope
[2023-05-11 11:17:53]   --> /cargo_home/registry/src/github.com-1ecc6299db9ec823/libp2p-identity-0.1.2/src/peer_id.rs:69:27
[2023-05-11 11:17:53]    |
[2023-05-11 11:17:53] 69 |         let key_enc = key.encode_protobuf();
[2023-05-11 11:17:53]    |                           ^^^^^^^^^^^^^^^ method not found in `&PublicKey`
[2023-05-11 11:17:53] 

encode_protobuf is clearly present...

the import for libp2p-identity for sc-consensus is defined as libp2p-identity = { version = "0.1.2", features = ["peerid"] }. If you include ed25519 as a feature, does it work then?

@altonen
Copy link
Contributor

altonen commented May 11, 2023

It looks like it worked because now it fails in sc-peerset while cargo-check for sc-consensus passed

@melekes
Copy link
Contributor Author

melekes commented May 12, 2023

Don't understand why libp2p-identity fails to compile:

[2023-05-11 11:17:53]     Checking libp2p-identity v0.1.2
[2023-05-11 11:17:53] error[E0599]: no method named `encode_protobuf` found for reference `&PublicKey` in the current scope
[2023-05-11 11:17:53]   --> /cargo_home/registry/src/github.com-1ecc6299db9ec823/libp2p-identity-0.1.2/src/peer_id.rs:69:27
[2023-05-11 11:17:53]    |
[2023-05-11 11:17:53] 69 |         let key_enc = key.encode_protobuf();
[2023-05-11 11:17:53]    |                           ^^^^^^^^^^^^^^^ method not found in `&PublicKey`
[2023-05-11 11:17:53] 

encode_protobuf is clearly present...

the import for libp2p-identity for sc-consensus is defined as libp2p-identity = { version = "0.1.2", features = ["peerid"] }. If you include ed25519 as a feature, does it work then?

Yes 🙌 Thanks for your help 🙏

@melekes
Copy link
Contributor Author

melekes commented May 12, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 4ce0b74 into paritytech:master May 12, 2023
3 checks passed
@melekes melekes deleted the anton/13537-upgrade-to-libp2p-0.51.0 branch May 12, 2023 07:12
pgherveou pushed a commit that referenced this pull request May 12, 2023
* client/network: upgrade to libp2p 0.51.0

* make discovery.rs compile

* make peer_info.rs compile

* changes to notifications and request-response proto

* make service.rs compile

* towards making request_responses.rs compile

* make request_responses.rs compile

* make request_responses.rs compile

* fix notifications/behaviour.rs tests

* fix warnings

* remove old code

* allow deprecated code (temporary)

* upgrade to libp2p 0.51.1

* add TODO for behaviour tests

* return empty vec if peer_id is absent

#13587 (comment)

fyi: I don't really know what the old behaviour was.

* update comment to reflect new defaults

Closes #13338

* Revert "update comment to reflect new defaults"

This reverts commit 7a981ab.

* remove config.rs (from wrong merge)

* upgrade to libp2p 0.51.2

* fix formatting

* use handle_pending_outbound_connection in networt_state RPC

* update deps

* use re-exports when we use other libp2p packages

* Apply suggestions from code review

Co-authored-by: Dmitry Markin <dmitry@markin.tech>

* format code

* handle potential errors in network_state RPC

* only update libp2p crate

* update libp2p-core

* fix docs

* use libp2p-identity instead of libp2p

where it's possible. libp2p-identity is much smaller, hence makes sense
to use it instead of larger libp2p crate.

* Update client/network/src/discovery.rs

Co-authored-by: Aaro Altonen <48052676+altonen@users.noreply.github.com>

* update Cargo.lock

* add comment for per_connection_event_buffer_size

current value is somewhat arbitrary and needs to be tweaked depending on
memory usage and network worker sleep stats.

* fix link format

* update Cargo.lock

* upgrade to libp2p 0.51.3

* deprecate mplex

* Revert "deprecate mplex"

This reverts commit 9e25820.

* Revert "upgrade to libp2p 0.51.3"

This reverts commit 6544dd4.

* use new libp2p version in `statement` crate

* pin version temporarily

* libp2p 0.51.3

* deprecate mplex

* deprecate legacy noise handshake

* fix build error

* update libp2p-identity

* enable libp2p-identity:ed25519 feature in sc-consensus

* enable ed25519 for peerset as well

---------

Co-authored-by: Dmitry Markin <dmitry@markin.tech>
Co-authored-by: Aaro Altonen <48052676+altonen@users.noreply.github.com>
Co-authored-by: parity-processbot <>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* client/network: upgrade to libp2p 0.51.0

* make discovery.rs compile

* make peer_info.rs compile

* changes to notifications and request-response proto

* make service.rs compile

* towards making request_responses.rs compile

* make request_responses.rs compile

* make request_responses.rs compile

* fix notifications/behaviour.rs tests

* fix warnings

* remove old code

* allow deprecated code (temporary)

* upgrade to libp2p 0.51.1

* add TODO for behaviour tests

* return empty vec if peer_id is absent

paritytech#13587 (comment)

fyi: I don't really know what the old behaviour was.

* update comment to reflect new defaults

Closes paritytech#13338

* Revert "update comment to reflect new defaults"

This reverts commit 7a981ab.

* remove config.rs (from wrong merge)

* upgrade to libp2p 0.51.2

* fix formatting

* use handle_pending_outbound_connection in networt_state RPC

* update deps

* use re-exports when we use other libp2p packages

* Apply suggestions from code review

Co-authored-by: Dmitry Markin <dmitry@markin.tech>

* format code

* handle potential errors in network_state RPC

* only update libp2p crate

* update libp2p-core

* fix docs

* use libp2p-identity instead of libp2p

where it's possible. libp2p-identity is much smaller, hence makes sense
to use it instead of larger libp2p crate.

* Update client/network/src/discovery.rs

Co-authored-by: Aaro Altonen <48052676+altonen@users.noreply.github.com>

* update Cargo.lock

* add comment for per_connection_event_buffer_size

current value is somewhat arbitrary and needs to be tweaked depending on
memory usage and network worker sleep stats.

* fix link format

* update Cargo.lock

* upgrade to libp2p 0.51.3

* deprecate mplex

* Revert "deprecate mplex"

This reverts commit 9e25820.

* Revert "upgrade to libp2p 0.51.3"

This reverts commit 6544dd4.

* use new libp2p version in `statement` crate

* pin version temporarily

* libp2p 0.51.3

* deprecate mplex

* deprecate legacy noise handshake

* fix build error

* update libp2p-identity

* enable libp2p-identity:ed25519 feature in sc-consensus

* enable ed25519 for peerset as well

---------

Co-authored-by: Dmitry Markin <dmitry@markin.tech>
Co-authored-by: Aaro Altonen <48052676+altonen@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade libp2p to 0.51.0
5 participants