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

v0.1.0 #14

Merged
merged 16 commits into from
Oct 21, 2017
Merged

v0.1.0 #14

merged 16 commits into from
Oct 21, 2017

Conversation

dryajov
Copy link
Member

@dryajov dryajov commented Aug 16, 2017

NOTE: This issue comment is here for historic purposes, all further tracking is happening in ipfs/js-ipfs#830

dependent PRs:


Relevant issues:


Tasks:

  • Implement the circuit transport
    • Integrated circuit transport in swarm
    • enable swarm dialing over circuit transport
    • proper error handling
    • Implement dialing
      • Implement dialing on relay
      • Implement dialing over a specific relay
      • Implement onion dialing on chained relay addresses
      • Implement telescope dialing of chained relay addresses (we can do with onion dialing for now?)
    • Implement listening - /libp2p/circuit/relay/1.0.0/stop
      • Implement listening on specific circuit addresses
      • Announce circuit addresses over identify
      • check for src address lenght
  • Implement the circuit-relay
    • listen for connections relay requests - /libp2p/circuit/relay/1.0.0/hop
    • dial the requested destination
    • implement active dialing
    • implement passive dialing
    • implement proper error handling
      • check for dst address length
  • make a full fledged transport
  • move tests to proper place
    • add more test coverage
  • integrate with libp2p
    • figure out the best place to plug circuit-relay
  • add relay config section to ipfs
  • add relay tests to js-ipfs
  • latest master for peer-id and peer-info published to npm

Tests:

  • Dialer

    • should dial over relay if no common transport exists
    • should dial over specific relay
      • should dial over specific peer routed relay
      • should dial over specific relay on the provided transport
      • should dial over chained relay address
  • Listener

    • Should receive relayed connections
  • Circuit relay

    • Should listen on /libp2p/circuit/relay/1.0.0/hop and forward connections to dst

dmitriy ryajov added 2 commits April 11, 2017 11:24
chore: adding default readme

feat: reworking as a transport

feat: getting peers communicating over relay (wip)

feat: address in swarm [wip]

feat: adding onion dial

feat: adding onion dialing and tests

feat: make circuit a full fledged transport

refactor: split transport dialer and circuit logic

test: adding dial tests

feat: adding passive/active dialing test

test: adding relay tests

fix: several isues

feat: consolidate and cleanup dialing

feat: handle listenning circuit addresses correctly

feat: make utils a factory

feat: adding StreamHandler to aid with pull-stream read/write

refactor: clean up and refactor relay and listener

tests: adding more relay and listener tests

tests: moving long multiaddr to a fixture

feat: adding _writeErr to handle returning errors in relay.js

fix: cleanup, moving setup code outside of dialer

refactor: renaming Relay to Hop

chore: fixing license

chore: removing custom eslint config

refactor: extract listening functionality into Stop handler

fix: removing unused priority functionality

fix: correct import of safe-buffer

lint: remove unused import

refactor: use async/setImmediate

fix: remove unused constant

refactor: move active/passive check out of _readDstAddr

feat: initial readme write up

fix: output correct circuit addresses

feat: initial reimplementation of /hop /stop as a protobufs

feat: reworking with protobufs

feat: moving SUCCESS response to HOP from STOP

feat: removing multihop (onion) dialing

0.0.3

fix: go-js integration test changes

feat: adding dialer tests
@dryajov dryajov mentioned this pull request Aug 17, 2017
53 tasks
@daviddias daviddias changed the title Feat/v0.1.0 v0.1.0 Aug 23, 2017
@daviddias
Copy link
Member

@dryajov could you update the first comment? Either remove the non-used checkboxes or fix them

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Please update to latest aegir, @dryajov

@daviddias daviddias requested review from victorb and removed request for a user, dignifiedquire and whyrusleeping October 13, 2017 15:35
@daviddias
Copy link
Member

@victorbjelkholm mind reviewing the whole PR and also check if the overall structure matches our common best practices?

@ghost ghost assigned dryajov Oct 15, 2017
@victorb
Copy link
Member

victorb commented Oct 17, 2017

Will review this first thing tomorrow.

circle.yml Outdated
- sudo dpkg -i libnss3*.deb
- sudo apt-get install -f
- sudo apt-get install --only-upgrade lsb-base
- sudo dpkg -i google-chrome.deb
Copy link
Member

Choose a reason for hiding this comment

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

This whole thing looks like a lot of fun, there is no simpler way? I seem to remember that one apt-get install -f should be enough after trying to install the .deb, then try to install the .deb again.

Or, possibly try using gdebi to figure out and install dependencies.

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 think that file is pretty old, I'll grab a more recent one from some related project.

const strMa = ma.toString()
if (!strMa.includes('/p2p-circuit')) {
log.err('invalid circuit address')
throw new Error('invalid circuit address')
Copy link
Member

Choose a reason for hiding this comment

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

Should return the error in the callback here

- circumventing NAT layers
- route mangling for better privacy (matreshka/shallot dialing).

It's also possible to use it for clients that implement exotic transports such as devices that only have bluetooth radios to be reachable over bluethoot enabled relays and become full p2p nodes.
Copy link
Member

Choose a reason for hiding this comment

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

tiny typo: bluethoot

Copy link
Member Author

Choose a reason for hiding this comment

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

that readme needs some serious work tho...


### libp2p-circuit and IPFS

Prior to `libp2p-circuit` there was a rift in the IPFS network, were IPFS nodes could only access content from nodes that speak the same protocol, for example TCP only nodes could only dial to other TCP only nodes, same for any other protocol combination. In practice, this limitation was most visible in JS-IPFS browser nodes, since they can only dial out but not be dialed in over WebRTC or WebSockets, hence any content that the browser node held was not reachable by the rest of the network even through it was announced on the DHT. Non browser IPFS nodes would usually speak more than one protocol such as TCP, WebSockets and/or WebRTC, this made the problem less severe outside of the browser. `libp2p-circuit` solves this problem completely, as long as there are `relay nodes` capable of routing traffic between those nodes their content should be available to the rest of the IPFS network.
Copy link
Member

Choose a reason for hiding this comment

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

TCP only nodes could only dial to other TCP only nodes I don't think this is true (or badly worded at least). A node with a websocket and TCP transport could be dialed to from a TCP only node, or a websocket only node.

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 was trying to convey the fact that if only speaking one protocol nodes could be only dialed over that protocol. Awesome stuff tho... I need to really rework the readme.


### Implementation rational

This module is not a transport, however it implements `interface-transport` interface in order to allow circuit to be plugged with `libp2p-swarm`. The rational behind it is that, `libp2p-circuit` has a dial and listen flow, which fits nicely with other transports, moreover, it requires the _raw_ connection to be encrypted and muxed just as a regular transport's connection does. All in all, `interface-transport` ended up being the correct level of abstraction for circuit, as well as allowed us to reuse existing integration points in `libp2p-swarm` and `libp2p` without adding any ad-hoc logic. All parts of `interface-transport` are used, including `.getAddr` which returns a list of `/p2p-circuit` addresses that circuit is currently listening.
Copy link
Member

Choose a reason for hiding this comment

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

It looks like a transport, it quacks like a transport but it's not a transport?

Copy link
Member Author

Choose a reason for hiding this comment

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

hrm... yeah :)

Copy link
Member Author

Choose a reason for hiding this comment

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

So, it doesn't implement any transport protocol itself, it only hooks into the flow and delegates the communication to the underlying transports through swarm... There was some back and forth on why the interface was chousen, hence the explanation. This might need a bit more clarification as to why it is not a real transport.

README.md Outdated

## License

[MIT](LICENSE) © 2015-2016 Protocol Labs
Copy link
Member

Choose a reason for hiding this comment

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

Just put 2017 here instead

})

it(`should source and dest`, () => {
expect(message.srcPeer).to.not.be.null()
Copy link
Member

Choose a reason for hiding this comment

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

should be deep.equal check rather than not.be.null

Copy link
Member Author

@dryajov dryajov Oct 18, 2017

Choose a reason for hiding this comment

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

hmm, there is nothing to deep.equal to in this test, the message is the result of serializing an object literal to protobuf. I don't think it make sense to deep equal it to itself or de-serialize it one more time and then deep equal? Tho, I'm open to suggestions. 👌 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

actually, I take that back! It makes perfect sense here ;-)

id: `QmQvM2mpqkjyXWbTHSUidUAWN26GgdMphTh9iGDdjgVXCy`,
addrs: [`/ipfs/QmQvM2mpqkjyXWbTHSUidUAWN26GgdMphTh9iGDdjgVXCy`]
}
}, new StreamHandler(conn), (conn) => {
Copy link
Member

Choose a reason for hiding this comment

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

seems the callback is missing err here. Can we also assert that conn is what we expect it to be? Same for the next test handle request with invalid multiaddr

@daviddias daviddias merged commit ebce8cd into master Oct 21, 2017
@daviddias daviddias deleted the feat/v0.1.0 branch October 21, 2017 07:26
@ghost ghost removed the in progress label Oct 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants