feat: implement auto swap and burn for taker fee #16
Conversation
x/gamm/keeper/taker_fee.go
Outdated
|
|
||
| //build new subroute to swap takerFeeCoin to base denom | ||
| var newRoutes []poolmanagertypes.SwapAmountInRoute | ||
| for i, route := range routes { |
There was a problem hiding this comment.
seems like the routes are coming here from the routes used to make the switch. but why do we assume that if those routes don't provide a way to dym there isn't other routes that do? seems like we're not checking all routes possible.
e.g the switch USDC -> UDST, which may have a direct route USDC -> USDT, doesn't mean there isnt UDSC -> X -> DYM - but if I understand correctly the routes for this won't be contained in the check.
There was a problem hiding this comment.
there's no definitive requirement regarding it.
we don't manage this route mapping, and I'm not sure it's a good idea to randomly pick pools to swap the taker fee.
The issue is that the requirement is to allow pools with different base assets (DYM / USDC) but the taker fee has specific logic for DYM.
if we could treat base assets equally, there will be less edge cases
anyway, more holistic approach will probably be allowing gas with different tokens, then swap them to DYM in gov decided pools. something like that i think
There was a problem hiding this comment.
there's no definitive requirement regarding it.
from my understanding of the requirement, every non-dym taker fee should be swapped to dym assuming there is some route which can facilitate this swap.
we don't manage this route mapping, and I'm not sure it's a good idea to randomly pick pools to swap the taker fee.
I believe this can be done similar to how the client calculates the route. or alternatively this client can also calculate if there is a route to swap to dym and pass it also in the request.
…route-with-dym feat: for taker fee, search for route with dym to swap and burn
Closes: #XXX
What is the purpose of the change
(E.g.: This pull request improves documation of area A by adding ....
Brief Changelog
(for example:)
Testing and Verifying
(Please pick one of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Documentation and Release Note
Unreleasedsection inCHANGELOG.md? (yes / no)x/<module>/spec/) / Osmosis docs repo / not documented)