imp(erc20: Emit additional approval event in case of transferFrom #2088
Conversation
MalteHerrmann
requested review from
facs95 and
GAtom22
and removed request for
a team
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2088 +/- ##
==========================================
+ Coverage 70.14% 70.22% +0.07%
==========================================
Files 339 353 +14
Lines 25499 25931 +432
==========================================
+ Hits 17887 18209 +322
- Misses 6675 6776 +101
- Partials 937 946 +9
|
fedekunze
deleted the
malte/emit-additional-approve-event-in-transfer
branch
| // FIXME: This is not working for the case where we transfer from the own account | ||
| // because the allowance is not removed on the SDK side. | ||
| is.ExpectNoSendAuthzForContract( | ||
| callType, contractsData, | ||
| owner.Addr, owner.Addr, |
There was a problem hiding this comment.
The case where we transferFrom the owner address will not be possible to fully support without adjusting our Cosmos SDK fork, since in Cosmos it's not supported to create authorizations for the same account. This is possible in ERC20s though.
However, the current alignment leads to the shown problem in this test - once set up through the EVM extension, the allowance is not reduced because in tx.go we use p.AuthzKeeper.DispatchActions(...). In that function, the case where granter == grantee is being skipped:
https://github.com/evmos/cosmos-sdk/blob/v0.47.5-evmos/x/authz/keeper/keeper.go#L97-L133
Description
This PR aligns our ERC20 EVM extension further with the Solidity implementations by emitting an additional approval event in case of a
transferFromtransaction.