-
Notifications
You must be signed in to change notification settings - Fork 112
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9776 +/- ##
============================================
+ Coverage 92.23% 92.25% +0.02%
- Complexity 7628 7769 +141
============================================
Files 937 951 +14
Lines 32117 32467 +350
Branches 4071 4113 +42
============================================
+ Hits 29622 29953 +331
- Misses 1542 1546 +4
- Partials 953 968 +15 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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
This is an example of the /accounts response: 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. |
7415cf2
to
54dda1f
Compare
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
54dda1f
to
fe35df4
Compare
… API Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
fe35df4
to
ede7fd4
Compare
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
ede7fd4
to
bacbf07
Compare
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
Signed-off-by: sdimitrov9 <stoyan.dimitrov@limechain.tech>
Quality Gate passedIssues Measures |
@@ -52,8 +52,13 @@ describe('NetworkNodeViewModel', () => { | |||
stakeRewarded: 4, | |||
stakingPeriod: '1654991999999999999', | |||
}, | |||
node: { | |||
adminKey: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test should populate a valid key
@@ -367,6 +367,7 @@ | |||
"responseJson": { | |||
"nodes": [ | |||
{ | |||
"admin_key": null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All specs have the admin_key as null. We should populate valid admin keys for at least one file like this one. It should include different variations like ED25519, ECDSA, 3200 immutable key, and one complex key.
Description:
Adding admin_key to the get /network/nodes REST API
Related issue(s):
Fixes #8757
Notes for reviewer:
Checklist