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

refactor: circuit relay to async #477

Merged
merged 9 commits into from
Nov 29, 2019
Merged

Conversation

jacobheun
Copy link
Contributor

@jacobheun jacobheun commented Nov 14, 2019

Relay

  • Refactors Circuit Relay to use async/await
  • Makes Circuit Relay abide by the Transport Interface
    • Note: I left out the interface tests for the time being because they'll need to be hacked a bit to get them working because they only leverage a single transport/peer. Circuit needs 3 to work properly.
  • All Circuit Relay connections are now created as libp2p Connections. Their MultiaddrConnection is the circuit relay stream of the underlying connection they are using.
  • new Adding a known relay address to your multiaddrs on startup (/p2p-circuit/<relay multiaddr>/p2p/<relay peer id> will cause the node to dial that relay.
  • new If a relay dial fails and we were not already connected to the relay, we will now properly disconnect. This wasn't the case before.

Note: This does not include active relay dialing (Relays wont dial unconnected destination peers). However, this is a low priority. In most instances we shouldn't try dialing a peer we're not connected to, because this means the dialing peer should also be able to connect to them. The only instance this is not the case, is if the relay is in a private network with the destination node and is the only exposed node. Private networks should be explicitly connecting to their exposed relay in this instance.

Other fixes

  • The Registrar was not getting rid of its tracked connections when libp2p stopped. It now has a close method which is called on libp2p.stop().
  • Transport Manager was clearing it's listeners map on stop, which is created on libp2p instantiation, not startup which caused issues with restarts. This is fixed.
  • If an internal dial error already had an error code it was being overridden with a generic dial error code. This makes debugging more difficult. Now the error will be rethrown if it already has a code

@jacobheun jacobheun changed the title refactor: circuit relay to async [WIP] refactor: circuit relay to async Nov 14, 2019
@jacobheun jacobheun changed the title [WIP] refactor: circuit relay to async refactor: circuit relay to async Nov 28, 2019
@jacobheun jacobheun marked this pull request as ready for review November 28, 2019 15:31
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

Great work @jacobheun

Found smaller stuff, but this should be almost ready to go

src/circuit/circuit/hop.js Outdated Show resolved Hide resolved
src/circuit/circuit/hop.js Outdated Show resolved Hide resolved
*/
getRawConn () {
return this.conn
return this.stream
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 leverage this to better name this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not being used anywhere so I will just remove it.

src/circuit/index.js Outdated Show resolved Hide resolved
src/circuit/index.js Outdated Show resolved Hide resolved
src/circuit/listener.js Outdated Show resolved Hide resolved
src/circuit/listener.js Outdated Show resolved Hide resolved
src/circuit/stream-to-conn.js Outdated Show resolved Hide resolved
src/circuit/stream-to-conn.js Outdated Show resolved Hide resolved
@jacobheun
Copy link
Contributor Author

I switched tests over to use chai-as-promised to get better visibility into test failures, and then fixed an issue with needing to reset the multiaddrs of the test nodes for the relay suite due to the fix in #485

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@jacobheun jacobheun merged commit 3dd5ff4 into refactor/async-await Nov 29, 2019
@jacobheun jacobheun deleted the refactor/circuit branch November 29, 2019 15:41
jacobheun added a commit that referenced this pull request Dec 12, 2019
* refactor: add dialing over relay support

* chore: fix lint

* fix: dont clear listeners on close

* fix: if dial errors already have codes, just rethrow them

* fix: clear the registrar when libp2p stops

* fix: improve connection maintenance with circuit

* chore: correct feedback

* test: use chai as promised

* test(fix): reset multiaddrs on dial test
jacobheun added a commit that referenced this pull request Jan 24, 2020
* refactor: add dialing over relay support

* chore: fix lint

* fix: dont clear listeners on close

* fix: if dial errors already have codes, just rethrow them

* fix: clear the registrar when libp2p stops

* fix: improve connection maintenance with circuit

* chore: correct feedback

* test: use chai as promised

* test(fix): reset multiaddrs on dial test
maschad pushed a commit to maschad/js-libp2p that referenced this pull request Jun 21, 2023
This dep is used, unsure why dep-check says it isn't.
maschad pushed a commit to maschad/js-libp2p that referenced this pull request Jun 21, 2023
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.

2 participants