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

api: Implement missing/empty fields for MVP #289

Merged
merged 12 commits into from
Jan 26, 2023
Merged

Conversation

mitjat
Copy link
Contributor

@mitjat mitjat commented Jan 23, 2023

Resolves #256

Implements all the fields that were added to the API in #247.

Most of the fields were straightforward, except for runtime balances. Changes introduced there:

  • Spin-off PR Track native balances in runtimes. Per-denomination events from accounts module.  #288 to implement tracking of oasis-sdk balances in runtimes
  • Moved /emerald/tokens to /emerald/evm_tokens; the former should be reserved for oasis-sdk tokens (currently only ROSE); internally renamed RuntimeToken->EvmToken
  • Expanded /consensus/account to return the balances across all paratimes, simultaneously for oasis-sdk tokens and EVM tokens.

The last two points are not entirely coherent with each other: one splits the two kinds of tokens across two different URLs, while the other one unifies them. Both approaches have pluses and minuses; in the end, I decided for this combo because I think it best mirrors what the frontend needs, and we can always add endpoints that slice and dice and present (almost) the same information slightly differently.

I recommend reviewing commit by commit, as many of them are boilerplate.

@mitjat mitjat changed the title Add index to RuntimeTransaction api: Implement missing/empty fields for MVP Jan 23, 2023
storage/client/queries.go Show resolved Hide resolved
storage/client/client.go Show resolved Hide resolved
api/spec/v1.yaml Show resolved Hide resolved
storage/client/client.go Outdated Show resolved Hide resolved
@mitjat mitjat force-pushed the mitjat/track-runtime-balance branch from 4286a23 to eadefee Compare January 25, 2023 20:41
Base automatically changed from mitjat/track-runtime-balance to main January 25, 2023 20:46
Copy link
Contributor Author

@mitjat mitjat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Andy!

storage/client/client.go Outdated Show resolved Hide resolved
api/spec/v1.yaml Show resolved Hide resolved
storage/client/client.go Show resolved Hide resolved
storage/client/queries.go Show resolved Hide resolved
@mitjat mitjat requested a review from Andrew7234 January 25, 2023 21:26
@mitjat mitjat force-pushed the mitjat/mvp_missing_api_fields branch from 67ab076 to c19dd29 Compare January 25, 2023 21:31
Copy link
Collaborator

@Andrew7234 Andrew7234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good pending one comment.

Would appreciate if someone more familiar with the token logic could take a second pass through

storage/client/queries.go Show resolved Hide resolved
@mitjat mitjat force-pushed the mitjat/mvp_missing_api_fields branch 4 times, most recently from 18d985a to 1af4b4e Compare January 26, 2023 09:57
@mitjat
Copy link
Contributor Author

mitjat commented Jan 26, 2023

Thank again, Andy.
In the end, I added one more commit/change to the PR: I tweaked the presentation (= API output) of runtime tokens in that I split the formerly single list into two lists: oasis-sdk tokens and EVM tokens. The former merging always felt a little forced (as also alluded to in the PR description). Now I noticed that our UI mocks require yet another EVM-token specific field (name, which oasis-sdk tokens don't have) and that pushed me to fully separate the EvmBalance and SdkBalance types.

@mitjat mitjat requested a review from buberdds January 26, 2023 10:20
@mitjat mitjat force-pushed the mitjat/mvp_missing_api_fields branch 3 times, most recently from dfc65d6 to 8ab16d6 Compare January 26, 2023 12:40
@mitjat mitjat force-pushed the mitjat/mvp_missing_api_fields branch from 9037f01 to 4390ae3 Compare January 26, 2023 18:30
@mitjat mitjat enabled auto-merge January 26, 2023 18:30
@mitjat mitjat merged commit 987a446 into main Jan 26, 2023
@mitjat mitjat deleted the mitjat/mvp_missing_api_fields branch January 26, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Export readily available data through API for MVP
2 participants