substrate
substrate copied to clipboard
paritytech
`reserve` operation should ensure `repatriate_reserved` works.
Any pallet implementing ReservableCurrency or fungible::MutateHold should ensure that the reserve/hold operation can be followed by a slash or repatriation of the reserved amount without bringing the state becoming invalid. I.e. something like this should always succeed:
Balances::make_free_balance_be(1, 1000);
if Balances::reserve(1, 1000).is_ok() {
System::inc_consumers(1);
assert_ok!(Balances::slash_reserved(1, 1000);
assert_eq!(System::consumers(1), 1);
assert_eq!(System::providers(1), 1);
}
Right now, this will fail as the balance is both reserved/slashed and used for a provider ref.
The solution is to ensure that any reserved amounts are not considered toward the ED (only the free, unlocked balance); if a transfer, reserve or lock operation were to take the unlocked free balance below the ED then it should fail.
some work towards that goal: https://github.com/paritytech/substrate/pull/12004 planning to take over this sometime soon
@gavofyork this is done here: https://github.com/paritytech/substrate/pull/12004 All this PR needs is test fixes, which is what I'm going to take care of shortly. Meanwhile I would appreciate a quick check if the solution is a correct one.
According to the specs of this ticket it should be. In my opinion it could be better, since we are basically duplicating some of that logic between ensure_can_withdraw and can_hold.
On the other hand, if we're planning to deprecate Currency altogether - that's probably not such a bad thing. Now that I look at this code it makes little sense that both can_reserve and reserve were calling ensure_can_withdraw.
I could not find any usage occurrences of can_reserve in any code in substrate, which leads me to believe that the initial idea was to call that in reserve, which is what we are doing now.
Should be fundamentally fixed with https://github.com/paritytech/substrate/pull/12951
This issue 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
