pallet balance `ensure_can_withdraw` can be invalid if provider is required

[gui1117]

the method ensure_can_withdraw in the trait Curreny says:

Returns Ok iff the account is able to make a withdrawal of the given amount for the given reason. Basically, it's just a dry-run of withdraw.

In the pallet balance implementation, if an account has a consumer and withdraw all its balance, the withdraw will fail due to provider being required. However the check ensure_can_withdraw will succeed.

Fixing it is not straightforward because the method ensure_can_withdraw has in input how much is the new_balance but not how much is the new reserved_balance. If we consider that the reserved balance is the one currently hold, then a fix can be:

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
index cc7b6351c2..d027401f57 100644
--- a/frame/balances/src/lib.rs
+++ b/frame/balances/src/lib.rs
@@ -1031,6 +1031,12 @@ impl<T: Config<I>, I: 'static> Currency<T::AccountId> for Pallet<T, I> where
                if amount.is_zero() { return Ok(()) }
                let min_balance = Self::account(who).frozen(reasons.into());
                ensure!(new_balance >= min_balance, Error::<T, I>::LiquidityRestrictions);
+
+               let ed = T::ExistentialDeposit::get();
+               let allow_death = !system::Pallet::<T>::is_provider_required(who);
+               let reserved = T::AccountStore::get(who).reserved;
+               ensure!(allow_death || new_balance + reserved >= ed, Error::<T, I>::KeepAlive);
+
                Ok(())
        }

[gui1117]

It seems to me that fn withdraw make use of ensure_can_withdraw without further checks on providers requirement, thus it is also wrong.

[stale[bot]]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

[gui1117]

still relevant to me, but people can use the new token traits which doesn't have this error