Add lint and test workflows (#3)

Co-authored-by: Colin Kennedy <[email protected]>

.github/workflows/lint.yml

@@ -0,0 +1,27 @@
+name: lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - main
+  pull_request:
+  workflow_dispatch:
+
+permissions:
+  contents: read
+  pull-requests: read
+
+jobs:
+  golangci:
+    name: golangci
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v4
+        with:
+          version: v1.57.2

.github/workflows/test.yml

@@ -0,0 +1,28 @@
+name: test
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  test:
+    name: coverage profile
+    strategy:
+      fail-fast: false
+      matrix:
+        go: [ '1.22' ]
+        os: [ ubuntu-latest, macos-latest, self-hosted-linux-arm64]
+    runs-on: ${{ matrix.os }}
+    steps:
+        - uses: actions/checkout@v4
+        - uses: actions/setup-go@v5
+          with:
+            go-version-file: go.mod
+        - name: generate test coverage
+          run: make install-go-test-coverage && make cover.out
+        - name: check test coverage
+          uses: vladopajic/go-test-coverage@v2
+          with:
+            config: .testcoverage.yml

.golangci.yaml

@@ -0,0 +1,130 @@
+linters-settings:
+  depguard:
+    rules:
+      logger:
+        deny:
+          - pkg: github.com/sirupsen/logrus
+  dupl:
+    threshold: 100
+  goconst:
+    min-len: 2
+    min-occurrences: 3
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated
+      - performance
+      - style
+    disabled-checks:
+      - sloppyReassign
+      - dupImport # https://github.com/go-critic/go-critic/issues/845
+      - ifElseChain
+      - octalLiteral
+      - whyNoLint
+      - unnamedResult
+  goimports:
+    local-prefixes: github.com/golangci/golangci-lint
+  gomnd:
+    # don't include the "operation" and "assign"
+    checks:
+      - argument
+      - case
+      - condition
+      - return
+    ignored-numbers:
+      - '0'
+      - '1'
+      - '2'
+      - '3'
+    ignored-functions:
+      - strings.SplitN
+
+  govet:
+    shadow: true
+    settings:
+      printf:
+        funcs:
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
+  lll:
+    line-length: 140
+  misspell:
+    locale: US
+  nolintlint:
+    allow-unused: false # report any unused nolint directives
+    require-explanation: false # don't require an explanation for nolint directives
+    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
+
+linters:
+  disable-all: true
+  enable:
+    - bodyclose
+    - depguard
+    - dogsled
+    - dupl
+    - errcheck
+    - exportloopref
+    - gochecknoinits
+    - goconst
+    - gocritic
+    - gofumpt
+    - gomnd
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - lll
+    - misspell
+    - nakedret
+    - noctx
+    - nolintlint
+    - staticcheck
+    - stylecheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - whitespace
+    - tparallel
+    - gci
+
+  # don't enable:
+  # - asciicheck
+  # - funlen
+  # - gocyclo
+  # - scopelint
+  # - gochecknoglobals
+  # - gocognit
+  # - godot
+  # - godox
+  # - goerr113
+  # - interfacer
+  # - maligned
+  # - nestif
+  # - prealloc
+  # - testpackage
+  # - revive
+  # - wsl
+
+issues:
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        - gomnd
+        - funlen
+        - lll
+        - gosec
+
+    - linters:
+        - lll
+      source: "^//go:generate "
+  exclude-dirs:
+    - app # This code is left over and will be changed soon.
+
+run:
+  timeout: 5m

.testcoverage.yml

@@ -0,0 +1,12 @@
+profile: cover.out
+local-prefix: github.com/polymerdao/monomer
+threshold:
+  package: 65
+  total: 70
+exclude:
+  paths:
+    - \.pb\.go$
+    - ^app
+    - ^x/rollup
+    - ^cmd/monomer
+    - ^engine

Makefile

@@ -1,7 +1,37 @@
-.PHONY: monomer
-monomer:
-	@go build -o monomer ./cmd/monomer/
+GOBIN ?= $$(go env GOPATH)/bin
+BINARY_NAME ?= monomer
+COVER_OUT ?= cover.out
+COVER_HTML ?= cover.html
+
+.PHONY: $(BINARY_NAME)
+$(BINARY_NAME):
+	go build -o $(BINARY_NAME) ./cmd/monomer/
+
+.PHONY: install-golangci-lint
+install-golangci-lint:
+	go install github.com/golangci/golangci-lint/cmd/[email protected]
+
+.PHONY: lint
+lint: install-golangci-lint
+	${GOBIN}/golangci-lint run
+
+.PHONY: install-go-test-coverage
+install-go-test-coverage:
+	go install github.com/vladopajic/go-test-coverage/[email protected]
+
+$(COVER_OUT):
+	go test ./... -coverprofile=$@ -covermode=atomic -coverpkg=./...
+	${GOBIN}/go-test-coverage --config=./.testcoverage.yml
+
+$(COVER_HTML): $(COVER_OUT)
+	go tool cover -html=$< -o $@
+
+.PHONY: test
+test: 
+	go test ./...
 
 .PHONY: clean
 clean:
-	@if [ -f ./monomer ]; then rm ./monomer; fi
+	if [ -f $(BINARY_NAME) ]; then rm $(BINARY_NAME); fi
+	if [ -f $(COVER_OUT) ]; then rm $(COVER_OUT); fi
+	if [ -f $(COVER_HTML) ]; then rm $(COVER_HTML); fi

app/peptide/payloadstore/payloadstore_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/ethereum-optimism/optimism/op-service/eth"
+	"github.com/ethereum/go-ethereum/beacon/engine"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/polymerdao/monomer"
@@ -17,29 +18,34 @@ func dummyPayload(height int64) *monomer.Payload {
 		GasLimit: &gaslimit,
 	}
 	hash := common.HexToHash(fmt.Sprintf("0x%x", height))
-	return eetypes.NewPayload(&attrs, hash, height)
+	return &monomer.Payload{
+		Timestamp:             uint64(attrs.Timestamp),
+		PrevRandao:            attrs.PrevRandao,
+		SuggestedFeeRecipient: attrs.SuggestedFeeRecipient,
+		Withdrawals:           attrs.Withdrawals,
+		NoTxPool:              attrs.NoTxPool,
+		GasLimit:              uint64(*attrs.GasLimit),
+		ParentBeaconBlockRoot: attrs.ParentBeaconBlockRoot,
+		ParentHash:            hash,
+		Height:                height,
+		Transactions:          attrs.Transactions,
+	}
 }
 
 func TestRollback(t *testing.T) {
 	ps := NewPayloadStore()
-	ids := make([]*eetypes.PayloadID, 10)
+	ids := make([]*engine.PayloadID, 10)
 
 	for h := int64(0); h < 10; h++ {
 		payload := dummyPayload(h)
-		require.NoError(t, ps.Add(payload))
-		id, err := payload.GetPayloadID()
-		require.NoError(t, err)
-		ids[h] = id
+		ps.Add(payload)
+		ids[h] = payload.ID()
 	}
 
 	for h := int64(0); h < 10; h++ {
 		newpayload, ok := ps.Get(*ids[h])
 		require.True(t, ok)
-
-		id, err := newpayload.GetPayloadID()
-		require.NoError(t, err)
-
-		require.Equal(t, ids[h], id)
+		require.Equal(t, ids[h], newpayload.ID())
 	}
 
 	require.NoError(t, ps.RollbackToHeight(5))

builder/builder.go

@@ -25,15 +25,15 @@ type Builder struct {
 }
 
 func New(
-	mempool *mempool.Pool,
+	mpool *mempool.Pool,
 	app monomer.Application,
 	blockStore store.BlockStore,
 	txStore txstore.TxStore,
 	eventBus *bfttypes.EventBus,
 	chainID monomer.ChainID,
 ) *Builder {
 	return &Builder{
-		mempool:    mempool,
+		mempool:    mpool,
 		app:        app,
 		blockStore: blockStore,
 		txStore:    txStore,
@@ -63,10 +63,15 @@ func (b *Builder) Rollback(head, safe, finalized common.Hash) error {
 	if err := b.blockStore.RollbackToHeight(targetHeight); err != nil {
 		return fmt.Errorf("rollback block store: %v", err)
 	}
-	// Ignore errors from UpdateLabel since we assume all hashes exist in the block store already.
-	b.blockStore.UpdateLabel(eth.Unsafe, head)
-	b.blockStore.UpdateLabel(eth.Safe, safe)
-	b.blockStore.UpdateLabel(eth.Finalized, finalized)
+	if err := b.blockStore.UpdateLabel(eth.Unsafe, head); err != nil {
+		return fmt.Errorf("update unsafe label: %v", err)
+	}
+	if err := b.blockStore.UpdateLabel(eth.Safe, safe); err != nil {
+		return fmt.Errorf("update safe label: %v", err)
+	}
+	if err := b.blockStore.UpdateLabel(eth.Finalized, finalized); err != nil {
+		return fmt.Errorf("update finalized label: %v", err)
+	}
 
 	if err := b.txStore.RollbackToHeight(targetHeight, currentHeight); err != nil {
 		return fmt.Errorf("rollback tx store: %v", err)
@@ -120,10 +125,10 @@ func (b *Builder) Build(payload *Payload) error {
 	header := &monomer.Header{
 		ChainID:    b.chainID,
 		Height:     currentHeight + 1,
-		Time:       uint64(payload.Timestamp), // TODO should we validate timestamp is greater than the previous timestamp? Probably should be done in engine api, if at all.
+		Time:       payload.Timestamp,
 		ParentHash: currentHead.Header.Hash,
 		AppHash:    info.GetLastBlockAppHash(),
-		GasLimit:   uint64(payload.GasLimit),
+		GasLimit:   payload.GasLimit,
 	}
 
 	// BeginBlock, DeliverTx, EndBlock, Commit

builder/builder_test.go

@@ -152,11 +152,9 @@ func TestBuild(t *testing.T) {
 			genesisBlock := blockStore.BlockByNumber(preBuildInfo.GetLastBlockHeight())
 			require.NotNil(t, genesisBlock)
 			gotBlock := blockStore.HeadBlock()
-			var allTxs [][]byte
-			if test.noTxPool {
-				allTxs = inclusionListTxs
-			} else {
-				allTxs = append(inclusionListTxs, mempoolTxs...)
+			allTxs := append([][]byte{}, inclusionListTxs...)
+			if !test.noTxPool {
+				allTxs = append(allTxs, mempoolTxs...)
 			}
 			wantBlock := &monomer.Block{
 				Header: &monomer.Header{

cmd/monomer/main.go

@@ -38,8 +38,7 @@ type Config struct {
 }
 
 func main() {
-	ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
-	defer cancel()
+	ctx, _ := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
 	if err := run(ctx); err != nil {
 		fmt.Fprintf(os.Stderr, "error: %v\n", err)
 		os.Exit(1)
@@ -57,11 +56,11 @@ func parseFlags() (*Config, error) {
 	var engineHost string
 	flag.StringVar(&engineHost, "engine-host", "127.0.0.1", "")
 	var enginePort uint64
-	flag.Uint64Var(&enginePort, "engine-port", 8888, "")
+	flag.Uint64Var(&enginePort, "engine-port", 8888, "") //nolint:gomnd
 	var ethHost string
 	flag.StringVar(&ethHost, "eth-host", "127.0.0.1", "")
 	var ethPort uint64
-	flag.Uint64Var(&ethPort, "eth-port", 8888, "")
+	flag.Uint64Var(&ethPort, "eth-port", 8889, "") //nolint:gomnd
 	var genesisFile string
 	flag.StringVar(&genesisFile, "genesis-file", "", "")
 
@@ -118,7 +117,7 @@ func run(ctx context.Context) error {
 	}()
 	blockStore := store.NewBlockStore(blockdb)
 
-	if err := prepareBlockStoreAndApp(config.Genesis, blockStore, app); err != nil {
+	if err = prepareBlockStoreAndApp(config.Genesis, blockStore, app); err != nil {
 		return err
 	}