Skip to content

feat(dapi)!: split getStatus into two endpoints - #1812

Merged
markin-io merged 19 commits into
v1.0-devfrom
feat/dapi/get-status-improvements
Apr 24, 2024
Merged

feat(dapi)!: split getStatus into two endpoints#1812
markin-io merged 19 commits into
v1.0-devfrom
feat/dapi/get-status-improvements

Conversation

@markin-io

@markin-io markin-io commented Apr 11, 2024

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

Reworked getStatus query to not do Masternode related RPC reuqests

What was done?

Removed unnecessary and performance heavy getMnSync('status') and getMasternode('status') Core RPC calls from getStatus handler

How Has This Been Tested?

CI

Breaking Changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@markin-io markin-io changed the title feat(dapi): lightweight getStatus feat!(dapi): lightweight getStatus Apr 11, 2024
@markin-io markin-io added this to the v1.0.0 milestone Apr 11, 2024
@markin-io markin-io changed the title feat!(dapi): lightweight getStatus feat(dapi): lightweight getStatus Apr 11, 2024
@markin-io markin-io changed the title feat(dapi): lightweight getStatus feat(dapi)!: lightweight getStatus Apr 11, 2024
@riongull

riongull commented Apr 13, 2024

Copy link
Copy Markdown

Hey @markin-io could you give some more context about what this is about? It's a pretty big PR.

What's the motivation, what are the removals being replaced with, is this part of a bigger architectural change, etc.

Thanks!

@markin-io

markin-io commented Apr 15, 2024

Copy link
Copy Markdown
Contributor Author

Hey @riongull.
The essence of this change can be seen here.

We got rid of getMnSync('status') and getMasternode('status') RPC calls since information they are providing not needed by JS wallet, but adding extra load to Core RPC.

Most of the files affected are either autogenerated protobuf files or tests.

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was speaking with some people and community and it seems masternode status is something what people is using. Shall we just move it to a separate endpoint instead of removing? getMasternodeStatus? In this case it's probably make sense to rename getStatus to getBlockchainStatus.

Comment thread packages/dapi-grpc/clients/core/v0/nodejs/CorePromiseClient.js Outdated
shumkov
shumkov previously approved these changes Apr 23, 2024

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

@shumkov shumkov changed the title feat(dapi)!: lightweight getStatus feat(dapi)!: split getStatus into two endpoints Apr 23, 2024
@markin-io
markin-io merged commit 5404679 into v1.0-dev Apr 24, 2024
@markin-io
markin-io deleted the feat/dapi/get-status-improvements branch April 24, 2024 08:14
@thephez thephez added the dapi-endpoint DAPI endpoint addition or modification label Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dapi-endpoint DAPI endpoint addition or modification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants