Skip to content
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

Addressing smartnode-issue-572 #621

Open
wants to merge 2 commits into
base: v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions client/wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ func (r *WalletRequester) Masquerade(address common.Address) (*types.ApiResponse
}

// Rebuild the validator keys associated with the wallet
func (r *WalletRequester) Rebuild() (*types.ApiResponse[api.WalletRebuildData], error) {
return client.SendGetRequest[api.WalletRebuildData](r, "rebuild", "Rebuild", nil)
func (r *WalletRequester) Rebuild(enablePartialRebuild bool) (*types.ApiResponse[api.WalletRebuildData], error) {
return client.SendGetRequest[api.WalletRebuildData](r, "rebuild", "Rebuild", map[string]string{"enable-partial-rebuild": strconv.FormatBool(enablePartialRebuild)})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting. bool types are native json types- do we need to use a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I was just following convention on this one. For example, the skip-validator-key-recovery parameter uses a similar code flow to translate the string to boolean.

I could see the perspective that this is a bit silly since the parameter is a string, gets converted to a bool to call the method then is immediately converted back to a string for the payload. I also cringe at the idea of passing a string around which is serving as a boolean, so neither way is elegant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SendGetRequest method takes a map[string]string. Refactoring this down could make this more generic, but for now requires parameters as strings.

}

// Recover wallet
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ require (
rsc.io/tmplfunc v0.0.3 // indirect
)

require golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8

replace github.com/wealdtech/go-merkletree v1.0.1-0.20190605192610-2bb163c2ea2a => github.com/rocket-pool/go-merkletree v1.0.1-0.20220406020931-c262d9b976dd

//replace github.com/rocket-pool/node-manager-core => ../node-manager-core
3 changes: 3 additions & 0 deletions rocketpool-cli/commands/wallet/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ func RegisterCommands(app *cli.App, name string, aliases []string) {
Name: "rebuild",
Aliases: []string{"b"},
Usage: "Rebuild validator keystores from derived keys",
Flags: []cli.Flag{
enablePartialRebuild,
},
Action: func(c *cli.Context) error {
// Validate args
utils.ValidateArgCount(c, 0)
Expand Down
57 changes: 36 additions & 21 deletions rocketpool-cli/commands/wallet/rebuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,41 +54,56 @@ func rebuildWallet(c *cli.Context) error {
}(customKeyPasswordFile)
}

var enablePartialRebuildValue = false
if enablePartialRebuild.Name != "" {
enablePartialRebuildValue = c.Bool(enablePartialRebuild.Name)
}

// Log
fmt.Println("Rebuilding node validator keystores...")
fmt.Printf("Partial rebuild enabled: %s.\n", enablePartialRebuild.Value)

// Rebuild wallet
response, err := rp.Api.Wallet.Rebuild()
response, err := rp.Api.Wallet.Rebuild(enablePartialRebuildValue)
if err != nil {
return err
}

// Log & return
fmt.Println("The node wallet was successfully rebuilt.")
if len(response.Data.ValidatorKeys) > 0 {
fmt.Println("Validator keys:")
for _, key := range response.Data.ValidatorKeys {
fmt.Println(key.Hex())
// Handle and print failure reasons with associated public keys
if len(response.Data.FailureReasons) > 0 {
fmt.Println("Some keys could not be recovered. You may need to import them manually, as they are not " +
"associated with your node wallet mnemonic. See the documentation for more details.")
fmt.Println("Failure reasons:")
SolezOfScience marked this conversation as resolved.
Show resolved Hide resolved
for pubkey, reason := range response.Data.FailureReasons {
fmt.Printf("Public Key: %s - Failure Reason: %s\n", pubkey.Hex(), reason)
}
fmt.Println()
} else {
fmt.Println("No validator keys were found.")
fmt.Println("No failures reported.")
}

if !utils.Confirm("Would you like to restart your Validator Client now so it can attest with the recovered keys?") {
fmt.Println("Please restart the Validator Client manually at your earliest convenience to load the keys.")
return nil
}
if len(response.Data.RebuiltValidatorKeys) > 0 {
fmt.Println("Validator keys:")
for _, key := range response.Data.RebuiltValidatorKeys {
fmt.Println(key.Hex())
}

// Restart the VC
fmt.Println("Restarting Validator Client...")
_, err = rp.Api.Service.RestartVc()
if err != nil {
fmt.Printf("Error restarting Validator Client: %s\n", err.Error())
fmt.Println("Please restart the Validator Client manually at your earliest convenience to load the keys.")
return nil
if !utils.Confirm("Would you like to restart your Validator Client now so it can attest with the recovered keys?") {
fmt.Println("Please restart the Validator Client manually at your earliest convenience to load the keys.")
return nil
}

// Restart the VC
fmt.Println("Restarting Validator Client...")
_, err = rp.Api.Service.RestartVc()
if err != nil {
fmt.Printf("Error restarting Validator Client: %s\n", err.Error())
fmt.Println("Please restart the Validator Client manually at your earliest convenience to load the keys.")
return nil
}
fmt.Println("Validator Client restarted successfully.")
} else {
fmt.Println("No validator keys were found.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a slight preference to see this moved up to L84 and the conditional inverted. An early return can prevent the need for an else block.

}
fmt.Println("Validator Client restarted successfully.")

return nil
}
5 changes: 5 additions & 0 deletions rocketpool-cli/commands/wallet/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ var (
Aliases: []string{"a"},
Usage: "If you are recovering a wallet that was not generated by the Smartnode and don't know the derivation path or index of it, enter the address here. The Smartnode will search through its library of paths and indices to try to find it.",
}
enablePartialRebuild = &cli.StringSliceFlag{
Name: "enable-partial-rebuild",
Aliases: []string{"p"},
Usage: "Allows the wallet rebuild process to partially succeed, responding with public keys for successfully rebuilt targets and errors for rebuild failures",
}
)

// Prompt for a new wallet password
Expand Down
15 changes: 10 additions & 5 deletions rocketpool-daemon/api/wallet/rebuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package wallet

import (
"fmt"
"net/url"

"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/gorilla/mux"
"github.com/rocket-pool/node-manager-core/utils/input"
"github.com/rocket-pool/smartnode/v2/rocketpool-daemon/common/validator"
"net/url"

"github.com/rocket-pool/node-manager-core/api/server"
"github.com/rocket-pool/node-manager-core/api/types"
Expand All @@ -24,7 +25,9 @@ func (f *walletRebuildContextFactory) Create(args url.Values) (*walletRebuildCon
c := &walletRebuildContext{
handler: f.handler,
}
return c, nil
inputError := server.ValidateOptionalArg("enable-partial-rebuild", args, input.ValidateBool, &c.enablePartialRebuild, nil)

return c, inputError
}

func (f *walletRebuildContextFactory) RegisterRoute(router *mux.Router) {
Expand All @@ -38,12 +41,14 @@ func (f *walletRebuildContextFactory) RegisterRoute(router *mux.Router) {
// ===============

type walletRebuildContext struct {
handler *WalletHandler
handler *WalletHandler
enablePartialRebuild bool
}

func (c *walletRebuildContext) PrepareData(data *api.WalletRebuildData, opts *bind.TransactOpts) (types.ResponseStatus, error) {
sp := c.handler.serviceProvider
vMgr := sp.GetValidatorManager()
keyRecoveryManager := validator.NewKeyRecoveryManager(vMgr, c.enablePartialRebuild, false)

// Requirements
err := sp.RequireWalletReady()
Expand All @@ -56,7 +61,7 @@ func (c *walletRebuildContext) PrepareData(data *api.WalletRebuildData, opts *bi
}

// Recover validator keys
data.ValidatorKeys, err = vMgr.RecoverMinipoolKeys(false)
data.RebuiltValidatorKeys, data.FailureReasons, err = keyRecoveryManager.RecoverMinipoolKeys()
if err != nil {
return types.ResponseStatus_Error, fmt.Errorf("error recovering minipool keys: %w", err)
}
Expand Down
188 changes: 188 additions & 0 deletions rocketpool-daemon/common/validator/key-recovery-manager.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
package validator

import (
"fmt"
"github.com/rocket-pool/node-manager-core/beacon"
"github.com/rocket-pool/node-manager-core/utils"
"golang.org/x/exp/maps"
"strings"
)

type KeyRecoveryManager struct {
manager *ValidatorManager
partialEnabled bool
testOnly bool
}

func NewKeyRecoveryManager(m *ValidatorManager, partialEnabled bool, testOnly bool) *KeyRecoveryManager {
return &KeyRecoveryManager{
manager: m,
partialEnabled: partialEnabled,
testOnly: testOnly,
}
}

func (s *KeyRecoveryManager) RecoverMinipoolKeys() ([]beacon.ValidatorPubkey, map[beacon.ValidatorPubkey]error, error) {
publicKeys, err := s.manager.GetMinipools()
if err != nil {
return []beacon.ValidatorPubkey{}, map[beacon.ValidatorPubkey]error{}, err
}

recoveredCustomPublicKeys, unrecoverableCustomPublicKeys, err := s.checkForAndRecoverCustomKeys(publicKeys)
if err != nil && !s.partialEnabled {
return maps.Keys(recoveredCustomPublicKeys), unrecoverableCustomPublicKeys, err
}

recoveredConventionalPublicKeys, unrecoveredPublicKeys := s.recoverConventionalKeys(publicKeys)

var allRecoveredPublicKeys []beacon.ValidatorPubkey
allRecoveredPublicKeys = append(allRecoveredPublicKeys, maps.Keys(recoveredCustomPublicKeys)...)
allRecoveredPublicKeys = append(allRecoveredPublicKeys, recoveredConventionalPublicKeys...)

for publicKey, err := range unrecoveredPublicKeys {
unrecoverableCustomPublicKeys[publicKey] = err
}

return allRecoveredPublicKeys, unrecoveredPublicKeys, nil
}

func (s *KeyRecoveryManager) checkForAndRecoverCustomKeys(
publicKeys map[beacon.ValidatorPubkey]bool,
) (map[beacon.ValidatorPubkey]bool, map[beacon.ValidatorPubkey]error, error) {

recoveredKeys := make(map[beacon.ValidatorPubkey]bool)
recoveryFailures := make(map[beacon.ValidatorPubkey]error)
var passwords map[string]string

keyFiles, err := s.manager.LoadFiles()
if err != nil {
return recoveredKeys, recoveryFailures, err
}

if len(keyFiles) == 0 {
return recoveredKeys, recoveryFailures, nil
}

passwords, err = s.manager.LoadCustomKeyPasswords()
if err != nil {
return recoveredKeys, recoveryFailures, err
}

for _, file := range keyFiles {
keystore, err := s.manager.ReadCustomKeystore(file)
if err != nil {
if s.partialEnabled {
continue
}
return recoveredKeys, recoveryFailures, err
}

if _, exists := publicKeys[keystore.Pubkey]; !exists {
err := fmt.Errorf("custom keystore for pubkey %s not found in minipool keyset", keystore.Pubkey.Hex())
recoveryFailures[keystore.Pubkey] = err
if s.partialEnabled {
continue
}
return recoveredKeys, recoveryFailures, err
}

formattedPublicKey := strings.ToUpper(utils.RemovePrefix(keystore.Pubkey.Hex()))
password, exists := passwords[formattedPublicKey]
if !exists {
err := fmt.Errorf("custom keystore for pubkey %s needs a password, but none was provided", keystore.Pubkey.Hex())
recoveryFailures[keystore.Pubkey] = err
if s.partialEnabled {
continue
}
return recoveredKeys, recoveryFailures, err
}

privateKey, err := s.manager.DecryptCustomKeystore(keystore, password)
if err != nil {
err := fmt.Errorf("error recreating private key for validator %s: %w", keystore.Pubkey.Hex(), err)
recoveryFailures[keystore.Pubkey] = err
if s.partialEnabled {
continue
}
return recoveredKeys, recoveryFailures, err
}

reconstructedPublicKey := beacon.ValidatorPubkey(privateKey.PublicKey().Marshal())
if reconstructedPublicKey != keystore.Pubkey {
err := fmt.Errorf("private keystore file %s claims to be for validator %s but it's for validator %s", file.Name(), keystore.Pubkey.Hex(), reconstructedPublicKey.Hex())
recoveryFailures[keystore.Pubkey] = err
if s.partialEnabled {
continue
}
return recoveredKeys, recoveryFailures, err
}

if !s.testOnly {
if err := s.manager.StoreValidatorKey(&privateKey, keystore.Path); err != nil {
recoveryFailures[keystore.Pubkey] = err
if s.partialEnabled {
continue
}
return recoveredKeys, recoveryFailures, err
}
}
recoveredKeys[reconstructedPublicKey] = true

delete(publicKeys, keystore.Pubkey)
}

return recoveredKeys, recoveryFailures, nil
}

func (s *KeyRecoveryManager) recoverConventionalKeys(publicKeys map[beacon.ValidatorPubkey]bool) ([]beacon.ValidatorPubkey, map[beacon.ValidatorPubkey]error) {
var recoveredPublicKeys []beacon.ValidatorPubkey
unrecoverablePublicKeys := map[beacon.ValidatorPubkey]error{}

bucketStart := uint64(0)
for {
if bucketStart >= bucketLimit {
break
}
bucketEnd := bucketStart + bucketSize
if bucketEnd > bucketLimit {
bucketEnd = bucketLimit
}

keys, err := s.manager.GetValidatorKeys(bucketStart, bucketEnd-bucketStart)
if err != nil {
return recoveredPublicKeys, map[beacon.ValidatorPubkey]error{beacon.ValidatorPubkey{}: fmt.Errorf("error getting node's validator keys")}
}

for _, validatorKey := range keys {
if exists := publicKeys[validatorKey.PublicKey]; exists {
delete(publicKeys, validatorKey.PublicKey)
if !s.testOnly {
if err := s.manager.SaveValidatorKey(validatorKey); err != nil {
unrecoverablePublicKeys[validatorKey.PublicKey] = err
if s.partialEnabled {
continue
}
return recoveredPublicKeys, unrecoverablePublicKeys
}
}
recoveredPublicKeys = append(recoveredPublicKeys, validatorKey.PublicKey)

} else {
err := fmt.Errorf("keystore for pubkey %s not found in minipool keyset", validatorKey.PublicKey)
unrecoverablePublicKeys[validatorKey.PublicKey] = err
if !s.partialEnabled {
return recoveredPublicKeys, unrecoverablePublicKeys
}
}
}

if len(publicKeys) == 0 {
// All keys have been recovered.
break
}

bucketStart = bucketEnd
}

return recoveredPublicKeys, unrecoverablePublicKeys
}
6 changes: 0 additions & 6 deletions rocketpool-daemon/common/validator/recover-keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ import (
"gopkg.in/yaml.v3"
)

const (
bucketSize uint64 = 20
bucketLimit uint64 = 2000
pubkeyBatchSize int = 500
)

func (m *ValidatorManager) RecoverMinipoolKeys(testOnly bool) ([]beacon.ValidatorPubkey, error) {
status, err := m.wallet.GetStatus()
if err != nil {
Expand Down
Loading