zeta-chain / node

ZetaChain’s blockchain node and an observer validator client
https://zetachain.com
MIT License
167 stars 108 forks source link

@zetachain/node-types improvements #1795

Open GMaiolo opened 8 months ago

GMaiolo commented 8 months ago

Is your feature request related to a problem? Please describe.

  1. It's hard to know where to look for a specific type due to a main index.d.ts not being present; ideally this index in the root of the package folder would export all types for easy imports.
  2. Not being able to find matching types for APIs For example when hitting /zeta-chain/crosschain/cctx/{CCTX_INDEX}, I see the node/typescript/crosschain/cross_chain_tx_pb.d.ts file, which has all types in camel case, but the API responds with snake case Also, enums are number-based, but the API responds with string literals, for example: image

Describe the solution you'd like

  1. A generated index.d.ts in the root exporting all types
  2. Update types to match exactly what the APIs return so these types can be used properly.
fadeev commented 8 months ago

We might need to switch to a different generator as this one supports only camel case seems like:

https://github.com/bufbuild/protobuf-es/blob/17327dbfd44fbff2f9c2f6c2fc0981b67153c2fe/docs/generated_code.md#field-names

And it seems like they're not planning on changing this:

https://github.com/bufbuild/protobuf-es/blob/17327dbfd44fbff2f9c2f6c2fc0981b67153c2fe/docs/faq.md#what-is-your-stance-on-adding-options-to-the-plugin

fadeev commented 8 months ago

@GMaiolo we need to switch to a different TS generator. I've been experimenting with Telescope and it seems to be working much better than generic proto-to-ts buf plugins.

Here's the generated client:

https://github.com/zeta-chain/node/tree/110646249ac485c85e9feaa3eea79aca2b247533/zetachaints/src/codegen/zetachain

For example, for foreign coins:

https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/fungible/foreign_coins

It generates proper types that match what the API is returning:

https://github.com/zeta-chain/node/blob/110646249ac485c85e9feaa3eea79aca2b247533/zetachaints/src/codegen/zetachain/fungible/foreign_coins.ts#L37-L48

GMaiolo commented 8 months ago

@fadeev nice! What about cctxs for example? I see this https://github.com/zeta-chain/node/blob/110646249ac485c85e9feaa3eea79aca2b247533/zetachaints/src/codegen/zetachain/crosschain/cross_chain_tx.ts

export enum CctxStatus {
  /** PendingInbound - some observer sees inbound tx */
  PendingInbound = 0,
  /** PendingOutbound - super majority observer see inbound tx */
  PendingOutbound = 1,
  /** OutboundMined - the corresponding outbound tx is mined */
  OutboundMined = 3,
  /** PendingRevert - outbound cannot succeed; should revert inbound */
  PendingRevert = 4,
  /** Reverted - inbound reverted. */
  Reverted = 5,
  /** Aborted - inbound tx error or invalid paramters and cannot revert; just abort. But the amount can be refunded to zetachain using and admin proposal */
  Aborted = 6,
  UNRECOGNIZED = -1,
}

With a function down below to map it to the actual string (cctxStatusFromJSON), would it be possible to have the enum correctly?

fadeev commented 8 months ago

@GMaiolo I think the issue is with:

https://github.com/zeta-chain/node/blob/9c1139e8ec3e129c05cd7e2917fefff7e427aad2/proto/crosschain/cross_chain_tx.proto#L10

We're using a custom option that instructs the proto transpiler to use strings. From what I've tested, none of the proto-to-ts tools handle this option.

@lumtis @kingpinXD can we consider not using exotic options to make sure that frontend tooling can process proto files correctly?

GMaiolo commented 8 months ago

@fadeev Brainstorming: could we custom parse all enums that have the option

option (gogoproto.goproto_enum_stringer) = true;

And parse them with same value as key?

fadeev commented 8 months ago

@GMaiolo we're not doing the parsing ourselves and instead of rely on third-party tools/parsers.

lumtis commented 7 months ago

Is option (gogoproto.goproto_enum_stringer) = true; the only problematic option so far?

fadeev commented 7 months ago

Is option (gogoproto.goproto_enum_stringer) = true; the only problematic option so far?

I believe so, yes.