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

fix(relay): external addr not expiring when relayed listen addr expire #5577

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

stormshield-frb
Copy link
Contributor

@stormshield-frb stormshield-frb commented Aug 29, 2024

Description

We encountered a bug when our app is connected and listening through a relay and that it decided to stop listening through the relay but still let the connection with the relay open (some other streams are opened with this peer), then the previously confirmed ExternalAddress does not expire.

The PR solves this problem and simplify a little bit the way to emit ExternalAddrConfirmed and ExternalAddrExpired by only watching NewListenAddr and ExpiredListenAddr events if they are notifying about a relayed listen address.

Also, when renewing reservation, NewListenAddr should not be emitted again if the same listen_addr is given, but it should emit a ListenAddrExpired event if a previously reserved one was not reserved again.

Notes & open questions

I have also included a correlated change by removing the /p2p/<local_peer_id> of the relayed listen addresses and external addresses (corresponding to a listen addresses). Indeed, every other transport does not include it so I think it is better that the relay transport acts the same. If necessary, I can do a separated PR.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@stormshield-frb stormshield-frb force-pushed the fix/relay-not-expiring-external-address branch from 96bff78 to 6aff4f8 Compare August 29, 2024 08:19
@stormshield-frb stormshield-frb changed the title fix(relay): relayed external address not expiring when stopping to listen through relay fix(relay): external address not expiring when stopping to listen relay Aug 29, 2024
@stormshield-frb stormshield-frb changed the title fix(relay): external address not expiring when stopping to listen relay fix(relay): external addr not expiring when relayed listen addr expire Aug 29, 2024
@stormshield-frb stormshield-frb marked this pull request as draft August 29, 2024 08:34
@stormshield-frb stormshield-frb force-pushed the fix/relay-not-expiring-external-address branch 3 times, most recently from 5bceae6 to d24cf31 Compare August 29, 2024 15:33
@stormshield-frb stormshield-frb marked this pull request as ready for review August 29, 2024 15:34
@stormshield-frb stormshield-frb force-pushed the fix/relay-not-expiring-external-address branch 2 times, most recently from eeefabb to cfb5cd1 Compare August 29, 2024 20:02
@stormshield-frb stormshield-frb force-pushed the fix/relay-not-expiring-external-address branch from cfb5cd1 to 2918c54 Compare August 29, 2024 20:05
@dariusc93
Copy link
Member

Thanks! Just as an fyi, there is no need to force push as the commits will all be squashed when it is merged.

@dariusc93 dariusc93 requested a review from jxs September 14, 2024 17:28
@stormshield-frb stormshield-frb force-pushed the fix/relay-not-expiring-external-address branch from 479b360 to bac825e Compare October 24, 2024 08:29
Copy link
Contributor

mergify bot commented Nov 20, 2024

This pull request has merge conflicts. Could you please resolve them @stormshield-frb? 🙏

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.

3 participants