op-service: RPC server wraps http-util server now, and factor out RPC handler

op-alt-da/daserver.go

@@ -14,7 +14,7 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/ethereum-optimism/optimism/op-service/rpc"
+	"github.com/ethereum-optimism/optimism/op-service/httputil"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/log"
 )
@@ -30,7 +30,7 @@ type DAServer struct {
 	log            log.Logger
 	endpoint       string
 	store          KVStore
-	tls            *rpc.ServerTLSConfig
+	tls            *httputil.ServerTLSConfig
 	httpServer     *http.Server
 	listener       net.Listener
 	useGenericComm bool

op-node/p2p/gossip_test.go

@@ -128,13 +128,11 @@ func TestVerifyBlockSignatureWithRemoteSigner(t *testing.T) {
 		"127.0.0.1",
 		0,
 		"test",
-		oprpc.WithAPIs([]rpc.API{
-			{
-				Namespace: "opsigner",
-				Service:   remoteSigner,
-			},
-		}),
 	)
+	server.AddAPI(rpc.API{
+		Namespace: "opsigner",
+		Service:   remoteSigner,
+	})
 
 	require.NoError(t, server.Start())
 	defer func() {

op-node/rollup/interop/managed/system.go

@@ -65,14 +65,12 @@ func NewManagedMode(log log.Logger, cfg *rollup.Config, addr string, port int, j
 	out.srv = rpc.NewServer(addr, port, "v0.0.0",
 		rpc.WithWebsocketEnabled(),
 		rpc.WithLogger(log),
-		rpc.WithJWTSecret(jwtSecret[:]),
-		rpc.WithAPIs([]gethrpc.API{
-			{
-				Namespace:     "interop",
-				Service:       &InteropAPI{backend: out},
-				Authenticated: true,
-			},
-		}))
+		rpc.WithJWTSecret(jwtSecret[:]))
+	out.srv.AddAPI(gethrpc.API{
+		Namespace:     "interop",
+		Service:       &InteropAPI{backend: out},
+		Authenticated: true,
+	})
 	return out
 }
 

op-service/httputil/options.go

@@ -0,0 +1,58 @@
+package httputil
+
+import (
+	"crypto/tls"
+	"net/http"
+
+	optls "github.com/ethereum-optimism/optimism/op-service/tls"
+)
+
+type config struct {
+	// listenAddr is the configured address to listen to when started.
+	// use listener.Addr to retrieve the address when online.
+	listenAddr string
+
+	tls *ServerTLSConfig
+
+	handler http.Handler
+
+	httpOpts []HTTPOption
+}
+
+func (c *config) ApplyOptions(opts ...Option) {
+	for _, opt := range opts {
+		opt(c)
+	}
+}
+
+// Option is a general config option.
+type Option func(cfg *config)
+
+// HTTPOption applies a change to an HTTP server, just before standup.
+// HTTPOption options are be re-executed on server shutdown/startup cycles,
+// for each new underlying Go *http.Server instance.
+type HTTPOption func(config *http.Server) error
+
+func WithHTTPOptions(options ...HTTPOption) Option {
+	return func(cfg *config) {
+		cfg.httpOpts = append(cfg.httpOpts, options...)
+	}
+}
+
+func WithMaxHeaderBytes(max int) HTTPOption {
+	return func(srv *http.Server) error {
+		srv.MaxHeaderBytes = max
+		return nil
+	}
+}
+
+type ServerTLSConfig struct {
+	Config    *tls.Config
+	CLIConfig *optls.CLIConfig // paths to certificate and key files
+}
+
+func WithServerTLS(tlsCfg *ServerTLSConfig) Option {
+	return func(cfg *config) {
+		cfg.tls = tlsCfg
+	}
+}

op-service/httputil/server.go

@@ -6,29 +6,65 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	"sync/atomic"
+	"sync"
+	"time"
 )
 
 // HTTPServer wraps a http.Server, while providing conveniences
 // like exposing the running state and address.
+//
+// It can be started with HTTPServer.Start and closed with
+// HTTPServer.Stop, HTTPServer.Close and HTTPServer.Shutdown (convenience functions for different gracefulness).
+//
+// The addr contains both host and port. A 0 port may be used to make the system bind to an available one.
+// The resulting address can be retrieved with HTTPServer.Addr or HTTPServer.HTTPEndpoint.
+//
+// The server may be started, stopped and started back up.
 type HTTPServer struct {
+	// mu is the lock used for bringing the server online/offline, and accessing the address of the server.
+	mu sync.RWMutex
+
+	// listener that the server is bound to. Nil if online.
 	listener net.Listener
-	srv      *http.Server
-	closed   atomic.Bool
+
+	srv *http.Server
+
+	// used as BaseContext of the http.Server
+	srvCtx    context.Context
+	srvCancel context.CancelFunc
+
+	config *config
 }
 
-// HTTPOption applies a change to an HTTP server
-type HTTPOption func(srv *HTTPServer) error
+// NewHTTPServer creates an HTTPServer that serves the given HTTP handler.
+// The server is inactive and has to be started explicitly.
+func NewHTTPServer(addr string, handler http.Handler, opts ...Option) *HTTPServer {
+	cfg := &config{
+		listenAddr: addr,
+		tls:        nil,
+		handler:    handler,
+		httpOpts:   nil,
+	}
+	cfg.ApplyOptions(opts...)
+	return &HTTPServer{config: cfg}
+}
 
-func StartHTTPServer(addr string, handler http.Handler, opts ...HTTPOption) (*HTTPServer, error) {
-	listener, err := net.Listen("tcp", addr)
-	if err != nil {
-		return nil, fmt.Errorf("failed to bind to address %q: %w", addr, err)
+func StartHTTPServer(addr string, handler http.Handler, opts ...Option) (*HTTPServer, error) {
+	out := NewHTTPServer(addr, handler, opts...)
+	return out, out.Start()
+}
+
+// Start starts the server, and checks if it comes online fully.
+func (s *HTTPServer) Start() error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.srv != nil {
+		return errors.New("already have existing server")
 	}
 
 	srvCtx, srvCancel := context.WithCancel(context.Background())
 	srv := &http.Server{
-		Handler:           handler,
+		Handler:           s.config.handler,
 		ReadTimeout:       DefaultTimeouts.ReadTimeout,
 		ReadHeaderTimeout: DefaultTimeouts.ReadHeaderTimeout,
 		WriteTimeout:      DefaultTimeouts.WriteTimeout,
@@ -37,28 +73,56 @@ func StartHTTPServer(addr string, handler http.Handler, opts ...HTTPOption) (*HT
 			return srvCtx
 		},
 	}
-	out := &HTTPServer{listener: listener, srv: srv}
-	for _, opt := range opts {
-		if err := opt(out); err != nil {
+
+	if s.config.tls != nil {
+		srv.TLSConfig = s.config.tls.Config
+	}
+
+	for _, opt := range s.config.httpOpts {
+		if err := opt(srv); err != nil {
 			srvCancel()
-			return nil, errors.Join(fmt.Errorf("failed to apply HTTP option: %w", err), listener.Close())
+			return fmt.Errorf("failed to apply HTTP option: %w", err)
 		}
 	}
-	go func() {
-		err := out.srv.Serve(listener)
+
+	listener, err := net.Listen("tcp", s.config.listenAddr)
+	if err != nil {
 		srvCancel()
-		// no error, unless ErrServerClosed (or unused base context closes, or unused http2 config error)
-		if errors.Is(err, http.ErrServerClosed) {
-			out.closed.Store(true)
+		return fmt.Errorf("failed to bind to address %q: %w", s.config.listenAddr, err)
+	}
+	s.listener = listener
+
+	s.srv = srv
+	s.srvCtx = srvCtx
+	s.srvCancel = srvCancel
+
+	// cap of 1, to not block on non-immediate shutdown
+	errCh := make(chan error, 1)
+	go func() {
+		if s.config.tls != nil {
+			errCh <- s.srv.ServeTLS(s.listener, "", "")
 		} else {
-			panic(fmt.Errorf("unexpected serve error: %w", err))
+			errCh <- s.srv.Serve(s.listener)
 		}
 	}()
-	return out, nil
+
+	// verify that the server comes up
+	standupTimer := time.NewTimer(10 * time.Millisecond)
+	defer standupTimer.Stop()
+
+	select {
+	case err := <-errCh:
+		s.cleanup()
+		return fmt.Errorf("http server failed: %w", err)
+	case <-standupTimer.C:
+		return nil
+	}
 }
 
 func (s *HTTPServer) Closed() bool {
-	return s.closed.Load()
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return s.srv == nil
 }
 
 // Stop is a convenience method to gracefully shut down the server, but force-close if the ctx is cancelled.
@@ -73,28 +137,73 @@ func (s *HTTPServer) Stop(ctx context.Context) error {
 	return nil
 }
 
+func (s *HTTPServer) cleanup() {
+	s.srv = nil
+	s.listener = nil
+	s.srvCtx = nil
+	s.srvCancel = nil
+}
+
 // Shutdown shuts down the HTTP server and its listener,
 // but allows active connections to close gracefully.
 // If the function exits due to a ctx cancellation the listener is closed but active connections may remain,
 // a call to Close() can force-close any remaining active connections.
 func (s *HTTPServer) Shutdown(ctx context.Context) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.srv == nil {
+		return nil
+	}
+	s.srvCancel()
 	// closes the underlying listener too.
-	return s.srv.Shutdown(ctx)
+	err := s.srv.Shutdown(ctx)
+	if err != nil {
+		return err
+	}
+	s.cleanup()
+	return nil
 }
 
 // Close force-closes the HTTPServer, its listener, and all its active connections.
 func (s *HTTPServer) Close() error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.srv == nil {
+		return nil
+	}
+	s.srvCancel()
 	// closes the underlying listener too
-	return s.srv.Close()
+	err := s.srv.Close()
+	if err != nil {
+		return err
+	}
+	s.cleanup()
+	return nil
 }
 
+// Addr returns the address that the server is listening on.
+// It returns nil if the server is not online.
 func (s *HTTPServer) Addr() net.Addr {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	if s.listener == nil {
+		return nil
+	}
 	return s.listener.Addr()
 }
 
-func WithMaxHeaderBytes(max int) HTTPOption {
-	return func(srv *HTTPServer) error {
-		srv.srv.MaxHeaderBytes = max
-		return nil
+// HTTPEndpoint returns the http(s) endpoint the server is serving.
+// It returns an empty string if the server is not online.
+func (s *HTTPServer) HTTPEndpoint() string {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	if s.listener == nil {
+		return ""
+	}
+	addr := s.listener.Addr().String()
+	if s.config.tls != nil {
+		return "https://" + addr
+	} else {
+		return "http://" + addr
 	}
 }