-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Update x/auth to use Any #6165
Update x/auth to use Any #6165
Conversation
👋 Thanks for creating a PR! Before we can merge this PR, please make sure that all the following items have been For contributor use:
For admin use:
Thank you for your contribution to the Cosmos-SDK! 🚀 |
Codecov Report
@@ Coverage Diff @@
## master #6165 +/- ##
==========================================
+ Coverage 54.72% 54.75% +0.02%
==========================================
Files 443 442 -1
Lines 26918 26960 +42
==========================================
+ Hits 14732 14763 +31
- Misses 11152 11162 +10
- Partials 1034 1035 +1 |
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.
great work @sahith-narahari!
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 good. minor comments
&ModuleAccount{}, | ||
) | ||
} | ||
|
||
// RegisterKeyTypeCodec registers an external concrete type defined in | ||
// another module for the internal ModuleCdc. | ||
func RegisterKeyTypeCodec(o interface{}, name string) { |
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.
@fedekunze would you still need this for ethermint, even after removing auth/Codec
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.
ACK
// | ||
// TODO:/XXX: Using a package-level global isn't ideal and we should consider | ||
// refactoring the module manager to allow passing in the correct module codec. |
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.
// | |
// TODO:/XXX: Using a package-level global isn't ideal and we should consider | |
// refactoring the module manager to allow passing in the correct module codec. |
registry.RegisterInterface( | ||
"cosmos_sdk.auth.v1.AccountI", | ||
(*AccountI)(nil), | ||
&BaseAccount{}, | ||
&ModuleAccount{}, | ||
) |
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.
q: if I want to add another account type? do I register is as follows? @aaronc :
registry.RegisterInterface(
"cosmos_sdk.auth.v1.AccountI",
(*AccountI)(nil),
&EthermintAccount{},
)
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.
asking because the interface will already be registered if the auth is called first
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.
you can just use RegisterImplementations
. the name in RegisterInterface
is just informational. maybe we should get rid of the ability to add implementations with that method so that it's less confusing
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.
yeah, it's definitely confusing
* Migrate keeper codec to use marshaler * Migrate AccountI to types * Did go imports * Fix tests for x/auth * Cleanup std/codec * Sort imports * Fix legacy codec * Add godoc for RegisterInterfaces * Add RegisterInterfaces to std * Fix typo * Fixed merge changes * Eliminate vesting import in auth * Fix lint issues * Fix tests * Addressed comments * Rename interfaces in RegisterInterfaces * Removed codec.proto from std * Minor code cleanup Co-authored-by: Aaron Craelius <[email protected]>
Description
ref: #6081