Remove Keymanageropts Pattern from Wallets and Remove Enable/Disable Feature for V1 CLI #7831
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
rauljordan
force-pushed
the
bye-km-opts
branch
from
e8f5915
to
47b55ad
Compare
rauljordan
commented
Nov 17, 2020
| @@ -387,30 +378,6 @@ func (w *Wallet) ReadKeymanagerConfigFromDisk(_ context.Context) (io.ReadCloser, | |||
|
|
|||
| } | |||
|
|
|||
| // LockWalletConfigFile lock read and write to wallet file in order to prevent | |||
There was a problem hiding this comment.
Now we don't have anything to lock. Slashing protection will take care of this. Additionally, we allow importing keys dynamically at runtime which would be invalidated by a lock
rauljordan
added
Accounts
Cleanup
rauljordan
commented
Nov 17, 2020
| @@ -0,0 +1,65 @@ | |||
| package imported | |||
There was a problem hiding this comment.
Please review carefully
| @@ -0,0 +1,240 @@ | |||
| package imported | |||
There was a problem hiding this comment.
Please review these tests
rauljordan
changed the title
prestonvanloon
previously approved these changes
Nov 17, 2020
prestonvanloon
previously approved these changes
Nov 17, 2020
prestonvanloon
previously approved these changes
Nov 17, 2020
|
Thanks for cc me. This is much better indeed! |
prestonvanloon
previously approved these changes
Nov 17, 2020
prestonvanloon
approved these changes
Nov 17, 2020
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.
No tracking issue. This PR removes the legacy
KeymanagerOptspattern we used in our wallets. We were basically storing a json file full of versioning and configuration options for a wallet that is underutilized and ultimately unnecessary in Prysm. This leads to more complex code in our wallet and makes it harder for people to programmatically manipulate their wallet accounts on disk in cloud deployments. This PR completely removes the pattern.This PR also adds comprehensive tests to accounts enable + disable, but opts to remove the features from our CLI entrypoints for Prysm v1 until we have confidence in prod. cc @fabdarice