oasisprotocol / explorer

Official explorer for the Oasis Network.
https://explorer.oasis.io
Apache License 2.0
8 stars 7 forks source link

[Bug]: No type safety around different types of addresses #711

Open csillag opened 1 year ago

csillag commented 1 year ago

Is there an existing issue for this?

SUMMARY

Internally in the code, we are using different address formats, but everything is a string

STEPS TO REPRODUCE

In our codebase, we are working with multiple types of addresses; most importantly Oasis addresses and ETH addresses. The problem is that both are represented using the string type, and so there is no type safety about using them the wrong way, therefore we can easily supply an ETH address where on Oasis address is expected, or vica versa.

Ideally, we should represent (wrap?) the different address types using different types, so that it would be impossible to use the wrong one everywhere.

EXPECTED BEHAVIOR

No response

csillag commented 1 year ago

Btw, this was also the root cause of the issue fixed in https://github.com/oasisprotocol/explorer/pull/710

mitjat commented 6 months ago

Discussed in call today: The nexus openapi spec includes some nonstandard tags that are used by the Go code generator to map oasis1 addresses to special Go types, rather than to string. If your codegen tool supports similar annotations, we can add them.

Heads up: There are some data structures that nexus directly dumps from the node as JSON, without checking for types internally and without specifying the types in the spec. Notably, those are tx and event bodies. When an address appears in one of those JSON subtrees, the openapi spec will not and cannot hint at its types.

lukaw3d commented 6 months ago

@mitjat can the spec replace

x-type-annotations:
  staking-address: &StakingAddressType
    type: string
    pattern: '^oasis1[a-z0-9]{40}$'
    example: "oasis1qpg2xuz46g53737343r20yxeddhlvc2ldqsjh70p"
    x-go-type: staking.Address
    x-go-type-import: { name: staking, path: "github.com/oasisprotocol/nexus/coreapi/v22.2.11/staking/api" }
    description: An Oasis-style (bech32) address.

...

            <<: *StakingAddressType

with

components:
  schemas:
    StakingAddressType:
      type: string
      pattern: '^oasis1[a-z0-9]{40}$'
      example: "oasis1qpg2xuz46g53737343r20yxeddhlvc2ldqsjh70p"
      x-go-type: staking.Address
      x-go-type-import: { name: staking, path: "github.com/oasisprotocol/nexus/coreapi/v22.2.11/staking/api" }
      description: An Oasis-style (bech32) address.

...

            $ref: '#/components/schemas/StakingAddressType'
mitjat commented 5 months ago

can the spec replace [yaml refs with $ref]

Sort of. There are tradeoffs. I took a stab at it in https://github.com/oasisprotocol/nexus/pull/641

lukaw3d commented 5 months ago

Update: Tradeoffs were solved. Address and StakingAddress are now extracted as aliases that we can post-process easily https://github.com/oasisprotocol/explorer/blob/8c5f0c51584739c752d1c431dfdc1813025ca034/src/oasis-nexus/generated/api.ts#L1761-L1771

lukaw3d commented 5 months ago

Next steps:

lukaw3d commented 2 months ago

Postprocessing: https://github.com/oasisprotocol/explorer/compare/master...lw/abandoned/stricter-addresses TODO: use strict types throughout the codebase to fix 22 type errors