Removed / missing MsgCreatePool (v7)

[lukanus]

Seems like the version 7 has gamm.MsgCreatePool message type completely removed.
Without this structure the node wouldn't be able to return/decode the old data. It would be also problematic for the block explorers to fetch the data using new binary.
This seems like the quite serious bug :)

[ValarDragon]

It got renamed to gamm.MsgCreateBalancerPool.

How do block explorers / integrators parse old states at the moment? Is there some codec message they run, and we need to provide a 'backwards compatible codec' somewhere?

I was under the impression that these amino routes are intended to not matter, and they only get serialized as an unfortunate side effect if you serialzie an interface.

[lukanus]

@ValarDragon So doesn't look like it: https://github.com/osmosis-labs/osmosis/blob/main/x/gamm/types/tx.pb.go :)
try searching for "create".

[ValarDragon]

Its in this file: https://github.com/osmosis-labs/osmosis/blob/main/x/gamm/pool-models/balancer/tx.pb.go

[lukanus]

@ValarDragon so after longer investigation it really is failing as I expected.

So If you try to return any previous transaction of that type with the latest code, the node that's failing. Because obviously it cannot just solve the GRPC type :

So, if you run query like:
grpcurl -plaintext -d '{"events":["tx.height=3513"]}' localhost:9090 cosmos.tx.v1beta1.Service.GetTxsEvent
You get node error ERROR:Message: unable to resolve type URL /osmosis.gamm.v1beta1.MsgCreatePool: tx parse error

It is quite serious as I originally thought

[ValarDragon]

Thanks for the reference for grpcurl!

This means we need to find a solution for keeping all legacy amino -> proto mappings for messages in the latest codebase. This is quite unfortunate :(

I wonder if theres a way we can get old messages to be parseable but not touch the state machine. I'd prefer to not have to have code in the msg server for blocking old messages. cc @marbar3778

[tac0turtle]

The approach we have talked about is a binary that houses all the types for previous versions then users are able to query different versions.

After an upgrade in some cases you will not be able to query old state, but the safest way to offer this is to version your proto files. Its passes a minor issue onto the explorers of updating the proto files when they change but with dynamic-cosmos this goes away.

Id highly recommend versioning your proto files, otherwise this could be come a larger issue later on.

[lukanus]

@marbar3778 @ValarDragon Well it's not really about block explorers even, currently the node is not able to return the data because it just cannot decode it's own state. It's also not really related to amino, but to type.Any in GRPC implementation. So even the Tendermint wrapper for event works fine - it's impossible for it to decode it's own state, because this type of event is not registered (the name of this event was changed).
Also I don't think keeping this type actually was equal to enabling it for users :)

For now would be great if the types for the one linked above and also for /osmosis.lockup.MsgUnlockPeriodLock that seems to be also removed got back on their place :) so the node wouldn't really fail returning it

[ValarDragon]

Part of the issue is its not just versioning the proto files, but also versioning the amino routes, unless we can have multiple amino routes point to the same thing concrete struct.

Is the best solution that we need to have legacy folders in every type, where we keep all old protobuf messages and items in the state? And then register these on the codecs? Seems pretty inelegant :/

Any way we can make grpcurl not fail to return, but instead return something saying 'failed to decode', provide the raw bytes and let people decode it otherwise? Or is this a bad idea, and we should register every old struct for all time?

[xoac]

It looks like MsgCreateBalancerPool can't be decoded also with current version. At least at block 3504952

{
  "code": 3,
  "message": "unable to resolve type URL /osmosis.gamm.v1beta1.MsgCreateBalancerPool: tx parse error: invalid request",
  "details": [
  ]
}