-
Notifications
You must be signed in to change notification settings - Fork 155
Add wallet masquerade and end-masquerade commands #779
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
Conversation
jshufro
left a comment
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.
A few comments/questions
| } | ||
|
|
||
| // Load the wallet store from disk and decrypt it | ||
| func (w *masqueradeWallet) loadStore() (bool, error) { |
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.
hmm, does this ever get called? Seems like we should probably just return an error here- can't imagine why we'd want to read the wallet store while masquerading.
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.
It gets called in the NewMasqueradeWallet() constructor and fails without an err in the case that a wallet is not loaded.
The only purpose it serves is to give you a nice printout of your loaded node wallet address in rp wallet status while you're masquerading.
The node wallet is initialized, but you are currently masquerading as 0xf6c1dCabF160d9A45691C67EfAA20EB2e89113e0
Wallet Address: 0xE8325F5f4486c2FF2AC7B522Fbc9eB249d46C936
Now that I'm taking another look, I'm pretty sure we don't need to be calling w.am.LoadAddress()within loadStore(): here and here
It already gets called within GetNodeAccount() when we're masquerading and need to fetch the masquerade address.
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.
Sounds like a reasonable vector to simplify this a bit, right?
…e constructors for each wallet type
…stead of GetAddress(). Delete isLoaded field from AddressManager
Co-authored-by: Jacob Shufro <[email protected]>
…dress, return err in TestRecovery while masquerading
This PR introduces two commands:
rocketpool wallet masquerade - Change your node's effective address to a different one. Your node will not be able to submit transactions or sign messages since you don't have the corresponding wallet's private key.rocketpool wallet end-masquerade - End a masquerade, restoring your node's effective address back to your wallet address if one is loaded.While masquerading, the node and watchtower daemon will continue to submit tasks using the initialized (stored on disk) wallet.
Additionally, the
NewWalletconstructor in the wallet package has been refactored to return an interface rather than a concrete struct, allowing flexibility for future wallet implementations.