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

P2P topic IDs should not include Minor/Patch versions of the RuntimeCommitteeProtocol #3311

Closed
ptrus opened this issue Sep 23, 2020 · 2 comments · Fixed by #3346
Closed

P2P topic IDs should not include Minor/Patch versions of the RuntimeCommitteeProtocol #3311

ptrus opened this issue Sep 23, 2020 · 2 comments · Fixed by #3346
Assignees
Labels
c:breaking/runtime Category: breaking runtime changes c:bug Category: bug c:p2p Category: P2P

Comments

@ptrus
Copy link
Member

ptrus commented Sep 23, 2020

P2P topic IDs include RuntimeCommitteeProtocol version:

func runtimeIDToTopicID(runtimeID common.Namespace) string {
return version.RuntimeCommitteeProtocol.String() + "/" + runtimeID.String()
}

However since we are following SemVer, Minor and Patch version changes to the RuntimeCommitteeProtocol should be backwards compatible therefore also not resulting in a changed topic ID.

@ptrus ptrus added c:bug Category: bug c:p2p Category: P2P labels Sep 23, 2020
@ptrus
Copy link
Member Author

ptrus commented Sep 23, 2020

The topics should probably also include chain IDs (or something similar to separate different network meshes).

@kostko
Copy link
Member

kostko commented Sep 23, 2020

Yeah so while the messages themselves use the chain domain separation context so they wouldn't be valid, we should probably separate the meshes to avoid needless messages.

@kostko kostko added c:breaking/consensus Category: breaking consensus changes c:breaking/runtime Category: breaking runtime changes and removed c:breaking/consensus Category: breaking consensus changes labels Sep 24, 2020
@ptrus ptrus self-assigned this Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:breaking/runtime Category: breaking runtime changes c:bug Category: bug c:p2p Category: P2P
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants