ci: stronger and more explicit linter options (#267)

* stronger and more explicit linter options
* exclude mempool and state/indexer from golangci-lint

Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>

Author: tzdybal

.golangci.yml

@@ -1,9 +1,31 @@
 run:
   timeout: 5m
   modules-download-mode: readonly
+  # mempool and indexer code is borrowed from Tendermint
+  skip-dirs:
+    - mempool
+    - state/indexer
+    - state/txindex
 
 linters:
   enable:
     - gofmt
     - goimports
     - misspell
+    - revive
+    - deadcode
+    - errcheck
+    - gofmt
+    - goimports
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unused
+    - varcheck
+
+issues:
+  exclude-use-default: false

block/manager.go

@@ -80,6 +80,7 @@ func getInitialState(store store.Store, genesis *tmtypes.GenesisDoc) (types.Stat
 	return s, err
 }
 
+// NewManager creates new block Manager.
 func NewManager(
 	proposerKey crypto.PrivKey,
 	conf config.BlockManagerConfig,
@@ -153,11 +154,13 @@ func getAddress(key crypto.PrivKey) ([]byte, error) {
 	return tmcrypto.AddressHash(rawKey), nil
 }
 
+// SetDALC is used to set DataAvailabilityLayerClient used by Manager.
 func (m *Manager) SetDALC(dalc da.DataAvailabilityLayerClient) {
 	m.dalc = dalc
 	m.retriever = dalc.(da.BlockRetriever)
 }
 
+// AggregationLoop is responsible for aggregating transactions into rollup-blocks.
 func (m *Manager) AggregationLoop(ctx context.Context) {
 	timer := time.NewTimer(0)
 	for {
@@ -175,6 +178,10 @@ func (m *Manager) AggregationLoop(ctx context.Context) {
 	}
 }
 
+// SyncLoop is responsible for syncing blocks.
+//
+// SyncLoop processes headers gossiped in P2p network to know what's the latest block height,
+// block data is retrieved from DA layer.
 func (m *Manager) SyncLoop(ctx context.Context) {
 	daTicker := time.NewTicker(m.conf.DABlockTime)
 	for {

block/manager_test.go

@@ -87,7 +87,7 @@ func TestInitialState(t *testing.T) {
 }
 
 func getMockDALC(logger log.Logger) da.DataAvailabilityLayerClient {
-	dalc := &mockda.MockDataAvailabilityLayerClient{}
+	dalc := &mockda.DataAvailabilityLayerClient{}
 	_ = dalc.Init(nil, nil, logger)
 	_ = dalc.Start()
 	return dalc

config/config.go

@@ -43,6 +43,9 @@ type BlockManagerConfig struct {
 	NamespaceID   [8]byte `mapstructure:"namespace_id"`
 }
 
+// GetViperConfig reads configuration parameters from Viper instance.
+//
+// This method is called in cosmos-sdk.
 func (nc *NodeConfig) GetViperConfig(v *viper.Viper) error {
 	nc.Aggregator = v.GetBool(flagAggregator)
 	nc.DALayer = v.GetString(flagDALayer)
@@ -59,6 +62,9 @@ func (nc *NodeConfig) GetViperConfig(v *viper.Viper) error {
 	return nil
 }
 
+// AddFlags adds Optimint specific configuration options to cobra Command.
+//
+// This function is called in cosmos-sdk.
 func AddFlags(cmd *cobra.Command) {
 	def := DefaultNodeConfig
 	cmd.Flags().Bool(flagAggregator, def.Aggregator, "run node in aggregator mode")

config/rpc.go

@@ -1,5 +1,6 @@
 package config
 
+// RPCConfig holds RPC configuration params.
 type RPCConfig struct {
 	ListenAddress string
 

conv/abci/block.go

@@ -102,10 +102,10 @@ func ToABCIBlockMeta(block *types.Block) (*tmtypes.BlockMeta, error) {
 	if err != nil {
 		return nil, err
 	}
-	blockId := tmtypes.BlockID{Hash: tmblock.Hash()}
+	blockID := tmtypes.BlockID{Hash: tmblock.Hash()}
 
 	return &tmtypes.BlockMeta{
-		BlockID:   blockId,
+		BlockID:   blockID,
 		BlockSize: tmblock.Size(),
 		Header:    tmblock.Header,
 		NumTxs:    len(tmblock.Txs),

da/celestia/celestia.go

@@ -27,13 +27,15 @@ type DataAvailabilityLayerClient struct {
 var _ da.DataAvailabilityLayerClient = &DataAvailabilityLayerClient{}
 var _ da.BlockRetriever = &DataAvailabilityLayerClient{}
 
+// Config stores Celestia DALC configuration parameters.
 type Config struct {
 	BaseURL     string        `json:"base_url"`
 	Timeout     time.Duration `json:"timeout"`
 	GasLimit    uint64        `json:"gas_limit"`
 	NamespaceID [8]byte       `json:"namespace_id"`
 }
 
+// Init initializes DataAvailabilityLayerClient instance.
 func (c *DataAvailabilityLayerClient) Init(config []byte, kvStore store.KVStore, logger log.Logger) error {
 	c.logger = logger
 
@@ -44,23 +46,26 @@ func (c *DataAvailabilityLayerClient) Init(config []byte, kvStore store.KVStore,
 	return nil
 }
 
+// Start prepares DataAvailabilityLayerClient to work.
 func (c *DataAvailabilityLayerClient) Start() error {
 	c.logger.Info("starting Celestia Data Availability Layer Client", "baseURL", c.config.BaseURL)
 	var err error
 	c.client, err = cnc.NewClient(c.config.BaseURL, cnc.WithTimeout(c.config.Timeout))
 	return err
 }
 
+// Stop stops DataAvailabilityLayerClient.
 func (c *DataAvailabilityLayerClient) Stop() error {
 	c.logger.Info("stopping Celestia Data Availability Layer Client")
 	return nil
 }
 
+// SubmitBlock submits a block to DA layer.
 func (c *DataAvailabilityLayerClient) SubmitBlock(block *types.Block) da.ResultSubmitBlock {
 	blob, err := block.MarshalBinary()
 	if err != nil {
 		return da.ResultSubmitBlock{
-			DAResult: da.DAResult{
+			BaseResult: da.BaseResult{
 				Code:    da.StatusError,
 				Message: err.Error(),
 			},
@@ -71,7 +76,7 @@ func (c *DataAvailabilityLayerClient) SubmitBlock(block *types.Block) da.ResultS
 
 	if err != nil {
 		return da.ResultSubmitBlock{
-			DAResult: da.DAResult{
+			BaseResult: da.BaseResult{
 				Code:    da.StatusError,
 				Message: err.Error(),
 			},
@@ -80,47 +85,49 @@ func (c *DataAvailabilityLayerClient) SubmitBlock(block *types.Block) da.ResultS
 
 	if txResponse.Code != 0 {
 		return da.ResultSubmitBlock{
-			DAResult: da.DAResult{
+			BaseResult: da.BaseResult{
 				Code:    da.StatusError,
 				Message: fmt.Sprintf("Codespace: '%s', Code: %d, Message: %s", txResponse.Codespace, txResponse.Code, txResponse.RawLog),
 			},
 		}
 	}
 
 	return da.ResultSubmitBlock{
-		DAResult: da.DAResult{
+		BaseResult: da.BaseResult{
 			Code:     da.StatusSuccess,
 			Message:  "tx hash: " + txResponse.TxHash,
 			DAHeight: uint64(txResponse.Height),
 		},
 	}
 }
 
+// CheckBlockAvailability queries DA layer to check data availability of block at given height.
 func (c *DataAvailabilityLayerClient) CheckBlockAvailability(dataLayerHeight uint64) da.ResultCheckBlock {
 	shares, err := c.client.NamespacedShares(context.TODO(), c.config.NamespaceID, dataLayerHeight)
 	if err != nil {
 		return da.ResultCheckBlock{
-			DAResult: da.DAResult{
+			BaseResult: da.BaseResult{
 				Code:    da.StatusError,
 				Message: err.Error(),
 			},
 		}
 	}
 
 	return da.ResultCheckBlock{
-		DAResult: da.DAResult{
+		BaseResult: da.BaseResult{
 			Code:     da.StatusSuccess,
 			DAHeight: dataLayerHeight,
 		},
 		DataAvailable: len(shares) > 0,
 	}
 }
 
+// RetrieveBlocks gets a batch of blocks from DA layer.
 func (c *DataAvailabilityLayerClient) RetrieveBlocks(dataLayerHeight uint64) da.ResultRetrieveBlocks {
 	data, err := c.client.NamespacedData(context.TODO(), c.config.NamespaceID, dataLayerHeight)
 	if err != nil {
 		return da.ResultRetrieveBlocks{
-			DAResult: da.DAResult{
+			BaseResult: da.BaseResult{
 				Code:    da.StatusError,
 				Message: err.Error(),
 			},
@@ -139,7 +146,7 @@ func (c *DataAvailabilityLayerClient) RetrieveBlocks(dataLayerHeight uint64) da.
 		err := blocks[i].FromProto(&block)
 		if err != nil {
 			return da.ResultRetrieveBlocks{
-				DAResult: da.DAResult{
+				BaseResult: da.BaseResult{
 					Code:    da.StatusError,
 					Message: err.Error(),
 				},
@@ -148,7 +155,7 @@ func (c *DataAvailabilityLayerClient) RetrieveBlocks(dataLayerHeight uint64) da.
 	}
 
 	return da.ResultRetrieveBlocks{
-		DAResult: da.DAResult{
+		BaseResult: da.BaseResult{
 			Code:     da.StatusSuccess,
 			DAHeight: dataLayerHeight,
 		},

da/celestia/mock/messages.go

@@ -9,30 +9,30 @@ import (
 // TODO(tzdybal): if we stop using `/namespaced_shares` we can get rid of this file.
 
 const (
-	ShareSize     = 256
-	NamespaceSize = 8
-	MsgShareSize  = ShareSize - NamespaceSize
+	shareSize     = 256
+	namespaceSize = 8
+	msgShareSize  = shareSize - namespaceSize
 )
 
 // splitMessage breaks the data in a message into the minimum number of
 // namespaced shares
 func splitMessage(rawData []byte, nid []byte) []NamespacedShare {
 	shares := make([]NamespacedShare, 0)
 	firstRawShare := append(append(
-		make([]byte, 0, ShareSize),
+		make([]byte, 0, shareSize),
 		nid...),
-		rawData[:MsgShareSize]...,
+		rawData[:msgShareSize]...,
 	)
 	shares = append(shares, NamespacedShare{firstRawShare, nid})
-	rawData = rawData[MsgShareSize:]
+	rawData = rawData[msgShareSize:]
 	for len(rawData) > 0 {
-		shareSizeOrLen := min(MsgShareSize, len(rawData))
+		shareSizeOrLen := min(msgShareSize, len(rawData))
 		rawShare := append(append(
-			make([]byte, 0, ShareSize),
+			make([]byte, 0, shareSize),
 			nid...),
 			rawData[:shareSizeOrLen]...,
 		)
-		paddedShare := zeroPadIfNecessary(rawShare, ShareSize)
+		paddedShare := zeroPadIfNecessary(rawShare, shareSize)
 		share := NamespacedShare{paddedShare, nid}
 		shares = append(shares, share)
 		rawData = rawData[shareSizeOrLen:]
@@ -80,16 +80,16 @@ func marshalDelimited(data []byte) ([]byte, error) {
 // appendToShares appends raw data as shares.
 // Used to build shares from blocks/messages.
 func appendToShares(shares []NamespacedShare, nid []byte, rawData []byte) []NamespacedShare {
-	if len(rawData) <= MsgShareSize {
+	if len(rawData) <= msgShareSize {
 		rawShare := append(append(
 			make([]byte, 0, len(nid)+len(rawData)),
 			nid...),
 			rawData...,
 		)
-		paddedShare := zeroPadIfNecessary(rawShare, ShareSize)
+		paddedShare := zeroPadIfNecessary(rawShare, shareSize)
 		share := NamespacedShare{paddedShare, nid}
 		shares = append(shares, share)
-	} else { // len(rawData) > MsgShareSize
+	} else { // len(rawData) > msgShareSize
 		shares = append(shares, splitMessage(rawData, nid)...)
 	}
 	return shares

da/celestia/mock/server.go

@@ -20,21 +20,24 @@ import (
 	"github.com/celestiaorg/optimint/types"
 )
 
+// Server mocks celestia-node HTTP API.
 type Server struct {
-	mock      *mockda.MockDataAvailabilityLayerClient
+	mock      *mockda.DataAvailabilityLayerClient
 	blockTime time.Duration
 	server    *http.Server
 	logger    log.Logger
 }
 
+// NewServer creates new instance of Server.
 func NewServer(blockTime time.Duration, logger log.Logger) *Server {
 	return &Server{
-		mock:      new(mockda.MockDataAvailabilityLayerClient),
+		mock:      new(mockda.DataAvailabilityLayerClient),
 		blockTime: blockTime,
 		logger:    logger,
 	}
 }
 
+// Start starts HTTP server with given listener.
 func (s *Server) Start(listener net.Listener) error {
 	err := s.mock.Init([]byte(s.blockTime.String()), store.NewDefaultInMemoryKVStore(), s.logger)
 	if err != nil {
@@ -53,6 +56,7 @@ func (s *Server) Start(listener net.Listener) error {
 	return nil
 }
 
+// Stop shuts down the Server.
 func (s *Server) Stop() {
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
 	defer cancel()

da/da.go

@@ -20,7 +20,8 @@ const (
 	StatusError
 )
 
-type DAResult struct {
+// BaseResult contains basic information returned by DA layer.
+type BaseResult struct {
 	// Code is to determine if the action succeeded.
 	Code StatusCode
 	// Message may contain DA layer specific information (like DA block height/hash, detailed error message, etc)
@@ -31,22 +32,23 @@ type DAResult struct {
 
 // ResultSubmitBlock contains information returned from DA layer after block submission.
 type ResultSubmitBlock struct {
-	DAResult
+	BaseResult
 	// Not sure if this needs to be bubbled up to other
 	// parts of Optimint.
 	// Hash hash.Hash
 }
 
 // ResultCheckBlock contains information about block availability, returned from DA layer client.
 type ResultCheckBlock struct {
-	DAResult
+	BaseResult
 	// DataAvailable is the actual answer whether the block is available or not.
 	// It can be true if and only if Code is equal to StatusSuccess.
 	DataAvailable bool
 }
 
+// ResultRetrieveBlocks contains batch of blocks returned from DA layer client.
 type ResultRetrieveBlocks struct {
-	DAResult
+	BaseResult
 	// Block is the full block retrieved from Data Availability Layer.
 	// If Code is not equal to StatusSuccess, it has to be nil.
 	Blocks []*types.Block
@@ -58,15 +60,18 @@ type DataAvailabilityLayerClient interface {
 	// Init is called once to allow DA client to read configuration and initialize resources.
 	Init(config []byte, kvStore store.KVStore, logger log.Logger) error
 
+	// Start is called once, after Init. It's implementation should start operation of DataAvailabilityLayerClient.
 	Start() error
+
+	// Stop is called once, when DataAvailabilityLayerClient is no longer needed.
 	Stop() error
 
 	// SubmitBlock submits the passed in block to the DA layer.
 	// This should create a transaction which (potentially)
 	// triggers a state transition in the DA layer.
 	SubmitBlock(block *types.Block) ResultSubmitBlock
 
-	// CheckBlockAvailability queries DA layer to check data availability of block corresponding to given header.
+	// CheckBlockAvailability queries DA layer to check data availability of block corresponding at given height.
 	CheckBlockAvailability(dataLayerHeight uint64) ResultCheckBlock
 }