Skip to content

Commit

Permalink
refactor: cleaner routing using gorilla mux regexps (#185)
Browse files Browse the repository at this point in the history
* refactor: GET routes to use gorilla mux regexp patterns

* refactor: server into files routing/handlers/middleware

* style: fix lint

* refactor: use gorilla mux for POST routes

* routing: fix incorrect error msgs for simple commitments

* cleanup: old unused function

* tests: fix e2e tests

* docs: add docstrings to handlers

* style: use vars for the gorilla mux routing variables instead of hardcoded strings

* logging: better logging msgs in op handlers

* metrics: use "unknown" instead of "noCommitment" in metrics middleware when missing meta info

* logging(handlers): simplify by moving "processing request" logs to shared handlers only
  • Loading branch information
samlaf authored Oct 20, 2024
1 parent df3a6f8 commit 0035323
Show file tree
Hide file tree
Showing 12 changed files with 562 additions and 482 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ For `alt-da` clients running on Optimism, the following commitment schema is sup

Both `keccak256` (i.e, S3 storage using hash of pre-image for commitment value) and `generic` (i.e, EigenDA) are supported to ensure cross-compatibility with alt-da storage backends if desired by a rollup operator.

OP Stack itself only has a conception of the first byte (`commit type`) and does no semantical interpretation of any subsequent bytes within the encoding. The `da layer type` byte for EigenDA is always `0x0`. However it is currently unused by OP Stack with name space values still being actively [discussed](https://github.com/ethereum-optimism/specs/discussions/135#discussioncomment-9271282).
OP Stack itself only has a conception of the first byte (`commit type`) and does no semantical interpretation of any subsequent bytes within the encoding. The `da layer type` byte for EigenDA is always `0x00`. However it is currently unused by OP Stack with name space values still being actively [discussed](https://github.com/ethereum-optimism/specs/discussions/135#discussioncomment-9271282).

### Simple Commitment Mode
For simple clients communicating with proxy (e.g, arbitrum nitro), the following commitment schema is supported:
Expand Down
31 changes: 0 additions & 31 deletions commitments/mode.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package commitments

import (
"encoding/hex"
"fmt"
)

Expand Down Expand Up @@ -32,36 +31,6 @@ func StringToCommitmentMode(s string) (CommitmentMode, error) {
}
}

func StringToDecodedCommitment(key string, c CommitmentMode) ([]byte, error) {
offset := 0
if key[:2] == "0x" {
offset = 2
}

b, err := hex.DecodeString(key[offset:])
if err != nil {
return nil, err
}

if len(b) < 3 {
return nil, fmt.Errorf("commitment is too short")
}

switch c {
case OptimismKeccak: // [op_type, ...]
return b[1:], nil

case OptimismGeneric: // [op_type, da_provider, cert_version, ...]
return b[3:], nil

case SimpleCommitmentMode: // [cert_version, ...]
return b[1:], nil

default:
return nil, fmt.Errorf("unknown commitment type")
}
}

func EncodeCommitment(b []byte, c CommitmentMode) ([]byte, error) {
switch c {
case OptimismKeccak:
Expand Down
22 changes: 10 additions & 12 deletions e2e/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,21 +199,19 @@ func TestProxyClientServerIntegration(t *testing.T) {
require.NoError(t, err)
})

t.Run("get data edge cases", func(t *testing.T) {
testCert := []byte("")
t.Run("get data edge cases - unsupported version byte 01", func(t *testing.T) {
testCert := []byte{1}
_, err := daClient.GetData(ts.Ctx, testCert)
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(),
"commitment is too short") && !isNilPtrDerefPanic(err.Error()))

testCert = []byte{1}
_, err = daClient.GetData(ts.Ctx, testCert)
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(),
"commitment is too short") && !isNilPtrDerefPanic(err.Error()))
assert.True(t,
strings.Contains(err.Error(), "unsupported version byte 01") && !isNilPtrDerefPanic(err.Error()))
})

testCert = []byte(e2e.RandString(10000))
_, err = daClient.GetData(ts.Ctx, testCert)
t.Run("get data edge cases - huge cert", func(t *testing.T) {
// TODO: we need to add the 0 version byte at the beginning.
// should this not be done automatically by the simple_commitment client?
testCert := append([]byte{0}, []byte(e2e.RandString(10000))...)
_, err := daClient.GetData(ts.Ctx, testCert)
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(),
"failed to decode DA cert to RLP format: rlp: expected input list for verify.Certificate") &&
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/ethereum/go-ethereum v1.14.11
github.com/go-redis/redis/v8 v8.11.5
github.com/golang/mock v1.2.0
github.com/gorilla/mux v1.8.0
github.com/joho/godotenv v1.5.1
github.com/minio/minio-go/v7 v7.0.77
github.com/prometheus/client_golang v1.20.4
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+
github.com/googleapis/gax-go v2.0.0+incompatible/go.mod h1:SFVmujtThgffbyetf+mdk2eWhX2bMyUtNHzFKcPA9HY=
github.com/googleapis/gax-go/v2 v2.0.3/go.mod h1:LLvjysVCY1JZeum8Z6l8qUty8fiNwE08qbEPm1M08qg=
github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI=
github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=
Expand Down
5 changes: 2 additions & 3 deletions server/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ func (me MetaError) Error() string {
me.Meta.CertVersion)
}

// NewMetaError creates a new MetaError
func NewMetaError(err error, meta commitments.CommitmentMeta) MetaError {
return MetaError{Err: err, Meta: meta}
func (me MetaError) Unwrap() error {
return me.Err
}
209 changes: 209 additions & 0 deletions server/handlers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
package server

import (
"context"
"encoding/hex"
"errors"
"fmt"
"io"
"net/http"

"github.com/Layr-Labs/eigenda-proxy/commitments"
"github.com/Layr-Labs/eigenda-proxy/store"
"github.com/gorilla/mux"
)

func (svr *Server) handleHealth(w http.ResponseWriter, _ *http.Request) error {
w.WriteHeader(http.StatusOK)
return nil
}

// =================================================================================================
// GET ROUTES
// =================================================================================================

// handleGetSimpleCommitment handles the GET request for simple commitments.
func (svr *Server) handleGetSimpleCommitment(w http.ResponseWriter, r *http.Request) error {
versionByte, err := parseVersionByte(w, r)
if err != nil {
return fmt.Errorf("error parsing version byte: %w", err)
}
commitmentMeta := commitments.CommitmentMeta{
Mode: commitments.SimpleCommitmentMode,
CertVersion: versionByte,
}

rawCommitmentHex, ok := mux.Vars(r)[routingVarNameRawCommitmentHex]
if !ok {
return fmt.Errorf("commitment not found in path: %s", r.URL.Path)
}
commitment, err := hex.DecodeString(rawCommitmentHex)
if err != nil {
return fmt.Errorf("failed to decode commitment %s: %w", rawCommitmentHex, err)
}

return svr.handleGetShared(r.Context(), w, commitment, commitmentMeta)
}

// handleGetOPKeccakCommitment handles the GET request for optimism keccak commitments.
func (svr *Server) handleGetOPKeccakCommitment(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)[routingVarNameRawCommitmentHex]
if !ok {
return fmt.Errorf("commitment not found in path: %s", r.URL.Path)
}
commitment, err := hex.DecodeString(rawCommitmentHex)
if err != nil {
return fmt.Errorf("failed to decode commitment %s: %w", rawCommitmentHex, err)
}

return svr.handleGetShared(r.Context(), w, commitment, commitmentMeta)
}

// handleGetOPGenericCommitment handles the GET request for optimism generic commitments.
func (svr *Server) handleGetOPGenericCommitment(w http.ResponseWriter, r *http.Request) error {
versionByte, err := parseVersionByte(w, r)
if err != nil {
return fmt.Errorf("error parsing version byte: %w", err)
}
commitmentMeta := commitments.CommitmentMeta{
Mode: commitments.OptimismGeneric,
CertVersion: versionByte,
}

rawCommitmentHex, ok := mux.Vars(r)[routingVarNameRawCommitmentHex]
if !ok {
return fmt.Errorf("commitment not found in path: %s", r.URL.Path)
}
commitment, err := hex.DecodeString(rawCommitmentHex)
if err != nil {
return fmt.Errorf("failed to decode commitment %s: %w", rawCommitmentHex, err)
}

return svr.handleGetShared(r.Context(), w, commitment, commitmentMeta)
}

func (svr *Server) handleGetShared(ctx context.Context, w http.ResponseWriter, comm []byte, meta commitments.CommitmentMeta) error {
svr.log.Info("Processing GET request", "commitment", hex.EncodeToString(comm), "commitmentMeta", meta)
input, err := svr.router.Get(ctx, comm, meta.Mode)
if err != nil {
err = MetaError{
Err: fmt.Errorf("get request failed with commitment %v: %w", comm, err),
Meta: meta,
}
if errors.Is(err, ErrNotFound) {
http.Error(w, err.Error(), http.StatusNotFound)
} else {
http.Error(w, err.Error(), http.StatusInternalServerError)
}
return err
}

svr.writeResponse(w, input)
return nil
}

// =================================================================================================
// POST ROUTES
// =================================================================================================

// handlePostSimpleCommitment handles the POST request for simple commitments.
func (svr *Server) handlePostSimpleCommitment(w http.ResponseWriter, r *http.Request) error {
commitmentMeta := commitments.CommitmentMeta{
Mode: commitments.SimpleCommitmentMode,
CertVersion: byte(commitments.CertV0), // TODO: hardcoded for now
}
return svr.handlePostShared(w, r, nil, commitmentMeta)
}

// handlePostOPKeccakCommitment handles the POST request for optimism keccak commitments.
func (svr *Server) handlePostOPKeccakCommitment(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)[routingVarNameRawCommitmentHex]
if !ok {
return fmt.Errorf("commitment not found in path: %s", r.URL.Path)
}
commitment, err := hex.DecodeString(rawCommitmentHex)
if err != nil {
return fmt.Errorf("failed to decode commitment %s: %w", rawCommitmentHex, err)
}

return svr.handlePostShared(w, r, commitment, commitmentMeta)
}

// handlePostOPGenericCommitment handles the POST request for optimism generic commitments.
func (svr *Server) handlePostOPGenericCommitment(w http.ResponseWriter, r *http.Request) error {
commitmentMeta := commitments.CommitmentMeta{
Mode: commitments.OptimismGeneric,
CertVersion: byte(commitments.CertV0), // TODO: hardcoded for now
}
return svr.handlePostShared(w, r, nil, commitmentMeta)
}

func (svr *Server) handlePostShared(w http.ResponseWriter, r *http.Request, comm []byte, meta commitments.CommitmentMeta) error {
svr.log.Info("Processing POST request", "commitment", hex.EncodeToString(comm), "commitmentMeta", meta)
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 err
}

commitment, err := svr.router.Put(r.Context(), meta.Mode, comm, input)
if err != nil {
err = MetaError{
Err: fmt.Errorf("put request failed with commitment %v (commitment mode %v): %w", comm, meta.Mode, err),
Meta: meta,
}
if errors.Is(err, store.ErrEigenDAOversizedBlob) || errors.Is(err, store.ErrProxyOversizedBlob) {
// we add here any error that should be returned as a 400 instead of a 500.
// currently only includes oversized blob requests
http.Error(w, err.Error(), http.StatusBadRequest)
} else {
http.Error(w, err.Error(), http.StatusInternalServerError)
}
return err
}

responseCommit, err := commitments.EncodeCommitment(commitment, meta.Mode)
if err != nil {
err = MetaError{
Err: fmt.Errorf("failed to encode commitment %v (commitment mode %v): %w", commitment, meta.Mode, err),
Meta: meta,
}
http.Error(w, err.Error(), http.StatusInternalServerError)
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 nil
}
66 changes: 66 additions & 0 deletions server/middleware.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package server

import (
"errors"
"fmt"
"net/http"
"time"

"github.com/Layr-Labs/eigenda-proxy/commitments"
"github.com/Layr-Labs/eigenda-proxy/metrics"
"github.com/ethereum/go-ethereum/log"
)

// withMetrics is a middleware that records metrics for the route path.
func withMetrics(
handleFn func(http.ResponseWriter, *http.Request) error,
m metrics.Metricer,
mode commitments.CommitmentMode,
) func(http.ResponseWriter, *http.Request) error {
return func(w http.ResponseWriter, r *http.Request) error {
recordDur := m.RecordRPCServerRequest(r.Method)

err := handleFn(w, r)
if err != nil {
var metaErr MetaError
if errors.As(err, &metaErr) {
recordDur(w.Header().Get("status"), string(metaErr.Meta.Mode), string(metaErr.Meta.CertVersion))
} else {
recordDur(w.Header().Get("status"), string("unknown"), string("unknown"))
}
return err
}
// we assume that every route will set the status header
versionByte, err := parseVersionByte(w, r)
if err != nil {
return fmt.Errorf("metrics middleware: error parsing version byte: %w", err)
}
recordDur(w.Header().Get("status"), string(mode), string(versionByte))
return nil
}
}

// withLogging is a middleware that logs information related to each request.
// It does not write anything to the response, that is the job of the handlers.
// Currently we cannot log the status code because go's default ResponseWriter interface does not expose it.
// TODO: implement a ResponseWriter wrapper that saves the status code: see https://github.com/golang/go/issues/18997
func withLogging(
handleFn func(http.ResponseWriter, *http.Request) error,
log log.Logger,
) func(http.ResponseWriter, *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
start := time.Now()
err := handleFn(w, r)
var metaErr MetaError
//nolint:gocritic // ifElseChain is not a good replacement with errors.As
if errors.As(err, &metaErr) {
log.Info("request", "method", r.Method, "url", r.URL, "duration", time.Since(start),
"err", err, "status", w.Header().Get("status"),
"commitment_mode", metaErr.Meta.Mode, "cert_version", metaErr.Meta.CertVersion)
} else if err != nil {
log.Info("request", "method", r.Method, "url", r.URL, "duration", time.Since(start), "err", err)
} else {
log.Info("request", "method", r.Method, "url", r.URL, "duration", time.Since(start))
}
}
}
Loading

0 comments on commit 0035323

Please sign in to comment.