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

Solving the Stream Muxer interoperability between Go and JS implementations #721

Closed
diasdavid opened this Issue Jan 22, 2017 · 10 comments

Comments

Projects
None yet
4 participants
@diasdavid
Member

diasdavid commented Jan 22, 2017

I'm creating this issue to keep track of all the work that has been put towards this endeavour, as it is becoming harder to have a clear picture of what is left.

  • Available Stream Muxers
    • js-libp2p-multiplex
    • js-libp2p-spdy
    • spdystream (Go) - equivalent to js-libp2p-spdy
    • yamux (Go)
    • muxado (Go)
    • go-multiplex (Go) - equivalent to js-libp2p-multiplex

With https://github.com/libp2p/interface-stream-muxer, we got a standard interface for Stream Multiplex and with it, a very comprehensive battery of tests, which can be found:

The following table presents the current state of each multiplex:

Module interface tests stress tests mega stress test interop
js-libp2p-spdy ✔️ ✔️ ✔️ 𝙓
js-libp2p-multiplex ✔️ ✔️ ✔️ 𝙓
spdystream ✔️ ?? ?? 𝙓
yamux ✔️ ✔️ ✔️ N/A
muxado ✔️ ?? ?? N/A
go-multiplex ✔️ ?? ?? 𝙓

Threads with information about this issue:

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Jan 22, 2017

Member

@VictorBjelkholm how is the state of testing? Have we confirmed that go-multiplex actually passes all the tests like yamux?

What about the tests just between js-libp2p-ipfs-node.js and go-libp2p, do those show any interesting behaviour?

Member

diasdavid commented Jan 22, 2017

@VictorBjelkholm how is the state of testing? Have we confirmed that go-multiplex actually passes all the tests like yamux?

What about the tests just between js-libp2p-ipfs-node.js and go-libp2p, do those show any interesting behaviour?

@diasdavid diasdavid added ready in progress and removed ready labels Jan 23, 2017

@VictorBjelkholm VictorBjelkholm referenced this issue Jan 23, 2017

Closed

Sprint: Browser accesses go-ipfs content #310

26 of 50 tasks complete
@VictorBjelkholm

This comment has been minimized.

Show comment
Hide comment
@VictorBjelkholm

VictorBjelkholm Jan 23, 2017

Member

@diasdavid

@VictorBjelkholm how is the state of testing? Have we confirmed that go-multiplex actually passes all the tests like yamux?

I cannot confirm that either go-multiplex or yamux is passing the tests since I cannot run them. Wrote a comment about this here: libp2p/go-stream-muxer#10 (comment)

Gonna skip muxando for now.

What about the tests just between js-libp2p-ipfs-node.js and go-libp2p, do those show any interesting behaviour?

So, with spdy everything is fine and seemingly working, when testing the echo example, however, when using multiplex, libp2p-nodejs fails with:

/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/pull-pair/index.js:11
      throw new Error('already piped')
      ^

Error: already piped
    at sink (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/pull-pair/index.js:11:13)
    at Connection.consume (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/pull-defer/sink.js:7:17)
    at pull (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/pull-stream/pull.js:43:9)
    at Dialer.handle (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/multistream-select/src/dialer/index.js:47:5)
    at protocolHandshake (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/libp2p-swarm/src/dial.js:183:10)
    at attemptMuxerUpgrade (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/libp2p-swarm/src/dial.js:60:11)
    at ms.select (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/libp2p-swarm/src/dial.js:147:17)
    at f (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/once/once.js:25:25)
    at select (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/multistream-select/src/dialer/index.js:76:16)
    at pullLP.decodeFromReader (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/multistream-select/src/select.js:25:14)

I'm now on a OS where I can reproduce the test-failures we saw with libp2p-swarm and libp2p-nodejs when multiplex is activated, so gonna look into and see if I can figure out why.

Member

VictorBjelkholm commented Jan 23, 2017

@diasdavid

@VictorBjelkholm how is the state of testing? Have we confirmed that go-multiplex actually passes all the tests like yamux?

I cannot confirm that either go-multiplex or yamux is passing the tests since I cannot run them. Wrote a comment about this here: libp2p/go-stream-muxer#10 (comment)

Gonna skip muxando for now.

What about the tests just between js-libp2p-ipfs-node.js and go-libp2p, do those show any interesting behaviour?

So, with spdy everything is fine and seemingly working, when testing the echo example, however, when using multiplex, libp2p-nodejs fails with:

/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/pull-pair/index.js:11
      throw new Error('already piped')
      ^

Error: already piped
    at sink (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/pull-pair/index.js:11:13)
    at Connection.consume (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/pull-defer/sink.js:7:17)
    at pull (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/pull-stream/pull.js:43:9)
    at Dialer.handle (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/multistream-select/src/dialer/index.js:47:5)
    at protocolHandshake (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/libp2p-swarm/src/dial.js:183:10)
    at attemptMuxerUpgrade (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/libp2p-swarm/src/dial.js:60:11)
    at ms.select (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/libp2p-swarm/src/dial.js:147:17)
    at f (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/once/once.js:25:25)
    at select (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/multistream-select/src/dialer/index.js:76:16)
    at pullLP.decodeFromReader (/Users/user/projects/ipfs/js-libp2p-ipfs-nodejs/node_modules/multistream-select/src/select.js:25:14)

I'm now on a OS where I can reproduce the test-failures we saw with libp2p-swarm and libp2p-nodejs when multiplex is activated, so gonna look into and see if I can figure out why.

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Jan 24, 2017

Member

Started a 'how to repro' issue to help everyone debug here: https://github.com/ipfs/js-libp2p-ipfs-nodejs/issues/59

Member

diasdavid commented Jan 24, 2017

Started a 'how to repro' issue to help everyone debug here: https://github.com/ipfs/js-libp2p-ipfs-nodejs/issues/59

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@jackkleeman

This comment has been minimized.

Show comment
Hide comment
@jackkleeman

jackkleeman Feb 23, 2017

Any news on the status of this in browsers? I have no issues from node in spdy, but in browser both spdy and multiplex (after enabling multiplex on the go listener) fails during the handshake, after selecting /spdy/3.1.0 or selecting /mplex/6.7.0. Well, it doesn't fail with an error, it just stalls.

jackkleeman commented Feb 23, 2017

Any news on the status of this in browsers? I have no issues from node in spdy, but in browser both spdy and multiplex (after enabling multiplex on the go listener) fails during the handshake, after selecting /spdy/3.1.0 or selecting /mplex/6.7.0. Well, it doesn't fail with an error, it just stalls.

@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Feb 23, 2017

Member

The version on master is fully interoperable with go-ipfs@0.4.6-rc1 using multiplex on node.js. I am still working through testing multiplex in the browser.

Member

dignifiedquire commented Feb 23, 2017

The version on master is fully interoperable with go-ipfs@0.4.6-rc1 using multiplex on node.js. I am still working through testing multiplex in the browser.

@jackkleeman

This comment has been minimized.

Show comment
Hide comment
@jackkleeman

jackkleeman Feb 23, 2017

Yes it looks like my problem is the outdated ipfs fork used by OpenBazaar. Thanks!

jackkleeman commented Feb 23, 2017

Yes it looks like my problem is the outdated ipfs fork used by OpenBazaar. Thanks!

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Mar 6, 2017

Member

@jackkleeman keep us up to date with OB on top of js-ipfs and go-ipfs. If there are tests we can run before releasing new versions, that would be sweet to make sure we don't break anything by mistake (the same way we do with orbit) :)

Member

diasdavid commented Mar 6, 2017

@jackkleeman keep us up to date with OB on top of js-ipfs and go-ipfs. If there are tests we can run before releasing new versions, that would be sweet to make sure we don't break anything by mistake (the same way we do with orbit) :)

@jackkleeman

This comment has been minimized.

Show comment
Hide comment
@jackkleeman

jackkleeman Mar 6, 2017

Will do, I am in the early stages of porting to JS. We have a dht relay for offline messaging, but aim to build online messaging with js ipfs for outbound and the new relay circuit for inbound. I doubt we will have a friendly test suite for some time :p

jackkleeman commented Mar 6, 2017

Will do, I am in the early stages of porting to JS. We have a dht relay for offline messaging, but aim to build online messaging with js ipfs for outbound and the new relay circuit for inbound. I doubt we will have a friendly test suite for some time :p

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Mar 6, 2017

Member

all right :) Considering this issue done then :)

Member

diasdavid commented Mar 6, 2017

all right :) Considering this issue done then :)

@diasdavid diasdavid closed this Mar 6, 2017

@diasdavid diasdavid removed the ready label Mar 6, 2017

@diasdavid diasdavid referenced this issue Mar 16, 2017

Closed

🚀 0.23 Release 🌟 #795

18 of 22 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment