# Pulsar WG 10/6/21
- [x] Frojdi's interface service
- [ ] Tyler's proto methods - https://github.com/cosmos/cosmos-proto/blob/ty/proto_methods/features/fastreflection/proto_message.go#L284
https://github.com/cosmos/cosmos-proto/blob/ty/proto_methods/testpb/1.pulsar.go#L973
- [ ] Backlog grooming
### Notes
having context available would be a good thing in interface methods, sometimes not
since we need to have context on what the protobuf any representation is, its better to have a wrappper type with private methods, so people cannot create these interfaces without using some specialiazed constructor and check the interface registery to see if its possible to create this interface type or not. it follows the any convention where you cant create an any without a type resolver
message any is gonna implement the interface and end up in the codegen
msg will be a concrete type which implements the interface
constructing this message will require you to pass in the interface and an interface registry.
this impl keeps up in a place where we have to be explicit
codegen will take care of protomethods for you
we will need an any cache
the registry will have the interface implemeneter, which will know of the interface implmeneters
maybe a global registry is not enough cause certain chains want to accept some implementations and not others
advantages:
* you get an any cache
* (alternative: getters and setters on struct)
* having getters and setters makes things easier than having to modify fields
* wrapping AnyMsg makes more sense
theres a new constructor that populates a private field
tx object stateful with interface registry there? (option)
aaron: make the behavior consistent
if we say the interface registry is only gonna check on marshal but not protoreflection, as long as we are consistent, its probably not a problem
frojdi: agree with that approach, if we say you are gonna work with the concrete type, you'll be safe, but if you work with ProtoReflect, you have to be sure the type you are gonna be setting is an interface implementer but you cannot be sure that the interface registry will accept it or not. thats what we can assure to other devs
aaron: if we use adr 33, this interface registry isnt checked. it doesnt apply to inter module communication.
frojdi: its only proto reflect set. im not seeing a context in which devs are using protoreflect set, when you are sending messages youre sending concrete types not protoreflect types. for adr 33 its a non-issue. its only going to be an issue for those modules or anything that is working with arbitrary proto types. i feel its a non-issue.
aaron: there are some tradeoffs in this approach. it makes sense but also its hard to make everyone happy, sometimes people dont understand why things are done a certain way and frustrated it wasnt done another way.
frojdi: it needs to be clear its not possible to get best of both worlds. need some form of tradeoff. either have complete safety, or bad dev ux, choices. we could keep things the way they are right now and it'll be safe right now. but people can still set arbitrary anys in interfaces, and we are gonna detect that only when we unmarshal/marshaling. but here we provide some protection when setting, but we dont have protection against protoreflect. but we are improving things, not making them worse
sam: one thing i think could be useful - be clear about who is subject to the bad parts - devs/users/things that use the sdk. who does this affect?
frojdi: you are getting unsafety with protoreflect,
aaron: its library devs. its up to you to check the interface registry when using the code
frojdi: we can put in the codec a check for the interface. we can use protobuf options to determine where the interface is and if its a correct option or not. we can abstract it away. its an advanced use case. only place we'll see this is ORM. even ORM doesnt even need to do this check cause it'll marshal bytes. marshal will check for it. its a problem if you're using protoreflect and you're not gonna marshal after.
aaron: another side to consider, using any message wrapper, but not returning an error and not returning a registry. we could not check it until marshal.
frojdi: if you do that you'll have adr 33 problems. having that error check is gonna protect you because in this case you have no way of setting this thing without like using a specialized constructor which accepts an interface registry. you have no way of doing this, so you are safe here. you are unsafe only if you do this thing: msg.Protoreflect().Set(...)
aaron: say theres a module that creates a vesting account, calls another module to create a vesting account, but that vesting account not supported on chain, once a client tries to unmarshal it, the vesting account wont work.
frojdi: you are saying we'll have this problem if you are trying to marshal/unmarshal a vesting account, the concrete type is not gonna give issues
aaron: im trying to see what would be the problem if we didnt deal with the interface registry in the any message wrapper.
frojdi: you could have this happening -
frojdi: you're dealing with things the state machine should not be dealing with. unexpected beahavior. having a constructor which always chcks if the chain supports that type is most likely better.
aaron: it makes sense
frojdi: its not the best devx but its better than what we have right now
aaron: its an imporvement and more correct. correct by default increase devx even if its not obvious
aaron: interface in the sdk. if we're moving in the direction of having the protobuf types in the go module, and we're saying this is just the types modules, and its just a version of the protobuf files codegen'd. theres a bit of a challenge with interfaces where the interfaces are implmentation. and we are putting the implmentation in the same package as the generated code. is there a way we can separate the impl of these interfaces from the concrete proto types that are used to represent them? could we have a msg send proto type that implments a msg service interface *outside* of that package. we could have multiple impl. of that service.
frojdi: you'd need to separate functions that are impl the interface from the type itself, then teh concern of how you are calling those methods, cause you cannot call directly on the type itself. so idk, it would be counter intuitive from how we deal with interfaces in golang.
aaron: if we are using a wrapper and you pass the registry to the wrapper, you could pull the impl into the wrapper type and then you have both conc. type and impl. theres ways you could make it better
frojdi: the only thing that would look weird is how you are actually implmenting these interfaces. you are not just registring the type but the functions that execute on the type. if you wanna separate conc. type from interface logic. it could be done, but most likely for devs it would be hard to understand
aaron: is there value to the concern that you are tightly tied to the interface impl. - it feels a bit fragile from a semantic versioning persspective.
frojdi: i agree with this. even in general/sdk i would not push people to make interface types with anys. i would avoid them as much as possible. in the sdk context where we deal with types as interfaces, we can deal with them a different way. for exmaple crisis. even evidence handlers could be handlers. we are sending a message that is an evidence and a separate handler that is dealing with that message. there are contexts where it might be useful to have interface and have diff impls. but i would try to avoid them as much as possibe. i would avoid interfaces as much as possible for sdk types. i see a use case for crypto keys, accounts, but there are also a lot of use cases where we can deal without interfaces, such as crisis, evidence. they could just be handlers. handlers of messages. rather than messages that implement an inteface which is a handler.
aaron: messages in the transaction,they have handlers. but we decided to pull ou these things, one is get signers, validate basic. there was both handler and a something defined on the concrete type. instead of interface its just a handler.
frojdi: in favor of that approach, i prefer it over everything else. even if ou take the most obvious example, which is sdk.Msg with validate basic, validate basic in my book is just a handler that accepts that message and validates it. the message should not implmenet the validation handler itself. in that context, you would enable the use case where you might have the same message where you oculd have multiple handlers on the same message, idk how you will pick which one to use. but yeah that would be a better approach
aaron: you could have those in the interface registry and they would always get registered
frojdi: how are yo ugonna call it from a message. are you using some sort of client?
aaron: current validate basic is fragile. get signers is one of the reasons we dont have a good client ux. we cant have some of the good things we want like semantic versioning. get signers requires a global. the whole message interfaces, theres reasons it is this way. lets just fix those problems and design it the right way
frojdi: couldnt you simply mark which fields need to be signed with protobuf options? the only problem you can not do and/or/if logic.
we should limit use of interfaces
use interface with things that actually need them