Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: upgrade go-libp2p #851

Closed
wants to merge 1 commit into from
Closed

Conversation

richard-ramos
Copy link
Member

Description

Upgrades go-libp2p to v0.3.2

@status-im-auto
Copy link

status-im-auto commented Oct 31, 2023

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ ba54a90 #1 2023-10-31 16:48:32 ~15 sec nix-flake 📄log
ba54a90 #1 2023-10-31 16:49:19 ~1 min linux 📄log
✖️ ba54a90 #1 2023-10-31 16:49:41 ~1 min tests 📄log
✖️ ba54a90 #1 2023-10-31 16:49:45 ~1 min tests 📄log
ba54a90 #1 2023-10-31 16:51:02 ~2 min android 📄log
ba54a90 #1 2023-10-31 16:51:26 ~3 min ios 📄log
✖️ 84c0a61 #2 2023-10-31 16:55:51 ~21 sec tests 📄log
84c0a61 #2 2023-10-31 16:56:26 ~59 sec linux 📄log
✖️ 84c0a61 #2 2023-10-31 16:56:52 ~1 min nix-flake 📄log
✖️ 84c0a61 #2 2023-10-31 16:56:54 ~1 min tests 📄log
84c0a61 #2 2023-10-31 16:57:30 ~2 min android 📄log
84c0a61 #2 2023-10-31 16:57:33 ~2 min ios 📄log

@richard-ramos
Copy link
Member Author

With the latest go-libp2p, this WARN is being printed:

2023-10-31T12:36:25.909-0400    WARN    p2p-config      config/config.go:304    rcmgr limit conflicts with connmgr limit: conn manager high watermark limit: 192, exceeds the system connection limit of: 1

I opened the following issue libp2p/go-libp2p#2628 to ask about what it means

@richard-ramos
Copy link
Member Author

This PR has to wait until libp2p/go-libp2p-pubsub#547 is solved.

Copy link
Collaborator

@chaitanyaprem chaitanyaprem left a comment

Choose a reason for hiding this comment

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

Looks good, once the pubsub module is updated we can merge.

libp2p.Muxer("/yamux/1.0.0", yamux.DefaultTransport),
libp2p.Muxer("/mplex/6.7.0", mplex.DefaultTransport),
),
libp2p.DefaultMuxers,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like mplex was removed by default in latest version.
Maybe we should still enable mplex explicitly so that nodes which don't support yamux still work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes

Copy link
Collaborator

Choose a reason for hiding this comment

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

@waku-org/js-waku-developers @waku-org/nwaku-developers shall we move to yamux?

Copy link

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

I think yes, it seems straightforward for js-waku

Copy link

Choose a reason for hiding this comment

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

Copy link

@danisharora099 danisharora099 Jan 9, 2024

Choose a reason for hiding this comment

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

yes, sounds good.
yamux is recommended to be used over mplex by libp2p -- allows some stream handling enhancements over mplex & is fully supported by js-libp2p
not entirely sure about the compatibility between the two though

@richard-ramos
Copy link
Member Author

PR replaced by #987

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants