-
Notifications
You must be signed in to change notification settings - Fork 729
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
Fungibles: migrate Democracy pallet #1861
base: master
Are you sure you want to change the base?
Conversation
@@ -1065,7 +1067,7 @@ impl pallet_democracy::Config for Runtime { | |||
// only do it once and it lasts only for the cool-off period. | |||
type VetoOrigin = pallet_collective::EnsureMember<AccountId, TechnicalCollective>; | |||
type CooloffPeriod = CooloffPeriod; | |||
type Slash = Treasury; | |||
type Slash = (); |
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.
We may want to adapt treasury to accept slashes from pallet_democracy here...
substrate/frame/democracy/src/migrations/unlock_and_unreserve_all_funds.rs
Outdated
Show resolved
Hide resolved
The CI pipeline was cancelled due to failure one of the required jobs. |
* MaxValues limit for storage maps in the pallet-bridge-grandpa * remove use from the future PR
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@pgherveou do you plan to finish this? |
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
I think it's ready for review |
Emitting one event per iteration could be problematic when a migration does thousands of steps per block, and hardly that useful. We therefore only emit one MigrationAdvanced event at the end of the block. Signed-off-by: Oliver Tale-Yazdi <[email protected]>
# Goal The goal of this PR is to replace the `Currency` trait with the `fungible` trait in the `capacity` pallet. Closes #942 Closes #1532 # Discussion The following Parity issues/PRs were used as references for changes: [Deprecate Currency - PR 12951](paritytech/substrate#12951) (Explanation of necessary changes.) [FRAME: Move pallets over to use fungible traits](paritytech/polkadot-sdk#226) (Issue to track Parity's efforts to update their pallets.) [pallet vesting / update to use fungible](https://github.com/paritytech/polkadot-sdk/pull/1760/files) (Example of some necessary changes.) [Fungibles: migrate Democracy pallet](paritytech/polkadot-sdk#1861) (Example of storage migration for Locks->Freezes.) # Changes - `.cargo-deny.toml` Added "multiformats" for `cid` crate to fix warning. - `vscode/settings.json` Added script that sets up the source map for the rust std library files for debugging. - `Cargo.lock` cargo updated. - `e2e/package-lock.json` npm updated. - Replaced traits as needed: Use `tokens::fungible::` - `InspectFungible` for `balance()` and `reducible_balance()` - `InspectFreeze` for `balance_frozen()` - `Mutate` for `set_balance()`, `mint_into()` - `MutateFreeze` for `set_freeze()`, and `thaw()` - Added `pub enum FreezeReason` to support `freezes` - Updated error handling as `set_freeze()` and `thaw()` can fail, so errors needed to be propagated. - Updated pallets/mocks to set MaxFreezes to 2, one for Capacity, one for TimeRelease - Updated runtime pallet configs to use BalancesMaxXXXXXs - Updated tests with `.expect()` where `set_freeze()` or `thaw()` can fail. - `FreezeIdentifier` and `RuntimeFreezeReason` configured with defaults. - Added v3 migration to Capacity. # Storage Migrations The value of `BalancesMaxFreezes` has been updated, which will impact the storage of the Balances pallet by changing `T::MaxFreezes`, see the code here: [substrate/frame/balances/src/lib.rs:480](https://github.com/paritytech/polkadot-sdk/blob/release-polkadot-v1.1.0/substrate/frame/balances/src/lib.rs#L480) ```rust /// Freeze locks on account balances. #[pallet::storage] pub type Freezes<T: Config<I>, I: 'static = ()> = StorageMap< _, Blake2_128Concat, T::AccountId, BoundedVec<IdAmount<T::FreezeIdentifier, T::Balance>, T::MaxFreezes>, ValueQuery, >; ``` The previous value of `T::MaxFreezes` was `0` so no data could be stored in `Freezes`, therefore no storage migration for `Freezes` is needed for this change. Even if there was data in storage, it would only need to be migrated if `T::MaxFreezes` is *decreased*. However, the current chain has data in `Locks` that needs to migrated to `Freezes`. Testing has shown that these `Locks` will no longer be accessible once the new traits are in place. The `Balances` pallet is configured to store account data using the `System` pallet. Therefore, these two pallets must be included when using `try-runtime` for testing. The migration for `Capacity` will access its storage to determine which accounts have `Locks` that need to be translated to `Freezes`. Then, the old `Currency` trait is used to remove the `Locks` and the new `fungible` trait is used to set the `Freeze`. # How to Review - [ ] Read through [Deprecate Currency - PR 12951](paritytech/substrate#12951) to understand context and check that Currency traits were properly replaced with fungible traits. - [ ] Check impact of changing to `set_freeze()` and `thaw()` which can now fail and make sure all error states are propagated correctly without possibility for `panic` - [ ] Check if `balance()` is used correctly, or should be changed to `reducible_balance()`. The calculations evaluating the Existential Deposit (ED) have been updated and Parity comments indicate that `reducible_balance()` is most likely the value needed. - [ ] Ensure that the migration weights calculations are correct. (Please let me know if you would like to walk through the migration code path together). # How to Test Runtime Migrations [Install the CLI version of try-runtime](https://paritytech.github.io/try-runtime-cli/try_runtime/#installation), then run try-runtime to test the migration against Frequency Rococo: ```bash cargo build --release --features frequency-rococo-testnet,try-runtime && \ try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade live --uri wss://rpc.rococo.frequency.xyz:443 -pallet Capacity --pallet Balances --pallet System ``` Alternatively, you can use the non-release version for faster compiles: ```bash cargo build --features frequency-rococo-testnet,try-runtime && \ try-runtime --runtime ./target/debug/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade live --uri wss://rpc.rococo.frequency.xyz:443 -pallet Capacity --pallet Balances --pallet System ``` You should see output like this: ```bash [2023-12-16T17:03:57Z INFO runtime::capacity] migrated 344 [2023-12-16T17:03:57Z INFO runtime::capacity] migrated 345 [2023-12-16T17:03:57Z INFO runtime::capacity] migrated 346 [2023-12-16T17:03:57Z INFO runtime::capacity] migrated 347 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migration finished [2023-12-16T17:03:57Z INFO runtime::capacity] Migration calculated weight = Weight { ref_time: 78200000000, proof_size: 0 } [2023-12-16T17:03:57Z INFO runtime::capacity] ✅ migration post_upgrade checks passed // end of Capacity v2 migration, v3 migration follows [2023-12-16T17:03:57Z INFO runtime::capacity] Running pre_upgrade... [2023-12-16T17:03:57Z INFO runtime::capacity] Finish pre_upgrade for 347 records [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 Capacity Locks->Freezes migration started [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0xac4e66be328c4d235be27fcb0af8ccbe12dce375236f9ccc5516780522bc8870, amount:1500000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x3048b40a2e7185e510b695c7ba15f31218a1d4a501c6a71596e4f60317fc180f, amount:800000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x663ea97a6b40a51ed79f3328f9629423e5525c84bb178e63a854e4ce497fde25, amount:900000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x1488b10710285607257cbaa5f2ee273b50613358c50e443b8e43790ba7e15705, amount:900000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x283ac37222b6e34afb4b187e41e8e828c2e2d2b7f5bc027a2bf98eccbca76d47, amount:200000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x1a038c038ece2448a22096697a35905a98cc14138666432a1767142e67f76414, amount:900000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0xa848c35ccb975405ceec7d275d2cbbfe0df577f54a4e87caee5a4bf64f68ad01, amount:500000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x2aa39545e21212127c97ce1059988eb7f8dabe94f61e023bde1b85fd9625a638, amount:1000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x9404948a1b48c6818117f5803e5b868515fc88d1bcbd6fa27a1083089e5b5604, amount:900000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0xa0e340ec2c110a262b3f730c1cd333d52d6bb9fcb0b149bdd7216fb8cd4b2468, amount:100000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x28d9dadd32d1864fa4aa553a74ff82637acb131079bfec5b2777df7dea4a2e5d, amount:900000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0x4c54252a95ae9e334be70375d14d51d4d89a6ae2deeaf9ca47091d797f319620, amount:200000000 [2023-12-16T17:03:57Z INFO runtime::capacity] 🔄 migrated account 0xe2133699b4fdc8444b98f9fddc7039374302a9a41e9ce53eafab113fd236c739, amount:200000000 ``` The total weight calculated for the Capacity migration on testnet: ```bash [2023-12-18T14:50:36Z INFO runtime::capacity] total accounts migrated from locks to freezes: 347 [2023-12-18T14:50:36Z INFO runtime::capacity] 🔄 Capacity Locks->Freezes migration finished [2023-12-18T14:50:36Z INFO runtime::capacity] Capacity Migration calculated weight = Weight { ref_time: 260375000000, proof_size: 0 } [2023-12-18T14:50:36Z INFO runtime::capacity] ✅ migration post_upgrade checks passed ``` The total weight calculated for the Capacity migration on main net: ```bash [2023-12-18T14:57:04Z INFO runtime::capacity] 🔄 Capacity Locks->Freezes migration finished [2023-12-18T14:57:04Z INFO runtime::capacity] Capacity Migration calculated weight = Weight { ref_time: 1625000000, proof_size: 0 } [2023-12-18T14:57:04Z INFO runtime::capacity] ✅ migration post_upgrade checks passed ``` # Upgrade Notes 1. `scripts/upgrade_accounts.py` should be executed to ensure that all accounts have been upgraded before running the migration. # Checklist - [x] Chain spec updated - [ ] Custom RPC OR Runtime API added/changed? Updated js/api-augment. - [ ] Design doc(s) updated - [x] Tests added - [ ] Benchmarks added - [x] Weights updated --------- Co-authored-by: Matthew Orris <--help> Co-authored-by: Enddy Dumbrique <[email protected]> Co-authored-by: Frequency CI [bot] <[email protected]>
…1818) # Goal The goal of this PR is to replace the `Currency` trait with the `fungible` trait in the `time-release` pallet. This work was split from PR #1779. Closes #942 Closes #1833 # Discussion The following Parity issues/PRs were used as references for changes: [Deprecate Currency - PR 12951](paritytech/substrate#12951) (Explanation of necessary changes.) [FRAME: Move pallets over to use fungible traits](paritytech/polkadot-sdk#226) (Issue to track Parity's efforts to update their pallets.) [pallet vesting / update to use fungible](https://github.com/paritytech/polkadot-sdk/pull/1760/files) (Example of some necessary changes.) [Fungibles: migrate Democracy pallet](paritytech/polkadot-sdk#1861) (Example of storage migration for Locks->Freezes.) # Changes - Replaced traits as needed: Use `tokens::fungible::` - `InspectFungible` for `balance()` and `reducible_balance()` - `InspectFreeze` for `balance_frozen()` - `Mutate` for `set_balance()`, `mint_into()` - `MutateFreeze` for `set_freeze()`, and `thaw()` - Added `pub enum FreezeReason` to support `freezes` - Updated error handling as `set_freeze()` and `thaw()` can fail, so errors needed to be propagated. - Updated runtime pallet configs to use BalancesMaxXXXXXs - Updated tests with `.expect()` where `set_freeze()` or `thaw()` can fail. - `FreezeIdentifier` and `RuntimeFreezeReason` configured with defaults. - Updated time-release pallet to propagate errors from set_freeze/thaw. - Added v2 migration to TimeRelease. # Storage Migrations The value of `BalancesMaxFreezes` has been updated, which will impact the storage of the Balances pallet by changing `T::MaxFreezes`, see the code here: [substrate/frame/balances/src/lib.rs:480](https://github.com/paritytech/polkadot-sdk/blob/release-polkadot-v1.1.0/substrate/frame/balances/src/lib.rs#L480) ```rust /// Freeze locks on account balances. #[pallet::storage] pub type Freezes<T: Config<I>, I: 'static = ()> = StorageMap< _, Blake2_128Concat, T::AccountId, BoundedVec<IdAmount<T::FreezeIdentifier, T::Balance>, T::MaxFreezes>, ValueQuery, >; ``` The previous value of `T::MaxFreezes` was `0` so no data could be stored in `Freezes`, therefore no storage migration for `Freezes` is needed for this change. Even if there was data in storage, it would only need to be migrated if `T::MaxFreezes` is *decreased*. However, the current chain has data in `Locks` that needs to migrated to `Freezes`. Testing has shown that these `Locks` will no longer be accessible once the new traits are in place. The `Balances` pallet is configured to store account data using the `System` pallet. Therefore, these two pallets must be included when using `try-runtime` for testing. The migration for `TimeRelease` will access its storage to determine which accounts have `Locks` that need to be translated to `Freezes`. Then, the old `Currency` trait is used to remove the `Locks` and the new `fungible` trait is used to set the `Freeze`. # How to Review - [x] Read through [Deprecate Currency - PR 12951](paritytech/substrate#12951) to understand context and check that Currency traits were properly replaced with fungible traits. - [ ] Check impact of changing to `set_freeze()` and `thaw()` which can now fail and make sure all error states are propagated correctly without possibility for `panic` - [ ] Check if `balance()` is used correctly, or should be changed to `reducible_balance()`. The calculations evaluating the Existential Deposit (ED) have been updated and Parity comments indicate that `reducible_balance()` is most likely the value needed. - [ ] Ensure that the migration weights calculations are correct. (Please let me know if you would like to walk through the migration code path together). # How to Test Runtime Migrations [Install the CLI version of try-runtime](https://paritytech.github.io/try-runtime-cli/try_runtime/#installation), then run try-runtime to test the migration against Frequency Rococo: ```bash cargo build --release --features frequency-rococo-testnet,try-runtime && \ try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade live --uri wss://rpc.rococo.frequency.xyz:443 --pallet TimeRelease --pallet Balances --pallet System ``` Alternatively, you can use the non-release version for faster compiles: ```bash cargo build --features frequency-rococo-testnet,try-runtime && \ try-runtime --runtime ./target/debug/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade live --uri wss://rpc.rococo.frequency.xyz:443 --pallet TimeRelease --pallet Balances --pallet System ``` And for testing on main-net: ```bash cargo build --features frequency,try-runtime && \ try-runtime --runtime ./target/debug/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade live --uri wss://1.rpc.frequency.xyz:443 --pallet Balances --pallet System --pallet TimeRelease ``` Testing with a snapshot: ```bash # create a snapshot (or use existing one) try-runtime create-snapshot --uri https:://rpc.rococo.frequency.xyz:443 testnet-all-pallets.state # use the testnet snapshot cargo build --features frequency-rococo-testnet,try-runtime && \ try-runtime --runtime ./target/debug/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade \ snap --path testnet-all-pallets.state # use the mainnet snapshot cargo build --features frequency,try-runtime && \ try-runtime --runtime ./target/debug/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade \ snap --path mainnet-all-pallets.state ``` You should see output like this: ```bash [2023-12-20T22:06:50Z INFO runtime::time-release] Running pre_upgrade... [2023-12-20T22:06:50Z INFO runtime::time-release] Finish pre_upgrade for 6 records [2023-12-20T22:06:50Z INFO runtime::time-release] Running storage migration... [2023-12-20T22:06:50Z INFO runtime::time-release] 🔄 Time Release Locks->Freezes migration started [2023-12-20T22:06:50Z INFO runtime::time-release] 🔄 migrated account 0x702cfcc9149d3c6f65728d3a5312d66a83fb70ed942cedb8e6450f4198ce7a77, amount:1000000000 [2023-12-20T22:06:50Z INFO runtime::time-release] 🔄 migrated account 0xce3bcb8ac19cdeb3ee14173be5f474292ff11ae56d4d0fa3f2bdaf24b4ef5842, amount:10000000000000 [2023-12-20T22:06:50Z INFO runtime::time-release] 🔄 migrated account 0x041a99f3614052bdd5b0aed6ed5805f592aacbcc0d5a443821f3b4339c44c11f, amount:400000050 [2023-12-20T22:06:50Z INFO runtime::time-release] 🔄 migrated account 0x26db9b4eeb5b5d511abd26903a25578f355e34e33316aeb2d34e846045cc7e45, amount:10000000000 [2023-12-20T22:06:50Z INFO runtime::time-release] 🔄 migrated account 0xc8d6262ff9fc322e59bcd36a36e310cfc7c50134e309a82f4330648e2eff7368, amount:1411 [2023-12-20T22:06:50Z INFO runtime::time-release] 🔄 migrated account 0x485dc3b17bcebba3013e47150e588c941a1f9778378367125e98a2e8f140325e, amount:3000000000 [2023-12-20T22:06:50Z INFO runtime::time-release] total accounts migrated from locks to frozen 6 [2023-12-20T22:06:50Z INFO runtime::time-release] 🔄 Time Release migration finished [2023-12-20T22:06:50Z INFO runtime::time-release] Time Release Migration calculated weight = Weight { ref_time: 4525000000, proof_size: 0 } [2023-12-20T22:06:50Z INFO runtime::time-release] ✅ migration post_upgrade checks passed ``` The total weight calculated for the TimeRelease migration on testnet: ```bash [2023-12-18T14:50:36Z INFO runtime::time-release] total accounts migrated from locks to frozen 6 [2023-12-18T14:50:36Z INFO runtime::time-release] 🔄 Time Release migration finished [2023-12-18T14:50:36Z INFO runtime::time-release] Time Release Migration calculated weight = Weight { ref_time: 4525000000, proof_size: 0 } [2023-12-18T14:50:36Z INFO runtime::time-release] ✅ migration post_upgrade checks passed ``` The total weight calculated for the TimeRelease migration on mainnet: ```bash [2023-12-18T14:57:04Z INFO runtime::time-release] 🔄 Time Release migration finished [2023-12-18T14:57:04Z INFO runtime::time-release] Time Release Migration calculated weight = Weight { ref_time: 16525000000, proof_size: 0 } [2023-12-18T14:57:04Z INFO runtime::time-release] ✅ migration post_upgrade checks passed ``` # Upgrade Notes 1. `scripts/upgrade_accounts.py` should be executed to ensure that all accounts have been upgraded before running the migration. This step may be a no-op, if all accounts have previously been upgraded. # Checklist - [x] Chain spec updated - [ ] Custom RPC OR Runtime API added/changed? Updated js/api-augment. - [ ] Design doc(s) updated - [ ] Tests added - [ ] Benchmarks added - [ ] Weights updated --------- Co-authored-by: Matthew Orris <--help> Co-authored-by: Aramik <[email protected]> Co-authored-by: Robert La Ferla <[email protected]>
Partial for #226
The (Reservable/Lockable)Currency traits are deprecated after this paritytech/substrate#12951 in favour of the fungible traits.
A few things have changed:
ReservableCurrency::(reserve/unreserve)
becomesfungible::MutateHold::(hold/release)
LockableCurrency::(extend_lock/remove_lock/set_lock)
becomesfungible::MutateFreeze::(extend_freeze/thaw/set_freeze)
Holds and Freezes
Holds are roughly analagous to reserves, but are now explicitly designed to be infallibly slashed. They do not contribute to the ED but do require a provider reference, removing any possibility of account reference counting from being problematic for a slash. They are also always named, ensuring different holds do not accidentally slash each other's balances.
Freezes are essentially the same as locks, except that they overlap with Holds. Since Holds are designed to be infallibly slashed, this means that any logic using a Freeze must handle the possibility of the frozen amount being reduced, potentially to zero. A permissionless function should be provided in order to allow bookkeeping to be updated in this instance.
Both Holds and Freezes require an identifier which is configurable and is expected to be an enum aggregated across all pallet instances of the runtime.
In the Pallet Democracy context we define
HoldReason::Proposal
andFreezeReason::Proposal
.Upgrade notes
The
pallet_democracy::Config
trait now requires new typeRuntimeHoldReason
andRuntimeFreezeReason
:Also,
construct_runtime
needspallet_democracy::HoldReason
and :pallet_democracy::FreezeReason
:Migration
A (multi-block) migration is provided with this PR, which will convert all existing locks to holds and freezes.