Skip to content

Commit

Permalink
fix: Commitment decoding bug && server handler tests (#112)
Browse files Browse the repository at this point in the history
* fix: Commitment decoding bug && svr handler tests

* fix: Commitment decoding bug && svr handler tests - lint fix

* fix: Commitment decoding bug && svr handler tests - mv gosec to golangci lint

* fix: Commitment decoding bug && svr handler tests - rm false positives
  • Loading branch information
epociask authored Sep 5, 2024
1 parent 7d15f38 commit 7959e8a
Show file tree
Hide file tree
Showing 14 changed files with 385 additions and 32 deletions.
11 changes: 0 additions & 11 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,3 @@ jobs:
version: v1.60
args: --timeout 3m

go-sec:
runs-on: ubuntu-latest
env:
GO111MODULE: on
steps:
- name: Checkout Source
uses: actions/checkout@v3
- name: Run Gosec Security Scanner
uses: securego/gosec@master
with:
args: ./...
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ linters:
disable-all: true
enable:
## enabled by default
- gosec # Golang Security Checker
- errcheck # checking for unchecked errors, these unchecked errors can be critical bugs in some cases
- gosimple # specializes in simplifying a code
- ineffassign # detects when assignments to existing variables are not used
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ lint:

@golangci-lint run

go-gen-mocks:
@echo "generating go mocks..."
@GO111MODULE=on go generate --run "mockgen*" ./...

install-lint:
@echo "Installing golangci-lint..."
@sh -c $(GET_LINT_CMD)
Expand Down
8 changes: 4 additions & 4 deletions commitments/mode.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ func StringToCommitmentMode(s string) (CommitmentMode, error) {
}

func StringToDecodedCommitment(key string, c CommitmentMode) ([]byte, error) {
if len(key) <= 2 {
return nil, fmt.Errorf("commitment is empty")
}

offset := 0
if key[:2] == "0x" {
offset = 2
Expand All @@ -41,6 +37,10 @@ func StringToDecodedCommitment(key string, c CommitmentMode) ([]byte, error) {
return nil, err
}

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

switch c {
case OptimismGeneric: // [op_type, ...]
return b[1:], nil
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/consensys/gnark-crypto v0.12.1
github.com/ethereum-optimism/optimism v1.9.0
github.com/ethereum/go-ethereum v1.14.0
github.com/golang/mock v1.2.0
github.com/joho/godotenv v1.5.1
github.com/minio/minio-go/v7 v7.0.74
github.com/prometheus/client_golang v1.19.1
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:tluoj9z5200jBnyusfRPU2LqT6J+DAorxEvtC7LHB+E=
github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
github.com/golang/mock v1.2.0 h1:28o5sBqPkBsMGnC6b4MvE2TzSr5/AT4c/1fLqVGIwlk=
github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
Expand Down
95 changes: 95 additions & 0 deletions mocks/router.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion server/load_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

// LoadStoreRouter ... creates storage backend clients and instruments them into a storage routing abstraction
func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (*store.Router, error) {
func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store.IRouter, error) {
// create S3 backend store (if enabled)
var err error
var s3 *store.S3Store
Expand Down
10 changes: 7 additions & 3 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ const (
type Server struct {
log log.Logger
endpoint string
router *store.Router
router store.IRouter
m metrics.Metricer
tls *rpc.ServerTLSConfig
httpServer *http.Server
listener net.Listener
}

func NewServer(host string, port int, router *store.Router, log log.Logger,
func NewServer(host string, port int, router store.IRouter, log log.Logger,
m metrics.Metricer) *Server {
endpoint := net.JoinHostPort(host, strconv.Itoa(port))
return &Server{
Expand Down Expand Up @@ -266,7 +266,7 @@ func ReadCommitmentMode(r *http.Request) (commitments.CommitmentMode, error) {
}

commit := path.Base(r.URL.Path)
if len(commit) > 0 && commit != "put" { // provided commitment in request params
if len(commit) > 0 && commit != "put" { // provided commitment in request params (op keccak256)
if !strings.HasPrefix(commit, "0x") {
commit = "0x" + commit
}
Expand All @@ -276,6 +276,10 @@ func ReadCommitmentMode(r *http.Request) (commitments.CommitmentMode, error) {
return commitments.SimpleCommitmentMode, err
}

if len(decodedCommit) < 3 {
return commitments.SimpleCommitmentMode, fmt.Errorf("commitment is too short")
}

switch decodedCommit[0] {
case byte(commitments.GenericCommitmentType):
return commitments.OptimismAltDA, nil
Expand Down
Loading

0 comments on commit 7959e8a

Please sign in to comment.