Skip to content
This repository has been archived by the owner on Aug 1, 2023. It is now read-only.

Feat/circuit relay #6

Merged
merged 50 commits into from
May 28, 2018
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
81b1e99
feat: port to new ipfsd-ctl
dryajov Dec 21, 2017
7013f88
fix: removing factory
dryajov Dec 23, 2017
65295bf
feat: adopting latest changes to ipfsd-ctl
dryajov Jan 11, 2018
4052ffa
chore: upping ipfsd-ctl version
dryajov Jan 17, 2018
e444539
test: increase timeouts
dryajov Jan 17, 2018
21eaeac
test: fixing circuit tests
dryajov Jan 18, 2018
6cc1628
test: run kad-dht tests in browser
dryajov Jan 18, 2018
4b8907b
wip: adding browser relay tests
dryajov Jan 24, 2018
d40a5bf
increase timeouts
dryajov Jan 25, 2018
9243b65
feat: add missing circuit tests from node runs
dryajov Jan 27, 2018
7a62388
fix: avoid using default API and Gateway ports
dryajov Jan 29, 2018
c61c5fd
feat: adding go to js test
dryajov Jan 29, 2018
90b7320
fix: /ipv4 -> /ip4
dryajov Jan 29, 2018
e4e1790
fix: cant relay if both the relay and one of the nodes are browser
dryajov Jan 29, 2018
46f172f
test: skip `get directory` tests on windows
dryajov Jan 29, 2018
98a7d04
fix: skip repo tests on windows
dryajov Jan 29, 2018
e9e8f73
fix: skip browser tests in node
dryajov Jan 30, 2018
655be31
fix: remove repo tests from browser runs
dryajov Jan 30, 2018
9f91929
fix: timeouts
dryajov Jan 30, 2018
4dd9b5f
fix: timeouts
dryajov Jan 30, 2018
b124d28
feat: increase browser inactivity timeout to make windows hapy
dryajov Jan 30, 2018
d504b65
feat: adding all missing browser tests
dryajov Jan 30, 2018
e938f61
fix: use IPFS_REUSEPORT untille new version of go-ipfs is released
dryajov Feb 2, 2018
d02d608
experiment: remove individual timeouts to rule time out related failures
dryajov Feb 2, 2018
0af7ac5
tests: reworking
dryajov Feb 3, 2018
645bf5f
remove runaway only
dryajov Mar 3, 2018
08d168c
lint
dryajov Mar 3, 2018
14059ed
even better tests
dryajov Mar 3, 2018
4f7451f
test: connect browser nodes with tiemout
dryajov Mar 3, 2018
35253ac
parametrize timeouts
dryajov Mar 3, 2018
1a947d3
reenable skipped test
dryajov Mar 3, 2018
3bb68f9
skip go-browser-browser
dryajov Mar 4, 2018
7157e5d
timeout
dryajov Mar 5, 2018
6ff891e
cleanup based on review
dryajov Mar 6, 2018
e5f617a
fix: naming of funcs
dryajov Mar 6, 2018
59e6a85
fix: naming of funcs
dryajov Mar 6, 2018
e2a007f
split circuit tests
dryajov Mar 6, 2018
c29f3a9
chore: rename files
dryajov Mar 6, 2018
9e9c38c
increase timeout
dryajov Mar 6, 2018
d5dd246
fix: missing import
dryajov Mar 6, 2018
4b3ee06
chore: remove .only
dryajov Mar 6, 2018
efcb48e
chore: add TODO regarding CAN_HOP
dryajov Mar 6, 2018
51a7118
several small fixes
dryajov Mar 6, 2018
06a4b9f
fix: pass options
dryajov Mar 7, 2018
1c261a7
review last commit
dryajov Mar 7, 2018
650876d
chore: updated todo
dryajov Mar 7, 2018
26daf07
chore: up deps
dryajov Mar 17, 2018
2a686e4
fix: daemon type in tests
dryajov Mar 18, 2018
b0e669a
Merge branch 'master' into feat/circuit-relay
daviddias May 28, 2018
9a2efbc
test: skip circuit tests. waiting for go-ipfs 0.4.15
daviddias May 28, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions .aegir.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
'use strict'

const createServer = require('ipfsd-ctl').createServer
const parallel = require('async/parallel')
const rendezvous = require('libp2p-websocket-star-rendezvous')

let rzserver

const server = createServer()
module.exports = {
Expand All @@ -16,8 +20,28 @@ module.exports = {
},
hooks: {
browser: {
pre: server.start.bind(server),
post: server.stop.bind(server)
pre: (done) => {
parallel([
(cb) => server.start(cb),
(cb) => {
rendezvous.start({
port: 24642
}, (err, _rzserver) => {
if (err) {
return done(err)
}
rzserver = _rzserver
cb()
})
}
], done)
},
post: (done) => {
parallel([
(cb) => server.stop(cb),
(cb) => rzserver.stop(cb)
], done)
}
}
}
}
11 changes: 7 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
},
"scripts": {
"lint": "aegir lint",
"test": "aegir test -t node -t browser --no-cors",
"test:node": "aegir test -t node -f test/node.js",
"test:browser": "aegir test -t browser --no-cors -f test/browser.js"
"test": "cross-env IPFS_REUSEPORT=false aegir test -t node -t browser --no-cors",
"test:node": "cross-env IPFS_REUSEPORT=false aegir test -t node -f test/node.js",
"test:browser": "cross-env IPFS_REUSEPORT=false aegir test -t browser --no-cors -f test/browser.js"
},
"pre-commit": [
"lint"
Expand All @@ -45,6 +45,7 @@
"buffer-loader": "0.0.1",
"chai": "^4.1.2",
"cids": "^0.5.2",
"cross-env": "^5.1.3",
"detect-node": "^2.0.3",
"dir-compare": "^1.4.0",
"dirty-chai": "^2.0.1",
Expand All @@ -53,10 +54,12 @@
"form-data": "^2.3.1",
"go-ipfs-dep": "^0.4.13",
"hat": "0.0.3",
"ipfs": "~0.27.7",
"ipfs": "^0.28.0",
Copy link
Member

Choose a reason for hiding this comment

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

Keep using the ~.

Copy link
Member Author

Choose a reason for hiding this comment

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

npm install annoyingly changes the ^ and ~ :(

Copy link
Member

Choose a reason for hiding this comment

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

I know, you just need to check on submitting the PR. Reviewing one's PRs can go a long way.

"ipfs-api": "^18.1.1",
"ipfsd-ctl": "~0.29.1",
"left-pad": "^1.2.0",
"libp2p-websocket-star": "^0.7.2",
Copy link
Member

Choose a reason for hiding this comment

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

Why did you need to import the transport?

"libp2p-websocket-star-rendezvous": "^0.2.2",
"lodash": "^4.17.4",
"mocha": "^4.0.1",
"ncp": "^2.0.0",
Expand Down
3 changes: 2 additions & 1 deletion test/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@
// require('./exchange-files')
// require('./pubsub')
require('./kad-dht')
require('./repo')
require('./circuit')
// require('./repo')
290 changes: 290 additions & 0 deletions test/circuit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,290 @@
/* eslint max-nested-callbacks: ["error", 8] */
/* eslint-env mocha */
'use strict'

const chai = require('chai')
const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)

const parallel = require('async/parallel')
const series = require('async/series')

const isNode = require('detect-node')

const utils = require('./utils/circuit')

const proc = utils.setUpProcNode
const js = utils.setUpJsNode
const go = utils.setUpGoNode

const ws = utils.wsAddr
const star = utils.wsStarAddr
const circuit = utils.circuitAddr

const send = utils.send

const base = '/ip4/127.0.0.1/tcp/0'

const connect = (nodeA, nodeB, relay, timeout, callback) => {
if (typeof timeout === 'function') {
callback = timeout
timeout = 1000
}

series([
(cb) => nodeA.ipfsd.api.swarm.connect(ws(relay.addrs), cb),
(cb) => setTimeout(cb, timeout),
Copy link
Member

Choose a reason for hiding this comment

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

Why Timeout? Swarm should just return after the connect is done.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've tried without the timeouts but it fails in a lot of cases. I believe it is related to WS somehow - but just a hunch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought about it some more and remembered the main reason this is there. Upon establishing a muxed connection, circuit will try to send the CAN_HOP to the incoming peer to identify it as a relay, that I believe, takes time. I'm trying to think of a better way of confirming this, but it sounds plausible so far.

Copy link
Member

Choose a reason for hiding this comment

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

That doesn't make sense. If there is any action caused and needed by swarm.connect, it won't finish (aka call the callback) until it's finished. Adding a timeout after the fact means that that swarm.connect is returning before the connection was finished, which is not the expected behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the action doesn't happen as part of the swarm connect. It uses peer-mux-established event to send the CAN_HOP, thats why the timeout works.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW - adding that to the flow would not be an issue at all, we already have a method that does the discovery (canHop), it would just be a matter of calling it.

Copy link
Member

Choose a reason for hiding this comment

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

Not that. I'm saying that PeerInfo -- https://github.com/libp2p/js-peer-info/blob/master/src/index.js -- should contain the information about the Protocol we know are supported and also if the node can be a relay, so that you can scan through your Peers set and see if the nodes you are connected are indeed relay nodes. This would remove any race condition.

Copy link
Member Author

Choose a reason for hiding this comment

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

should contain the information about the Protocol we know are supported

The PeerInfo should already have all the multiaddrs that the peer is listening on, so that information is there.

and also if the node can be a relay, so that you can scan through your Peers set and see if the nodes you are connected are indeed relay nodes.

AFAIU, this would imply having a flag in the PeerInfo announcing that the peer is a relay?Wouldn't this require changing go's PeerInfo as well?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. Better track this in a separate issue and add a TODO on the timeout that points to it :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, agree! We desperately need a sane service discovery mechanism...

(cb) => nodeB.ipfsd.api.swarm.connect(ws(relay.addrs), cb),
(cb) => setTimeout(cb, timeout),
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

(cb) => nodeA.ipfsd.api.swarm.connect(circuit(nodeB.addrs), cb)
], callback)
}

const timeout = 80 * 1000
const baseTest = {
connect,
send,
timeout
}

let tests = {
Copy link
Member

Choose a reason for hiding this comment

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

Why let and not const?

Copy link
Member Author

Choose a reason for hiding this comment

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

gets merged with the browser tests. I'll separated into two different files as you suggested bellow.

'go-go-go': {
create: (callback) => series([
(cb) => go([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb)
], callback)
},
'js-go-go': {
create: (callback) => series([
(cb) => js([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb)
], callback)
},
'go-go-js': {
create: (callback) => series([
(cb) => go([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb)
], callback)
},
'js-go-js': {
create: (callback) => series([
(cb) => js([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb)
], callback)
},
'go-js-go': {
create: (callback) => series([
(cb) => go([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb)
], callback)
},
'js-js-go': {
create: (callback) => series([
(cb) => js([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb)
], callback)
},
'go-js-js': {
create: (callback) => series([
(cb) => go([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb)
], callback)
},
'js-js-js': {
create: (callback) => series([
(cb) => js([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb)
], callback)
}
}

const connWithTimeout = (timeout) => {
return (nodeA, nodeB, relay, callback) => {
connect(nodeA, nodeB, relay, timeout, callback)
}
}
const browser = {
Copy link
Member

Choose a reason for hiding this comment

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

Can we have the browser ones in a separate file? This file is huge.

'browser-go-js': {
Copy link
Member

@victorb victorb Mar 5, 2018

Choose a reason for hiding this comment

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

Keeping track of these mutations for a human is not super easy. Maybe we could generate the combinations given a list of implementations to test?

For example, passing ['js', 'go', 'browser'] to a function that can figure out all permutations of those elements. Would also be really useful to have a parameter that would output combinations of two elements rather than three (circuit needs three elements, middle is relay, but for the rests of interop tests, we only need two elements connecting to each other).

Might be a bit out-of-scope for this PR though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I like pre-generating as well. It gets a little tricky because of how nodes connect - nodes to relay or relay to node. I took a stab at a more DSLish approach here - 0af7ac5#diff-3049b1205b403a9fbacb7734aa167858, which demostrates the amount of data I had to encode in order to perform the spawning and connecting the nodes. I'll play around with it a bit more and I'm sure we can clean this up further, but I think this is "good enough" for now.

create:
(callback) => series([
(cb) => proc([], cb),
(cb) => go([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb)
], callback),
connect: connWithTimeout(1500)
Copy link
Member

Choose a reason for hiding this comment

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

Why do browser connects need a special timeout?

},
'browser-go-go': {
create: (callback) => series([
(cb) => proc([], cb),
(cb) => go([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb)
], callback),
connect: connWithTimeout(1500)
},
'browser-js-js': {
create: (callback) => series([
(cb) => proc([], cb),
(cb) => js([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb)
], callback),
connect: connWithTimeout(1500)
},
'browser-js-go': {
create: (callback) => series([
(cb) => proc([], cb),
(cb) => js([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb)
], callback),
connect: connWithTimeout(1500)
},
'js-go-browser': {
create: (callback) => series([
(cb) => js([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb),
(cb) => proc([], cb)
], callback),
connect: connWithTimeout(1500)
},
'go-go-browser': {
create: (callback) => series([
(cb) => go([`${base}/ws`], cb),
(cb) => go([`${base}/ws`], cb),
(cb) => proc([], cb)
], callback),
connect: connWithTimeout(1500)
},
'js-js-browser': {
create: (callback) => series([
(cb) => js([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb),
(cb) => proc([], cb)
], callback),
connect: connWithTimeout(1500)
},
'go-js-browser': {
create: (callback) => series([
(cb) => go([`${base}/ws`], cb),
(cb) => js([`${base}/ws`], cb),
(cb) => proc([], cb)
], callback),
connect: connWithTimeout(1500)
},
'go-browser-browser': {
create: (callback) => series([
(cb) => go([`${base}/ws`], cb),
(cb) => proc([`/ip4/127.0.0.1/tcp/24642/ws/p2p-websocket-star`], cb),
(cb) => proc([`/ip4/127.0.0.1/tcp/24642/ws/p2p-websocket-star`], cb)
], callback),
connect: (nodeA, nodeB, relay, callback) => {
series([
(cb) => relay.ipfsd.api.swarm.connect(ws(nodeA.addrs), cb),
(cb) => setTimeout(cb, 2000),
(cb) => relay.ipfsd.api.swarm.connect(star(nodeB.addrs), cb),
(cb) => setTimeout(cb, 2000),
Copy link
Member

Choose a reason for hiding this comment

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

Again, there shouldn't be a need for timeouts here.

(cb) => nodeA.ipfsd.api.swarm.connect(circuit(nodeB.addrs), cb)
], callback)
},
timeout: 100 * 1000,
skip: () => true
},
'js-browser-browser': {
create: (callback) => series([
(cb) => js([`${base}/ws`], cb),
(cb) => proc([`/ip4/127.0.0.1/tcp/24642/ws/p2p-websocket-star`], cb),
(cb) => proc([`/ip4/127.0.0.1/tcp/24642/ws/p2p-websocket-star`], cb)
], callback),
connect: (nodeA, nodeB, relay, callback) => {
series([
(cb) => relay.ipfsd.api.swarm.connect(ws(nodeA.addrs), cb),
(cb) => setTimeout(cb, 2000),
(cb) => relay.ipfsd.api.swarm.connect(star(nodeB.addrs), cb),
(cb) => setTimeout(cb, 2000),
(cb) => nodeA.ipfsd.api.swarm.connect(circuit(nodeB.addrs), cb)
], callback)
},
timeout: 100 * 1000
},
'browser-browser-go': {
create: (callback) => series([
(cb) => proc([`/ip4/127.0.0.1/tcp/24642/ws/p2p-websocket-star`], cb),
(cb) => proc([`/ip4/127.0.0.1/tcp/24642/ws/p2p-websocket-star`], cb),
(cb) => go([`${base}/ws`], cb)
], callback),
connect: (nodeA, nodeB, relay, callback) => {
series([
(cb) => relay.ipfsd.api.swarm.connect(star(nodeA.addrs), cb),
(cb) => setTimeout(cb, 2000),
(cb) => relay.ipfsd.api.swarm.connect(ws(nodeB.addrs), cb),
(cb) => setTimeout(cb, 2000),
(cb) => nodeA.ipfsd.api.swarm.connect(circuit(nodeB.addrs), cb)
], callback)
},
timeout: 100 * 1000,
skip: () => true
},
'browser-browser-js': {
create: (callback) => series([
(cb) => proc([`/ip4/127.0.0.1/tcp/24642/ws/p2p-websocket-star`], cb),
(cb) => proc([`/ip4/127.0.0.1/tcp/24642/ws/p2p-websocket-star`], cb),
(cb) => js([`${base}/ws`], cb)
], callback),
connect: (nodeA, nodeB, relay, callback) => {
series([
(cb) => relay.ipfsd.api.swarm.connect(star(nodeA.addrs), cb),
(cb) => setTimeout(cb, 2000),
(cb) => relay.ipfsd.api.swarm.connect(ws(nodeB.addrs), cb),
(cb) => setTimeout(cb, 2000),
(cb) => nodeA.ipfsd.api.swarm.connect(circuit(nodeB.addrs), cb)
], callback)
}
}
}

describe('circuit', () => {
if (!isNode) {
tests = Object.assign([], tests, browser)
}

Object.keys(tests).forEach((test) => {
let nodes
let nodeA
let relay
let nodeB

tests[test] = Object.assign({}, baseTest, tests[test])
const dsc = tests[test].skip && tests[test].skip() ? describe.skip : describe
dsc(test, function () {
this.timeout(tests[test].timeout)

before((done) => {
tests[test].create((err, _nodes) => {
expect(err).to.not.exist()
nodes = _nodes.map((n) => n.ipfsd)
nodeA = _nodes[0]
relay = _nodes[1]
nodeB = _nodes[2]
done()
})
})

after((done) => parallel(nodes.map((ipfsd) => (cb) => ipfsd.stop(cb)), done))

it('connect', (done) => {
tests[test].connect(nodeA, nodeB, relay, done)
})

it('send', (done) => {
tests[test].send(nodeA.ipfsd.api, nodeB.ipfsd.api, done)
})
})
})
})
7 changes: 7 additions & 0 deletions test/exchange-files.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const join = require('path').join
const os = require('os')
const hat = require('hat')

const isWindows = os.platform() === 'win32'

const rmDir = promisify(rimraf)

const DaemonFactory = require('ipfsd-ctl')
Expand Down Expand Up @@ -175,6 +177,11 @@ describe('exchange files', () => {

// TODO these tests are not fetching the full dir??
describe('get directory', () => dirs.forEach((num) => {
// skipping until https://github.com/ipfs/interop/issues/9 is addressed
if (isWindows) {
return
}

it(`go -> js: depth: 5, num: ${num}`, function () {
this.timeout(50 * 1000)
const dir = tmpDir()
Expand Down
Loading