single PR on polkadot-sdk version update

[muharem]

Problem:

An engineer seeking to integrate a feature from a new polkadot-sdk version must submit a pull request (PR) in which the versions of all crates from polkadot-sdk are updated. A single crate in most cases can not be updated, since they are interdependent. This necessitates the engineer to first address all breaking changes, resulting in a single PR covering various topics. This, in turn, leads to an extended review process for the PR.

I believe it's important to agree on how to handle such updates to ensure that everyone has the right expectations.

Examples: #87 #56

Possible Solutions:

There are several approaches that an engineer can take to tackle this:

[updated on 09.01.2024]
please see new proposal in the comments - #101 (comment), #101 (comment)
[updated

Option 1: minimal integration / flexible
In this approach, an engineer updating to a new polkadot-sdk version will address, at the very least, all breaking changes in a manner that prepares it for production. The extent of additional changes is left to the discretion of the engineer.

Option 2: exhaustive
In this scenario, one or more engineers ensure that all anticipated and reasonable features and fixes from the polkadot-sdk are integrated.

Option 3: regressive
Here, an engineer tasked with updating all crates within the runtimes repository applies minimal changes with the objective of making the crates compile. New features are disabled, and adjustments to contracts or their items are made in a restrictive manner. The PR is opened and merged to unblock subsequent PRs. Engineers responsible for specific domains or those who have worked on new features then follow up with their own PRs.
To support this, restrictive default pallet configs may be introduced on FRAME level.

Any other alternatives?

[bkchr]

Option 3: regressive
Here, an engineer tasked with updating all crates within the runtimes repository applies minimal changes with the objective of making the crates compile. New features are disabled, and adjustments to contracts or their items are made in a restrictive manner. The PR is opened and merged to unblock subsequent PRs. Engineers responsible for specific domains or those who have worked on new features then follow up with their own PRs.
To support this, restrictive default pallet configs may be introduced on FRAME level.

IMO this almost the same as Option 1 and I would go this road. The process right now is quite tedious. I hope that it improves with prdocs and better changelogs to make it easier for a dev to integrate the changes here. Explicit changes to the configuration should be done in other subsequent prs that are only focused at this one issue.

[xlc]

we really should engineer polkadot-sdk in such way that it doesn't have to be a breaking change on every release and have nightly release

[muharem]

we really should engineer polkadot-sdk in such way that it doesn't have to be a breaking change on every release and have nightly release

@xlc I totally agree. However, there will be breaking upgrades, and as someone who wants to introduce a new feature from a new release, you may not know what steps to take. This uncertainty might discourage contributions, and others might not anticipate such extensive PRs encompassing all changes. Ultimately, this can slow down the process.

@bkchr for example, a situation where I see the difference in these options is when making a change to an existing config's type parameter. Previously, it was Currency, but now it's Consideration trait bound. In Option 1, I would provide an implementation that's ready for production, while in Option 2, I can set it to a unit type with the expectation that it will be configured in subsequent PRs.

I'm not fond of Option 3 due to the risk of inadvertently releasing something disabled that was not intended to be.

[muharem]

I would better put it as follows,
A new release can encompass the following:

1. breaking changes to existing functionality.
2. breaking changes introducing new functionality.
3. non-breaking changes to existing functionality.
4. new functionality.

The first two can be configured or disabled, while the last two can be setup or left unspecified. Where it is configured or setup as ready for production.

[muharem]

I would vote for the approach where,

1. must be configured as ready for prod
2. should be disabled
3. should be left unset
4. should be left unset

for example in a last such PR, the new weight fee impl should be left unset, and a separate PR should be raised

[bkontur]

There is an upcoming PR with Snowbridge that will require polkadot-sdk 1.6 release. Currently, the fellows' runtimes are aligned with 1.3, so we need to bump to 1.4 and 1.5 before.

I would like to start a PR for the bump to 1.4 and 1.5, similar to what @svyatonik did for the bump from 1.0 to 1.3 here and see how it goes.

Is there a consensus on how to proceed with bumping to 1.4 and 1.5?

[muharem]

@bkchr
Since there is no agreement yet, I think you as an author are free to decide which approach to take.
I would advertise my proposal from above, curious to see how it will go.

I would vote for the approach where,

1. must be configured as ready for prod
2. should be disabled
3. should be left unset
4. should be left unset

for example in a last such PR, the new weight fee impl should be left unset, and a separate PR should be raised

[gilescope]

When polkadot-sdk needs to be upgraded, that should be done as a separate PR (along the lines of option 1) and then the additional functionality that wants to be enabled should be done as an additional PR that builds upon that first PR. Having a mishmash of changing things and upgrading polkadot-sdk seems messy (and makes changes harder to rollback). Ideally there should be boring PRs that upgrade polkadot-sdk for each released node version so that the PRs are easy to review.

[bkontur]

very dumb question, actual repo is bumped to [email protected], but actual latest is [email protected],
so do we need to create at first PR for bumping to [email protected] + fix compilation + fix/add all features and prepare for release and then do the same for 1.4 to 1.5? Or can we just bump from 1.3 to [email protected]?

@muharem how did you bump to 1.2 here? Manually increased major version by 1 or did you use any script or command?

iiuc, @svyatonik Slava did bump from 1.2 to 1.3 with cargo upgrade --pinned --incompatible here.

So, I did cargo upgrade (the same as Slava) from 1.3 -> 1.5 (latest) here: #137.

Sorry, maybe I've missed it, is there any guidance/readme for releasing fellows runtimes?

[muharem]

@bkontur I did it just with find and replace, but there is probably a better way. I would just bump it to the latest.

[bkontur]

My two cents for the discussion: points or lessons learned from a practical point of view when bumping to [email protected]/1.6/1.7

CI for "everything" code-related.

Definitely, we must have reliable CI pipelines for "everything" code-related. Currently, we are missing:

• Checking benchmarks - refer to [CI] Add pipeline for checking benchmarks #197 (I spent so much time fixing benchmarks 😄)
• Verifying transaction_version - see Add transaction_version bump check #69
• Integration testing - see Integration Tests #232
• (Nice-to-have) CI job for regenerating weights.

Release checklist

• We have examples for 1.2.0 or 1.1.0.
• I would suggest creating a GitHub issue template:
  • For a new/next release, we create an issue from this template to ensure nothing is forgotten.
  • It can include best practices/steps, such as:
    • How to bump the repository - see point 3. below.
    • Manual tasks that need to be done (e.g., checking benchmarks, transaction bump). With fixed CI, this step will go away.
    • Suggestions from @xlc: 1.2.0 Release #140 (comment)

Bump polkadot-sdk

When the fellows' repository is behind several polkadot-sdk releases, we should bump to the latest one and avoid creating semi-bumps (unless there is a reason):

• Simply run the command:

  cargo upgrade --pinned --incompatible
  

• There may be some clever solutions, such as utilizing dependabot to check polkadot-sdk crates. When we release a new polkadot-sdk, dependabot can create a pull request to the fellows' repository with actual bump, and so on. It may be worth investigating this possibility.

My story with bumping to 1.5 / 1.6 / 1.7

This is what I did for 1.2.0 release, along with suggestions for the next release, 1.3.0:

• Run cargo upgrade --pinned --incompatible.
• Add the polkadot-sdk version to the changelog, under Based on Polkadot-SDK, for example: CHANGELOG.md#based-on-polkadot-sdk.
• Revert encointer-kusama/Cargo.toml and creating an issue for @brenzi to bump Encointer and its pallets as a separate PR (because actual Encointer pallets depends on the older polkdot-sdk version - I had to fix Encointer runtime to avoid dependencies collisions for system-parachain-constants and chain-spec-builder).
• Run cargo build to ensure Cargo.lock reflects changes (then commit/push).
• Open a draft PR.
• Compile everything in the following loop:
  • Check for compilation errors:
    • Search polkadot-sdk release notes, for example: 1.7.0 or 1.6.0...
      • If an issue wasn't mentioned in the release notes (R0-silent), I looked for similar code in the polkadot-sdk release branch/tag and identified the author of that code/update to ask.
    • Fix obvious and easy errors (then commit/push).
    • Address remaining issues:
      • Implemented fixes with defaults or copied/pasted from polkadot-sdk runtimes.
        • If unsure, added a TODO and tracking issue (or a bullet point referencing the code) and ask the original implementor to check/fix.
    • Repeat the process until everything compiles (manually checking benchmarks has to be replaced by a CI job).

I can't say if this is Option 1, Option 2, or Option 3, it's probably a combination. However, I would choose the pragmatic approach: compile as soon as possible to unblock others from creating additional PRs. Nonetheless, it can't be merged without approvals, and you don't want to merge code that doesn't compile.

[muharem]

we should bump to the latest one and avoid creating semi-bumps

Do you mean we should upgrade to the final sdk version we plan for release, for 1.2 it would be one PR with sdk 1.7 instead three PRs with 1.5, 1.6, 1.7 upgrades?
If yes, I agree with it.
Not sure how this matches with your suggestion for dependancy bot that would create PR for every new version of sdk.

(Nice-to-have) CI job for regenerating weights.

For me this is not nice to have, but we should have it. May be not CI job for every PR, but some command that can be manually initiated.

[bkontur]

we should bump to the latest one and avoid creating semi-bumps

Do you mean we should upgrade to the final sdk version we plan for release, for 1.2 it would be one PR with sdk 1.7 instead three PRs with 1.5, 1.6, 1.7 upgrades? If yes, I agree with it.

Yes, I mean, when doing manually, there is no reason to do semi-bumps and waste time, so just bumping directly to the actual latest polkadot-sdk release is enough.

Not sure how this matches with your suggestion for dependancy bot that would create PR for every new version of sdk.

But, if we want to do it automatically, we can use dependabot (or other tool).

How's the situation today? We've released fellows 1.2.0 with polkadot-sdk 1.7.0 (plus several patches). In the meantime, we've also released polkadot-sdk 1.8, 1.9, 1.10, and this week 1.11 is coming. So, who's working on bumping the fellows repo? Well, unfortunately, nobody. And why?

For instance, if we set up dependabot (or another tool), it could at least create a branch with bumped dependencies for 1.8, 1.9, and/or 1.10. There's a higher chance that somebody could pick it up if they see that some easy stuff is not compiling or whatever (at least a few commits could help). It's a better starting point than having no branch at all, because I guess that people don't even know how to start this bumping manually.

And maybe one day (in an ideal world and with the right alignment of stars and moons), we could have a polkadot-sdk release that makes no breaking changes (yes, we are working on that with DefaultConfigs, semver and other stuff, which is cool). Then, we would be able to merge directly the bumped branch from dependabot, similar to how we do it for other dependencies, like Bump anyhow from 1.0.75 to 1.0.81.

[xlc]

I will suggest a regular release pipeline. e.g. we try to make a Polkadot runtime regular release every month. We can have a bot or something to create a release branch and everything not yet merged will require extra justification for it to be included. And then the same bot also can create a PR to bump polkadot-sdk and ready for someone to pick it up.

[muharem]

@bkontur I think dependabot might result a very similar experience with what you had with 1.5/1.6/1.7 PRs. But I am not against trying. For now I was thinking that we could first choose a final sdk version for the next runtime release. And after we have that agreement we open one PR to upgrade runtimes to a chosen sdk version.

[ggwpez]

I went for the exhaustive single-MR approach and it worked fine for 1.7->1.13 and 1.13->1.14. The trick is to ping every developer and delegate the changes in their submodule, since i have no clue how some things should be configured