Skip to content

Commit f74461e

Browse files
mattheworrisaramikmrlaferla
authored
Replace Currency->fungible trait migration for time-release pallet (#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]>
1 parent bb96e13 commit f74461e

File tree

13 files changed

+1394
-709
lines changed

13 files changed

+1394
-709
lines changed

Cargo.lock

Lines changed: 114 additions & 123 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ lint-audit:
8181
format-lint: format lint
8282

8383
.PHONY: ci-local
84-
ci-local: check lint lint-audit test e2e-tests
84+
ci-local: check lint lint-audit test js e2e-tests
8585

8686
.PHONY: upgrade
8787
upgrade-local:

e2e/package-lock.json

Lines changed: 696 additions & 432 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pallets/time-release/Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "pallet-time-release"
3-
description = "Provides scheduled balance locking mechanism, in a *graded release* way."
3+
description = "Provides scheduled balance freezing mechanism, in a *graded release* way."
44
authors = ["Frequency"]
55
edition = "2021"
66
homepage = "https://frequency.xyz"
@@ -23,11 +23,12 @@ sp-io = { workspace = true }
2323
sp-runtime = { workspace = true }
2424
sp-std = { workspace = true }
2525
frame-benchmarking = { workspace = true, optional = true }
26+
sp-core = { workspace = true }
27+
log = { workspace = true, default-features = false }
2628

2729
[dev-dependencies]
2830
chrono = { workspace = true }
2931
pallet-balances = { workspace = true }
30-
sp-core = { workspace = true }
3132
common-primitives = { default-features = false, path = "../../common/primitives" }
3233

3334
[features]

pallets/time-release/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ This pallet is a fork of the [ORML-vesting]( [vesting](https://github.com/open-w
44

55
## Overview
66

7-
Time-release module provides a means of scheduled balance lock on an account. It uses the *graded release* way, which unlocks a specific amount of balance every period of time, until all balance unlocked.
7+
The `time-release` module offers a method for scheduling a balance freeze on an account. It employs a *graded release* approach, which thaws a specific amount of balance every period of time, until all balance is thawed.
88

99
### Release Schedule
1010

11-
The schedule of a release on hold is described by data structure `ReleaseSchedule`: from the block number of `start`, for every `period` amount of blocks, `per_period` amount of balance would unlocked, until number of periods `period_count` reached. Note in release schedules, *time* is measured by block number. All `ReleaseSchedule`s under an account could be queried in chain state.
11+
The schedule of a release on hold is described by the data structure `ReleaseSchedule`. Starting from the specified block number denoted as `start`, the schedule operates on a periodic basis. For each `period` amount of blocks, a designated `per_period` amount of balance is unfrozen. This process continues until the specified number of periods, denoted as `period_count`, is reached. It's important to highlight that in release schedules, the concept of time is measured in terms of block numbers. Accessing all `ReleaseSchedule` instances associated with an account is possible through querying the chain state.

pallets/time-release/src/benchmarking.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use super::*;
44
use crate::Pallet as TimeReleasePallet;
55

66
use frame_benchmarking::{account, benchmarks, whitelist_account, whitelisted_caller};
7-
use frame_support::traits::{Currency, Imbalance};
87
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
98
use sp_runtime::{traits::TrailingZeroInput, SaturatedConversion};
109
use sp_std::prelude::*;
@@ -17,9 +16,8 @@ pub type Schedule<T> = ReleaseSchedule<BlockNumberFor<T>, BalanceOf<T>>;
1716
const SEED: u32 = 0;
1817

1918
fn set_balance<T: Config>(who: &T::AccountId, balance: BalanceOf<T>) {
20-
let deposit_result = T::Currency::deposit_creating(who, balance.saturated_into());
21-
let actual_deposit = deposit_result.peek();
22-
assert_eq!(balance, actual_deposit);
19+
let actual_deposit = T::Currency::mint_into(&who, balance.saturated_into());
20+
assert_eq!(balance, actual_deposit.unwrap());
2321
}
2422

2523
fn lookup_of_account<T: Config>(
@@ -76,7 +74,7 @@ benchmarks! {
7674
}: _(RawOrigin::Signed(to.clone()))
7775
verify {
7876
assert_eq!(
79-
T::Currency::free_balance(&to),
77+
T::Currency::balance(&to),
8078
schedule.total_amount().unwrap() * BalanceOf::<T>::from(i) ,
8179
);
8280
}
@@ -103,7 +101,7 @@ benchmarks! {
103101
}: _(RawOrigin::Root, to_lookup, schedules)
104102
verify {
105103
assert_eq!(
106-
T::Currency::free_balance(&to),
104+
T::Currency::balance(&to),
107105
schedule.total_amount().unwrap() * BalanceOf::<T>::from(i)
108106
);
109107
}

0 commit comments

Comments
 (0)