Deprecate `Currency`; introduce holds and freezing into `fungible` traits

[gavofyork]

Related to https://github.com/paritytech/substrate/issues/12918 Fixes https://github.com/paritytech/substrate/issues/12701 Polkadot Companion: https://github.com/paritytech/polkadot/pull/6780

Introduces Freezing and Holds: two new facilities in Balances pallet and an interface for them through fungible and fungibles. Also reworks some internals of Balances pallet to make it safer.

Some renaming:

• Balances::set_balance becomes Balances::force_set_balance
• Balances::transfer becomes Balances::transfer_allow_death

In general, it's now better to use the fungible trait API for Balances rather than messing with the dispatchables directly. The long-term plan is to remove the dispatchables and have all pallet-access be through XCM.

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.

Balances Internals

Alter reserve functionality in Balances so that it does not contribute to the account's existential deposit and does use a consumer reference. Remove the concept of withdraw reasons from locks. Migration is lazy: accounts about to be mutated will be migrated prior. New accounts will automatically be mutated. A permissionless upgrade dispatchable is provided to upgrade dormant accounts.

Typing

Type-safety is improved by removing all naked bool parameters from fungible(s) API functions. In particular, best_effort and force which were both previously boolean are now typed as Precision and Fortitude. Lesser used arguments which have changed are on_hold which is typed as Restriction and minted which is typed as Provenance. This generally makes invocations much more readable.

Privatizations

One function mutate_account (in Balances pallet) has been privatized. It should never have been public since it ignores some important bookkeeping. If you're using it, you probably mean to be using make_free_balance_be instead.

Upgrading

Because reserved/held funds do not any longer contribute to the ED, you may find that certain test and benchmark code no longer works. This is generally due to the case where exactly as many funds were placed in an account as were later reserved. To fix this breakage, simply ensure that at least the currency's minimum_deposit is in the account in addition to any funds which are planned to be reserved. It's usually as simple as appending + T::Currency::minimum_balance() to the argument of make_free_balance_be.

In some circumstances, errors have changed to the more general errors.

After introducing a runtime with this change in it, chains should ensure that all accounts are upgraded by repeatedly calling update_accounts giving it account IDs which are not yet upgraded. A script which does this shall be provided.

Pallets which use the existing set of Currency traits should be refactored to use the fungible traits. Aside from the actual code change, a gateway function (ensure_upgraded) should be introduced and called prior to any mutating operations to ensure that any accounts using the old Currency traits are unreserved/unlocked and said funds are instead placed on hold/frozen. A permissionless fee-free dispatchable should also be introduced (upgrade_accounts), similar to that of the Balances pallet, which is able to do this for dormant accounts.

It is not crucial to get this done in any particular timeframe, however at some point in the future Currency traits, Locks and Reserves will be deprecated and removed, so it will need to happen before then.

Important Notes

Some functions in the Currency traits retain their name and parameter set in the fungible traits and have similar semantics. However, there are many functions with differences in naming, and some with differences in result types and their meaning.

• Currency::free_balance may be replaced with fungible::Inspect::balance.
• Currency::make_free_balance_be may be replaced with fungible::Mutate::set_balance.
• ReservableCurrency::reserved_balance may be replaced with fungible::hold::Inspect::total_balance_on_hold.
• NamedReservableCurrency::reserve may be replaced with fungible::hold::Mutate::hold.
• NamedReservableCurrency::unreserve may be replaced with fungible::hold::Mutate::release, however the returned Result must be handled and the inner of Ok has the opposite meaning (i.e. it is the amount released, rather than the amount not released, if any).
• LockableCurrency::set_lock may generally be replaced with fungible::freeze::Mutate::set_freeze.
• LockableCurrency::extend_lock may generally be replaced with fungible::freeze::Mutate::extend_freeze.
• LockableCurrency::remove_lock may be replaced with fungible::freeze::Mutate::thaw.

There are no equivalents for ReservableCurrency::reserve or ::unreserve: an identifier must always be used.

Functions which may result in an account being deleted accept a new Preservation value. This has three variants:

• Expendable: Equivalent to ExistenceRequirement::AllowDeath
• Protect: The account must not die. If there are multiple provider refs, then this may still allow the balance to be reduced to zero.
• Preserve: Not only must the account not die, but the provider reference of the Currency implementation must also be kept in place.

Pallet NIS

NOTE: This updates NIS to use only the fungible API rather than Currency. This is fine since NIS is not deployed anywhere. For other pallets already in production then a progressive migration will be needed.

Pallet NIS has been altered to use active_issuance rather than total_issuance. This means the IgnoredIssuance is whatever should be discounted against active_issuance. It should probably be replaced with ().

TODO

• [x] Create UnbalancedHold.
• [x] Implement UnbalancedHold for balances.
• [x] Implement all the other Hold traits for UnbalancedHold implementations.
• [x] Deprecate fee_frozen and WithdrawReasons; just have a single frozen field in AccountData.
• [x] Impl InspectFreeze and MutateFreeze for Balances.
  • [x] Main implementation
  • [x] Tests for MutateFreeze
  • [x] Tests for combination of freezing and locking
• [x] Support arbitrary IDs for freezes and holds.
• [x] Change freeze/hold mechanism to not be mutually exclusive.
  • [x] Introduce tests for this.
• [x] Permissionless fee-free dispatchable for running ensure_upgraded on a non-upgraded account.
  • [x] Benchmarking
  • [x] Test
• [x] Refactor fungible/fungibles
  • [x] Remove all blanket impls.
  • [x] Events for all mutator traits.
  • [x] Bring fungibles up to date.
  • [x] Make ItemOf work properly again.
  • [x] Make Assets pallet build.
• [x] Refactor fungible/fungibles/nonfungibles:
  • [x] Rename suspend/resume to shelve/restore.
• [x] Fix tests using Balances pallet owing to reserves now requiring a consumer ref and not counting toward ED.
• [x] Script for calling upgrade_accounts: https://github.com/ggwpez/substrate-scripts/blob/master/upgrade-accounts.py
• [x] Handle dust in legacy uses of mutate account.
• [x] Fix up remaining usages of old bool API.
• [x] Fix up assets and balances benchmarks.

Maybe

• [ ] Make Currency an optional feature.

Next up: https://github.com/paritytech/substrate/issues/13251

[gilescope]

Would this be a good opportunity to rename the Mutate trait from make_balance_be to set_balance to be more consistent and friendly to newcomers?

[gavofyork]

set_balance is already in use in the Unbalanced trait.

[gavofyork]

@muharem All points addressed - thanks for the review.

[gavofyork]

@muharem please confirm you're happy with the final queries.

[gavofyork]

@ruseinov please check changes are satisfactory and approve if so.

[gavofyork]

@kianenigma could do with a review.

[tonyalaribe]

A tiny update to the description for this PR

The line:

Functions which may result in an account being deleted accept a new KeepAlive value. This has three variants:

Should be

Functions which may result in an account being deleted accept a new Preservation value. This has three variants:

since it's no longer KeepAlive, but Preservation in the latest changes.

[gavofyork]

@tonyalaribe, @ruseinov, @KiChjang, @ggwpez, @gilescope thanks for the review/suggestions - they've been addressed. Once you're happy please provide approvals.

[gavofyork]

bot merge force

[paritytech-processbot[bot]]

Error: "Check reviews" status is not passing for https://github.com/paritytech/polkadot/pull/6780

[ggwpez]

bot bench $ pallet dev pallet_balances

[command-bot[bot]]

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2539949 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_balances. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 75-eb738593-98d7-4261-8558-03f5e1a04234 to cancel this command or bot cancel to cancel all commands in this pull request.

[gavofyork]

bot merge force

[paritytech-processbot[bot]]

Error: "Check reviews" status is not passing for https://github.com/paritytech/polkadot/pull/6780

[gavofyork]

bot merge force

[paritytech-processbot[bot]]

Error: "Check reviews" status is not passing for https://github.com/paritytech/polkadot/pull/6780

[gavofyork]

bot merge force

[paritytech-processbot[bot]]

Error: "Check reviews" status is not passing for https://github.com/paritytech/polkadot/pull/6780

[command-bot[bot]]

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_balances has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2539949 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2539949/artifacts/download.

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs. Job name: test-linux-stable Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2540531

[gavofyork]

bot merge force

[paritytech-processbot[bot]]

Error: "Check reviews" status is not passing for https://github.com/paritytech/polkadot/pull/6780

[gavofyork]

bot merge force

[Polkadot-Forum]

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-41-v0-9-42/2828/1

[shawntabrizi]

within 3 months at which point the transfer alias will be removed.

Looks like it should be removed by now

[liamaharon]

within 3 months at which point the transfer alias will be removed.

Looks like it should be removed by now

cc @juangirini

[juangirini]

within 3 months at which point the transfer alias will be removed.

Looks like it should be removed by now

https://github.com/paritytech/polkadot-sdk/issues/148 cc @liamaharon