Onboard validator's Beacon REST API usage to e2e tests (#11704)

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* Onboard validator's Beacon REST API usage to e2e tests

* Remove unused variables

* Remove use_beacon_api tags

* Fix DeepSource errors

* Revert unneeded changes

* Revert evaluator changes

* Revert import reordering

* Address PR comments

* Remove all REST API e2e tests except minimal one

* Fix validator pointing to inexisting beacon node port

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

.bazelrc

@@ -230,9 +230,3 @@ build --modify_execution_info='GoStdlib.*=+no-remote-cache'
 
 # Set bazel gotag
 build --define gotags=bazel
-
-# Build the binary with Beacon API calls for the validator
-build --flag_alias=use_beacon_api=//validator/client/validator-client-factory:use_beacon_api
-
-build:beacon_api --use_beacon_api
-build:beacon_api --define=gotags=use_beacon_api

cmd/validator/flags/BUILD.bazel

@@ -5,10 +5,7 @@ go_library(
     srcs = [
         "flags.go",
         "interop.go",
-    ] + select({
-        "//validator/client/validator-client-factory:beacon_api_usage": ["beacon_api_flags.go"],
-        "//conditions:default": ["grpc_flags.go"],
-    }),
+    ],
     importpath = "github.com/prysmaticlabs/prysm/v3/cmd/validator/flags",
     visibility = [
         "//cmd/prysmctl:__subpackages__",

cmd/validator/flags/beacon_api_flags.go

@@ -1,8 +0,0 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
-package flags
-
-const (
-	BuiltWithBeaconApi = true
-)

cmd/validator/flags/grpc_flags.go

@@ -1,8 +0,0 @@
-//go:build !use_beacon_api
-// +build !use_beacon_api
-
-package flags
-
-const (
-	BuiltWithBeaconApi = false
-)

cmd/validator/main.go

@@ -49,6 +49,7 @@ func startNode(ctx *cli.Context) error {
 var appFlags = []cli.Flag{
 	flags.BeaconRPCProviderFlag,
 	flags.BeaconRPCGatewayProviderFlag,
+	flags.BeaconRESTApiProviderFlag,
 	flags.CertFlag,
 	flags.GraffitiFlag,
 	flags.DisablePenaltyRewardLogFlag,
@@ -112,11 +113,6 @@ var appFlags = []cli.Flag{
 }
 
 func init() {
-	// Append the Beacon REST API flags
-	if flags.BuiltWithBeaconApi {
-		appFlags = append(appFlags, flags.BeaconRESTApiProviderFlag)
-	}
-
 	appFlags = cmd.WrapFlags(append(appFlags, features.ValidatorFlags...))
 }
 

cmd/validator/usage.go

@@ -86,6 +86,7 @@ var appHelpFlagGroups = []flagGroup{
 		Flags: []cli.Flag{
 			flags.BeaconRPCProviderFlag,
 			flags.BeaconRPCGatewayProviderFlag,
+			flags.BeaconRESTApiProviderFlag,
 			flags.CertFlag,
 			flags.EnableWebFlag,
 			flags.DisablePenaltyRewardLogFlag,
@@ -128,16 +129,6 @@ var appHelpFlagGroups = []flagGroup{
 }
 
 func init() {
-	// Append the Beacon REST API flags
-	if flags.BuiltWithBeaconApi {
-		for groupIndex := range appHelpFlagGroups {
-			group := &appHelpFlagGroups[groupIndex]
-			if group.Name == "validator" {
-				group.Flags = append(group.Flags, flags.BeaconRESTApiProviderFlag)
-			}
-		}
-	}
-
 	cli.AppHelpTemplate = appHelpTemplate
 
 	type helpData struct {

config/features/config.go

@@ -47,6 +47,7 @@ type Flags struct {
 	WriteWalletPasswordOnWebOnboarding  bool // WriteWalletPasswordOnWebOnboarding writes the password to disk after Prysm web signup.
 	EnableDoppelGanger                  bool // EnableDoppelGanger enables doppelganger protection on startup for the validator.
 	EnableHistoricalSpaceRepresentation bool // EnableHistoricalSpaceRepresentation enables the saving of registry validators in separate buckets to save space
+	EnableBeaconRESTApi                 bool // EnableBeaconRESTApi enables experimental usage of the beacon REST API by the validator when querying a beacon node
 	// Logging related toggles.
 	DisableGRPCConnectionLogs bool // Disables logging when a new grpc client has connected.
 	EnableFullSSZDataLogging  bool // Enables logging for full ssz data on rejected gossip messages
@@ -290,6 +291,10 @@ func ConfigureValidator(ctx *cli.Context) error {
 		logEnabled(enableDoppelGangerProtection)
 		cfg.EnableDoppelGanger = true
 	}
+	if ctx.Bool(EnableBeaconRESTApi.Name) {
+		logEnabled(EnableBeaconRESTApi)
+		cfg.EnableBeaconRESTApi = true
+	}
 	cfg.KeystoreImportDebounceInterval = ctx.Duration(dynamicKeyReloadDebounceInterval.Name)
 	Init(cfg)
 	return nil

config/features/flags.go

@@ -128,6 +128,10 @@ var (
 		Name:  "enable-full-ssz-data-logging",
 		Usage: "Enables displaying logs for full ssz data on rejected gossip messages",
 	}
+	EnableBeaconRESTApi = &cli.BoolFlag{
+		Name:  "enable-beacon-rest-api",
+		Usage: "Experimental enable of the beacon REST API when querying a beacon node",
+	}
 )
 
 // devModeFlags holds list of flags that are set when development mode is on.
@@ -145,6 +149,7 @@ var ValidatorFlags = append(deprecatedFlags, []cli.Flag{
 	attestTimely,
 	enableSlashingProtectionPruning,
 	enableDoppelGangerProtection,
+	EnableBeaconRESTApi,
 }...)
 
 // E2EValidatorFlags contains a list of the validator feature flags to be tested in E2E.

hack/update-mockgen.sh

@@ -80,7 +80,7 @@ for ((i = 0; i < ${#beacon_api_mocks[@]}; i++)); do
     file=${beacon_api_mocks[i]% *};
     source=${beacon_api_mocks[i]#* };
     echo "generating $file for file: $source";
-    GO11MODULE=on mockgen -package=mock --build_flags="--tags=use_beacon_api" -source="validator/client/beacon-api/$source" -destination="$file"
+    GO11MODULE=on mockgen -package=mock -source="validator/client/beacon-api/$source" -destination="$file"
 done
 
 goimports -w "$beacon_api_mock_path/."

testing/endtoend/BUILD.bazel

@@ -69,6 +69,7 @@ common_deps = [
     "//testing/require:go_default_library",
     "//testing/slasher/simulator:go_default_library",
     "//testing/util:go_default_library",
+    "//validator/helpers:go_default_library",
     "@com_github_pkg_errors//:go_default_library",
     "@com_github_sirupsen_logrus//:go_default_library",
     "@com_github_sirupsen_logrus//hooks/test:go_default_library",

testing/endtoend/components/validator.go

@@ -213,6 +213,7 @@ func (v *ValidatorNode) Start(ctx context.Context) error {
 		fmt.Sprintf("--%s=%d", flags.MonitoringPortFlag.Name, e2e.TestParams.Ports.ValidatorMetricsPort+index),
 		fmt.Sprintf("--%s=%d", flags.GRPCGatewayPort.Name, e2e.TestParams.Ports.ValidatorGatewayPort+index),
 		fmt.Sprintf("--%s=localhost:%d", flags.BeaconRPCProviderFlag.Name, beaconRPCPort),
+
 		fmt.Sprintf("--%s=%s", flags.GrpcHeadersFlag.Name, "dummy=value,foo=bar"), // Sending random headers shouldn't break anything.
 		fmt.Sprintf("--%s=%s", cmdshared.VerbosityFlag.Name, "debug"),
 		fmt.Sprintf("--%s=%s", flags.ProposerSettingsFlag.Name, proposerSettingsPathPath),
@@ -220,6 +221,18 @@ func (v *ValidatorNode) Start(ctx context.Context) error {
 		"--" + cmdshared.E2EConfigFlag.Name,
 		"--" + cmdshared.AcceptTosFlag.Name,
 	}
+
+	if v.config.UseBeaconRestApi {
+		beaconRestApiPort := e2e.TestParams.Ports.PrysmBeaconNodeGatewayPort + index
+		if beaconRestApiPort >= e2e.TestParams.Ports.PrysmBeaconNodeGatewayPort+e2e.TestParams.BeaconNodeCount {
+			// Point any extra validator clients to a node we know is running.
+			beaconRestApiPort = e2e.TestParams.Ports.PrysmBeaconNodeGatewayPort
+		}
+
+		args = append(args, fmt.Sprintf("--%s=http://localhost:%d", flags.BeaconRESTApiProviderFlag.Name, beaconRestApiPort))
+		args = append(args, fmt.Sprintf("--%s", features.EnableBeaconRESTApi.Name))
+	}
+
 	// Only apply e2e flags to the current branch. New flags may not exist in previous release.
 	if !v.config.UsePrysmShValidator {
 		args = append(args, features.E2EValidatorFlags...)

testing/endtoend/minimal_scenario_e2e_test.go

@@ -18,6 +18,10 @@ func TestEndToEnd_MinimalConfig_Web3Signer(t *testing.T) {
 	e2eMinimal(t, types.WithRemoteSigner()).run()
 }
 
+func TestEndToEnd_MinimalConfig_ValidatorRESTApi(t *testing.T) {
+	e2eMinimal(t, types.WithCheckpointSync(), types.WithValidatorRESTApi()).run()
+}
+
 func TestEndToEnd_ScenarioRun_EEOffline(t *testing.T) {
 	t.Skip("TODO(#10242) Prysm is current unable to handle an offline e2e")
 	runner := e2eMinimal(t)

testing/endtoend/types/types.go

@@ -36,6 +36,12 @@ func WithValidatorCrossClient() E2EConfigOpt {
 	}
 }
 
+func WithValidatorRESTApi() E2EConfigOpt {
+	return func(cfg *E2EConfig) {
+		cfg.UseBeaconRestApi = true
+	}
+}
+
 // E2EConfig defines the struct for all configurations needed for E2E testing.
 type E2EConfig struct {
 	TestCheckpointSync      bool
@@ -47,6 +53,7 @@ type E2EConfig struct {
 	TestDeposits            bool
 	UseFixedPeerIDs         bool
 	UseValidatorCrossClient bool
+	UseBeaconRestApi        bool
 	EpochsToRun             uint64
 	Seed                    int64
 	TracingSinkEndpoint     string

validator/client/beacon-api/BUILD.bazel

@@ -1,6 +1,5 @@
 load("@prysm//tools/go:def.bzl", "go_library", "go_test")
 
-# gazelle:build_tags use_beacon_api
 go_library(
     name = "go_default_library",
     srcs = [
@@ -33,7 +32,6 @@ go_library(
     ],
 )
 
-# gazelle:build_tags use_beacon_api
 go_test(
     name = "go_default_test",
     size = "small",
@@ -50,7 +48,6 @@ go_test(
         "wait_for_chain_start_test.go",
     ],
     embed = [":go_default_library"],
-    gotags = ["use_beacon_api"],
     deps = [
         "//api/gateway/apimiddleware:go_default_library",
         "//beacon-chain/rpc/apimiddleware:go_default_library",

validator/client/beacon-api/activation.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/activation_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/attestation_data.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/attestation_data_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/beacon_api_helpers.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/beacon_api_helpers_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/beacon_api_validator_client.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/beacon_api_validator_client_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/domain_data.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/domain_data_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (
@@ -66,11 +63,12 @@ func TestGetDomainData_GenesisError(t *testing.T) {
 
 	// Make sure that GetGenesis() is called exactly once
 	genesisProvider := mock.NewMockgenesisProvider(ctrl)
-	genesisProvider.EXPECT().GetGenesis().Return(nil, nil, errors.New("")).Times(1)
+	genesisProvider.EXPECT().GetGenesis().Return(nil, nil, errors.New("foo error")).Times(1)
 
 	validatorClient := &beaconApiValidatorClient{genesisProvider: genesisProvider}
 	_, err := validatorClient.getDomainData(epoch, domainType)
 	assert.ErrorContains(t, "failed to get genesis info", err)
+	assert.ErrorContains(t, "foo error", err)
 }
 
 func TestGetDomainData_InvalidGenesisRoot(t *testing.T) {

validator/client/beacon-api/genesis.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/genesis_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/index.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/index_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/json_rest_handler.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/json_rest_handler_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/state_validators.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/state_validators_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/beacon-api/wait_for_chain_start_test.go

@@ -1,6 +1,3 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package beacon_api
 
 import (

validator/client/grpc-api/grpc_validator_client_test.go

@@ -1,6 +1,3 @@
-//go:build !use_beacon_api
-// +build !use_beacon_api
-
 package grpc_api
 
 import (

validator/client/validator-client-factory/BUILD.bazel

@@ -1,32 +1,15 @@
 load("@prysm//tools/go:def.bzl", "go_library")
-load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
-
-bool_flag(
-    name = "use_beacon_api",
-    build_setting_default = False,
-)
-
-config_setting(
-    name = "beacon_api_usage",
-    flag_values = {
-        ":use_beacon_api": "true",
-    },
-)
 
 go_library(
     name = "go_default_library",
-    srcs = select({
-        ":beacon_api_usage": ["beacon_api_validator_client_factory.go"],
-        "//conditions:default": ["grpc_validator_client_factory.go"],
-    }),
+    srcs = ["validator_client_factory.go"],
     importpath = "github.com/prysmaticlabs/prysm/v3/validator/client/validator-client-factory",
     visibility = ["//visibility:public"],
     deps = [
+        "//config/features:go_default_library",
+        "//validator/client/beacon-api:go_default_library",
         "//validator/client/grpc-api:go_default_library",
         "//validator/client/iface:go_default_library",
         "//validator/helpers:go_default_library",
-    ] + select({
-        ":beacon_api_usage": ["//validator/client/beacon-api:go_default_library"],
-        "//conditions:default": [],
-    }),
+    ],
 )

validator/client/validator-client-factory/grpc_validator_client_factory.go

@@ -1,14 +0,0 @@
-//go:build !use_beacon_api
-// +build !use_beacon_api
-
-package validator_client_factory
-
-import (
-	grpcApi "github.com/prysmaticlabs/prysm/v3/validator/client/grpc-api"
-	"github.com/prysmaticlabs/prysm/v3/validator/client/iface"
-	validatorHelpers "github.com/prysmaticlabs/prysm/v3/validator/helpers"
-)
-
-func NewValidatorClient(validatorConn validatorHelpers.NodeConnection) iface.ValidatorClient {
-	return grpcApi.NewGrpcValidatorClient(validatorConn.GetGrpcClientConn())
-}

validator/client/validator-client-factory/validator_client_factory.go

@@ -1,9 +1,7 @@
-//go:build use_beacon_api
-// +build use_beacon_api
-
 package validator_client_factory
 
 import (
+	"github.com/prysmaticlabs/prysm/v3/config/features"
 	beaconApi "github.com/prysmaticlabs/prysm/v3/validator/client/beacon-api"
 	grpcApi "github.com/prysmaticlabs/prysm/v3/validator/client/grpc-api"
 	"github.com/prysmaticlabs/prysm/v3/validator/client/iface"
@@ -11,6 +9,12 @@ import (
 )
 
 func NewValidatorClient(validatorConn validatorHelpers.NodeConnection) iface.ValidatorClient {
-	fallbackClient := grpcApi.NewGrpcValidatorClient(validatorConn.GetGrpcClientConn())
-	return beaconApi.NewBeaconApiValidatorClientWithFallback(validatorConn.GetBeaconApiUrl(), validatorConn.GetBeaconApiTimeout(), fallbackClient)
+	grpcClient := grpcApi.NewGrpcValidatorClient(validatorConn.GetGrpcClientConn())
+	featureFlags := features.Get()
+
+	if featureFlags.EnableBeaconRESTApi {
+		return beaconApi.NewBeaconApiValidatorClientWithFallback(validatorConn.GetBeaconApiUrl(), validatorConn.GetBeaconApiTimeout(), grpcClient)
+	} else {
+		return grpcClient
+	}
 }