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: circuit v2 #1166

Closed
wants to merge 22 commits into from
Closed

feat: circuit v2 #1166

wants to merge 22 commits into from

Conversation

mpetrunic
Copy link
Member

@mpetrunic mpetrunic commented Feb 25, 2022

closes #1029
Implemented:

  • hop protocol + unit tests
  • stop protocol + unit tests
  • integrated into transport with v1 fallback
  • real reservation store
  • sending reservations and maintaining relays
  • reservation client - establishing reservations on relay

Open questions:

  • ~~ what to use as storage for reservation store? ~~
  • the best way to do periodic refreshing of reservations?
  • is there a way for tagging connections as priority so they don't get removed?

At this point, test/relay/relay.node.js should pass but it doesn't. From my debugging, it breaks at upgrading connection but cannot figure out why (it hangs and then aborts) so I would appreciate somebody with more knowledge of how it works to take a look at it. - fixed

@mxinden
Copy link
Member

mxinden commented Feb 28, 2022

the best way to do periodic refreshing of reservations?

Not sure whether this is a js-libp2p specific question, or a general question. In case it is of some help, below is the reservation renewal state machine in rust-libp2p:

https://github.com/libp2p/rust-libp2p/blob/fd31d61a7f9ca652e4adb006e2a16a8a5a32ccea/protocols/relay/src/v2/client/handler.rs#L588-L716

@mpetrunic
Copy link
Member Author

the best way to do periodic refreshing of reservations?

Not sure whether this is a js-libp2p specific question, or a general question. In case it is of some help, below is the reservation renewal state machine in rust-libp2p:

https://github.com/libp2p/rust-libp2p/blob/fd31d61a7f9ca652e4adb006e2a16a8a5a32ccea/protocols/relay/src/v2/client/handler.rs#L588-L716

Tnx that is certainly useful. But yeah, seems like it's too part question, as I'm also curious where to put that code inside js-libp2p 😅

@mpetrunic
Copy link
Member Author

@achingbrain Could you help me with test/dialing/resolver.spec.js It seems like it's trying to use libp2p from aegir.js as a relay but in circuiv2 it takes too long to create a reservation with that libp2p and test fails before that happens. Any ideas on how to fix/circumvent that.

I could easily fix it if I could get access to that libp2p variable as I could stub reservation before test starts.

@achingbrain achingbrain mentioned this pull request May 18, 2022
@mpetrunic mpetrunic marked this pull request as ready for review July 30, 2022 17:54
@mpetrunic
Copy link
Member Author

@achingbrain @wemeetagain finally got this in a working state. There is probably a lot of weird stuff as this PR has gone trough numerous merges with a lot of conflicts

@BigLep
Copy link
Contributor

BigLep commented Sep 27, 2022

@mpetrunic @achingbrain : what are the next steps for getting this over the line? Is it just about code review?

@mpetrunic
Copy link
Member Author

This is still missing the ability to purge expired reservations but not sure how to architect this as I would need to figure out where to put the reservation store so that it became Startable.

@p-shahi p-shahi mentioned this pull request Oct 17, 2022
@ckousik ckousik mentioned this pull request Nov 7, 2022
1 task
@p-shahi
Copy link
Member

p-shahi commented Nov 7, 2022

closing in favor of #1475

@p-shahi p-shahi closed this Nov 7, 2022
@achingbrain achingbrain deleted the feat/circuit-v2 branch May 18, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Implement Circuit Relay v2 in JS
5 participants