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.

Sign up for GitHub

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

Jump to bottom

Add yamux as first streamMuxer #5804

Open
dapplion opened this issue Jul 26, 2023 · 1 comment · Fixed by #5886
Open

Add yamux as first streamMuxer #5804

dapplion opened this issue Jul 26, 2023 · 1 comment · Fixed by #5886
Assignees
@wemeetagain
Labels
meta-feature-request Issues to track feature requests. meta-investigate Issues found that require further investigation and may not have a specific resolution/fix prio-medium Resolve this some time soon (tm).

Comments

@dapplion
Copy link
Contributor

Problem description

Beacon chain spec expects implementations to support yamux.

Yamux is a better multiplexer than yamux per-spec as it supports per stream back-pressure. However Javascript mplex implementation is more battle tested and has served us well, while yamux is new and may have bugs or performance issues

Solution description

@chainsafe/libp2p-yamux is fully developed now, we should attempt to incorporate it by heavily testing it

Additional context

No response
@dapplion dapplion added the meta-feature-request Issues to track feature requests. label Jul 26, 2023
@dapplion dapplion mentioned this issue Jul 26, 2023
Closed
4 tasks
@wemeetagain wemeetagain mentioned this issue Aug 14, 2023
Merged
@philknows philknows added this to the v1.12.0 milestone Aug 15, 2023
@philknows philknows modified the milestones: v1.12.0, v1.13.0 Oct 10, 2023
@philknows philknows added prio-medium Resolve this some time soon (tm). meta-investigate Issues found that require further investigation and may not have a specific resolution/fix labels Nov 7, 2023
@philknows philknows removed this from the v1.13.0 milestone Aug 8, 2024
@philknows
Copy link
Member

Some suggestions from ACDC 139 based on our testing done for yamux. Because yamux and mplex are not working well together, there were suggestions to:

  • Have options for users to switch between yamux and mplex rather than enabling both.
  • On startup, we can base the first streamMuxer on how many peers we get using either option and can set some threshold to switch over.

@philknows philknows reopened this Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wemeetagain wemeetagain

Labels
meta-feature-request Issues to track feature requests. meta-investigate Issues found that require further investigation and may not have a specific resolution/fix prio-medium Resolve this some time soon (tm).
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add yamux stream muxer ChainSafe/lodestar
3 participants
@wemeetagain @dapplion @philknows