Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polkadot v0.9.42 #2560

Merged
merged 19 commits into from
Jul 4, 2023
Merged

Polkadot v0.9.42 #2560

merged 19 commits into from
Jul 4, 2023

Conversation

wangjj9219
Copy link
Member

@wangjj9219 wangjj9219 commented Jun 15, 2023

pallet-balances breaking change

[email protected] uses new provider/consumers mechanism, there's breaking change: now pallet-balances Account supply provider based on its free_balance instead of total_balance, and no-zero reserved_balance will as consumer.

It will affect some case, if the ED of native token is 100:

if a account endowed by 200(>ed) free_balance, its provider inc 1, the (provider, consumer) is (1, 0) . Then reserve its 200 free_balance to reserved balance, according to new rules, consumer will inc 1, and free_balance changes to 0, it's below ED, provider will dec 1, However, due to consumer is 1, the provider cannot become 0. So the reserve operation will fail.

For users, when they do some operations, their account generally has sufficient Native token, or has other tokens assets as providers. BUT some functions of Acala will use some escrow accounts, which are only used to store reserved ACA/KAR:

1. create NFT class

It will do:

  1. transfer xxx ACA as deposit to the class account
  2. then class account reserve xxx ACA

now it will fail when reserve.

Solution

For create NFT class, transfer total deposit + ACA ED to class account. after reserve total deposit, free_balance is still >= ED (provider, consumer is 1, 0 ) .
For mint nft and transfer nft, if the receiver's free_balance is <= ed, transfer ED to receiver.

2. deploy contract or call contract in EVM+

the situation is much more complicated:

  • deploy a contract or call contract will charge(reserve) storage deposit fee
  • when deploy a contract on EVM+, will inc_provider directly for contract account (provider, consumer is 1, 0 ) before charge storage.
  • charge storage will do:
  1. if the storage deposit delta is positive,
    1.1 transfer delta from origin caller to contract account
    1.2 reserve delta amount free_balance to reserved_balance
  2. if the storage deposit delta is negative,
    2.1. repatriate_reserved the delta reserved balance from contract account to origin caller

Here are the results for each case:
a. deploy a new contract, the delta is positive and >= ED, it will succeed. before the reserve operation , provider, consumer is 2, 0, 1 is added by inc_provider directly, 1 is added by transfer operation. After reserve operation, provider, consumer is 1, 1

b. call contract, the delta is positive and >= ED, it will succeed. provider, consumer will change from 1, 1 to 2, 1 when transfer operation, after reserve operation, it becomes to 1, 1

c. deploy a new contract or call contract , delta is positive , but below ED. !!! It will fail !!! , due to the TokenError::BelowMinimum error when transfer delta to contract account. the latest impl for transfer not allowed receiver's free_balance is below ED after transfer, even if it's total balance is >= ED.

d. call contract, delta is negative. Usually, the free balance of the origin caller is greater than ED, so here it will be successful.

solution

We add SystemAccountStore to instead of the impl for StoredMap of System, it regards existed frame_system::Account also exists pallet_balances::Account, even if it's AccountData is default.
so that repatriate_reserved can transfer reserved balance to initialized contract account, which is created
by inc_provider has default AccountData.

Then use repatriate_reserved to instead of following operation:

  1. unreserve storage deposit from caller
  2. caller transfer storage deposit to contract account
  3. contract account reserve storage deposit

module-currencies refactor fungibles traits impl

In this pr module-currencies refactor the impl for fungibles traits:

  1. In impl traits, each function should judge the asset_id, if it is erc20, it will be processed separately, if it is orml-tokens asset or native token, it will be passed to the corresponding function of Tokens and Balances respectively.

  2. the fungibles in the substrate change the functions at the fund operation level to the default implementation(such as transfer, burn_from, mint_into...). module-currencies still overwrite these functions, it's necessary to compatible fungibles traits with ERC20. So, if use fungibles traits in code, should only use these overwrited impl functions.

@xlc
Copy link
Member

xlc commented Jun 18, 2023

we should do this to handle ref count #2495 and replace most of our reserves to RefProvider

@wangjj9219
Copy link
Member Author

we should do this to handle ref count #2495 and replace most of our reserves to RefProvider

How to prevent reserve operation fail for free_balance < ED after reserve, then dec_provider fail?

Now we have come to a crossroads, should we use the current PR solution or do this:
#2495 (comment)

@wangjj9219 wangjj9219 marked this pull request as ready for review June 20, 2023 15:50
@xlc xlc requested review from ermalkaleci and zjb0807 June 20, 2023 23:38
@xlc
Copy link
Member

xlc commented Jun 22, 2023

we also need to kill dust on account killed

modules/currencies/src/lib.rs Show resolved Hide resolved
modules/currencies/src/lib.rs Show resolved Hide resolved
modules/currencies/src/lib.rs Show resolved Hide resolved
modules/currencies/src/tests.rs Show resolved Hide resolved
@zjb0807 zjb0807 mentioned this pull request Jun 26, 2023
@xlc xlc merged commit 637aea4 into master Jul 4, 2023
@xlc xlc deleted the polkadot-v0.9.42 branch July 4, 2023 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants