Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decide on use of int64 in JSON responses. #1609

Closed
6 tasks done
abuiles opened this issue Aug 19, 2019 · 6 comments
Closed
6 tasks done

Decide on use of int64 in JSON responses. #1609

abuiles opened this issue Aug 19, 2019 · 6 comments
Assignees
Labels
horizon horizon-api Issues or features related to the Horizon API

Comments

@abuiles
Copy link
Contributor

abuiles commented Aug 19, 2019

This is a follow up of #1363 - we have multiple places where we return int64 in JSON responses, which could lead to issues in JS applications. We fixed the problem reported in #1363, but the question now is, should we do the same for the following struct and moving forward, does it make sense to avoid int64 on Horizon structs? We haven't been consistent with this.

  • go/protocols/horizon/operations/main.go:139:10: OfferID int64 json:"offer_id"
  • go/protocols/horizon/operations/main.go:146:10: OfferID int64 json:"offer_id"
  • go/protocols/horizon/effects/main.go:260:20: OfferID int64 json:"offer_id"
  • go/protocols/horizon/main.go:228:21: ID int64 json:"id"
  • go/protocols/horizon/main.go:376:16: Timestamp int64 json:"timestamp"
  • go/protocols/horizon/main.go:377:16: TradeCount int64 json:"trade_count"
@abuiles abuiles added horizon horizon-api Issues or features related to the Horizon API labels Aug 19, 2019
@bartekn
Copy link
Contributor

bartekn commented Aug 20, 2019

@abuiles can you update the Horizon 0.20.0 CHANGELOG with this list and add Action needed tags (for 0.22.0 release)?

@bartekn bartekn added this to the Horizon 0.22.0 milestone Aug 21, 2019
@ire-and-curses
Copy link
Member

Because of our imminent special v12 protocol release (Horizon 0.22), these fields are now scheduled to change in 0.23.

@abuiles abuiles self-assigned this Oct 15, 2019
@ire-and-curses
Copy link
Member

After internal discussion we're going to change all of these in Horizon 0.25, to give SDKs enough time to handle it.

@leighmcculloch
Copy link
Member

Has a decision been made on this yet? I don't see a decision communicated here, but give SDKs enough time to handle it sounds like we've made one.

@ire-and-curses
Copy link
Member

Yes. These fields will change to string. We'll be raising issues with all the SDKs (ours + community). We'll aim to support the fields both before and after the change with the next SDK version, so that the Horizon update won't cause clients to break at a discrete point in time.

@abuiles
Copy link
Contributor Author

abuiles commented Jan 20, 2020

Fixed on #2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
horizon horizon-api Issues or features related to the Horizon API
Projects
None yet
Development

No branches or pull requests

4 participants