Skip to content

Commit

Permalink
Replace Currency->fungible trait migration for time-release pallet (#…
Browse files Browse the repository at this point in the history
…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 <aramikm@gmail.com>
Co-authored-by: Robert La Ferla <robert@onemsg.co>
  • Loading branch information
3 people authored Jan 5, 2024
1 parent 14d4388 commit 35c2346
Show file tree
Hide file tree
Showing 13 changed files with 1,394 additions and 709 deletions.
237 changes: 114 additions & 123 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ lint-audit:
format-lint: format lint

.PHONY: ci-local
ci-local: check lint lint-audit test e2e-tests
ci-local: check lint lint-audit test js e2e-tests

.PHONY: upgrade
upgrade-local:
Expand Down
1,128 changes: 696 additions & 432 deletions e2e/package-lock.json

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions pallets/time-release/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pallet-time-release"
description = "Provides scheduled balance locking mechanism, in a *graded release* way."
description = "Provides scheduled balance freezing mechanism, in a *graded release* way."
authors = ["Frequency"]
edition = "2021"
homepage = "https://frequency.xyz"
Expand All @@ -23,11 +23,12 @@ sp-io = { workspace = true }
sp-runtime = { workspace = true }
sp-std = { workspace = true }
frame-benchmarking = { workspace = true, optional = true }
sp-core = { workspace = true }
log = { workspace = true, default-features = false }

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

[features]
Expand Down
4 changes: 2 additions & 2 deletions pallets/time-release/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ This pallet is a fork of the [ORML-vesting]( [vesting](https://github.com/open-w

## Overview

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.
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.

### Release Schedule

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.
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.
10 changes: 4 additions & 6 deletions pallets/time-release/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use super::*;
use crate::Pallet as TimeReleasePallet;

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

fn set_balance<T: Config>(who: &T::AccountId, balance: BalanceOf<T>) {
let deposit_result = T::Currency::deposit_creating(who, balance.saturated_into());
let actual_deposit = deposit_result.peek();
assert_eq!(balance, actual_deposit);
let actual_deposit = T::Currency::mint_into(&who, balance.saturated_into());
assert_eq!(balance, actual_deposit.unwrap());
}

fn lookup_of_account<T: Config>(
Expand Down Expand Up @@ -76,7 +74,7 @@ benchmarks! {
}: _(RawOrigin::Signed(to.clone()))
verify {
assert_eq!(
T::Currency::free_balance(&to),
T::Currency::balance(&to),
schedule.total_amount().unwrap() * BalanceOf::<T>::from(i) ,
);
}
Expand All @@ -103,7 +101,7 @@ benchmarks! {
}: _(RawOrigin::Root, to_lookup, schedules)
verify {
assert_eq!(
T::Currency::free_balance(&to),
T::Currency::balance(&to),
schedule.total_amount().unwrap() * BalanceOf::<T>::from(i)
);
}
Expand Down
Loading

0 comments on commit 35c2346

Please sign in to comment.