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

Add admin key to get /network/nodes endpoint #9776

Merged
merged 8 commits into from
Nov 25, 2024

Conversation

sdimitrov9
Copy link
Contributor

@sdimitrov9 sdimitrov9 commented Nov 18, 2024

Description:
Adding admin_key to the get /network/nodes REST API

Related issue(s):

Fixes #8757

Notes for reviewer:
Screenshot 2024-11-18 at 11 38 06
Screenshot 2024-11-18 at 11 39 14

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)
Screenshot 2024-11-18 at 11 43 48

@sdimitrov9 sdimitrov9 added the rest Area: REST API label Nov 18, 2024
@sdimitrov9 sdimitrov9 self-assigned this Nov 18, 2024
@sdimitrov9 sdimitrov9 requested a review from a team as a code owner November 18, 2024 09:42
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.27%. Comparing base (0c38f14) to head (b89ce25).
Report is 28 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9776      +/-   ##
============================================
+ Coverage     92.23%   92.27%   +0.04%     
- Complexity     7628     7770     +142     
============================================
  Files           937      951      +14     
  Lines         32117    32457     +340     
  Branches       4071     4113      +42     
============================================
+ Hits          29622    29951     +329     
+ Misses         1542     1541       -1     
- Partials        953      965      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@sdimitrov9 sdimitrov9 added the enhancement Type: New feature label Nov 18, 2024
@SvetBorislavov
Copy link

SvetBorislavov commented Nov 18, 2024

Hey, the 'admin_key' value structure is not correct. The node's admin_key is of type 'Key', here is the definition inside the protobuf: https://github.com/hashgraph/hedera-protobufs/blob/main/services/state/addressbook/node.proto#L148
The mirror node currently returns this kind of 'Key' structure in the following format:

"key": {
        "_type": "ED25519",
        "key": "61f37fc1bbf3ff4453712ee6a305c5c7255955f7889ec3bf30426f1863158ef4"
      }

This is an example of the /accounts response:
Here is the whole request: https://testnet.mirrornode.hedera.com/api/v1/accounts?account.id=2159149

The structure should be such because the key may be a KeyList with multiple levels.

The key type is also shown in the screenshot you have attached.

@sdimitrov9 sdimitrov9 force-pushed the 8757-add-admin-key-to-get-nodes-endpoint branch from 7415cf2 to 54dda1f Compare November 18, 2024 14:58
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
@sdimitrov9 sdimitrov9 force-pushed the 8757-add-admin-key-to-get-nodes-endpoint branch from 54dda1f to fe35df4 Compare November 18, 2024 15:09
… API

Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
@sdimitrov9 sdimitrov9 force-pushed the 8757-add-admin-key-to-get-nodes-endpoint branch from fe35df4 to ede7fd4 Compare November 18, 2024 15:16
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
@sdimitrov9 sdimitrov9 force-pushed the 8757-add-admin-key-to-get-nodes-endpoint branch from ede7fd4 to bacbf07 Compare November 18, 2024 15:17
@sdimitrov9 sdimitrov9 marked this pull request as draft November 18, 2024 15:24
@steven-sheehy steven-sheehy added this to the 0.119.0 milestone Nov 18, 2024
hedera-mirror-rest/viewmodel/networkNodeViewModel.js Outdated Show resolved Hide resolved
hedera-mirror-rest/model/node.js Outdated Show resolved Hide resolved
hedera-mirror-rest/model/node.js Outdated Show resolved Hide resolved
hedera-mirror-rest/api/v1/openapi.yml Outdated Show resolved Hide resolved
hedera-mirror-rest/api/v1/openapi.yml Outdated Show resolved Hide resolved
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
@sdimitrov9 sdimitrov9 marked this pull request as ready for review November 19, 2024 14:57
@sdimitrov9 sdimitrov9 requested a review from a team November 19, 2024 14:57
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
@steven-sheehy steven-sheehy changed the title 8757 add admin key to get /network/nodes endpoint Add admin key to get /network/nodes endpoint Nov 19, 2024
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
Copy link

sonarcloud bot commented Nov 25, 2024

Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@edwin-greene edwin-greene 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

@steven-sheehy steven-sheehy merged commit 5f29238 into main Nov 25, 2024
34 checks passed
@steven-sheehy steven-sheehy deleted the 8757-add-admin-key-to-get-nodes-endpoint branch November 25, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Type: New feature rest Area: REST API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HIP-869 Add admin_key to the /network/nodes REST API
4 participants