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

Implement GetBlockchainInfo #1895

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

maxconway
Copy link
Contributor

  • Previously had placeholders in it

All the fields of the return are also available as other API endpoints. For transaction rate, tx block rate, and sharding_structure I delegated to the other APIs, but for the others I didn't, for a two main reasons:

  1. To avoid repeatedly locking and unlocking the Arc<Mutex<Node>>, which felt like it'd be slow
  2. Some of the APIs convert numbers to strings to return them, and I thought re-parsing them would be inelegant. I could have converted them all the wrappers but that seemed a bit too much.

This already has a test which I added in #1864

- Previously had placeholders in it

All the fields of the return are also available as other API endpoints.
For transaction rate, tx block rate, and sharding_structure I delegated to the other APIs, but for the others I didn't, for a two main reasons:
1. To avoid repeatedly locking and unlocking the `Arc<Mutex<Node>>`, which felt like it'd be slow
2. Some of the APIs convert numbers to strings to return them, and I thought re-parsing them would be inelegant. I could have converted them all the wrappers but that seemed a bit too much.

This already has a test which I added in #1864
@maxconway maxconway added the area:apis Related to externally-facing APIs label Nov 25, 2024
@maxconway maxconway marked this pull request as ready for review November 25, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:apis Related to externally-facing APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant