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(libp2p): command to wait for the relay to be ready #1525

Merged
merged 5 commits into from
Feb 7, 2024

Conversation

Larkooo
Copy link
Collaborator

@Larkooo Larkooo commented Feb 7, 2024

adds a command for the client to wait for the relay server to be ready. in cases where the client wants to make sure the relay is listening before trying to publish a message

Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Attention: 618 lines in your changes are missing coverage. Please review.

Comparison is base (374f2bf) 70.06% compared to head (c1b343f) 70.59%.
Report is 29 commits behind head on main.

Files Patch % Lines
bin/katana/src/main.rs 0.00% 94 Missing ⚠️
crates/katana/runner/src/logs.rs 10.00% 72 Missing ⚠️
bin/saya/src/main.rs 0.00% 56 Missing ⚠️
crates/saya/core/src/lib.rs 0.00% 50 Missing ⚠️
crates/katana/primitives/src/genesis/mod.rs 92.40% 35 Missing ⚠️
bin/saya/src/args/mod.rs 0.00% 33 Missing ⚠️
...es/saya/core/src/data_availability/celestia/mod.rs 0.00% 33 Missing ⚠️
crates/katana/runner/src/prefunded.rs 0.00% 31 Missing ⚠️
bin/saya/src/args/data_availability.rs 0.00% 24 Missing ⚠️
bin/sozo/src/ops/migration/mod.rs 30.30% 23 Missing ⚠️
... and 28 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1525      +/-   ##
==========================================
+ Coverage   70.06%   70.59%   +0.53%     
==========================================
  Files         236      257      +21     
  Lines       22531    24975    +2444     
==========================================
+ Hits        15786    17632    +1846     
- Misses       6745     7343     +598     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

@Larkooo should we consider a timeout here, or the client should not be stuck in such situation?

@Larkooo
Copy link
Collaborator Author

Larkooo commented Feb 7, 2024

@Larkooo should we consider a timeout here, or the client should not be stuck in such situation?

what do you mean by a timeout? like a naive approach where we wait a specific amount of time?
it wouldn't be sustainable because clients might act differently from others, for eg. the wasm client takes more time to receive a handshake from the relay server than a native platform client.

also imo we don't want to block clients - because in a game it would just be completely blocking and potentially introduce a freeze.

at first i was going to abstract the waiting into the publish command, which is just going to wait for the relay to be ready before publishing the message but i decided against that as it might be better if the client decides when to do that and the fact that it is explicit

@Larkooo
Copy link
Collaborator Author

Larkooo commented Feb 7, 2024

and also the waiting is needed only for publishing messages - to make sure that the relay server is ready to receive it so we dont lose a message if we try to publish it and the relay server is not ready yet

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Got you, thanks the context.

Perhaps for a subsequent PR we can specify a channel we are waiting the subscription from.
As for now any subscribed event will define the state as ready. I don't know the exact use case in client side you're having, it's in our case enough I guess?

@Larkooo
Copy link
Collaborator Author

Larkooo commented Feb 7, 2024

Perhaps for a subsequent PR we can specify a channel we are waiting the subscription from.

Oh yeah basically, the Subscribed event is only supposed to be coming from the relay server because that event is emitted when a remote peer subscribes to a channel, and the only remote peer we have is the relay server (yes it's weird but it's because the server acts as a relay). So basically for now it's safe to just assume that any subscription event we receive means that the server is ready - because we received its subscription confirmation (to the topic where our message is going to be broadcasted)

Basically the communication is

Client <> Relay server <> Other client

ClientA cannot klnow if ClientB is subscribed to a specific channel because it goes through the relay server - it basically only communicates with the relay server which then forwards those messages.

I hope its more clear

@Larkooo Larkooo merged commit 118ed84 into dojoengine:main Feb 7, 2024
12 checks passed
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