-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Add ability to pay tx fees with various tokens #28
Conversation
x/txfees/keeper/feetokens.go
Outdated
return sdk.NewCoin(baseDenom, spotPrice.MulInt(inputFee.Amount).RoundInt()), nil | ||
} | ||
|
||
// CalcFeeSpotPrice converts the provided tx fees into their equivalent value in the base denomination. |
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.
IMO redundant function as the logic is just in line 46 and the arguments required for it are already provided in the calling function (ConvertToBaseToken
). So just can replace line 23 with the actual call to the CalculateSpotPrice
.
x/txfees/keeper/feetokens.go
Outdated
} | ||
|
||
// CalcFeeSpotPrice converts the provided tx fees into their equivalent value in the base denomination. | ||
// Spot Price Calculation: spotPrice / (1 - swapFee), |
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.
not sure I understand this line. the spot price is calcualted in a different way. not sure what this calculation means.
x/txfees/ante/feedecorator.go
Outdated
return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") | ||
} | ||
|
||
// checks to make sure the module account has been set to collect fees in base token |
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.
is this comment correct? why just base token?
return ctx, fmt.Errorf("txfees module account (%s) has not been set", types.ModuleName) | ||
} | ||
|
||
// checks to make sure the module account has been set to collect fees in base token |
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.
also here. why just base token?
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.
not sure what u mean.
it checks the modules defined (txfees for non-udym, feecollector for udum)
x/txfees/keeper/hooks.go
Outdated
} | ||
_, err = k.poolManager.RouteExactAmountIn(ctx, moduleAddr, route, coinBalance, sdk.ZeroInt()) | ||
if err != nil { | ||
return err |
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.
if there is an error to swap one of the tokens it will exit the loop and the rest couldn't be switched. Also the token will still stay in the txFees module to cause problem for every consecutive swap.
…be-passed-to-txfees-module feat: taker fee should be passed to txfees module
consist of a few logical components:
feedecorator.go
- theMempoolFeeDecorator
andDeductFeeDecorator
to allow various fee tokens for gas feeshooks.go
-