ci: additional workflows and update template (#614)

.github/pull_request_template.md

@@ -4,6 +4,10 @@
 
 ---
 
+Close #XXX
+
+<-- Briefly describe the content of this pull request -->
+
 For Author:
 
 - [ ]  Targeted PR against correct branch

.github/workflows/codeql.yml

@@ -0,0 +1,41 @@
+name: "CodeQL"
+
+on:
+  pull_request:
+    paths:
+      - "**.go"
+  push:
+    # The branches below must be a subset of the branches above
+    branches:
+      - main
+      - release/**
+    paths:
+      - "**.go"
+
+jobs:
+  analyze:
+    name: Analyze
+    runs-on: ubuntu-latest
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+      - uses: actions/setup-go@v3
+        with:
+          go-version: "1.22"
+      # Initializes the CodeQL tools for scanning.
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v2
+        with:
+          languages: "go"
+          queries: crypto-com/cosmos-sdk-codeql@main,security-and-quality
+
+      - name: Build
+        run: make build
+
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v2

.github/workflows/markdown_lint.yml

@@ -0,0 +1,19 @@
+name: markdown-lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - main
+  pull_request:
+
+jobs:
+  markdownlint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: markdownlint-cli
+        uses: nosborn/[email protected]
+        with:
+          files: ./
+          config_file: .markdownlint.yaml

.golangci.yml

@@ -3,13 +3,6 @@ run:
   timeout: 5m
   modules-download-mode: readonly
   # mempool and indexer code is borrowed from Tendermint
-  skip-dirs:
-    - mempool
-    - state/indexer
-    - state/txindex
-  skip-files:
-    - da/celestia/mock/server.go
-    - ./*_test.go
 
 linters:
   disable-all: true
@@ -29,3 +22,10 @@ linters:
 
 issues:
   exclude-use-default: false
+  exclude-files: 
+    - da/celestia/mock/server.go
+    - ./*_test.go
+  exclude-dirs:
+    - mempool
+    - state/indexer
+    - state/txindex

.markdownlint.yaml

@@ -2,7 +2,6 @@ default: true
 MD010:
   code_blocks: false
 MD013: false
-MD024:
-  allow_different_nesting: true
+MD024: true
 MD033:
   allowed_elements: ["img"]

block/initchain.go

@@ -21,7 +21,7 @@ func (m *Manager) RunInitChain(ctx context.Context) error {
 	// This is a hack to make sure the chain can get both addresses without us needing to change comet signatures:
 	//  The RDK will save a sequencer, and delete the extra validator after initChain, so we also delete it here
 	//  to keep the state in sync.
-	
+
 	//get the proposer's consensus pubkey
 	proposer := m.settlementClient.GetProposer()
 	tmPubKey, err := cryptocodec.ToTmPubKeyInterface(proposer.PublicKey)

block/manager_test.go

@@ -31,28 +31,25 @@ import (
 	"github.com/dymensionxyz/dymint/store"
 )
 
-const (
-	connectionRefusedErrorMessage    = "connection refused"
-	errorValidatingBatchErrorMessage = "Error validating batch"
-)
-
 func TestInitialState(t *testing.T) {
+	var err error
 	assert := assert.New(t)
 	genesis := testutil.GenerateGenesis(123)
 	sampleState := testutil.GenerateState(1, 128)
 	key, _, _ := crypto.GenerateEd25519Key(rand.Reader)
 	conf := getManagerConfig()
 	logger := log.TestingLogger()
 	pubsubServer := pubsub.NewServer()
-	pubsubServer.Start()
+	err = pubsubServer.Start()
+	require.NoError(t, err)
 	proxyApp := testutil.GetABCIProxyAppMock(logger.With("module", "proxy"))
 	settlementlc := slregistry.GetClient(slregistry.Mock)
 	_ = settlementlc.Init(settlement.Config{}, pubsubServer, logger)
 
 	// Init empty store and full store
 	emptyStore := store.New(store.NewDefaultInMemoryKVStore())
 	fullStore := store.New(store.NewDefaultInMemoryKVStore())
-	_, err := fullStore.UpdateState(sampleState, nil)
+	_, err = fullStore.UpdateState(sampleState, nil)
 	require.NoError(t, err)
 
 	// Init p2p client
@@ -130,7 +127,8 @@ func TestProduceOnlyAfterSynced(t *testing.T) {
 		assert.NoError(t, err)
 		daResultSubmitBatch := manager.dalc.SubmitBatch(batch)
 		assert.Equal(t, daResultSubmitBatch.Code, da.StatusSuccess)
-		manager.settlementClient.SubmitBatch(batch, manager.dalc.GetClientType(), &daResultSubmitBatch)
+		err = manager.settlementClient.SubmitBatch(batch, manager.dalc.GetClientType(), &daResultSubmitBatch)
+		require.NoError(t, err)
 		nextBatchStartHeight = batch.EndHeight + 1
 		// Wait until daHeight is updated
 		time.Sleep(time.Millisecond * 500)
@@ -143,7 +141,14 @@ func TestProduceOnlyAfterSynced(t *testing.T) {
 	//enough time to sync and produce blocks
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second*4)
 	defer cancel()
-	go manager.Start(ctx, true)
+	// Capture the error returned by manager.Start.
+	errChan := make(chan error, 1)
+	go func() {
+		errChan <- manager.Start(ctx, true)
+		err := <-errChan
+		// Check for error from manager.Start.
+		assert.NoError(t, err, "Manager start should not produce an error")
+	}()
 	<-ctx.Done()
 	assert.True(t, manager.syncTarget == batch.EndHeight)
 	//validate that we produced blocks
@@ -271,7 +276,8 @@ func TestBlockProductionNodeHealth(t *testing.T) {
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			manager.pubsub.PublishWithEvents(context.Background(), c.healthStatusEventData, c.healthStatusEvent)
+			err := manager.pubsub.PublishWithEvents(context.Background(), c.healthStatusEventData, c.healthStatusEvent)
+			assert.NoError(err, "PublishWithEvents should not produce an error")
 			time.Sleep(500 * time.Millisecond)
 			blockHeight := manager.store.Height()
 			time.Sleep(500 * time.Millisecond)
@@ -425,7 +431,8 @@ func TestCreateNextDABatchWithBytesLimit(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			// Produce blocks
 			for i := 0; i < tc.blocksToProduce; i++ {
-				manager.produceBlock(ctx, true)
+				err := manager.produceBlock(ctx, true)
+				assert.NoError(err)
 			}
 
 			// Call createNextDABatch function

block/production_test.go

@@ -114,7 +114,14 @@ func TestCreateEmptyBlocksNew(t *testing.T) {
 	require.NoError(err)
 	require.NotNil(abciClient)
 	require.NoError(abciClient.Start())
-	defer abciClient.Stop()
+
+	defer func() {
+		// Capture the error returned by abciClient.Stop and handle it.
+		err := abciClient.Stop()
+		if err != nil {
+			t.Logf("Error stopping ABCI client: %v", err)
+		}
+	}()
 
 	mempoolCfg := tmcfg.DefaultMempoolConfig()
 	mempoolCfg.KeepInvalidTxsInCache = false

conv/config_test.go

@@ -16,22 +16,29 @@ func TestGetNodeConfig(t *testing.T) {
 	validDymint := "/ip4/127.0.0.1/tcp/1234"
 
 	cases := []struct {
-		name     string
-		input    *tmcfg.Config
-		expected config.NodeConfig
+		name        string
+		input       *tmcfg.Config
+		expected    config.NodeConfig
+		expectError bool
 	}{
-		{"empty", nil, config.NodeConfig{}},
-		{"Seeds", &tmcfg.Config{P2P: &tmcfg.P2PConfig{Seeds: "seeds"}}, config.NodeConfig{P2P: config.P2PConfig{Seeds: "seeds"}}},
+		{"empty", nil, config.NodeConfig{}, true},
+		{"Seeds", &tmcfg.Config{P2P: &tmcfg.P2PConfig{Seeds: validCosmos + "," + validCosmos}}, config.NodeConfig{P2P: config.P2PConfig{Seeds: validDymint + "," + validDymint}}, false},
 		//GetNodeConfig translates the listen address, so we expect the translated address
-		{"ListenAddress", &tmcfg.Config{P2P: &tmcfg.P2PConfig{ListenAddress: validCosmos}}, config.NodeConfig{P2P: config.P2PConfig{ListenAddress: validDymint}}},
-		{"RootDir", &tmcfg.Config{BaseConfig: tmcfg.BaseConfig{RootDir: "~/root"}}, config.NodeConfig{RootDir: "~/root"}},
-		{"DBPath", &tmcfg.Config{BaseConfig: tmcfg.BaseConfig{DBPath: "./database"}}, config.NodeConfig{DBPath: "./database"}},
+		{"ListenAddress", &tmcfg.Config{P2P: &tmcfg.P2PConfig{ListenAddress: validCosmos}}, config.NodeConfig{P2P: config.P2PConfig{ListenAddress: validDymint}}, false},
+		{"RootDir", &tmcfg.Config{BaseConfig: tmcfg.BaseConfig{RootDir: "~/root"}}, config.NodeConfig{RootDir: "~/root"}, false},
+		{"DBPath", &tmcfg.Config{BaseConfig: tmcfg.BaseConfig{DBPath: "./database"}}, config.NodeConfig{DBPath: "./database"}, false},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			var actual config.NodeConfig
-			GetNodeConfig(&actual, c.input)
+			err := GetNodeConfig(&actual, c.input)
+			if c.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, c.expected, actual)
+			}
 			assert.Equal(t, c.expected, actual)
 		})
 	}

da/avail/avail_test.go

@@ -129,7 +129,7 @@ func TestRetriveBatches(t *testing.T) {
 		avail.WithClient(mockSubstrateApiClient),
 	}
 	pubsubServer := pubsub.NewServer()
-	pubsubServer.Start()
+	err = pubsubServer.Start()
 	assert.NoError(err)
 	// Start the DALC
 	dalc := avail.DataAvailabilityLayerClient{}

da/celestia/celestia_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"math/rand"
+	cryptoRand "crypto/rand"
 	"testing"
 	"time"
 
@@ -294,9 +295,14 @@ func TestRetrievalWrongCommitment(t *testing.T) {
 }
 
 func setDAandMock(t *testing.T) (*mocks.CelestiaRPCClient, da.DataAvailabilityLayerClient, []byte, *header.ExtendedHeader) {
+	var err error
 	pubsubServer := pubsub.NewServer()
-	pubsubServer.Start()
-	defer pubsubServer.Stop()
+	err = pubsubServer.Start()
+	require.NoError(t, err)
+	defer func() {
+		err = pubsubServer.Stop()
+		require.NoError(t, err)
+	}()
 
 	require := require.New(t)
 
@@ -309,7 +315,7 @@ func setDAandMock(t *testing.T) (*mocks.CelestiaRPCClient, da.DataAvailabilityLa
 		GasLimit: 3000000,
 		Fee:      200000000,
 	}
-	err := config.InitNamespaceID()
+	err = config.InitNamespaceID()
 	require.NoError(err)
 	conf, err := json.Marshal(config)
 	require.NoError(err)
@@ -392,6 +398,6 @@ func getRandomTx() types.Tx {
 
 func getRandomBytes(n int) []byte {
 	data := make([]byte, n)
-	_, _ = rand.Read(data)
+	_, _ = cryptoRand.Read(data)
 	return data
 }

da/celestia/rpc_test.go

@@ -63,6 +63,7 @@ func TestSubmitBatch(t *testing.T) {
 	// build a proof for an NID that is within the namespace range of the tree
 	nID := []byte{1}
 	proof, err := tree.ProveNamespace(nID)
+	require.NoError(err)
 	blobProof := blob.Proof([]*nmt.Proof{&proof})
 
 	cases := []struct {
@@ -110,7 +111,8 @@ func TestSubmitBatch(t *testing.T) {
 		}
 		// Subscribe to the health status event
 		pubsubServer := pubsub.NewServer()
-		pubsubServer.Start()
+		err = pubsubServer.Start()
+		require.NoError(err, tc.name)
 		HealthSubscription, err := pubsubServer.Subscribe(context.Background(), "testSubmitBatch", da.EventQueryDAHealthStatus)
 		assert.NoError(err, tc.name)
 		// Start the DALC