open-runtime-module-library icon indicating copy to clipboard operation
open-runtime-module-library copied to clipboard

add erc20 compatible allowances

Open gregdhill opened this issue 2 years ago • 2 comments

I'm not sure if this has been considered before, but we are working on adding ERC20 precompiles here and we need some way to approve the spending of funds. Substrate already supports this in pallet-assets so we can follow their design approach. For instance they do require deposits which I have currently omitted from this PR for simplicity but let me know if that is something you'd like to see. Otherwise the current change itself is trivial, we have another storage map which tracks the total amount approved for spending which can be updated either by the set_allowance extrinsic or it will decreased when the spender calls transfer_allowance.

gregdhill avatar Sep 01 '23 16:09 gregdhill

Codecov Report

Merging #950 (b765fd2) into master (7e15fcf) will decrease coverage by 0.03%. The diff coverage is 73.33%.

@@            Coverage Diff             @@
##           master     #950      +/-   ##
==========================================
- Coverage   78.46%   78.43%   -0.03%     
==========================================
  Files         105      105              
  Lines       10819    10879      +60     
==========================================
+ Hits         8489     8533      +44     
- Misses       2330     2346      +16     
Files Changed Coverage Δ
tokens/src/weights.rs 0.00% <0.00%> (ø)
tokens/src/lib.rs 77.14% <75.75%> (-0.05%) :arrow_down:
tokens/src/tests.rs 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar Sep 01 '23 16:09 codecov[bot]

I just don't like the approve/transferFrom model. I understand we need it for ERC20 but I don't want to expose them in pallets. There is no reason user should use those calls and therefore we shouldn't have those calls.

There are two alternative approach: Something like Acala does, use EVM contracts to implement those. e.g. https://github.com/AcalaNetwork/predeploy-contracts/blob/master/contracts/token/Token.sol#L72 Implement them in a different pallet so other orml-tokens user don't need to have those code.

xlc avatar Sep 02 '23 05:09 xlc