Beacon node setup improvements (ethereum-optimism#9120)

* Beacon node setup improvements

* Update op-node/flags/flags.go

Co-authored-by: protolambda <[email protected]>

* Add option to not half op-node startup on failed check

* Update op-node/flags/flags.go

Co-authored-by: protolambda <[email protected]>

* op-node: improve beacon-API configuration handling

---------

Co-authored-by: protolambda <[email protected]>

op-e2e/e2eutils/fakebeacon/blobs.go

@@ -129,6 +129,15 @@ func (f *FakeBeacon) Start(addr string) error {
 			f.log.Error("blobs handler err", "err", err)
 		}
 	})
+	mux.HandleFunc("/eth/v1/node/version", func(w http.ResponseWriter, r *http.Request) {
+		err := json.NewEncoder(w).Encode(&eth.APIVersionResponse{Data: eth.VersionInformation{Version: "fakebeacon 1.2.3"}})
+		if err != nil {
+			w.WriteHeader(http.StatusInternalServerError)
+			f.log.Error("version handler err", "err", err)
+		} else {
+			w.WriteHeader(http.StatusOK)
+		}
+	})
 	f.beaconSrv = &http.Server{
 		Handler:           mux,
 		ReadTimeout:       time.Second * 20,

op-e2e/l1_beacon_client_test.go

@@ -0,0 +1,29 @@
+package op_e2e
+
+import (
+	"context"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/fakebeacon"
+	"github.com/ethereum-optimism/optimism/op-service/client"
+	"github.com/ethereum-optimism/optimism/op-service/sources"
+	"github.com/ethereum-optimism/optimism/op-service/testlog"
+	"github.com/ethereum/go-ethereum/log"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetVersion(t *testing.T) {
+	l := testlog.Logger(t, log.LvlInfo)
+
+	beaconApi := fakebeacon.NewBeacon(l, t.TempDir(), uint64(0), uint64(0))
+	t.Cleanup(func() {
+		_ = beaconApi.Close()
+	})
+	require.NoError(t, beaconApi.Start("127.0.0.1:0"))
+
+	cl := sources.NewL1BeaconClient(client.NewBasicHTTPClient(beaconApi.BeaconAddr(), l))
+
+	version, err := cl.GetVersion(context.Background())
+	require.NoError(t, err)
+	require.Equal(t, "fakebeacon 1.2.3", version)
+}

op-node/flags/flags.go

@@ -43,6 +43,19 @@ var (
 		Destination: new(string),
 	}
 	/* Optional Flags */
+	BeaconAddr = &cli.StringFlag{
+		Name:     "l1.beacon",
+		Usage:    "Address of L1 Beacon-node HTTP endpoint to use.",
+		Required: false,
+		EnvVars:  prefixEnvVars("L1_BEACON"),
+	}
+	BeaconCheckIgnore = &cli.BoolFlag{
+		Name:     "l1.beacon.ignore",
+		Usage:    "When false, halts op-node startup if the healthcheck to the Beacon-node endpoint fails.",
+		Required: false,
+		Value:    false,
+		EnvVars:  prefixEnvVars("L1_BEACON_IGNORE"),
+	}
 	SyncModeFlag = &cli.GenericFlag{
 		Name:    "syncmode",
 		Usage:   fmt.Sprintf("IN DEVELOPMENT: Options are: %s", openum.EnumString(sync.ModeStrings)),
@@ -252,6 +265,8 @@ var requiredFlags = []cli.Flag{
 }
 
 var optionalFlags = []cli.Flag{
+	BeaconAddr,
+	BeaconCheckIgnore,
 	SyncModeFlag,
 	RPCListenAddr,
 	RPCListenPort,

op-node/node/client.go

@@ -31,6 +31,8 @@ type L1EndpointSetup interface {
 
 type L1BeaconEndpointSetup interface {
 	Setup(ctx context.Context, log log.Logger) (cl client.HTTP, err error)
+	// ShouldIgnoreBeaconCheck returns true if the Beacon-node version check should not halt startup.
+	ShouldIgnoreBeaconCheck() bool
 	Check() error
 }
 
@@ -173,7 +175,8 @@ func (cfg *PreparedL1Endpoint) Check() error {
 }
 
 type L1BeaconEndpointConfig struct {
-	BeaconAddr string // Address of L1 User Beacon-API endpoint to use (beacon namespace required)
+	BeaconAddr        string // Address of L1 User Beacon-API endpoint to use (beacon namespace required)
+	BeaconCheckIgnore bool   // When false, halt startup if the beacon version endpoint fails
 }
 
 var _ L1BeaconEndpointSetup = (*L1BeaconEndpointConfig)(nil)
@@ -183,8 +186,12 @@ func (cfg *L1BeaconEndpointConfig) Setup(ctx context.Context, log log.Logger) (c
 }
 
 func (cfg *L1BeaconEndpointConfig) Check() error {
-	if cfg.BeaconAddr == "" {
-		return errors.New("expected beacon address, but got none")
+	if cfg.BeaconAddr == "" && !cfg.BeaconCheckIgnore {
+		return errors.New("expected L1 Beacon API endpoint, but got none")
 	}
 	return nil
 }
+
+func (cfg *L1BeaconEndpointConfig) ShouldIgnoreBeaconCheck() bool {
+	return cfg.BeaconCheckIgnore
+}

op-node/node/config.go

@@ -126,12 +126,13 @@ func (cfg *Config) Check() error {
 	if err := cfg.L2.Check(); err != nil {
 		return fmt.Errorf("l2 endpoint config error: %w", err)
 	}
-	if cfg.Beacon != nil {
+	if cfg.Rollup.EcotoneTime != nil {
+		if cfg.Beacon == nil {
+			return fmt.Errorf("the Ecotone upgrade is scheduled but no L1 Beacon API endpoint is configured")
+		}
 		if err := cfg.Beacon.Check(); err != nil {
-			return fmt.Errorf("beacon endpoint config error: %w", err)
+			return fmt.Errorf("misconfigured L1 Beacon API endpoint: %w", err)
 		}
-	} else if cfg.Rollup.EcotoneTime != nil {
-		return fmt.Errorf("ecotone upgrade scheduled but no beacon endpoint is configured")
 	}
 	if err := cfg.Rollup.Check(); err != nil {
 		return fmt.Errorf("rollup config error: %w", err)

op-node/node/node.go

@@ -292,19 +292,62 @@ func (n *OpNode) initRuntimeConfig(ctx context.Context, cfg *Config) error {
 }
 
 func (n *OpNode) initL1BeaconAPI(ctx context.Context, cfg *Config) error {
-	if cfg.Beacon == nil {
-		n.log.Warn("No beacon endpoint configured. Configuration is mandatory for the Ecotone upgrade")
+	// If Ecotone upgrade is not scheduled yet, then there is no need for a Beacon API.
+	if cfg.Rollup.EcotoneTime == nil {
 		return nil
 	}
+	// Once the Ecotone upgrade is scheduled, we must have initialized the Beacon API settings.
+	if cfg.Beacon == nil {
+		return fmt.Errorf("missing L1 Beacon Endpoint configuration: this API is mandatory for Ecotone upgrade at t=%d", *cfg.Rollup.EcotoneTime)
+	}
+
+	// We always initialize a client. We will get an error on requests if the client does not work.
+	// This way the op-node can continue non-L1 functionality when the user chooses to ignore the Beacon API requirement.
 	httpClient, err := cfg.Beacon.Setup(ctx, n.log)
 	if err != nil {
-		return fmt.Errorf("failed to setup L1 beacon client: %w", err)
+		return fmt.Errorf("failed to setup L1 Beacon API client: %w", err)
 	}
+	n.beacon = sources.NewL1BeaconClient(httpClient)
 
-	cl := sources.NewL1BeaconClient(httpClient)
-	n.beacon = cl
-
-	return nil
+	// Retry retrieval of the Beacon API version, to be more robust on startup against Beacon API connection issues.
+	beaconVersion, missingEndpoint, err := retry.Do2[string, bool](ctx, 5, retry.Exponential(), func() (string, bool, error) {
+		ctx, cancel := context.WithTimeout(ctx, time.Second*10)
+		defer cancel()
+		beaconVersion, err := n.beacon.GetVersion(ctx)
+		if err != nil {
+			if errors.Is(err, client.ErrNoEndpoint) {
+				return "", true, nil // don't return an error, we do not have to retry when there is a config issue.
+			}
+			return "", false, err
+		}
+		return beaconVersion, false, nil
+	})
+	if missingEndpoint {
+		// Allow the user to continue if they explicitly ignore the requirement of the endpoint.
+		if cfg.Beacon.ShouldIgnoreBeaconCheck() {
+			n.log.Warn("This endpoint is required for the Ecotone upgrade, but is missing, and configured to be ignored. " +
+				"The node may be unable to retrieve EIP-4844 blobs data.")
+			return nil
+		} else {
+			// If the client tells us the endpoint was not configured,
+			// then explain why we need it, and what the user can do to ignore this.
+			n.log.Error("The Ecotone upgrade requires a L1 Beacon API endpoint, to retrieve EIP-4844 blobs data. " +
+				"This can be ignored with the --l1.beacon.ignore option, " +
+				"but the node may be unable to sync from L1 without this endpoint.")
+			return errors.New("missing L1 Beacon API endpoint")
+		}
+	} else if err != nil {
+		if cfg.Beacon.ShouldIgnoreBeaconCheck() {
+			n.log.Warn("Failed to check L1 Beacon API version, but configuration ignores results. "+
+				"The node may be unable to retrieve EIP-4844 blobs data.", "err", err)
+			return nil
+		} else {
+			return fmt.Errorf("failed to check L1 Beacon API version: %w", err)
+		}
+	} else {
+		n.log.Info("Connected to L1 Beacon API, ready for EIP-4844 blobs retrieval.", "version", beaconVersion)
+		return nil
+	}
 }
 
 func (n *OpNode) initL2(ctx context.Context, cfg *Config, snapshotLog log.Logger) error {

op-node/service.go

@@ -78,6 +78,7 @@ func NewConfig(ctx *cli.Context, log log.Logger) (*node.Config, error) {
 		L2:     l2Endpoint,
 		Rollup: *rollupConfig,
 		Driver: *driverConfig,
+		Beacon: NewBeaconEndpointConfig(ctx),
 		RPC: node.RPCConfig{
 			ListenAddr:  ctx.String(flags.RPCListenAddr.Name),
 			ListenPort:  ctx.Int(flags.RPCListenPort.Name),
@@ -114,6 +115,13 @@ func NewConfig(ctx *cli.Context, log log.Logger) (*node.Config, error) {
 	return cfg, nil
 }
 
+func NewBeaconEndpointConfig(ctx *cli.Context) node.L1BeaconEndpointSetup {
+	return &node.L1BeaconEndpointConfig{
+		BeaconAddr:        ctx.String(flags.BeaconAddr.Name),
+		BeaconCheckIgnore: ctx.Bool(flags.BeaconCheckIgnore.Name),
+	}
+}
+
 func NewL1EndpointConfig(ctx *cli.Context) *node.L1EndpointConfig {
 	return &node.L1EndpointConfig{
 		L1NodeAddr:       ctx.String(flags.L1NodeAddr.Name),

op-service/client/http.go

@@ -2,10 +2,10 @@ package client
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
-	"strings"
 	"time"
 
 	"github.com/ethereum/go-ethereum/log"
@@ -28,16 +28,19 @@ type BasicHTTPClient struct {
 }
 
 func NewBasicHTTPClient(endpoint string, log log.Logger) *BasicHTTPClient {
-	// Make sure the endpoint ends in trailing slash
-	trimmedEndpoint := strings.TrimSuffix(endpoint, "/") + "/"
 	return &BasicHTTPClient{
-		endpoint: trimmedEndpoint,
+		endpoint: endpoint,
 		log:      log,
 		client:   &http.Client{Timeout: DefaultTimeoutSeconds * time.Second},
 	}
 }
 
+var ErrNoEndpoint = errors.New("no endpoint is configured")
+
 func (cl *BasicHTTPClient) Get(ctx context.Context, p string, query url.Values, headers http.Header) (*http.Response, error) {
+	if cl.endpoint == "" {
+		return nil, ErrNoEndpoint
+	}
 	target, err := url.Parse(cl.endpoint)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse endpoint URL: %w", err)

op-service/eth/blobs_api.go

@@ -60,3 +60,11 @@ type ReducedConfigData struct {
 type APIConfigResponse struct {
 	Data ReducedConfigData `json:"data"`
 }
+
+type APIVersionResponse struct {
+	Data VersionInformation `json:"data"`
+}
+
+type VersionInformation struct {
+	Version string `json:"version"`
+}

op-service/sources/l1_beacon_client.go

@@ -19,6 +19,7 @@ import (
 )
 
 const (
+	versionMethod        = "eth/v1/node/version"
 	genesisMethod        = "eth/v1/beacon/genesis"
 	specMethod           = "eth/v1/config/spec"
 	sidecarsMethodPrefix = "eth/v1/beacon/blob_sidecars/"
@@ -169,3 +170,12 @@ func blobsFromSidecars(blobSidecars []*eth.BlobSidecar, hashes []eth.IndexedBlob
 	}
 	return out, nil
 }
+
+// GetVersion fetches the version of the Beacon-node.
+func (cl *L1BeaconClient) GetVersion(ctx context.Context) (string, error) {
+	var resp eth.APIVersionResponse
+	if err := cl.apiReq(ctx, &resp, versionMethod, nil); err != nil {
+		return "", err
+	}
+	return resp.Data.Version, nil
+}