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

feat: add support for specifying noise and secio #32

Closed
wants to merge 3 commits into from

Conversation

jacobheun
Copy link
Contributor

@jacobheun jacobheun commented Feb 9, 2020

Depends on libp2p/go-libp2p-daemon#158
Depends on libp2p/js-libp2p-daemon#32

The connect tests can now be run with different configurations to make it easier to run those suites for both secio and noise. It doesn't currently support the ability to test fallback strategies (ie: using secio when the other node doesn't know noise).

Using Local Daemons

I've added the ability to specify local daemons when running the tests to make verifying go daemon changes easier. Doing this is noted in the readme.

TODO

  • Get interop working. Note: Tests are failing in CI due to the current go-libp2p-daemon version not gracefully handling extraneous parameters. There is an underlying error

feat: add ability to specify local daemons

test: run connect tests with multiple configs
@jacobheun
Copy link
Contributor Author

jacobheun commented Feb 12, 2020

There is an issue with noise interop right now. js to go hangs, and go to js receives the following error: failed to negotiate security protocol: runHandshake_xx initiator stage 1 fail: xx_recvHandshakeMessage read length err=EOF

I tested this by installing the WIP go-libp2p-daemon branch and updating to the latest version of the noise repo go get -u github.com/libp2p/go-libp2p-noise.

cc: @yusefnapora @mpetrunic

@@ -131,6 +131,9 @@ class Daemon {
}

const daemon = execa(this._binPath, execOptions)
if (process.env.DEBUG) {
daemon.stderr.pipe(process.stderr)
Copy link
Member

Choose a reason for hiding this comment

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

Aren't library logs suppose to be in process.stdout?

Might be useful to pass custom env to execa, I think something like execa(this._binPath, execOptions, {env: {DEBUG: "libp2p:noise, libp2p:secio"}}) should be supported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All debug logs are going to stderr as we're just using the defaults in the debug module atm.

execa forwards env to the new process, so it shouldn't be necessary as we only need forwarding right now. We don't need the conditional though.

@vasco-santos
Copy link
Member

FYI, with the latest changes in master, I created #40 to replace this PR considering that with would be easier than rebasing this PR.

@jacobheun
Copy link
Contributor Author

Closing in favor of #40

@jacobheun jacobheun closed this May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants