Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Deprecate Currency; introduce holds and freezing into fungible traits #12951

Merged
merged 174 commits into from
Mar 18, 2023

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented Dec 16, 2022

Related to paritytech/polkadot-sdk#236
Fixes #12701
Polkadot Companion: paritytech/polkadot#6780
Cumulus Companion: paritytech/cumulus#2334

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. Requires Existential Deposit requirements to be made of the free balance, not the total balance.

Key Points

  • Introduces 4 new items in pallet_balances::Config trait:
    • type MaxHolds
    • type MaxFreezes
    • type HoldIdentifier
    • type FreezeIdentifier
    • Until you introduce pallets which use holds and freezes, then these can safely be set to ConstU32<0> in the case of the first two and () in the case of the second two.
  • Introduces a new dispatchable (extrinsic) in pallet_balances: upgrade_accounts.
  • Renames 2 dispatchables:
    • Balances::set_balance becomes Balances::force_set_balance and only accepts the free balance (not the reserved balance since this no longer makes sense).
    • Balances::transfer becomes Balances::transfer_allow_death
    • Uses of both within your codebases should be renamed as you upgrade the version of your Balances pallet.
  • Alters many function signatures of fungible/fungibles traits making them much more type-safe.
  • Merges the fungible::Transfer trait into fungible::Mutate (and same for fungibles).
  • Requires the fungible::Balanced trait to be explicitly implemented (and same for fungibles).
  • Introduces default implementations for almost all functionality in the fungible/fungibles traits.
  • Silently deprecates Currency traits in favour of fungible traits.
  • Accounts now require the Existential Deposit (Currency::minimum_balance()) in addition to any amount to be reserved. This is a BREAKING CHANGE and is likely to result in failures in tests and benchmarking.
  • An ExistentialDeposit of zero no longer works. If you need to allow any account to have a zero balance and retain associated account data, you must introduce a pallet which allows an unpermissioned frame_system::Pallet::<Runtime>::inc_providers().

Long Description

Some renaming:

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

In the latter case, since ecosystem tooling largely depends on using transfer, a compatibility stub for transfer remains, however this naming is DEPRECATED and ecosystem tools should move over to using transfer_allow_death within 3 months at which point the transfer alias will be removed.

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

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

Maybe

  • Make Currency an optional feature.

Next up: paritytech/polkadot-sdk#226

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 16, 2022
@gavofyork gavofyork marked this pull request as draft December 16, 2022 12:01
@gavofyork gavofyork added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Dec 16, 2022
dzmitry-lahoda added a commit to ComposableFi/composable-ibc that referenced this pull request Sep 15, 2023
Refactor packet sequence handling in query methods (#394)

* Refactor packet sequence handling in query methods

Replaced `HashSet` with `HashMap` for efficient and accurate packet sequence handling in `query_send_packets` and `query_received_packets` methods. The change optimizes the query process by ensuring that the latest block events are tracked correctly.

* add a comment

Ensure target block headers are within votes ancestries (#396)

* Ensure target block headers are within votes ancestries

Added necessary logic to fetch and include the target block header manually in the event it is not found within the returned justification's votes ancestries. This resolves potential issues where the target block header may not be immediately available within the ancestries of the justification.

* add the target only if the ancestries are not empty

chore: upgrade to polkadot-v0.9.43

upgrade hash-db

more hash-db

sp_consensus_grandpa rename

fungibles::Transfer becomes Mutate

paritytech/substrate#12951

update toolchain

keystore updates

benchmarks compiling

compilation errors

comment

cargo fmt

propagate dependencies

more updates

updates

nasty trick to make it compile

more

fmt

build network

rm rootevent

rm Root

rm more rootevent

life is hard

replace codec

fix codegen

update polkadot-launch

upgrade types to match version release-v9.10037.1 (#399)

* upgrade types to match version release-v9.10037.1

* fmt

change Keystore for MemoryStore

change preservation parameter in transfer function
from Preservation::Protect to Preservation::Expendable

rm logger

fix unused import

bullish

change wsPort for rpcPort on parachain-launch/config

fix ports

udeps is happy

make docker-compose work fine

Create config.json

Signed-off-by: dzmitry-lahoda <dzmitry@lahoda.pro>

fixed zombie chain id

Signed-off-by: dzmitry-lahoda <dzmitry@lahoda.pro>

more like PL config

Signed-off-by: dzmitry-lahoda <dzmitry@lahoda.pro>

script to ZN start

Signed-off-by: dzmitry-lahoda <dzmitry@lahoda.pro>

added zombienet guide

Signed-off-by: dzmitry-lahoda <dzmitry@lahoda.pro>

add zombienet to ci

fixing workflow to enable nix installation

merge install nix with installation for zombienet

nix before zombienet

change version and add nix-build

remove old nativ polka launch, added logs from it to ZN

fixing ci nix install

using other action

test nix first

br

propagate runtime-benchmarks

docker

thing is that need to disable broadcast discovery, to avoid connecting to wrong relay (similar to docker network isolation)

fixing nix ci

docker isntall

proper ports

probe

100% same nix install as in composable repo on official GH runners

runs

try nix-quick-install-actio

install manually

get {ZN, process-comopse} as binaries (ditching NIX)

more fixes

wrong pwd

fixed ports and run cluster in background

clean

rm nodejs installation

fix port of second network

oh go devs

no dashes

get some logs

chmod everybody

chmod polkadot

sleep before checking status of parachains

add BenchmarkHelper on mock::Test

query PARA_HOST

fix typo in script

reduced amount of blocks to wait, tested locally - ports are super ok and make READY status

wheriam

ports

no docker ...

no netstat

given same set of product, one cooks bad and one well

debug netsat

very high sleep just to debug

nohup

sleep after checks

changing step Wait until parachains are ready

don't run process compose

remove the nohup

fixing lines 132 and 133

revert v2 of actions

unify steps

relative path

avoid installing parachain-node

feature flag BenchmarkHelper and propagate runtime-benchmarks

feature flag BenchmarkHelper and propagate runtime-benchmarks

only one relaychain to rule them all

update url of static server

remove unsafe_cast_to_jsonrpsee_client

update the timestamp

fix build.

Revert "fix build."

This reverts commit 8cfb09d.

Revert "update the timestamp"

This reverts commit 2059cf3.

final fix for extrinsic tests

remove unsafe_cast_to_jsonrpsee_client

compilation issue

fix keystore

fix weight decoding

key is compact u32

increase timeouts

update ports on zombienet

output stack trace on fail

debug
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited T1-runtime This PR/Issue is related to the topic “runtime”. T2-API This PR/Issue is related to APIs.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

reserve operation should ensure repatriate_reserved works.