Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Figure out the situation with register_notifications_protocol #6827

Closed
tomaka opened this issue Aug 5, 2020 · 1 comment · Fixed by #7646
Closed

Figure out the situation with register_notifications_protocol #6827

tomaka opened this issue Aug 5, 2020 · 1 comment · Fixed by #7646

Comments

@tomaka
Copy link
Contributor

tomaka commented Aug 5, 2020 •
edited
Loading

The NetworkService::register_notifications_protocol method is a hack, as explained here.
The problem with this method is that we might have started already connecting to other nodes.

At the moment, if two nodes connect before register_notifications_protocol has been called, and if then we try to send a message on the given protocol, it will be sent on the legacy substream as a fallback.

This is however getting in the way of removing the legacy substream. cc #5670

The proper fix is to refactor Substrate to pass the list of protocols in the NetworkConfiguration, but that's a lot of work.
@tomaka tomaka mentioned this issue Aug 6, 2020
Merged
@tomaka tomaka mentioned this issue Aug 31, 2020
Merged
@tomaka tomaka mentioned this issue Nov 18, 2020
Merged
@tomaka
Copy link
Contributor Author

tomaka commented Nov 18, 2020

Update: the legacy substream is no longer in use. If two nodes connect before register_notifications_protocol has been called, messages are simply lost.

#6833 was done in order to prevent this situation from happening.

@tomaka tomaka mentioned this issue Dec 1, 2020
Merged
@ghost ghost closed this as completed in #7646 Dec 2, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@tomaka