In proto files, we use both customtype and casttype. We should use customtype everywhere.
They are similar, from the docs:
- customtype, It works with the Marshal and Unmarshal methods, to allow you to have your own types in your struct, but marshal to bytes. For example, custom.Uuid or custom.Fixed128
- casttype (beta), Changes the generated fieldtype. All generated code assumes that this type is castable to the protocol buffer field type. It does not work for structs or enums.
ref: https://github.com/cosmos/cosmos-sdk/pull/6978#discussion_r467868841
I started doing this in #6978, but I it's bigger than I thought, and I ran into some failing tests. So I decided to create a self-contained issue for it.
We need casttype for addresses. It won't work with customtype
Ok. Out of curiosity, why won't it work?
I thought we would just need to implement these methods on AccAddress, ValAddress and ConsAddress.
But why would we do that it's just extra work? casttype is simple and logical IMHO
My train of reasoning is:
customtype and casttype in proto filesGiven the 2 points above, we probably should just stick with one everywhere?
True, there's less impl on our side with casttype, but it's also beta (ref). If we decide on casttype, then should we use casttype everywhere?
Given the 2 points above, we probably should just stick with one everywhere?
But why? What does that buy us? I don't see any overhead in sticking with how things are and there is overhead in changing. So there's cost, but what is the benefit?
Also I would note that gogo proto is now unmaintained (https://github.com/gogo/protobuf/issues/691). We have a fork that we are responsible for maintaining if we want any changes. We have change that haven't been merged upstream because there is no maintainer. So everything is at the same level of alpha/beta regardless of what their docs say. We could choose to move away from gogo protobuf because of this or else take over maintainership. But so far, there does not be a strong motivation to do so because 1) it actually works pretty well and 2) is still faster than the official protobuf. We will probably wait until the community provides a compelling alternative which I expect will happen sometime in the next year. But I see no immediate urgency.
I think the main point here is _consistency_, but given that we use casttype on very few types, I think it's OK. Mind if we close this @amaurymartiny?
If I could go back in time, I would only implement casttype.
Most helpful comment
If I could go back in time, I would only implement casttype.