Web3signer: persistent public keys (#13682)

* WIP * broken and still wip * more wip improving saving * wip * removing cyclic dependency * gaz * fixes * fixing more tests and how files load * fixing wallet tests * fixing test * updating keymanager tests * improving how the web3signer keymanager works * WIP * updated keymanager to read from file * gaz * reuse readkeyfile function and add in duplicate keys check * adding in locks to increase safety * refactored how saving keys work, more tests needed: * fix test * fix tests * adding unit tests and cleaning up locks * fixing tests * tests were not fixed properly * removing unneeded files * Update cmd/validator/accounts/wallet_utils.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update validator/accounts/wallet/wallet.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * review feedback * updating flags and e2e * deepsource fix * resolving feedback * removing fatal test for now * addressing manu's feedback * gofmt * fixing tests * fixing unit tests * more idomatic feedback * updating log files * updating based on preston's suggestion * improving logs and event triggers * addressing comments from manu * truncating was not triggering key file reload * fixing unit test * removing wrong dependency * fix another broken unit test * fixing bad pathing on file * handle errors in test * fixing testdata dependency * resolving deepsource and comment around logs * removing unneeded buffer * reworking ux of web3signer file, unit tests to come * adding unit tests for file change retries * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * updating based on review feedback * missed err check * adding some aliases to make running easier * Update validator/keymanager/remote-web3signer/log.go Co-authored-by: Manu NALEPA <enalepa@offchainlabs.com> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * radek's review * Update validator/keymanager/remote-web3signer/internal/client.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Update validator/keymanager/remote-web3signer/keymanager.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * addressing more review feedback and linting * fixing tests * adding log * adding 1 more test * improving logs --------- Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> Co-authored-by: Manu NALEPA <enalepa@offchainlabs.com> Co-authored-by: Radosław Kapka <rkapka@wp.pl>
prysmaticlabs · Jun 26, 2024 · 539b981 · 539b981
1 parent aad29ff
commit 539b981
Show file tree

Hide file tree

Showing 32 changed files with 1,096 additions and 267 deletions.
diff --git a/cmd/validator/accounts/exit.go b/cmd/validator/accounts/exit.go
@@ -34,7 +34,7 @@ func Exit(c *cli.Context, r io.Reader) error {
 	beaconRPCProvider := c.String(flags.BeaconRPCProviderFlag.Name)
 	if !c.IsSet(flags.Web3SignerURLFlag.Name) && !c.IsSet(flags.WalletDirFlag.Name) && !c.IsSet(flags.InteropNumValidators.Name) {
 		return errors.Errorf("No validators found, please provide a prysm wallet directory via flag --%s "+
-			"or a web3signer location with corresponding public keys via flags --%s and --%s ",
+			"or a remote signer location with corresponding public keys via flags --%s and --%s ",
 			flags.WalletDirFlag.Name,
 			flags.Web3SignerURLFlag.Name,
 			flags.Web3SignerPublicValidatorKeysFlag,
@@ -62,7 +62,7 @@ func Exit(c *cli.Context, r io.Reader) error {
 		}
 		config, err := node.Web3SignerConfig(c)
 		if err != nil {
-			return errors.Wrapf(err, "could not configure web3signer")
+			return errors.Wrapf(err, "could not configure remote signer")
 		}
 		config.GenesisValidatorsRoot = resp.GenesisValidatorsRoot
 		w, km, err = walletWithWeb3SignerKeymanager(c, config)

diff --git a/cmd/validator/accounts/wallet_utils.go b/cmd/validator/accounts/wallet_utils.go
@@ -30,7 +30,7 @@ func walletWithKeymanager(c *cli.Context) (*wallet.Wallet, keymanager.IKeymanage
 }
 
 func walletWithWeb3SignerKeymanager(c *cli.Context, config *remote_web3signer.SetupConfig) (*wallet.Wallet, keymanager.IKeymanager, error) {
-	w := wallet.NewWalletForWeb3Signer()
+	w := wallet.NewWalletForWeb3Signer(c)
 	km, err := w.InitializeKeymanager(c.Context, iface.InitKeymanagerConfig{ListenForChanges: false, Web3SignerConfig: config})
 	if err != nil {
 		return nil, nil, err

diff --git a/cmd/validator/flags/flags.go b/cmd/validator/flags/flags.go
@@ -288,18 +288,30 @@ var (
 	// example:--validators-external-signer-url=http://localhost:9000
 	// web3signer documentation can be found in Consensys' web3signer project docs
 	Web3SignerURLFlag = &cli.StringFlag{
-		Name:  "validators-external-signer-url",
-		Usage: "URL for consensys' web3signer software to use with the Prysm validator client.",
-		Value: "",
+		Name:    "validators-external-signer-url",
+		Usage:   "URL for consensys' web3signer software to use with the Prysm validator client.",
+		Value:   "",
+		Aliases: []string{"remote-signer-url"},
 	}
 	// Web3SignerPublicValidatorKeysFlag defines a comma-separated list of hex string public keys or external url for web3signer to use for validator signing.
 	// example with external url: --validators-external-signer-public-keys= https://web3signer.com/api/v1/eth2/publicKeys
 	// example with public key: --validators-external-signer-public-keys=0xa99a...e44c,0xb89b...4a0b
 	// web3signer documentation can be found in Consensys' web3signer project docs```
 	Web3SignerPublicValidatorKeysFlag = &cli.StringSliceFlag{
-		Name:  "validators-external-signer-public-keys",
-		Usage: "Comma separated list of public keys OR an external url endpoint for the validator to retrieve public keys from for usage with web3signer.",
+		Name:    "validators-external-signer-public-keys",
+		Usage:   "Comma separated list of public keys OR an external url endpoint for the validator to retrieve public keys from for usage with web3signer.",
+		Aliases: []string{"remote-signer-keys"},
+	}
+
+	// Web3SignerKeyFileFlag defines a file for keys to persist to.
+	// example:--validators-external-signer-key-file=./path/to/keys.txt
+	Web3SignerKeyFileFlag = &cli.StringFlag{
+		Name:    "validators-external-signer-key-file",
+		Usage:   "A file path used to load remote public validator keys and persist them through restarts.",
+		Value:   "",
+		Aliases: []string{"remote-signer-keys-file"},
 	}
+
 	// KeymanagerKindFlag defines the kind of keymanager desired by a user during wallet creation.
 	KeymanagerKindFlag = &cli.StringFlag{
 		Name:  "keymanager-kind",

diff --git a/cmd/validator/main.go b/cmd/validator/main.go
@@ -79,6 +79,7 @@ var appFlags = []cli.Flag{
 	// Consensys' Web3Signer flags
 	flags.Web3SignerURLFlag,
 	flags.Web3SignerPublicValidatorKeysFlag,
+	flags.Web3SignerKeyFileFlag,
 	flags.SuggestedFeeRecipientFlag,
 	flags.ProposerSettingsURLFlag,
 	flags.ProposerSettingsFlag,

diff --git a/cmd/validator/usage.go b/cmd/validator/usage.go
@@ -127,6 +127,7 @@ var appHelpFlagGroups = []flagGroup{
 		Flags: []cli.Flag{
 			flags.Web3SignerURLFlag,
 			flags.Web3SignerPublicValidatorKeysFlag,
+			flags.Web3SignerKeyFileFlag,
 		},
 	},
 	{

diff --git a/io/file/BUILD.bazel b/io/file/BUILD.bazel
@@ -2,12 +2,16 @@ load("@prysm//tools/go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = ["fileutil.go"],
+    srcs = [
+        "fileutil.go",
+        "log.go",
+    ],
     importpath = "github.com/prysmaticlabs/prysm/v5/io/file",
     visibility = ["//visibility:public"],
     deps = [
         "//config/params:go_default_library",
         "@com_github_pkg_errors//:go_default_library",
+        "@com_github_sirupsen_logrus//:go_default_library",
     ],
 )
 

diff --git a/io/file/fileutil.go b/io/file/fileutil.go
@@ -382,3 +382,27 @@ func DirFiles(dir string) ([]string, error) {
 	}
 	return files, nil
 }
+
+// WriteLinesToFile writes a slice of strings to a file, each string on a new line.
+func WriteLinesToFile(lines []string, filename string) error {
+	// Open the file for writing. If the file does not exist, create it, or truncate it if it does.
+	f, err := os.Create(filepath.Clean(filename))
+	if err != nil {
+		return fmt.Errorf("error creating file: %w", err)
+	}
+	defer func(file *os.File) {
+		err := file.Close()
+		if err != nil {
+			log.Error(err.Error())
+		}
+	}(f)
+
+	// Iterate through all lines in the slice and write them to the file
+	for _, line := range lines {
+		if _, err := f.WriteString(line + "\n"); err != nil {
+			return fmt.Errorf("error writing line to file: %w", err)
+		}
+	}
+
+	return nil
+}
diff --git a/io/file/fileutil_test.go b/io/file/fileutil_test.go
@@ -24,6 +24,7 @@ import (
 	"os/user"
 	"path/filepath"
 	"sort"
+	"strings"
 	"testing"
 
 	"github.com/prysmaticlabs/prysm/v5/config/params"
@@ -567,3 +568,37 @@ func TestHasReadWritePermissions(t *testing.T) {
 		})
 	}
 }
+
+func TestWriteLinesToFile(t *testing.T) {
+	filename := filepath.Join(t.TempDir(), "testfile.txt")
+	t.Run("write to a new file", func(t *testing.T) {
+		lines := []string{"line1", "line2", "line3"}
+		require.NoError(t, file.WriteLinesToFile(lines, filename))
+		// Check file content
+		content, err := os.ReadFile(filepath.Clean(filename))
+		if err != nil {
+			t.Fatalf("failed to read file: %v", err)
+		}
+
+		// Join lines with newline for comparison
+		expectedContent := strings.Join(lines, "\n") + "\n"
+		if string(content) != expectedContent {
+			t.Errorf("file content = %q, want %q", string(content), expectedContent)
+		}
+	})
+	t.Run("overwrite existing file", func(t *testing.T) {
+		lines := []string{"line4", "line5"}
+		require.NoError(t, file.WriteLinesToFile(lines, filename))
+		// Check file content
+		content, err := os.ReadFile(filepath.Clean(filename))
+		if err != nil {
+			t.Fatalf("failed to read file: %v", err)
+		}
+
+		// Join lines with newline for comparison
+		expectedContent := strings.Join(lines, "\n") + "\n"
+		if string(content) != expectedContent {
+			t.Errorf("file content = %q, want %q", string(content), expectedContent)
+		}
+	})
+}
diff --git a/io/file/log.go b/io/file/log.go
@@ -0,0 +1,5 @@
+package file
+
+import "github.com/sirupsen/logrus"
+
+var log = logrus.WithField("prefix", "fileutil")
diff --git a/testing/endtoend/components/validator.go b/testing/endtoend/components/validator.go
@@ -198,7 +198,7 @@ func (v *ValidatorNode) Start(ctx context.Context) error {
 		beaconRPCPort = e2e.TestParams.Ports.PrysmBeaconNodeRPCPort
 	}
 
-	file, err := helpers.DeleteAndCreateFile(e2e.TestParams.LogPath, fmt.Sprintf(e2e.ValidatorLogFileName, index))
+	logFile, err := helpers.DeleteAndCreateFile(e2e.TestParams.LogPath, fmt.Sprintf(e2e.ValidatorLogFileName, index))
 	if err != nil {
 		return err
 	}
@@ -223,7 +223,7 @@ func (v *ValidatorNode) Start(ctx context.Context) error {
 	}
 	args := []string{
 		fmt.Sprintf("--%s=%s/eth2-val-%d", cmdshared.DataDirFlag.Name, e2e.TestParams.TestPath, index),
-		fmt.Sprintf("--%s=%s", cmdshared.LogFileName.Name, file.Name()),
+		fmt.Sprintf("--%s=%s", cmdshared.LogFileName.Name, logFile.Name()),
 		fmt.Sprintf("--%s=%s", flags.GraffitiFileFlag.Name, gFile),
 		fmt.Sprintf("--%s=%d", flags.MonitoringPortFlag.Name, e2e.TestParams.Ports.ValidatorMetricsPort+index),
 		fmt.Sprintf("--%s=%d", flags.GRPCGatewayPort.Name, e2e.TestParams.Ports.ValidatorGatewayPort+index),
@@ -258,7 +258,16 @@ func (v *ValidatorNode) Start(ctx context.Context) error {
 		// See: https://docs.teku.consensys.net/en/latest/HowTo/External-Signer/Use-External-Signer/
 		args = append(args,
 			fmt.Sprintf("--%s=http://localhost:%d", flags.Web3SignerURLFlag.Name, Web3RemoteSignerPort),
-			fmt.Sprintf("--%s=%s", flags.Web3SignerPublicValidatorKeysFlag.Name, strings.Join(validatorHexPubKeys, ",")))
+		)
+		if v.config.UsePersistentKeyFile {
+			keysPath := filepath.Join(e2e.TestParams.TestPath, "proposer-settings", fmt.Sprintf("validator_%d", index), "keys.txt")
+			if err := file.WriteLinesToFile(validatorHexPubKeys, keysPath); err != nil {
+				return err
+			}
+			args = append(args, fmt.Sprintf("--%s=%s", flags.Web3SignerKeyFileFlag.Name, keysPath))
+		} else {
+			args = append(args, fmt.Sprintf("--%s=%s", flags.Web3SignerPublicValidatorKeysFlag.Name, strings.Join(validatorHexPubKeys, ",")))
+		}
 	} else {
 		// When not using remote key signer, use interop keys.
 		args = append(args,

diff --git a/testing/endtoend/minimal_scenario_e2e_test.go b/testing/endtoend/minimal_scenario_e2e_test.go
@@ -20,6 +20,10 @@ func TestEndToEnd_MinimalConfig_Web3Signer(t *testing.T) {
 	e2eMinimal(t, types.InitForkCfg(version.Phase0, version.Deneb, params.E2ETestConfig()), types.WithRemoteSigner()).run()
 }
 
+func TestEndToEnd_MinimalConfig_Web3Signer_PersistentKeys(t *testing.T) {
+	e2eMinimal(t, types.InitForkCfg(version.Phase0, version.Deneb, params.E2ETestConfig()), types.WithRemoteSignerAndPersistentKeysFile()).run()
+}
+
 func TestEndToEnd_MinimalConfig_ValidatorRESTApi(t *testing.T) {
 	e2eMinimal(t, types.InitForkCfg(version.Phase0, version.Deneb, params.E2ETestConfig()), types.WithCheckpointSync(), types.WithValidatorRESTApi()).run()
 }

diff --git a/testing/endtoend/types/types.go b/testing/endtoend/types/types.go
@@ -26,6 +26,13 @@ func WithRemoteSigner() E2EConfigOpt {
 	}
 }
 
+func WithRemoteSignerAndPersistentKeysFile() E2EConfigOpt {
+	return func(cfg *E2EConfig) {
+		cfg.UseWeb3RemoteSigner = true
+		cfg.UsePersistentKeyFile = true
+	}
+}
+
 func WithCheckpointSync() E2EConfigOpt {
 	return func(cfg *E2EConfig) {
 		cfg.TestCheckpointSync = true
@@ -58,6 +65,7 @@ type E2EConfig struct {
 	UsePrysmShValidator     bool
 	UsePprof                bool
 	UseWeb3RemoteSigner     bool
+	UsePersistentKeyFile    bool
 	TestDeposits            bool
 	UseFixedPeerIDs         bool
 	UseValidatorCrossClient bool

diff --git a/validator/accounts/iface/wallet.go b/validator/accounts/iface/wallet.go
@@ -19,11 +19,15 @@ type InitKeymanagerConfig struct {
 type Wallet interface {
 	// Methods to retrieve wallet and accounts metadata.
 	AccountsDir() string
+	// Method to retrieve wallet directory.
+	Dir() string
 	Password() string
 	// Read methods for important wallet and accounts-related files.
 	ReadFileAtPath(ctx context.Context, filePath string, fileName string) ([]byte, error)
 	// Write methods to persist important wallet and accounts-related files to disk.
 	WriteFileAtPath(ctx context.Context, pathName string, fileName string, data []byte) (bool, error)
 	// Method for initializing a new keymanager.
 	InitializeKeymanager(ctx context.Context, cfg InitKeymanagerConfig) (keymanager.IKeymanager, error)
+	// Method for returning keymanager kind.
+	KeymanagerKind() keymanager.Kind
 }
diff --git a/validator/accounts/testing/mock.go b/validator/accounts/testing/mock.go
@@ -29,6 +29,8 @@ type Wallet struct {
 	UnlockAccounts    bool
 	lock              sync.RWMutex
 	HasWriteFileError bool
+	WalletDir         string
+	Kind              keymanager.Kind
 }
 
 // AccountNames --
@@ -47,6 +49,16 @@ func (w *Wallet) AccountsDir() string {
 	return w.InnerAccountsDir
 }
 
+// Dir for the wallet.
+func (w *Wallet) Dir() string {
+	return w.WalletDir
+}
+
+// KeymanagerKind --
+func (w *Wallet) KeymanagerKind() keymanager.Kind {
+	return w.Kind
+}
+
 // Exists --
 func (w *Wallet) Exists() (bool, error) {
 	return len(w.Directories) > 0, nil

diff --git a/validator/accounts/wallet/wallet.go b/validator/accounts/wallet/wallet.go
@@ -254,10 +254,11 @@ func OpenOrCreateNewWallet(cliCtx *cli.Context) (*Wallet, error) {
 }
 
 // NewWalletForWeb3Signer returns a new wallet for web3 signer which is temporary and not stored locally.
-func NewWalletForWeb3Signer() *Wallet {
+func NewWalletForWeb3Signer(cliCtx *cli.Context) *Wallet {
+	walletDir := cliCtx.String(flags.WalletDirFlag.Name)
 	// wallet is just a temporary wallet for web3 signer used to call initialize keymanager.
 	return &Wallet{
-		walletDir:      "",
+		walletDir:      walletDir, // it's ok if there's an existing wallet
 		accountsPath:   "",
 		keymanagerKind: keymanager.Web3Signer,
 		walletPassword: "",
@@ -318,6 +319,11 @@ func (w *Wallet) AccountsDir() string {
 	return w.accountsPath
 }
 
+// Dir for the wallet.
+func (w *Wallet) Dir() string {
+	return w.walletDir
+}
+
 // Password for the wallet.
 func (w *Wallet) Password() string {
 	return w.walletPassword

diff --git a/validator/accounts/wallet/wallet_test.go b/validator/accounts/wallet/wallet_test.go
@@ -61,7 +61,11 @@ func Test_IsValid_RandomFiles(t *testing.T) {
 }
 
 func TestWallet_InitializeKeymanager_web3Signer_HappyPath(t *testing.T) {
-	w := wallet.NewWalletForWeb3Signer()
+	app := cli.App{}
+	set := flag.NewFlagSet("test", 0)
+	newDir := filepath.Join(t.TempDir(), "new")
+	set.String(flags.WalletDirFlag.Name, newDir, "")
+	w := wallet.NewWalletForWeb3Signer(cli.NewContext(&app, set, nil))
 	ctx := context.Background()
 	root, err := hexutil.Decode("0x270d43e74ce340de4bca2b1936beca0f4f5408d9e78aec4850920baf659d5b69")
 	require.NoError(t, err)
@@ -70,7 +74,6 @@ func TestWallet_InitializeKeymanager_web3Signer_HappyPath(t *testing.T) {
 		Web3SignerConfig: &remoteweb3signer.SetupConfig{
 			BaseEndpoint:          "http://localhost:8545",
 			GenesisValidatorsRoot: root,
-			PublicKeysURL:         "http://localhost:8545/public_keys",
 		},
 	}
 	km, err := w.InitializeKeymanager(ctx, config)
@@ -79,7 +82,11 @@ func TestWallet_InitializeKeymanager_web3Signer_HappyPath(t *testing.T) {
 }
 
 func TestWallet_InitializeKeymanager_web3Signer_nilConfig(t *testing.T) {
-	w := wallet.NewWalletForWeb3Signer()
+	app := cli.App{}
+	set := flag.NewFlagSet("test", 0)
+	newDir := filepath.Join(t.TempDir(), "new")
+	set.String(flags.WalletDirFlag.Name, newDir, "")
+	w := wallet.NewWalletForWeb3Signer(cli.NewContext(&app, set, nil))
 	ctx := context.Background()
 	config := iface.InitKeymanagerConfig{
 		ListenForChanges: false,

diff --git a/validator/client/BUILD.bazel b/validator/client/BUILD.bazel
@@ -124,6 +124,7 @@ go_test(
         "//async/event:go_default_library",
         "//beacon-chain/core/signing:go_default_library",
         "//cache/lru:go_default_library",
+        "//cmd/validator/flags:go_default_library",
         "//config/features:go_default_library",
         "//config/fieldparams:go_default_library",
         "//config/params:go_default_library",
@@ -169,6 +170,7 @@ go_test(
         "@com_github_sirupsen_logrus//hooks/test:go_default_library",
         "@com_github_stretchr_testify//mock:go_default_library",
         "@com_github_tyler_smith_go_bip39//:go_default_library",
+        "@com_github_urfave_cli_v2//:go_default_library",
         "@com_github_wealdtech_go_eth2_util//:go_default_library",
         "@in_gopkg_d4l3k_messagediff_v1//:go_default_library",
         "@io_bazel_rules_go//go/tools/bazel:go_default_library",