migrate pallet-fast-unstake to fungible::* trait family
#1772
Conversation
kianenigma
added
the
R0-silent
kianenigma
changed the title
fungible::* trait family
|
I was under the impression that this pallet has some bound on the total size of the queue and therefore can be migrated in a one-off manner. But I see now that there is bound, so I need to rework this into a lazy migration. |
| @@ -247,8 +247,7 @@ pub mod pallet { | |||
| type Randomness: Randomness<Self::Hash, BlockNumberFor<Self>>; | |||
|
|
|||
| /// The fungible in which fees are paid and contract balances are held. | |||
| type Currency: Inspect<Self::AccountId> | |||
| + Mutate<Self::AccountId> | |||
| type Currency: Mutate<Self::AccountId> | |||
There was a problem hiding this comment.
Why was this change made? Seems totally independent
There was a problem hiding this comment.
I think it was a useless line as Inspect is a subtrait to Mutate
edit: nvm, just realized it's a whole different pallet.
|
The CI pipeline was cancelled due to failure one of the required jobs. |
| let _ = T::Currency::burn_all_held( | ||
| &HoldReason::FastUnstake.into(), | ||
| &stash, | ||
| Precision::BestEffort, | ||
| // This is not a system-level slash, so we politely do our best to slash. | ||
| Fortitude::Polite, | ||
| ) |
There was a problem hiding this comment.
slash_reserved created a negative imbalance, whereas burn_all_held just burns it. Don't we still need that negative imbalance?
There was a problem hiding this comment.
If we need to slash instead of burn, we might need a slash_all_held
* crate-level docs for the parachains pallet * fix typos
part of #226.
Note that many other pallets could possibly use the
Considerationapi if they are purely holding deposits. In the case of this pallet, I decided to tap into thefungible::*traits directly because the type of deposit held in this pallet is fixed, and very rudimentary. We don't want to care about iftype Depositeven changes. The pallet should at any point putDeposit::get()on hold, and upon successful operation, should release all of it.I think for these pallets the
fungiblebased approach is better because it gives you the ability to stop tracking the amount of deposit held. Contrary, in theConsiderationyou need to store theticketso that you can update or release the deposit.I might nonetheless try this approach as well.
The PR contains some logical changes to the pallet and a migration, namely because, as noted above, we no longer need to store the deposit amount.