Remove staking delay exits migration #662
Conversation
4meta5
added
A0-pleasereview
| let (reads, writes) = delay_nomination_exits_migration_execution::<T>(); | ||
| <T as frame_system::Config>::DbWeight::get().reads(reads) | ||
| + <T as frame_system::Config>::DbWeight::get().writes(writes) | ||
| + 5_000_000_000 // 1% of the max block weight, to account for computation |
There was a problem hiding this comment.
I didn't notice this until now. Nice actual weight accounting.
pallets/parachain-staking/src/lib.rs
Outdated
| /// True if executed, false by default | ||
| type DelayNominationExitsMigration<T: Config> = StorageValue<_, bool, ValueQuery>; |
There was a problem hiding this comment.
We're removing the type from code, but the data will still be stored in the on-chain storage. A single bool will definitely not bloat state in any practical way. Still I wonder if we should clean it up.
If so we could either add the cleanup logic to on_runtime_upgrade which would then have to be removed again later. Or we could do it manually with a set_storage extrinsic.
There was a problem hiding this comment.
Nice catch, I think removing it in on_runtime_upgrade is simple and makes sense.
There was a problem hiding this comment.
I added back the storage item, but killed whatever it stored in on_runtime_upgrade. In a future PR after this runtime upgrade is run, I'll remove the storage item itself and the on_runtime_upgrade.
Does kill() mean that it's still storing bool::default() == false in storage? Any suggestions here?
Maybe the extrinsic approach is better...
There was a problem hiding this comment.
kill() means it is not storing anything in storage. Since you declared it as a ValueQuery however, the runtime and tools like polkadot js will act as if it stored bool::default(). This should be fine though because the migration code is gone so nothing should be reading it.
There was a problem hiding this comment.
So we need 2 upgrades before removing the on_runtime_upgrade right ?
There was a problem hiding this comment.
Yes, one more after this one
crystalin
added
B7-runtimenoteworthy
Also adds
migrations.mdto root ofparachain-stakingpallet, which will link to migration creation/removal PRs to track migrations. Networks can use these PRs to write custom migrations when they need to jump versions i.e.Nominator1 -> Nominator3.