-
Notifications
You must be signed in to change notification settings - Fork 120
feat: redesign bank precompile for erc20 support #804
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
base: main
Are you sure you want to change the base?
Conversation
Closes: cosmos#505 Supersedes: cosmos#517 Use go-abi to simplify implementation. update erc20 source
| for denom, amount := range m { | ||
| coin := sdk.NewCoin(denom, sdkmath.NewInt(amount)) | ||
| if cb(coin) { | ||
| break | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for denom, amount := range k.supplies { | ||
| coin := sdk.NewCoin(denom, sdkmath.NewInt(amount)) | ||
| if cb(coin) { | ||
| break | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #804 +/- ##
==========================================
+ Coverage 64.99% 65.18% +0.18%
==========================================
Files 317 320 +3
Lines 21615 21827 +212
==========================================
+ Hits 14049 14228 +179
- Misses 6371 6415 +44
+ Partials 1195 1184 -11
🚀 New features to boost your workflow:
|
|
looks like we might need a |
|
Anything we need to do to get our code coverage up? |
I removed generated abi files from the coverage, the report looks good now. |
| Precompile: cmn.Precompile{ | ||
| KvGasConfig: storetypes.GasConfig{}, | ||
| TransientKVGasConfig: storetypes.GasConfig{}, | ||
| ContractAddress: common.HexToAddress(evmtypes.BankPrecompileAddress), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks need to set BalanceHandlerFactory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, it explicitly reject to handle native token in transferFrom method.
Description
Closes: #505
Supersedes: #517
Use go-abi to simplify implementation, see: #789
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
mainbranch