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

chore: do not mount relay more than once #1650

Merged
merged 1 commit into from
Apr 5, 2023
Merged

Conversation

alrevuelta
Copy link
Contributor

  • Prevents the relay from being mounted more than once.
  • This prevents mounting more relay protocols in the switch, which created unnecessary streams under the hood.

@alrevuelta alrevuelta requested review from LNSD and jm-clius April 5, 2023 11:03
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

From the issue description, it seems like this was a real problem noticed somewhere? I guess I'd first need to understand why mountRelay was called twice in the first place so we can prevent that? Or was this only in a test environment? Afaict this shouldn't be able to happen on any of our apps. If this is just a sanity check or to prevent node.mountRelay being called twice in new apps/tests, it MSTM.

Copy link
Contributor

@LNSD LNSD left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@alrevuelta
Copy link
Contributor Author

@jm-clius I found this issue while testing, but i think its a nice to have due to the implications it has.

what do you mean by MSTM? can't find it online.

@jm-clius
Copy link
Contributor

jm-clius commented Apr 5, 2023

but i think its a nice to have due to the implications it has

Indeed. I was just worried that there may be some bug in wakunode2 that causes relay to be mounted twice, which would be unexplained behaviour. It makes sense as a safety check.

Oh, MSTM = Makes Sense To Me. I thought it was used more widely, but I also now can't find it. 😄

Approving.

@alrevuelta alrevuelta merged commit 5d853b8 into master Apr 5, 2023
14 checks passed
@alrevuelta alrevuelta deleted the dont-mount-relay-twice branch April 5, 2023 13:12
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.

None yet

3 participants