-
Notifications
You must be signed in to change notification settings - Fork 42
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
fix: filter for wss in libp2p websocket transport #1989
Conversation
size-limit report 📦
|
ba38972
to
cbddc74
Compare
cbddc74
to
5ffe187
Compare
packages/sdk/src/utils/libp2p.ts
Outdated
@@ -64,11 +64,16 @@ export async function defaultLibp2p( | |||
? { metadata: wakuMetadata(shardInfo) } | |||
: {}; | |||
|
|||
const filter = | |||
typeof process !== "undefined" && process?.env?.NODE_ENV === "test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's handle this the same way it happens here -
js-waku/packages/sdk/rollup.config.js
Line 25 in 3d92f19
"process?.env?.NODE_ENV": JSON.stringify("production") |
this way we don't need to worry about process not being defined or even passing process.env.NODE_ENV
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated. tested it in light-js by making a release from this branch and made sure the replacement works
5ffe187
to
682cc66
Compare
Problem
Current libp2p transport filter allows all connections. This is fine for testing, but in production we want to limit transports to secure WebSockets (wss or tls/ws).
Solution
Allow all transport types when running tests by setting
process.env.NODE_ENV=test
but otherwise use the libp2p filter for secure websockets peersNotes
Contribution checklist:
!
in title if breaks public API;