-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
x/feegrant API Audit changes #9194
Conversation
-add comments to msg and query server -remove decorator from docs -add coments to msgs.go -remove decorator from godoc
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 seems like we're missing this point:
Same as in x/authz, we should simplify msg and types naming. Also
(a FeeAllowanceGrant) GetFeeGrant()
method naming is a bit confusing. Why not just useGetAllowance()
?
In particular, I guess FeeAllowanceGrant
could become AllowanceGrant
(or even just Grant
?), BasicFeeAllowance
-> BasicAllowance
, etc.
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
*refactor proto msg names and functions *add docs pertaining to auth's ante handler for deducted fees
…smos-sdk into ty-9115-feegrant_audit
Codecov Report
@@ Coverage Diff @@
## master #9194 +/- ##
=======================================
Coverage 60.14% 60.14%
=======================================
Files 595 595
Lines 37194 37194
=======================================
Hits 22369 22369
Misses 12846 12846
Partials 1979 1979
|
I slimmed down pretty much all the proto messages to be more to the point. |
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.
Looks okay from an API perspective. Are we removing height based expiration in a separate PR?
Yes @aleem1314 is taking care of that in another PR |
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.
LGTM!
@technicallyty Could you fix the conflicts? |
@technicallyty could you fix the lint issues? |
Description
-changes keeper methods and proto messages to be more streamlined
-add comments to msg and query server
-remove decorator from docs
-add coments to msgs.go
-remove decorator from godoc
ref: #9115
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes