Add transferrable balance to currency trait to fix balance inconsistency#2038
Closed
dnjscksdn98 wants to merge 1 commit intoparitytech:masterfrom
Closed
Add transferrable balance to currency trait to fix balance inconsistency#2038dnjscksdn98 wants to merge 1 commit intoparitytech:masterfrom
dnjscksdn98 wants to merge 1 commit intoparitytech:masterfrom
Conversation
|
User @dnjscksdn98, please sign the CLA here. |
alstjd0921
approved these changes
Oct 26, 2023
2bd73be to
3404a99
Compare
ggwpez
reviewed
Nov 23, 2023
| .unwrap_or_else(|_| SignedImbalance::Positive(Self::PositiveImbalance::zero())) | ||
| } | ||
|
|
||
| fn transferrable_balance(who: &T::AccountId, preservation: Preservation) -> Self::Balance { |
Member
There was a problem hiding this comment.
Would it be possible to factor this one function out into a LegacyCurrencyTransferrableBalance trait or something?
Than you can require that in pallet_evm like type Currency: Currency<Balance> + LegacyCurrencyTransferrableBalance<Balance>; or so.
It seems like a better solution than editing the existing trait. Do you think it could work?
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR will resolve the balance inconsistency between Substrate's runtime and Frontier's
pallet_evmthat occurred after this prior PR was merged. In the mentioned PR, a new concept has been introduced in the balance system such as holds and frozen. However, the update can be disabled by setting some config types to empty tuples in the runtime as below.Furthermore, the new interface has been moved to the fungible trait and the old interface is left in the currency trait. From this point it affected the issue to
pallet_evm. Currently,pallet_evm'sConfigtrait contains a Currency type, that will mostly be set topallet_balances. TheCurrencytype has a trait bound specified as below.The problem is that it contains the
Inspecttrait that is fromfungible, which is the target interface that has been updated to the new balance system. Previously,Inspectwas added in order to use thereducible_balance()method to fetch the current transferrable balance of an account. But now this method has been modified to follow the new balance system, which means that even though the new balance system is disabled for a moment,pallet_evmwill fetch transferrable balance through the modified logic. As a result, this will lead to balance inconsistency (In the case when an account has locked and reserved balance both).So, I added a new method to the
currencytrait named astransferrable_balance()that has the same functionality as the old version ofreducible_balance(). I know that thecurrencytrait is deprecated and will be removed further some day, but for a short term until the balance system is fully migrated, I think this will be a good to go for chains that implements Frontier for now. (This issue affects every chain that has implemented Frontier'spallet_evmsuch as Moonbeam.)And for more information, there is another side effect occurred by this issue. Since the evm balance are shown higher than expected, even though the actual balance is insufficient to pay, evm transactions are still capable to be sent. However, the transaction will be stuck in the mempool infinitely until the balance is filled enough to be payed.
Perhaps if this PR has been merged in any good way, the
pallet_evmmust be updated as below.Inspecttrait from the Configtransferrable_balance()