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

wasm-ext: Replace libp2p-wasm-ext with libp2p-websocket-websys #3611

Closed
Tracked by #2617
thomaseizinger opened this issue Mar 13, 2023 · 10 comments · Fixed by #4102
Closed
Tracked by #2617

wasm-ext: Replace libp2p-wasm-ext with libp2p-websocket-websys #3611

thomaseizinger opened this issue Mar 13, 2023 · 10 comments · Fixed by #4102

Comments

@thomaseizinger
Copy link
Contributor

@vincev built a pure Rust websys-based websocket transport here: https://github.com/vincev/libp2p-websys-transport

We currently offer something similar in https://github.com/libp2p/rust-libp2p/tree/master/transports/wasm-ext but involves Javascript.

The libp2p-wasm-ext transport is a bit more capable because it allows arbitrary transports to be plugged in. In reality, you can only use websocket though. Both WebRTC and WebTransport are not possible because they have a built-in security protocol and multiplexer.

@mxinden Should we retire our libp2p-wasm-ext module in favor of one with websys bindings?

@thomaseizinger thomaseizinger added the decision-pending Marks issues where a decision is pending before we can move forward. label Mar 13, 2023
@dariusc93
Copy link
Member

Honestly, I dont see a downside to migrating over to websys since it would be less code (eg js) to maintain.

@mxinden
Copy link
Member

mxinden commented Mar 23, 2023

I don't have much experience with either. Off the top of my head this sounds like the right step forward.

@thomaseizinger thomaseizinger added difficulty:moderate help wanted and removed decision-pending Marks issues where a decision is pending before we can move forward. labels Mar 23, 2023
@thomaseizinger
Copy link
Contributor Author

@vincev Would you be up for contributing your transport to the mono repo?

@vincev
Copy link

vincev commented Mar 23, 2023

@thomaseizinger sure no problem, how can I help?

@thomaseizinger
Copy link
Contributor Author

Really, all there is to do is deleting the existing libp2p-wasm-ext from this repository and moving your crate into the workspace! We'd also need you do add the @libp2p/rust-libp2p-maintainers teams as owners to the crate on crates.io so we can publish updates going forward.

If that works for you, it'd be great if you could send a PR!

@vincev
Copy link

vincev commented Mar 23, 2023

Sounds good, I'll create a PR in the next few days.

@vincev
Copy link

vincev commented Mar 26, 2023

How do I add you as owners? I tried the following but I am getting an error:

cargo owner -v --list libp2p
    Updating crates.io index
tomaka (Pierre Krieger)
mxinden (Max Inden)
github:paritytech:core-devs (Core devs)
github:paritytech:libp2p-devs (Libp2p devs)
github:libp2p:repos-rust (Repos - Rust)
github:libp2p:rust-libp2p-maintainers (rust-libp2p Maintainers)

and then when I try to add maintainers I get:

cargo owner --add github:libp2p:rust-libp2p-maintainers
    Updating crates.io index
error: failed to invite owners to crate `libp2p-websys-transport` on registry at https://crates.io

Caused by:
  the remote server responded with an error: could not find the github team libp2p/rust-libp2p-maintainers

@thomaseizinger
Copy link
Contributor Author

I am not sure about the exact syntax. @mxinden Can you help?

@thomaseizinger thomaseizinger changed the title wasm-ext: Should we replace libp2p-wasm-ext? wasm-ext: Replace libp2p-wasm-ext with libp2p-websys-transport Mar 29, 2023
@mxinden
Copy link
Member

mxinden commented Apr 5, 2023

Not sure whether this is still an issue.

cargo owner --add github:libp2p:rust-libp2p-maintainers
    Updating crates.io index
error: failed to invite owners to crate `libp2p-websys-transport` on registry at https://crates.io

Caused by:
  the remote server responded with an error: could not find the github team libp2p/rust-libp2p-maintainers

My first guess would be that you can only add a GitHub group in case you are part of that group. Would that make sense?

@vincev
Copy link

vincev commented Apr 5, 2023

My first guess would be that you can only add a GitHub group in case you are part of that group. Would that make sense?

Yes makes sense, but you are right we don't need to do this anymore as you now own libp2p-websys-websocket.

@thomaseizinger thomaseizinger changed the title wasm-ext: Replace libp2p-wasm-ext with libp2p-websys-transport wasm-ext: Replace libp2p-wasm-ext with libp2p-websocket-websys Sep 20, 2023
@thomaseizinger thomaseizinger self-assigned this Oct 1, 2023
@mergify mergify bot closed this as completed in #4102 Oct 10, 2023
mergify bot pushed a commit that referenced this issue Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment