Refactor handleValidatorExitCommand
#3344
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zah
suggested changes
Jan 31, 2022
beacon_chain/nimbus_beacon_node.nim
Outdated
| proc restValidatorExit(config: BeaconNodeConf) {.async.} = | ||
| let | ||
| address = if isNone(config.restUrlForExit): | ||
| resolveTAddress("127.0.0.1:47000")[0] |
There was a problem hiding this comment.
This is not the right default port (5052). The code here should really use the DefaultEth2RestPort constant from the spec/network module.
beacon_chain/nimbus_beacon_node.nim
Outdated
| quiting = "q" | ||
| confirmation = "I understand the implications of submitting a voluntary exit" | ||
|
|
||
| proc askForExitConfirmation(): string = |
There was a problem hiding this comment.
Since you have the enum above, why don't you use it as the return value of this function?
Make `validator exit command` work both with `JSON-RPC` and `REST` APIs Fix problem with specifying rest-url using `localhost` Change back exit error messages in `state_transition_block`
EmilIvanichkovv
force-pushed
the
validator-exits-fix
branch
from
491a705
to
3733edb
Compare
|
At some point, we should move this code to a separate module - trusted node sync started this transformation, and now that this command becomes long with multiple options etc, a new module that handles it would be appropriate. it's not a blocker for the PR, but certainly a nice-to-have |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make
validator exit commandwork both withJSON-RPCandRESTAPIsFix problem with specifying rest-url using
localhostChange back exit error messages in
state_transition_block