Storage Version Migration #3220
Conversation
…ps://github.com/litentry/litentry-parachain into p-1245-manul-migration-based-on-storage-version
| // along with Litentry. If not, see <https://www.gnu.org/licenses/>. | ||
|
|
||
| // By the time of this migration on Litentry 9220 (polkadot stable2407) | ||
| // The current storage version: pallet version: |
There was a problem hiding this comment.
Hmm I think we only have StorageVersion now, no PalletVersion?
As I saw there was once a migration from PalletVersion to the new StorageVersion: https://paritytech.github.io/polkadot-sdk/master/src/frame_support/migrations.rs.html#255-261
There was a problem hiding this comment.
Yes, apply this migration first then try-runtime feature.
pallet version is indeed storage version. And truth is we are missing/wrong on both.
There are few pallets (weird) using a special "storageVersion" StorageMap holding the info. But palletVersion query onchain is not bonding to this storage at all. (see my comment on transactionPayment, vesting pallets)
The PalletVersion migration you providing is mostly likely not going to work for us. It just clean the Pallet version storage (if any) and assign the latest storage version without doing any specific migration.
| // Balances V0 => V1 | ||
| // The official pallet migration is not need since we do not have any XCM deactive accounts | ||
| // But our onchain inactiveIssuance storage of pallet_balance is non-negative | ||
| // TODO: Where does this number come from? |
There was a problem hiding this comment.
I believe it's just calculated from total_issuance - active_issuance.
I googled a bit:
How Does It Emerge?
inactive issuance can accumulate in the following scenarios:
Locks Applied by Other Pallets: Pallets like pallet-staking or pallet-democracy can introduce locks on tokens, marking them as inactive until certain conditions are met (e.g., unlocking after a referendum).
Misconfiguration or Bugs: An incorrect runtime implementation or upgrade could inadvertently increase inactive issuance.
Custom Pallets or Extensions: Developers may implement additional features that lock balances in ways not accounted for during runtime upgrades or storage migrations.
When tokens are locked via these mechanisms, they are no longer counted as "active issuance" but still exist in the chain's total issuance.
| "StorageVersion".as_bytes(), | ||
| )); | ||
|
|
||
| InactiveIssuance::<T>::kill(); |
There was a problem hiding this comment.
IMO we should not touch it - just upgrading the storage version is enough
There was a problem hiding this comment.
Okay, I will remove this part of code for security purpose.
There was a problem hiding this comment.
Can you remove it? Then we are good to go I guess
| // TODO: Where does this number come from? | ||
| BalancesUpdateStorageVersionResetInactive<Runtime>, | ||
| // Democracy V0 => V1 | ||
| // This migration only effects onging proposal/referedum, NextExternal |
There was a problem hiding this comment.
Does it mean it's better to wait until there's no on-going ref/proposals to do this migration?
There was a problem hiding this comment.
Technically it should be the same whether or not there is any onging ones.
see extra comment
// The referedum info's proposal hash is migrated if the hash is in old form (In our case, even for an onging one it will do nothing)
new ongoing ref/proposal should already have new form, so it will do nothing.
But from a ghost story perspective, of course it will be better if no onging ones.
| // V4 to V5 | ||
| // Did nothing to storage | ||
| // Just checking MaxActiveOutboundChannels is not exceeded | ||
| // Our current Paseo Storage is 0 |
There was a problem hiding this comment.
Do you mean Litentry?
| // V3 to V4 | ||
| // XCMP QueueConfig has different default value | ||
| // Migration targeting at changing storage value to new default value if old value matched | ||
| // Our current Paseo setting has already hard-coded |
There was a problem hiding this comment.
Do you mean Litentry?
…ps://github.com/litentry/litentry-parachain into p-1245-manul-migration-based-on-storage-version
|
This comment is not yet resolved: Nor is this PR approved :( |
P-1245
Our Litentry, Paseo are Storage version outdated. It will block tools/CI like try-runtime.