-
Notifications
You must be signed in to change notification settings - Fork 46
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
[DO NOT MERGE] feat: 1.9.0 release #460
Conversation
## fixes [KILTProtocol/ticket#2255](https://github.com/KILTprotocol/ticket/issues/2255)
This reverts commit 0d0044b.
This reverts commit c8f4577.
we test first on peregrine and will later enable this feature on spiritnet
This reverts commit df5abe3.
This reverts commit 6d4fcc6.
Since rust-analyzer has been merged into the main rustlang repo, it is tested for breaking changes. This means that we don't need to pin the nightly to a specific version anymore, as using the latest nightly is always guaranteed to work with the latest version of the rust-analyzer extension (try it yourselves, for me it finally just works). This PR then does the following: - update the toolchain file to use the latest nightly, which gives us access to potentially new clippy warnings, and to better rust-analyzer performances - update the runtime builder to use Rust 1.64 as also nowadays done by Polkadot - remove the deprecated `cargo-features = ["workspace-inheritance"]`, since from 1.64 it has been stabilised (this could be caught only by using a recent enough nightly version) - Let the CI job use the same version as in the toolchain file, by removing the `+nightly` flag. This will ensure we have a consistent behaviour between local machine and CI server. The only thing to consider is that the nightly will differ from the version that srtool uses to build the runtime WASM. That was the case also before, and it will always be until substrate will support compilation with the stable toolchain.
## fixes KILTProtocol/ticket#2289 and KILTProtocol/ticket#2296 * Upgrades from Polkadot v0.9.29 to v0.9.32 * Adds missing feature implementations for all tomls (checked via `subalfred check features` in all crates) * Actually necessary for application of paritytech/substrate#10592 (see runtime changes in [dd81eac](dd81eac)) * Migrates Democracy, Preimage and Scheduler pallets to use bounded Calls, see below ## Summary of changes (Polkadot v0.9.30-0.9.32) * Weights v2 are not fully there yet, but the struct now includes the [second field for storage size](paritytech/substrate#12277) (tracking issue: https://github.com/paritytech/substrate/issues/12176) ### Breaking Changes * Breaking: Outer enums (paritytech/substrate#11981) * `Origin` --> `RuntimeOrigin` * `Call` --> `RuntimeCall` * `Event` --> `RuntimeEvent` * ~Convention seems to be to keep `Event`, `Call`, `Origin` for inner pallet usage, e.g. `Did::Origin`~ Update: We use `Runtime` prefix internally as well ### Noteworthy PRs * paritytech/substrate#12109 * paritytech/substrate#12328 * paritytech/cumulus#1585 * Following the effort of decoupling collators and full relay nodes, this PR adds the possibility of pointing the collator to an “external” (non in-process) relay node. This is still considered experimental, and the **relay node is suggested to run on the same machine than the collator for the moment**. * To specify the relay full node rpc: `polkadot-parachain --alice --collator --relay-chain-rpc-url <rpc-websocket-url>` * paritytech/substrate#12486 * Before this change only the interpreted WASM executor was included in per default compilations. Making the compiled executor opt-in, now, compiled WASM executor is set by default and an opt-out instead. This could lead to **big performance difference** between using these two, as more recent versions of the interpreter see a regression in performance. ### Scheduler, Preimage, Democracy Migration * paritytech/substrate#11649 * Referenda, Democracy, Scheduler and Preimage pallets are all now bounded in storage access footprint * Removed the concept of a "hard deadline" or weight-override priority and no longer guarantees that at least one scheduled item will be executed per block (since these are both dangerous to parachains which have a strict need of weight limits). This means you must ensure that scheduled items are below the MaximumWeight or they will not be executed. * Interesting comment: paritytech/substrate#11649 (comment) > There is migration code which moves existing proposals and referenda over to the new format. However IT DOES NOT MIGRATE EVERYTHING: > > * Preimages are **NOT** migrated. Any registered preimages in Democracy at the time of migration are dropped. Their balance is **NOT UNRESERVED**. > * The re-dispatcher used in the old Democracy implementation is removed. Any proposals scheduled for dispatch by Democracy **WILL NOT EXECUTE**. > > This means you SHOULD ensure that: > > * **the preimage for the runtime upgrade is placed as an imminent preimage, not with a deposit;** > * **no other preimages are in place at the time of upgrade;** > * **there are no other proposals scheduled for dispatch by Democracy at the time of upgrade.** > > The Democracy pallet will be marked as deprecated immediately once Referenda is considered production-ready. **ALL TEAMS ARE RECOMMENDED TO SWITCH SWAY FROM DEMOCRACY PALLET TO REFERENDA/CONVICTION-VOTING PALLETS ASAP** #### Result of `try-runtime` against Spiritnet on Friday Nov 18, 2022: ``` 2022-11-18 09:27:23.917 INFO main runtime::preimage::migration::v1: Migrating 0 images 2022-11-18 09:27:23.917 INFO main runtime::scheduler::migration: Trying to migrate 0 agendas... 2022-11-18 09:27:23.917 INFO main runtime::scheduler::migration: Migrated 0 agendas. 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: 0 public proposals will be migrated. 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: 25 referenda will be migrated. 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #7 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #20 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #13 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #5 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #8 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #1 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #19 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #9 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #16 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #14 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #21 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #15 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #24 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #22 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #2 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #10 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #0 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #6 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #11 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #3 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #17 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #18 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #23 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #4 2022-11-18 09:27:23.917 INFO main runtime::democracy::migration::v1: migrating referendum #25 2022-11-18 09:27:23.918 INFO main runtime::democracy::migration::v1: 0 public proposals migrated, 25 referenda migrated ``` ## Checklist: - [x] I have verified that the code works - [x] No panics! (checked arithmetic ops, no indexing `array[3]` use `get(3)`, ...) - [x] I have verified that the code is easy to understand - [ ] If not, I have left a well-balanced amount of inline comments - [x] I have [left the code in a better state](https://deviq.com/principles/boy-scout-rule) - [x] I have documented the changes (where applicable)
## fixes NO TICKET
New Relaychain spec, code substitute for peregrine Co-authored-by: William Freudenberger <[email protected]>
## related https://github.com/KILTprotocol/ticket/issues/2356 ## Checklist: - [ ] I have verified that the code works - [ ] No panics! (checked arithmetic ops, no indexing `array[3]` use `get(3)`, ...) - [ ] I have verified that the code is easy to understand - [ ] If not, I have left a well-balanced amount of inline comments - [ ] I have [left the code in a better state](https://deviq.com/principles/boy-scout-rule) - [ ] I have documented the changes (where applicable)
After the move to Polkadot, Spiritnet XCM config was still referring to Kusama as the relay chain. This should be updated now. Co-authored-by: Albrecht <[email protected]>
## fixes KILTProtocol/ticket#2351 Upgrade polkadot version to 0.9.36 ### Notable changes in 0.9.32 -> 0.9.33 [Full changelog](https://github.com/paritytech/polkadot/releases/tag/v0.9.33) This release is super small and only consists of removing the sp_tasks::spawn api, that wasn't used by us or any others (paritytech/substrate#12639). The only other change was an internal refactoring of the state database in paritytech/substrate#12239 ### Notable changes in 0.9.33 -> 0.9.34 [Full changelog](https://github.com/paritytech/polkadot/releases/tag/v0.9.34) This is a bigger runtime only release mainly about improvements to opengov and collectives. The only notable change that required code changes was the removal of the wasmtime feature flag in some crates paritytech/substrate#12684 ### Notable changes in 0.9.34 -> 0.9.35 [Full changelog](https://github.com/paritytech/polkadot/releases/tag/v0.9.35) Most notable are the two changes paritytech/substrate#12891 and paritytech/substrate#12868. Also there is now the possibility to use a new RPC server that supports http and websocket requests on the same server. We should look into this since this will eventually cause the older severs to be deprecated in the future (paritytech/substrate#12663). In paritytech/substrate#12485 a general message queue pallet was introduced that might be helpful for receivng XCM in the future. ### Notable changes in 0.9.35 -> 0.9.36 [Full changelog](https://github.com/paritytech/polkadot/releases/tag/v0.9.36) This is a hotfix release to address an issue in the dispute coordinator on the node level. ## How to test: Please provide a brief step-by-step instruction. If necessary provide information about dependencies (specific configuration, branches, database dumps, etc.) - Step 1 - Step 2 - etc. ## Checklist: - [ ] I have verified that the code works - [ ] No panics! (checked arithmetic ops, no indexing `array[3]` use `get(3)`, ...) - [ ] I have verified that the code is easy to understand - [ ] If not, I have left a well-balanced amount of inline comments - [ ] I have [left the code in a better state](https://deviq.com/principles/boy-scout-rule) - [ ] I have documented the changes (where applicable) Co-authored-by: Antonio <[email protected]>
## fixes KILTProtocol/ticket#2392 ## Breaking Changes for us ~~None! 🥳~~ Edit: Forgot to also check with try-runtime feature enabled. There is a small tweak necessary because of [This PR about on-runtime-upgrade](paritytech/substrate#13045) No database migrations, no runtime migrations and no new host functions. ## Polkadot Release Link https://github.com/paritytech/polkadot/releases/tag/v0.9.37 ## Release Analysis Forum Post https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-37/1736 ## Cool new stuff that might be useful (or not) * [frame_support::storage: Add StorageStreamIter](paritytech/substrate#12721) * If we have a StorageValue that contains something iterable, we can directly iterate over it, without copying the memory first by a regular get() call. * [Add ensure_* mathematical methods](paritytech/substrate#12754) * The checked_* family of calls returns an Option which is in 99% of the cases mapped to an error * ensure_* calls directly return an error which can be propagated using questionmark operator more easily * [Kusama shows how to express complex local origins in XCM messages](paritytech/polkadot#6273) * Perhaps the most interesting one in this release, would be a good idea for @weichweich and @ntn-x2 to have a look into this * [pallet_uniques successor NFTv2 is out! 🥳 😄 ](paritytech/substrate#12765) * Finally we can have NFTs with owner controlled metadata on our chain. * They even literally mention that this way users can write DIDs directly on their NFT!
Brings the two runtimes back to sync. The diff between the peregrine and spiritnet runtime is now minimal again. Also one comment was outdated and the public credentials are not part of the benchmarks for spiritnet.
Fixes KILTprotocol/ticket#2327 and fixes KILTprotocol/ticket#2325. Adds a creation block number to future CTypes, and it introduces a `set_block_number` extrinsic that can be called **by sudo on standalone, Peregrine and Spiritnet**. Co-authored-by: Albrecht <[email protected]>
## fixes KILTprotocol/ticket#2388 Since Peregrine fix tags didn't contain `-rc-` the CI pushed them as latest. This commit undo [this commit ](3a852e5) ## Checklist: - [ ] I have verified that the code works - [ ] No panics! (checked arithmetic ops, no indexing `array[3]` use `get(3)`, ...) - [ ] I have verified that the code is easy to understand - [ ] If not, I have left a well-balanced amount of inline comments - [ ] I have [left the code in a better state](https://deviq.com/principles/boy-scout-rule) - [ ] I have documented the changes (where applicable)
There were few things that, as far as I have understood, were not 100% consistent with what the chain does and especially does NOT do, right now. More details are provided as comments in the files.
A while ago, we have migrated to using the `StorageVersion` way of declaring the storage version for our pallets. Nevertheless, we were not aware that such a change would only be written to storage either explicitly in a storage migration for live chains, or at genesis for new chains. This means that we have few places where the declared storage version, which is a `const` inside each pallet, does not reflect what is written on chain in the relative entry. This means that, for the future, **every time we add a new pallet we must also include the storage migration to write the value of the storage version on chain**. ### Peregrine The `attestation` and `publicCredentials` pallet expose a `StorageVersion` of 1, but the on-chain value is 0. ### Spiritnet It has the same issue as Peregrine, with the addition of the `web3Names` pallet as well. This PR then exposes a pre-runtime hook that logs with a warning all pallets that suffer from this inconsistency, and a post-runtime check that verifies that everything has been fixed. #### Note The migration that updates the storage version has to be run last, so that pre- and post- runtime hooks don't trigger any unexpected behaviour. ## How to test Compile the `kilt-parachain` binary with `cargo build -p kilt-parachain --features try-runtime`. For Peregrine, run: ```bash ./target/debug/kilt-parachain try-runtime \ --runtime target/debug/wbuild/peregrine-runtime/peregrine_runtime.compact.compressed.wasm \ -lruntime=info \ on-runtime-upgrade --checks \ live --uri wss://peregrine.kilt.io:443/parachain-public-ws ``` For Spiritnet, run: ```bash ./target/debug/kilt-parachain try-runtime \ --runtime target/debug/wbuild/spiritnet-runtime/spiritnet_runtime.compact.compressed.wasm \ -lruntime=info \ on-runtime-upgrade --checks \ live --uri wss://spiritnet.kilt.io:443 ```
@@ -3,7 +3,7 @@ FROM docker.io/library/ubuntu:22.04 | |||
LABEL maintainer "[email protected]" | |||
LABEL description="This image contains tools for Substrate blockchains runtimes." | |||
|
|||
ARG RUSTC_VERSION="1.62.0" | |||
ARG RUSTC_VERSION="1.64.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't match with the latest srtool version (1.66.1) thats also used in the CI script
cc @weichweich once you're back. We will deploy on staging by end of the week but wait for your pair of eyes before merging and releasing on Peregrine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me this looks good, all important stuff seems to be there. I'd say we create an rc from this and continue testing on staging 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Some calls are heavier now, for no apparent reason.
.saturating_add(T::DbWeight::get().reads(2 as u64)) | ||
.saturating_add(T::DbWeight::get().writes(2 as u64)) | ||
} | ||
// Storage: Attestation Attestations (r:1 w:1) | ||
// Storage: System Account (r:1 w:1) | ||
fn reclaim_deposit() -> Weight { | ||
Weight::from_ref_time(51_952_000 as u64) | ||
Weight::from_ref_time(115_795_000 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The weights of remove
, reclaim_deposit
, change_deposit_owner
, update_deposit
increased a lot.
.saturating_add(T::DbWeight::get().reads(2 as u64)) | ||
.saturating_add(T::DbWeight::get().writes(2 as u64)) | ||
} | ||
// Storage: Did Did (r:1 w:0) | ||
/// The range of component `l` is `[1, 5242880]`. | ||
fn signature_verification_sr25519(l: u32, ) -> Weight { | ||
Weight::from_ref_time(29_998_000 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also increased a bit
.saturating_add(T::DbWeight::get().reads(2 as u64)) | ||
.saturating_add(T::DbWeight::get().writes(4 as u64)) | ||
} | ||
// Storage: System Account (r:1 w:1) | ||
// Storage: DidLookup ConnectedDids (r:1 w:1) | ||
// Storage: DidLookup ConnectedAccounts (r:0 w:2) | ||
fn associate_sender() -> Weight { | ||
Weight::from_ref_time(72_677_000 as u64) | ||
Weight::from_ref_time(172_109_000 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Increased and seems counter intuitive. associate_account
does 2 signature checks while associate_sender
only 1. but associate_sender
is significantly heavier.
.saturating_add(T::DbWeight::get().reads(2 as u64)) | ||
.saturating_add(T::DbWeight::get().writes(3 as u64)) | ||
} | ||
// Storage: DidLookup ConnectedDids (r:1 w:1) | ||
// Storage: System Account (r:1 w:1) | ||
// Storage: DidLookup ConnectedAccounts (r:0 w:1) | ||
fn remove_account_association() -> Weight { | ||
Weight::from_ref_time(54_561_000 as u64) | ||
Weight::from_ref_time(111_285_000 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These things also increased; remove_account_association
, change_deposit_owner
@@ -56,24 +56,24 @@ pub struct SubstrateWeight<T>(PhantomData<T>); | |||
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> { | |||
// Storage: System Account (r:1 w:1) | |||
fn on_initialize_mint_to_treasury() -> Weight { | |||
Weight::from_ref_time(37_539_000 as u64) | |||
Weight::from_ref_time(81_256_000 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These increases seem to be random.
// Standard Error: 2_000 | ||
.saturating_add(Weight::from_ref_time(10_000 as u64).saturating_mul(n as u64)) | ||
fn claim(_n: u32, ) -> Weight { | ||
Weight::from_ref_time(79_918_651 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This increased a little but is now independent from the length of the name
.saturating_add(T::DbWeight::get().reads(4 as u64)) | ||
.saturating_add(T::DbWeight::get().writes(3 as u64)) | ||
} | ||
// Storage: Web3Names Names (r:1 w:1) | ||
// Storage: Web3Names Owner (r:1 w:1) | ||
// Storage: System Account (r:1 w:1) | ||
fn release_by_owner() -> Weight { | ||
Weight::from_ref_time(56_091_000 as u64) | ||
Weight::from_ref_time(115_743_000 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
twice as high
// Standard Error: 1_000 | ||
.saturating_add(Weight::from_ref_time(40_000 as u64).saturating_mul(n as u64)) | ||
fn unban(_n: u32, ) -> Weight { | ||
Weight::from_ref_time(38_246_474 as u64) | ||
.saturating_add(T::DbWeight::get().reads(1 as u64)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also independent from the length now
@@ -52,41 +52,41 @@ impl<T: frame_system::Config> attestation::WeightInfo for WeightInfo<T> { | |||
// Storage: Attestation Attestations (r:1 w:1) | |||
// Storage: System Account (r:1 w:1) | |||
fn add() -> Weight { | |||
Weight::from_ref_time(56_234_000 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
twice as high
@@ -50,43 +50,43 @@ pub struct WeightInfo<T>(PhantomData<T>); | |||
impl<T: frame_system::Config> pallet_balances::WeightInfo for WeightInfo<T> { | |||
// Storage: System Account (r:2 w:2) | |||
fn transfer() -> Weight { | |||
Weight::from_ref_time(87_075_000 as u64) | |||
Weight::from_ref_time(207_389_000 as u64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
twice as high
@weichweich there has been quite some work on the benchmarking side, so I guess these results are a consequence of those. I have no other clear explanation for these new weights. Some weights have gone up, many others have gone down, and some even do not depend on the input anymore, which hints at a different way of fitting the numbers together. I would not be too worried. |
Fixes https://github.com/KILTprotocol/ticket/issues/2396.
PR shows the diff with the
master
branch.For better insights into what this release changes, please refer to the diff with develop - to see if we included all the latest features and fixes - and the diff with 1.8.0 to check exactly what we have changed
Edit
After going through past releases and other chain upgrade paths, I found out two migrations we did not consider:
check_out
and reactivates them oncheck_in
). We do not have that, and beyond the treasury have nothing else to be considered as inactive. Apparently Polkadot does not consider deposits as inactive. We could consider removing the tokens sent to the pure proxy as deactivated, perhaps in the next migration. It is not critical that we do it in the same migration.TODO