refactor: use gorilla mux for POST routes

server/handlers.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"path"
 
 	"github.com/Layr-Labs/eigenda-proxy/commitments"
 	"github.com/Layr-Labs/eigenda-proxy/store"
@@ -19,6 +18,10 @@ func (svr *Server) handleHealth(w http.ResponseWriter, _ *http.Request) error {
 	return nil
 }
 
+// =================================================================================================
+// GET ROUTES
+// =================================================================================================
+
 func (svr *Server) handleGetSimpleCommitment(w http.ResponseWriter, r *http.Request) error {
 	versionByte, err := parseVersionByte(r)
 	if err != nil {
@@ -111,45 +114,64 @@ func (svr *Server) handleGetShared(ctx context.Context, w http.ResponseWriter, c
 	return nil
 }
 
-// handlePut handles the PUT request for commitments.
-// Note: even when an error is returned, the commitment meta is still returned,
-// because it is needed for metrics (see the WithMetrics middleware).
-// TODO: we should change this behavior and instead use a custom error that contains the commitment meta.
-func (svr *Server) handlePut(w http.ResponseWriter, r *http.Request) (commitments.CommitmentMeta, error) {
-	meta, err := readCommitmentMeta(r)
+// =================================================================================================
+// PUT ROUTES
+// =================================================================================================
+
+func (svr *Server) handlePutSimpleCommitment(w http.ResponseWriter, r *http.Request) error {
+	svr.log.Info("Processing simple commitment")
+	commitmentMeta := commitments.CommitmentMeta{
+		Mode:        commitments.SimpleCommitmentMode,
+		CertVersion: byte(commitments.CertV0), // TODO: hardcoded for now
+	}
+	return svr.handlePutShared(w, r, nil, commitmentMeta)
+}
+
+// handleGetOPKeccakCommitment handles the GET request for optimism keccak commitments.
+func (svr *Server) handlePutOPKeccakCommitment(w http.ResponseWriter, r *http.Request) error {
+	// TODO: do we use a version byte in OPKeccak commitments? README seems to say so, but server_test didn't
+	// versionByte, err := parseVersionByte(r)
+	// if err != nil {
+	// 	err = fmt.Errorf("error parsing version byte: %w", err)
+	// 	http.Error(w, err.Error(), http.StatusBadRequest)
+	// 	return err
+	// }
+	commitmentMeta := commitments.CommitmentMeta{
+		Mode:        commitments.OptimismKeccak,
+		CertVersion: byte(commitments.CertV0),
+	}
+
+	rawCommitmentHex, ok := mux.Vars(r)["raw_commitment_hex"]
+	if !ok {
+		return fmt.Errorf("commitment not found in path: %s", r.URL.Path)
+	}
+	commitment, err := hex.DecodeString(rawCommitmentHex)
 	if err != nil {
-		err = fmt.Errorf("invalid commitment mode: %w", err)
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return commitments.CommitmentMeta{}, err
+		return fmt.Errorf("failed to decode commitment %s: %w", rawCommitmentHex, err)
+	}
+
+	svr.log.Info("Processing op keccak commitment", "commitment", rawCommitmentHex, "commitmentMeta", commitmentMeta)
+	return svr.handlePutShared(w, r, commitment, commitmentMeta)
+}
+
+func (svr *Server) handlePutOPGenericCommitment(w http.ResponseWriter, r *http.Request) error {
+	svr.log.Info("Processing simple commitment")
+	commitmentMeta := commitments.CommitmentMeta{
+		Mode:        commitments.OptimismGeneric,
+		CertVersion: byte(commitments.CertV0), // TODO: hardcoded for now
 	}
-	// ReadCommitmentMeta function invoked inside HandlePut will not return a valid certVersion
-	// Current simple fix is using the hardcoded default value of 0 (also the only supported value)
-	//TODO: smarter decode needed when there's more than one version
-	meta.CertVersion = byte(commitments.CertV0)
+	return svr.handlePutShared(w, r, nil, commitmentMeta)
+}
 
+func (svr *Server) handlePutShared(w http.ResponseWriter, r *http.Request, comm []byte, meta commitments.CommitmentMeta) error {
 	input, err := io.ReadAll(r.Body)
 	if err != nil {
 		err = MetaError{
 			Err:  fmt.Errorf("failed to read request body: %w", err),
 			Meta: meta,
 		}
 		http.Error(w, err.Error(), http.StatusBadRequest)
-		return commitments.CommitmentMeta{}, err
-	}
-
-	key := path.Base(r.URL.Path)
-	var comm []byte
-
-	if len(key) > 0 && key != Put { // commitment key already provided (keccak256)
-		comm, err = commitments.StringToDecodedCommitment(key, meta.Mode)
-		if err != nil {
-			err = MetaError{
-				Err:  fmt.Errorf("failed to decode commitment from key %v (commitment mode %v): %w", key, meta.Mode, err),
-				Meta: meta,
-			}
-			http.Error(w, err.Error(), http.StatusBadRequest)
-			return commitments.CommitmentMeta{}, err
-		}
+		return err
 	}
 
 	commitment, err := svr.router.Put(r.Context(), meta.Mode, comm, input)
@@ -165,7 +187,7 @@ func (svr *Server) handlePut(w http.ResponseWriter, r *http.Request) (commitment
 		} else {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 		}
-		return commitments.CommitmentMeta{}, err
+		return err
 	}
 
 	responseCommit, err := commitments.EncodeCommitment(commitment, meta.Mode)
@@ -175,13 +197,13 @@ func (svr *Server) handlePut(w http.ResponseWriter, r *http.Request) (commitment
 			Meta: meta,
 		}
 		http.Error(w, err.Error(), http.StatusInternalServerError)
-		return commitments.CommitmentMeta{}, err
+		return err
 	}
 
 	svr.log.Info(fmt.Sprintf("response commitment: %x\n", responseCommit))
 	// write commitment to resp body if not in OptimismKeccak mode
 	if meta.Mode != commitments.OptimismKeccak {
 		svr.writeResponse(w, responseCommit)
 	}
-	return meta, nil
+	return nil
 }

server/routing.go

@@ -46,10 +46,30 @@ func (svr *Server) registerRoutes(r *mux.Router) {
 			http.Error(w, fmt.Sprintf("unsupported commitment type %s", commitType), http.StatusBadRequest)
 		},
 	)
-	// we need to handle both: see https://github.com/ethereum-optimism/optimism/pull/12081
-	// /put is for generic commitments, and /put/ for keccak256 commitments
-	// TODO: we should probably separate their handlers?
-	// r.HandleFunc("/put", WithLogging(WithMetrics(svr.handlePut, svr.m), svr.log)).Methods("POST")
-	// r.HandleFunc("/put/", WithLogging(WithMetrics(svr.handlePut, svr.m), svr.log)).Methods("POST")
+
+	subrouterPOST := r.Methods("POST").PathPrefix("/put").Subrouter()
+	// simple commitments (for nitro)
+	subrouterPOST.HandleFunc("", // commitment is calculated by the server using the body data
+		withLogging(withMetrics(svr.handlePutSimpleCommitment, svr.m, commitments.SimpleCommitmentMode), svr.log),
+	).Queries("commitment_mode", "simple")
+	// op keccak256 commitments (write to S3)
+	subrouterPOST.HandleFunc("/"+
+		"{optional_prefix:(?:0x)?}"+ // commitments can be prefixed with 0x
+		// TODO: do we need this 0x00 byte? keccak commitments are the only ones that have anything after /put/
+		"{commit_type_byte_hex:00}"+ // 00 for keccak256 commitments
+		// TODO: should these be present..?? README says they should but server_test didn't have them
+		// "{da_layer_byte:[0-9a-fA-F]{2}}"+ // should always be 0x00 for eigenDA but we let others through to return a 404
+		// "{version_byte_hex:[0-9a-fA-F]{2}}"+ // should always be 0x00 for now but we let others through to return a 404
+		"{raw_commitment_hex}",
+		withLogging(withMetrics(svr.handlePutOPKeccakCommitment, svr.m, commitments.OptimismKeccak), svr.log),
+	)
+	// op generic commitments (write to EigenDA)
+	subrouterPOST.HandleFunc("", // commitment is calculated by the server using the body data
+		withLogging(withMetrics(svr.handlePutOPGenericCommitment, svr.m, commitments.OptimismGeneric), svr.log),
+	)
+	subrouterPOST.HandleFunc("/", // commitment is calculated by the server using the body data
+		withLogging(withMetrics(svr.handlePutOPGenericCommitment, svr.m, commitments.OptimismGeneric), svr.log),
+	)
+
 	r.HandleFunc("/health", withLogging(svr.handleHealth, svr.log)).Methods("GET")
 }

server/routing_test.go

@@ -0,0 +1,105 @@
+package server
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/Layr-Labs/eigenda-proxy/metrics"
+	"github.com/Layr-Labs/eigenda-proxy/mocks"
+	"github.com/ethereum/go-ethereum/log"
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
+)
+
+// TestRouting tests that the routes were properly encoded.
+// We should eventually replace this with autogenerated specmatic tests over an openapi spec.
+func TestRouting(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockRouter := mocks.NewMockIRouter(ctrl)
+
+	m := metrics.NewMetrics("default")
+	server := NewServer("localhost", 8080, mockRouter, log.New(), m)
+	err := server.Start()
+	require.NoError(t, err)
+
+	tests := []struct {
+		name         string
+		url          string
+		method       string
+		body         []byte
+		expectedCode int
+		expectedBody string
+	}{
+		{
+			name:   "Not Found - Must have a commitment key",
+			url:    "/get/0x",
+			method: http.MethodGet,
+			body:   nil,
+			// originally we returned 400 for these, but now we return 404 because
+			// not having a commitment is not a valid route.
+			expectedCode: http.StatusNotFound,
+			expectedBody: "404 page not found\n",
+		},
+		{
+			name: "Not Found - Op Mode InvalidCommitmentKey",
+			url:  "/get/0x1",
+			body: nil,
+			// originally we returned 400 for these, but now we return 404 because
+			// not having a commitment is not a valid route.
+			expectedCode: http.StatusNotFound,
+			expectedBody: "404 page not found\n",
+		},
+		{
+			name: "Not Found - Op Mode InvalidCommitmentKey",
+			url:  "/get/0x999",
+			body: nil,
+			// originally we returned 400 for these, but now we return 404 because
+			// not having a commitment is not a valid route.
+			expectedCode: http.StatusNotFound,
+			expectedBody: "404 page not found\n",
+		},
+		{
+			name:   "Not Found OP Keccak256 - TooShortCommitmentKey",
+			url:    "/put/0x",
+			method: http.MethodPut,
+			body:   []byte("some data"),
+			// originally we returned 400 for these, but now we return 404 because
+			// not having a commitment is not a valid route.
+			expectedCode: http.StatusNotFound,
+			expectedBody: "404 page not found\n",
+		},
+		{
+			name: "Not Found OP Keccak256 - TooShortCommitmentKey",
+			url:  "/put/0x1",
+			body: []byte("some data"),
+			// originally we returned 400 for these, but now we return 404 because
+			// not having a commitment is not a valid route.
+			expectedCode: http.StatusNotFound,
+			expectedBody: "404 page not found\n",
+		},
+		{
+			name: "Not Found OP Keccak256 - InvalidCommitmentPrefixBytes",
+			url:  fmt.Sprintf("/put/0x999%s", testCommitStr),
+			body: []byte("some data"),
+			// originally we returned 400 for these, but now we return 404 because
+			// not having a commitment is not a valid route.
+			expectedCode: http.StatusNotFound,
+			expectedBody: "404 page not found\n",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := httptest.NewRequest(tt.method, tt.url, nil)
+			rec := httptest.NewRecorder()
+			server.httpServer.Handler.ServeHTTP(rec, req)
+
+			require.Equal(t, tt.expectedCode, rec.Code)
+			require.Equal(t, tt.expectedBody, rec.Body.String())
+
+		})
+	}
+}

server/server.go

@@ -7,15 +7,12 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	"path"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/Layr-Labs/eigenda-proxy/commitments"
 	"github.com/Layr-Labs/eigenda-proxy/metrics"
 	"github.com/Layr-Labs/eigenda-proxy/store"
-	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/gorilla/mux"
 )
@@ -116,89 +113,6 @@ func (svr *Server) Port() int {
 	return port
 }
 
-// Read both commitment mode and version
-func readCommitmentMeta(r *http.Request) (commitments.CommitmentMeta, error) {
-	// label requests with commitment mode and version
-	ct, err := readCommitmentMode(r)
-	if err != nil {
-		return commitments.CommitmentMeta{}, fmt.Errorf("failed to read commitment mode: %w", err)
-	}
-	if ct == "" {
-		return commitments.CommitmentMeta{}, fmt.Errorf("commitment mode is empty")
-	}
-	cv, err := readCommitmentVersion(r, ct)
-	if err != nil {
-		// default to version 0
-		return commitments.CommitmentMeta{Mode: ct, CertVersion: cv}, err
-	}
-	return commitments.CommitmentMeta{Mode: ct, CertVersion: cv}, nil
-}
-
-func readCommitmentMode(r *http.Request) (commitments.CommitmentMode, error) {
-	query := r.URL.Query()
-	// if commitment mode is provided in the query params, use it
-	// eg. /get/0x123..?commitment_mode=simple
-	// TODO: should we only allow simple commitment to be set in the query params?
-	key := query.Get(CommitmentModeKey)
-	if key != "" {
-		return commitments.StringToCommitmentMode(key)
-	}
-
-	// else, we need to parse the first byte of the commitment
-	commit := path.Base(r.URL.Path)
-	if len(commit) > 0 && commit != Put { // provided commitment in request params (op keccak256)
-		if !strings.HasPrefix(commit, "0x") {
-			commit = "0x" + commit
-		}
-
-		decodedCommit, err := hexutil.Decode(commit)
-		if err != nil {
-			return "", err
-		}
-
-		if len(decodedCommit) < 3 {
-			return "", fmt.Errorf("commitment is too short")
-		}
-
-		switch decodedCommit[0] {
-		case byte(commitments.GenericCommitmentType):
-			return commitments.OptimismGeneric, nil
-
-		case byte(commitments.Keccak256CommitmentType):
-			return commitments.OptimismKeccak, nil
-
-		default:
-			return commitments.SimpleCommitmentMode, fmt.Errorf("unknown commit byte prefix")
-		}
-	}
-	return commitments.OptimismGeneric, nil
-}
-
-func readCommitmentVersion(r *http.Request, mode commitments.CommitmentMode) (byte, error) {
-	commit := path.Base(r.URL.Path)
-	if len(commit) > 0 && commit != Put { // provided commitment in request params (op keccak256)
-		if !strings.HasPrefix(commit, "0x") {
-			commit = "0x" + commit
-		}
-
-		decodedCommit, err := hexutil.Decode(commit)
-		if err != nil {
-			return 0, err
-		}
-
-		if len(decodedCommit) < 3 {
-			return 0, fmt.Errorf("commitment is too short")
-		}
-
-		if mode == commitments.OptimismGeneric || mode == commitments.SimpleCommitmentMode {
-			return decodedCommit[2], nil
-		}
-
-		return decodedCommit[0], nil
-	}
-	return 0, nil
-}
-
 func (svr *Server) GetEigenDAStats() *store.Stats {
 	return svr.router.GetEigenDAStore().Stats()
 }