New endpoint /node/loaded-keys #5943
Conversation
sstanculeanu
added
type:feature
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## rc/v1.6.next1 #5943 +/- ##
=================================================
+ Coverage 80.15% 80.17% +0.02%
=================================================
Files 709 709
Lines 94135 94172 +37
=================================================
+ Hits 75456 75506 +50
+ Misses 13331 13317 -14
- Partials 5348 5349 +1 ☔ View full report in Codecov by Sentry. |
facade/nodeFacade.go
Outdated
| @@ -36,7 +36,8 @@ import ( | |||
| const DefaultRestInterface = "localhost:8080" | |||
|
|
|||
| // DefaultRestPortOff is the default value that should be passed if it is desired | |||
| // to start the node without a REST endpoint available | |||
| // | |||
There was a problem hiding this comment.
indenting went wrong here and on L167
| func (nar *nodeApiResolver) GetManagedKeys() []string { | ||
| managedKeys := nar.managedPeersMonitor.GetManagedKeys() | ||
| return nar.parseKeys(managedKeys) | ||
| } | ||
|
|
||
| // GetLoadedKeys returns all keys that were loaded and will be managed by this node | ||
| func (nar *nodeApiResolver) GetLoadedKeys() []string { | ||
| loadedKeys := nar.managedPeersMonitor.GetLoadedKeys() |
There was a problem hiding this comment.
Question: should we append the main BLS key from crypto components to this list? Reason: maybe we are using the node in single key mode, not multikey and this route should return the main BLS key.
Or return that single key if the list returned by the managedPeersMonitor is empty. I guess this will be better as the multikey list won't be polluted by an observer BLS key.
There was a problem hiding this comment.
pushed
it will also return the public key for single key observers
|
|
||
| args := createMockArgsManagedPeersHolder() | ||
| holder, _ := keysManagement.NewManagedPeersHolder(args) | ||
| _ = holder.AddManagedPeer(skBytes1) |
There was a problem hiding this comment.
👍 testing out the sort call
sstanculeanu
changed the title
| allLoadedKeys := make([][]byte, 0, len(holder.data)) | ||
| for pk := range holder.data { | ||
| allLoadedKeys = append(allLoadedKeys, []byte(pk)) | ||
| } | ||
|
|
||
| sort.Slice(allLoadedKeys, func(i, j int) bool { | ||
| return string(allLoadedKeys[i]) < string(allLoadedKeys[j]) | ||
| }) |
There was a problem hiding this comment.
extract this logic into a separate func and used in ManagedKeysByCurrentNode and remove sorting logic from managePeersMonitor?
There was a problem hiding this comment.
ManagedKeysByCurrentNode has an extra check than GetLoadedKeysByCurrentNode, the one for validators.. thus the common code is just the sorting which can be done separate
There was a problem hiding this comment.
- allin ✅
- scenario ✅
- on multikey nodes the route prins all keys , and on single key nodes the route prints that one specific key ✅
Normal allin test: v1.6.15-dev-config-eb2e06c06d -> loaded-keys-ebeaab4359
--- Specific errors ---
block hash does not match 399
wrong nonce in block 295
miniblocks does not match 0
num miniblocks does not match 0
miniblock hash does not match 0
block bodies does not match 0
receipts hash missmatch 0
/------/
--- Statistics ---
Nr. of all ERRORS: 1
Nr. of all WARNS: 258
Nr. of new ERRORS: 1
Nr. of new WARNS: 23
Nr. of PANICS: 0
/------/
--- ERRORS ---
/------/
Reasoning behind the pull request
Proposed changes
Testing procedure
/node/loaded-keyson both single key(validator and observer) and multi keyPre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?