Incorrect casing on cosmos LCD query response types

[haveanicedavid]

I'm pretty ignorant of the mechanics of proto generation, and am not positive this is a telescope issue. I'm also working with the upcoming .46 cosmos release, which might be relevant

Basically, I have a repo (https://github.com/haveanicedavid/groups-ui) which is using a package (https://github.com/haveanicedavid/cosmos-groups-ts) generated through telescope, where I

1. pulled down the latest tagged SDK .46 release (v0.46.1)
2. copied the proto tags over from /proto in SDK to /proto in the telescope proj
3. pulled in missing proto files from the buf registry (buf export buf.build/cosmos/cosmos-sdk:$(curl -sS https://api.github.com/repos/cosmos/cosmos-sdk/commits/v0.46.1 | jq -r .sha) --output ./proto in the telescope proj)
4. ran the codegen script yarn codegen (fixing proto errors til it runs correctly) and published to NPM

With that, i've been able to generate an LCD client through the factory and successfully run queries, and the shape of response data on those queries is correct, but the casing is off because LCD / REST sends properties in snake_case. For example the GroupInfo interface:

compared to the response (totalWeight vs total_weight, created_at vs createdAt):

Again, I’m not sure this is an issue with telescope or on my end (there are a lot of moving parts), but it seems like the response data for LCD / REST is structured as snake_case: https://docs.cosmos.network/master/modules/group/#rest so presumably this could impact all LCD query clients generated with cosmos protos

[pyramation]

thank you @haveanicedavid — this is super detailed.

I'll dig into this. It could be we may want to visit the keepCasing option in the parser and then make sure that all works well with the existing protos.

[haveanicedavid]

cosmos/cosmos-sdk#8055 <- possibly relevant. If cosmos-SDK switches to camelCase for V2 of grpc gateway, this should be a non-issue I think

@pyramation in the immediate, if there’s an easy way to implement something like keepCasing and produce snake_case types, that would be helpful for me and presumably anyone else using LCD clients — but again, only seems worth your time if it’s easy given the above

[pyramation]

Thanks again btw! I pulled your repo down, the LCD endpoints seem to be in snake_case, not camelCase. Please read my findings below and let me know if I've misunderstood the problem.

Perhaps we just have some confusion? One idea I had... were you trying to type responses with GroupInfo instead of using the result directly from the typed response? Keep in mind, the LCD automatically types the responses with QueryGroupInfoResponse which is

export interface QueryGroupInfoResponse {
  /** info is the GroupInfo for the group. */
  info: GroupInfoRes;
}

export interface GroupInfoRes {
  /** id is the unique ID of the group. */
  id: Long;

  /** admin is the account address of the group's admin. */
  admin: string;

  /** metadata is any arbitrary metadata to attached to the group. */
  metadata: string;

  /**
   * version is used to track changes to a group's membership structure that
   * would break existing proposals. Whenever any members weight is changed,
   * or any member is added or removed this version is incremented and will
   * cause proposals based on older versions of this group to fail
   */
  version: Long;

  /** total_weight is the sum of the group members' weights. */
  total_weight: string;

  /** created_at is a timestamp specifying when a group was created. */
  created_at: string;
}

[pyramation]

ok so, maybe I did miss this manual commit: haveanicedavid/cosmos-groups-ts@913823e

so I take it this is why things are snake_case.

[haveanicedavid]

@pyramation Sorry, I should have clarified - yes, I recently updated those types manually for response values just to get back to the UI work - anywhere you see Res appended to the end of a definition (mostly within response.lcd.ts), that was manual

If you pull down https://github.com/haveanicedavid/cosmos-groups-ts and run a yarn codegen, I believe it will overwrite the manual definitions. Or you could dowgrade the comsos-groups-ts package to a prior version (like 0.0.12)

[pyramation]

@pyramation Sorry, I should have clarified - yes, I recently updated those types manually for response values just to get back to the UI work - anywhere you see Res appended to the end of a definition (mostly within response.lcd.ts), that was manual

thanks appreciate it! glad you can focus on the UI!

I'll see if I can explore the keepCasing w/o breaking other parts of the app.

[haveanicedavid]

@pyramation 🙏

Please don’t spend too much time on it - it’s not too hard to manually modify some of the types. FWIW, telescope has still been a huge timesaver, and the convenience methods are great!

[pyramation]

Please don’t spend too much time on it - it’s not too hard to manually modify some of the types. FWIW, telescope has still been a huge timesaver, and the convenience methods are great!

Wow thank you so much! That means a lot.

Well, I've got a PR I'll start working on, we can track it here: #195

It's possible that it works, so I could probably publish it with defaulting to the current casing, but it would allow us to test the original casing. If it breaks, at least other projects won't break since we can keep it under a flag/option, and then fix whatever needs to be in order to properly encode protos w the new casing.

[pyramation]

Successfully published:
 - @osmonauts/[email protected]

this has the prototypes.parser.keepCase option https://github.com/osmosis-labs/telescope#protobuf-parser

[pyramation]

maybe we can give it a whirl. I would guess there are likely issues, but if we can successfully make TXs, then this should work! Otherwise I'll keep chipping away at it ;) We should probably default to keepCase — it was inherited from ts-proto and we should use the original developer go protobuf casing anyhow.

[haveanicedavid]

oh awesome! I’ll try it out either later tonight or tomorrow morning. Thank you!

[haveanicedavid]

actually seems like TX creation through the MessageComposer works fine! But there is an issue making queries with snake_case params.

IE this (excerpt.. pseudo-codey to show snake keys`) successfully creates a msg I can use to complete a TX (🎉 ):

  return cosmosgroups.MessageComposer.withTypeUrl.createGroupWithPolicy({
    admin,
    decision_policy,
    group_policy_metadata: '',
    group_policy_as_admin: policyAsAdmin === 'true',
    group_metadata: JSON.stringify({
      name,
      description,
      forumLink,
      updatedAt: new Date().toString(),
      other: otherMetadata,
    }),
    members: groupMembers,
  })

but this query fails:

    const lcdClient = await cosmos.ClientFactory.createLCDClient({
      restEndpoint: Chain.active.rest,
    })
    lcdClient.cosmos.group.v1.groupMembers({ group_id })

— it expects a camelCase property. Bypassing TS, it returns data as expected:

    // @ts-ignore
    lcdClient.cosmos.group.v1.groupMembers({ groupId })

Poking around a bit more to see if I might know what’s going on, but this is still an improvement I think, because it’s much easier to change the casing on the Query objects in the lcd client than it is to change all return values

[pyramation]

oh interesting! so maybe we just need to fix the LCD. I'll dig in.

Very happy to hear that TXs work :)

[pyramation]

ok so looks like inside of the LCD, we're doing this:

  async groupInfo(params: QueryGroupInfoRequest): Promise<QueryGroupInfoResponse> {
    const endpoint = `cosmos/group/v1/group_info/${params.groupId}`;
    return await this.request<QueryGroupInfoResponse>(endpoint);
  }

so technically, if we fix cosmos/group/v1/group_info/${params.groupId} to be cosmos/group/v1/group_info/${params.group_id} it should work!

[haveanicedavid]

@pyramation I noticed that poking around last night, and was trying to figure out how that’s set within telescope but am still pretty new to the codebase. I should have time later tonight to poke around and see if I can hack together a potential fix, but pointers are welcome 😄

It does seem that if the value inside the LCD client is modified (ie to group_id) it should work!

[pyramation]

I'm working on a fix!

[pyramation]

#196

Successfully published:
 - @osmonauts/[email protected]

some new updates

[pyramation]

I think it may do what we need?

[haveanicedavid]

Awesome! Not only fixes the original issue, but also optional types aroudn pagination. Pulled in the new changes, and was able to run all the LCD queries and messages I have implemented without modifying the generated types at all 🥳 🎉

Closing the issue - thanks so much!

[pyramation]

amazing! great to hear!

I guess the question I have for telescope now is, how to manage when keepCase:false — I'll make a new issue ;)

[haveanicedavid]

Oh hope I didn't close prematurely! Feel free to re-open. Happy to help testing moving forward as well if you make a new issue - just tag me

[pyramation]

totally cool! I made an issue here we can track: #197