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

Upgrade Substrate #2424

Merged
merged 6 commits into from
Jan 18, 2024
Merged

Upgrade Substrate #2424

merged 6 commits into from
Jan 18, 2024

Conversation

nazar-pc
Copy link
Member

This is a pretty big upgrade and as such I limited it to upgrade of Substrate and those dependencies without which it didn't compile. There are many other dependencies to be updated that I'll take care of in a separate PR.

The first commit upgrades Substrate and changes everything that is broken, so it is a bit larger, further PRs address some deprecations an minor cleanups.

The only deprecation that is not fixed yet is GenericChainSpec::from_genesis, which I now think I know how to fix, but I spent way too much time on it already and will get back to local WIP branch once this PR lands.

Notable upstream changes in Substrate:

Smaller things in Substrate:

There were some small changes in Frontier, but nothing really notable, feel free to check the diff between subspace-v4 and subspace-v5 in our fork.

A few changes from our fork of Substrate were upstreamed, which is nice, but networking changes caused a bunch of conflicts for our patches, so I had to adapt synced state announcement. I also reconsidered approach for sync pausing that allows us to switch to DSN sync and instead of introducing new sync mode after massive changes in paritytech/polkadot-sdk#2467 that made porting of the previous solution painful and invasive and replaced with atomic boolean instead that strategically disables a single operation. Should have the same effect, and while potentially a bit fragile (they can change sync process to request stuff from more places that this boolean will not help with and everything will continue to compile), the change is limited and hopefully temporary.

Overall here are all our patches on top of upstream right now, one of the biggest and most invasive is paritytech/polkadot-sdk#1598 that upstream consistently ignores 😞

As you can see, there will be a few follow-up PRs with cleanup of dependencies and such, but this is the bulk of the work.

Going forward I think it will be interesting to consider Polkadot-SDK's releases so we don't need to patch Frontier, but some of our changes do unfortunately require modification of Frontier to compile at all, not just upstream changes.

I do encourage you to look through all upstream changes in Substrate and Frontier if you have time to do so to familiarize yourself with what is happening.

Code contributor checklist:

Copy link
Member

@NingLin-P NingLin-P left a comment

Choose a reason for hiding this comment

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

Just one nit

@@ -577,6 +577,8 @@ where
pub select_chain: FullSelectChain,
/// Network service.
pub network_service: Arc<NetworkService<Block, <Block as BlockT>::Hash>>,
/// Cross-domain gossip notification service.
pub cdm_gossip_notification_service: Box<dyn NotificationService>,
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps xdm_gossip_notification_service to keep consistency with other places

Copy link
Member Author

Choose a reason for hiding this comment

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

The function is called cdm_* that returns this thing. I have already discussed this with Ved, we will rename a few remaining things from cdm to xdm in a separate PR. Also feel free to push a commit here with renaming.

@nazar-pc nazar-pc added this pull request to the merge queue Jan 18, 2024
Merged via the queue into main with commit d5965cd Jan 18, 2024
10 checks passed
@nazar-pc nazar-pc deleted the upgrade-substrate branch January 18, 2024 21:45
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