-
Notifications
You must be signed in to change notification settings - Fork 48
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
fix: replace missing interfaces to x/profiles
module protobuf codec into app codec
#1270
Conversation
x/profiles
module protobuf codec
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1270 +/- ##
=======================================
Coverage 81.49% 81.49%
=======================================
Files 218 218
Lines 18270 18273 +3
=======================================
+ Hits 14889 14892 +3
Misses 2772 2772
Partials 609 609 ☔ View full report in Codecov by Sentry. |
x/profiles/types/codec.go
Outdated
// | ||
// The actual codec used for serialization should be provided to x/profiles and | ||
// defined at the application level. | ||
func ModuleCdc() *codec.ProtoCodec { |
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.
Why was this changed from a variable to a method? If we do this, each time we wall ModuleCdc
we are de-facto re-registering all the interfaces and returning a new coded. Which might impact performance issues. Why wasn't this kept as a variable and an init
method used instead if needed?
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.
The init
of models.pb.go runs after the codec.go init
.
It causes that the interfaces registration of x/profiles
is before proto message name registration so the typeURL of x/profiles
types are always /
, finally, it leads the error as follows:
panic: concrete type *types.Base58Address has already been registered under typeURL /, cannot register *types.HexAddress under same typeURL. This usually means that there are conflicting modules registering different concrete types for a same interface implementation
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.
I realize that the current app codec is already proto codec which registered all the required modules, so we can use it directly without any problems instead of maintaining another one for x/profiles
x/profiles
module protobuf codecx/profiles
module protobuf codec into app codec
Description
Closes: #XXXX
This PR fixes the registry with empty interfaces, it causes the
LinkChainAccountPacketData
can not be parsed properly with the error:Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change