-
Notifications
You must be signed in to change notification settings - Fork 607
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
refactor: clean up keeper interfaces #2394
Conversation
x/txfees/types/expected_keepers.go
Outdated
// TxFeesKeeper defines the expected transaction fee keeper | ||
type TxFeesKeeper interface { | ||
ConvertToBaseToken(ctx sdk.Context, inputFee sdk.Coin) (sdk.Coin, error) | ||
GetBaseDenom(ctx sdk.Context) (denom string, err error) | ||
GetFeeToken(ctx sdk.Context, denom string) (FeeToken, error) | ||
} |
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.
Lets keep this one, so other modules can import it, rather than having to redefine it locally
(I think types packages defining succinct, exported interfaces that other modules can directly reference is good when applicable)
Looks like we don't do this already, but we should have a type check for txfees keeper implementing this interface
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.
Awesome, this looks great other than my one comment!
* refactor: clean up keeper interfaces * rename community pool keeper * changelog * restore TxFeesKeeper and type check (cherry picked from commit 1313b63) # Conflicts: # CHANGELOG.md # app/keepers/keepers.go # x/incentives/keeper/gauge.go # x/incentives/keeper/keeper.go # x/txfees/types/expected_keepers.go
Co-authored-by: Roman <[email protected]>
Closes: #XXX
What is the purpose of the change
DistrKeeper
toCommunityPoolKeeper
Testing and Verifying
This change is a trivial rework / code cleanup without any test coverage.
Documentation and Release Note
Unreleased
section inCHANGELOG.md
? no