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

Complete migration to v2 primitives & parachain-host #4803

Closed
rphmeier opened this issue Jan 28, 2022 · 7 comments · Fixed by #5037
Closed

Complete migration to v2 primitives & parachain-host #4803

rphmeier opened this issue Jan 28, 2022 · 7 comments · Fixed by #5037
Labels
I8-refactor Code needs refactoring.

Comments

@rphmeier
Copy link
Contributor

We're currently using a mix of v1 and v2, which is not great. We should move everything to v2 and fully define v2. Re-exporting stuff from v1 is fine when it's still the same as v1.

@rphmeier rphmeier added the I8-refactor Code needs refactoring. label Jan 28, 2022
@rphmeier
Copy link
Contributor Author

This means updating everything to use polkadot_primitives::v2::... whenever it just works and adding code to handle the differences where necessary. Might need a runtime upgrade first. Further investigation needed, let's discuss here.

@eskimor
Copy link
Member

eskimor commented Jan 28, 2022

Actually, when reading through your Element message, it would make more sense to me if we did not import v1 primitives, but instead just start v2 as a copy of v1 and have modifications then. v1 should then be treated as immutable and we can delete it eventually without any issues.

@ordian
Copy link
Member

ordian commented Jan 28, 2022

Initially I made primitives::v2 re-export all of the primitives::v1 (except for SessionInfo). This way we'd switch all code to primitives::v2, but keep v1 immutable. But it turned out to be a pain for reviewers and for me to constantly resolve all the merge conflicts until it was merged. If we want to bundle a few other changes, I'd suggest introducing v3 and moving v1 and v2 into that.

@rphmeier
Copy link
Contributor Author

rphmeier commented Jan 29, 2022

instead just start v2 as a copy of v1 and have modifications then. v1 should then be treated as immutable and we can delete it eventually without any issues.

Yeah, this is the most 'pure' thing to do but it also means that all subsystems need to be totally adapted to the new v2 types.

I find that re-exporting means that we can adjust only affected subsystems as the v2 types would be exactly the same. But we probably want to refactor the RuntimeApiSubsystem to help us navigate the migrations anyway, and that would require a bit of rewriting as well.

@rphmeier
Copy link
Contributor Author

rphmeier commented Feb 2, 2022

For example, I think that the RuntimeApiRequest enum should be renamed RuntimeApiV1Request and that RuntimeApiMessage::Request should be named RuntmeApiMessage::RequestV1. Then we can have V1, V2, VStaging (VStaging is the 'workbench' spec which is flexible, whereas others aren't).

@rphmeier
Copy link
Contributor Author

rphmeier commented Feb 21, 2022

So the specific proposal for work here is

  1. Explicitly rename RuntimeApiRequest to RuntimeApiV2Request
  2. Add a RuntimeApiVersion request message to RuntimeApiVersion
  3. Update subsystem_util logic to place helpers for this stuff into a v2 module
  4. Update all imports of polkadot-primitives::v1::* to use v2
  5. Remove polkadot-primitives::v0 and polkadot-primitives::v1 and move everything necessary into v2
  6. Ensure that v2contains everything necessary

@ordian I guess this can be done as soon as the v2 API hits Kusama & Polkadot?

@ordian
Copy link
Member

ordian commented Feb 22, 2022

@ordian I guess this can be done as soon as the v2 API hits Kusama & Polkadot?

0.9.17 (for Polkadot) has been branched off so we're not blocked on this to be merged into master
kusama is already on v2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I8-refactor Code needs refactoring.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants