substrate icon indicating copy to clipboard operation
substrate copied to clipboard

Use fungible traits for treasury pallet

Open tonyalaribe opened this issue 1 year ago • 3 comments

Background

The current currency traits are deprecated and there's currently a move to modify the existing pallets to support the fungible traits. https://github.com/paritytech/polkadot-sdk/issues/226 This is an important step towards some more topics which are coming up, such as the Multi-Asset Treasury topic which will build on these fungible traits.

Review notes:

Since some of the fungible traits are fairly new and some methods appear to have similar uses, it's not always clear if i'm using the right traits and methods. For eg it's not always clear whether to use mint_into or set_balance in tests, as a replacement for Balances::make_free_balance_be()

It's also not always clear what the direct fungible equivalents of some Currency methods are. Eg Currency has deposit_into_existing and deposit_creating and only deposit exists under fungible with the Precision::Exact and Precision::BestEffort flags. Please keep this in mind while reviewing, to spot scenarios where this PR might be using the wrong fungible method equivalents.

The PR depends on the changes to the fungible traits in the gav-hold-freeze branch (https://github.com/paritytech/substrate/pull/12951), and would have to be reanchored on master when that branch has been merged

tonyalaribe avatar Feb 27 '23 12:02 tonyalaribe

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

paritytech-cicd-pr avatar Mar 06 '23 09:03 paritytech-cicd-pr

Thanks! @tonyalaribe this is very helpful PR to understand the whole change going on with https://github.com/paritytech/substrate/pull/12951 Does this PR require any migration? like converting existing balances which has been set with lock()/reserve() to be compatible with freeze()/hold() ?

vivekvpandya avatar Mar 14 '23 03:03 vivekvpandya

Thanks! @tonyalaribe this is very helpful PR to understand the whole change going on with paritytech/substrate#12951 Does this PR require any migration? like converting existing balances which has been set with lock()/reserve() to be compatible with freeze()/hold() ?

I believe so. But the need for migrations will be from the work on the balance pallet in paritytech/substrate#12951. So the migrations are more for the balance pallet than the treasury pallet.

tonyalaribe avatar Mar 14 '23 09:03 tonyalaribe