refactor(db, parser, ci): post-review improvements

.github/workflows/ci.yaml

@@ -6,22 +6,78 @@ on:
   pull_request:
     branches: [ main ]
 
+permissions:
+  contents: read
+
 jobs:
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    permissions:
+      contents: read
+      checks: write  # Required by golangci-lint-action v7+ for inline PR annotations
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4.2.2
+
+      - name: Set up Go
+        uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34  # v5.3.0
+        with:
+          go-version: '1.24'
+
+      - name: Run golangci-lint
+        uses: golangci/golangci-lint-action@1e7e51e771db61008b38414a730f564565cf7c20  # v9.2.0
+        with:
+          version: v2.10.1
+
+  test:
+    name: Test
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    services:
+      postgres:
+        image: postgres:16.9-alpine3.21
+        env:
+          POSTGRES_USER: yugabyte
+          POSTGRES_PASSWORD: yugabyte
+        ports:
+          - 5433:5432
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 5s
+          --health-timeout 5s
+          --health-retries 10
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4.2.2
+
+      - name: Set up Go
+        uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34  # v5.3.0
+        with:
+          go-version: '1.24'
+
+      - name: Run tests
+        run: go test -count=1 ./pkg/...
+
   check-sqlc-generation:
     name: Validate SQLC Generated Code
     runs-on: ubuntu-latest
+    timeout-minutes: 15
     steps:
       - name: Checkout code
-        uses: actions/checkout@v4
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4.2.2
 
       - name: Set up Go
-        uses: actions/setup-go@v5
+        uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34  # v5.3.0
         with:
           go-version: '1.24'
 
       - name: Install sqlc
         run: |
-          curl -L https://github.com/sqlc-dev/sqlc/releases/download/v1.30.0/sqlc_1.30.0_linux_amd64.tar.gz | tar xz
+          curl -sSfL -o sqlc.tar.gz https://github.com/sqlc-dev/sqlc/releases/download/v1.30.0/sqlc_1.30.0_linux_amd64.tar.gz
+          echo "468aecee071bfe55e97fcbcac52ea0208eeca444f67736f3b8f0f3d6a106132e  sqlc.tar.gz" | sha256sum --check
+          tar xzf sqlc.tar.gz
           sudo mv sqlc /usr/local/bin/
           sqlc version
 

.golangci.yml

@@ -45,6 +45,8 @@ linters:
           deny:
             - pkg: github.com/pkg/errors
               desc: github.com/pkg/errors is no longer maintained
+            - pkg: github.com/hyperledger/fabric/common/flogging
+              desc: use github.com/hyperledger/fabric-x-committer/utils/logging instead
     errcheck:
       check-type-assertions: true
     errorlint:
@@ -130,11 +132,11 @@ linters:
         Copyright IBM Corp. All Rights Reserved.
 
         SPDX-License-Identifier: Apache-2.0
-exclusions:
-  generated: lax
-  warn-unused: true
-  presets:
-    - common-false-positives
+  exclusions:
+    generated: lax
+    warn-unused: true
+    presets:
+      - common-false-positives
 formatters:
   enable:
     - gofumpt
@@ -147,7 +149,8 @@ formatters:
         - github.com/LF-Decentralized-Trust-labs/fabric-x-block-explorer
   exclusions:
     generated: lax
-sort-order:
-  - file
-  - severity
-  - linter
+output:
+  sort-order:
+    - file
+    - severity
+    - linter

Makefile

@@ -1,7 +1,10 @@
 # Copyright IBM Corp. All Rights Reserved.
 # SPDX-License-Identifier: Apache-2.0
 
-.PHONY: sqlc lint test test-no-db test-requires-db test-all coverage clean help
+.PHONY: sqlc lint test test-no-db test-requires-db test-all start-db stop-db coverage clean help
+
+DB_CONTAINER_NAME := sc_postgres_unit_tests
+DB_PORT           := 5433
 
 sqlc: ## Generate Go code from SQL using sqlc
 	@echo "Generating Go code from SQL files..."
@@ -20,20 +23,34 @@ test-no-db: ## Run tests that do not require a database (parser, types, util)
 		./pkg/types/... \
 		./pkg/util/...
 
-test-requires-db: ## Run DB tests using Docker (requires Docker)
-	@echo "Running tests with Docker..."
-	DB_TYPE=postgres go test -v -count=1 ./pkg/db/...
+start-db: ## Start a local PostgreSQL container for DB tests
+	@docker ps -aq -f name=$(DB_CONTAINER_NAME) | xargs -r docker rm -f
+	docker run --name $(DB_CONTAINER_NAME) \
+		-e POSTGRES_PASSWORD=yugabyte \
+		-e POSTGRES_USER=yugabyte \
+		-p $(DB_PORT):5432 \
+		-d postgres:16.9-alpine3.21
+	@echo "Waiting for Postgres to be ready..."
+	@until docker exec $(DB_CONTAINER_NAME) pg_isready -U yugabyte -q; do sleep 1; done
+	@echo "✅ Postgres is ready on localhost:$(DB_PORT)"
+
+stop-db: ## Stop and remove the local PostgreSQL container
+	docker ps -aq -f name=$(DB_CONTAINER_NAME) | xargs -r docker rm -f
+
+test-requires-db: ## Run DB tests (requires running Postgres: make start-db)
+	@echo "Running tests with database..."
+	DB_DEPLOYMENT=local go test -v -count=1 ./pkg/db/...
 
-test-all: ## Run all tests
+test-all: ## Run all tests (requires running Postgres: make start-db)
 	@echo "Running all tests..."
-	go test -v -count=1 ./pkg/...
+	DB_DEPLOYMENT=local go test -v -count=1 ./pkg/...
 
 test: test-all ## Alias for test-all
 
-coverage: ## Generate test coverage report
+coverage: ## Generate test coverage report (requires running Postgres: make start-db)
 	@echo "Generating coverage report..."
 	@mkdir -p coverage
-	go test -coverprofile=coverage/coverage.out ./pkg/...
+	DB_DEPLOYMENT=local go test -coverprofile=coverage/coverage.out ./pkg/...
 	go tool cover -html=coverage/coverage.out -o coverage/coverage.html
 	go tool cover -func=coverage/coverage.out
 	@echo ""

go.mod

@@ -4,7 +4,6 @@ go 1.24.3
 
 require (
 	github.com/cockroachdb/errors v1.12.0
-	github.com/hyperledger/fabric v2.1.1+incompatible
 	github.com/hyperledger/fabric-protos-go-apiv2 v0.3.7
 	github.com/hyperledger/fabric-x-committer v0.1.7
 	github.com/jackc/pgx/v5 v5.8.0

go.sum

@@ -93,8 +93,6 @@ github.com/hyperledger-labs/SmartBFT v0.0.0-20250503203013-eb005eef8866 h1:Mu/6N
 github.com/hyperledger-labs/SmartBFT v0.0.0-20250503203013-eb005eef8866/go.mod h1:9aNHNXsCVy/leGz2gpTC1eOL5QecxbSAGjqsLh4T1LM=
 github.com/hyperledger/aries-bbs-go v0.0.0-20240528084656-761671ea73bc h1:3Ykk6MtyfnlzMOQry9zkxsoLWpCWZwDPqehO/BJwArM=
 github.com/hyperledger/aries-bbs-go v0.0.0-20240528084656-761671ea73bc/go.mod h1:Kofn6A6WWea1ZM8Rys5aBW9dszwJ7Ywa0kyyYL0TPYw=
-github.com/hyperledger/fabric v2.1.1+incompatible h1:cYYRv3vVg4kA6DmrixLxwn1nwBEUuYda8DsMwlaMKbY=
-github.com/hyperledger/fabric v2.1.1+incompatible/go.mod h1:tGFAOCT696D3rG0Vofd2dyWYLySHlh0aQjf7Q1HAju0=
 github.com/hyperledger/fabric-amcl v0.0.0-20230602173724-9e02669dceb2 h1:B1Nt8hKb//KvgGRprk0h1t4lCnwhE9/ryb1WqfZbV+M=
 github.com/hyperledger/fabric-amcl v0.0.0-20230602173724-9e02669dceb2/go.mod h1:X+DIyUsaTmalOpmpQfIvFZjKHQedrURQ5t4YqquX7lE=
 github.com/hyperledger/fabric-config v0.3.0 h1:FS5/dc9GAniljP6RYxQRG92AaiBVoN2vTvtOvnWqeQs=

pkg/db/db_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// TestDatabaseTestEnv verifies that the test infrastructure works correctly.
 func TestDatabaseTestEnv(t *testing.T) {
 	t.Parallel()
 	env := NewDatabaseTestEnv(t)
@@ -34,7 +33,6 @@ func TestDatabaseTestEnv(t *testing.T) {
 	assert.True(t, tableExists, "blocks table should exist")
 }
 
-// TestNewPostgres verifies the NewPostgres function creates a valid connection pool.
 func TestNewPostgres(t *testing.T) {
 	t.Parallel()
 
@@ -54,7 +52,6 @@ func TestNewPostgres(t *testing.T) {
 	defer pool.Close()
 }
 
-// TestDatabaseHelpers verifies helper methods in DatabaseTestEnv.
 func TestDatabaseHelpers(t *testing.T) {
 	t.Parallel()
 	env := NewDatabaseTestEnv(t)

pkg/db/db_writer.go

@@ -11,7 +11,7 @@ import (
 	"encoding/hex"
 
 	"github.com/cockroachdb/errors"
-	"github.com/hyperledger/fabric/common/flogging"
+	"github.com/hyperledger/fabric-x-committer/utils/logging"
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgxpool"
 
@@ -20,7 +20,7 @@ import (
 	"github.com/LF-Decentralized-Trust-labs/fabric-x-block-explorer/pkg/util"
 )
 
-var logger = flogging.MustGetLogger("db")
+var logger = logging.New("db")
 
 // BlockWriter writes processed blocks to the database.
 type BlockWriter struct {
@@ -44,63 +44,69 @@ func (bw *BlockWriter) WriteProcessedBlock(ctx context.Context, pb *types.Proces
 	if pb == nil {
 		return errors.New("processed block is nil")
 	}
-
-	parsedData, ok := pb.Data.(*types.ParsedBlockData)
-	if !ok {
-		return errors.New("processed block Data is not *types.ParsedBlockData")
+	if pb.BlockInfo == nil {
+		return errors.New("processed block has nil BlockInfo")
+	}
+	if pb.Data == nil {
+		return errors.New("processed block has nil Data")
 	}
 
-	tx, err := bw.beginTx(ctx)
+	p, err := buildBatchParams(pb.BlockInfo.Number, pb.Data)
 	if err != nil {
 		return err
 	}
 
-	committed := false
-	defer func() {
-		if !committed {
-			_ = tx.Rollback(ctx)
-		}
-	}()
+	tx, rollbackFn, err := bw.beginTx(ctx)
+	if err != nil {
+		return err
+	}
+	defer rollbackFn()
 
 	q := dbsqlc.New(tx)
 
 	if err = q.InsertBlock(ctx, dbsqlc.InsertBlockParams{
-		BlockNum:     int64(pb.BlockInfo.Number), //nolint:gosec // block numbers fit in int64
-		TxCount:      int32(pb.Txns),             //nolint:gosec // tx count fits in int32
+		BlockNum:     int64(pb.BlockInfo.Number),       //nolint:gosec // block numbers fit in int64
+		TxCount:      int32(len(pb.Data.Transactions)), //nolint:gosec // tx count fits in int32
 		PreviousHash: pb.BlockInfo.PreviousHash,
 		DataHash:     pb.BlockInfo.DataHash,
 	}); err != nil {
 		return err
 	}
 
-	p, err := buildBatchParams(parsedData)
-	if err != nil {
-		return err
-	}
-
 	if err := p.flush(ctx, q); err != nil {
 		return err
 	}
 
 	if err := tx.Commit(ctx); err != nil {
 		return err
 	}
-	committed = true
 
-	logger.Debugf("stored block %d with %d transactions", pb.BlockInfo.Number, len(parsedData.Transactions))
+	logger.Debugf("stored block %d with %d transactions", pb.BlockInfo.Number, len(pb.Data.Transactions))
 	return nil
 }
 
-// beginTx starts a new database transaction using the available connection or pool.
-func (bw *BlockWriter) beginTx(ctx context.Context) (pgx.Tx, error) {
+// beginTx starts a new database transaction and returns the transaction and a rollback function.
+func (bw *BlockWriter) beginTx(ctx context.Context) (pgx.Tx, func(), error) {
+	var tx pgx.Tx
+	var err error
 	switch {
 	case bw.conn != nil:
-		return bw.conn.Begin(ctx)
+		tx, err = bw.conn.Begin(ctx)
 	case bw.pool != nil:
-		return bw.pool.Begin(ctx)
+		tx, err = bw.pool.Begin(ctx)
 	default:
-		return nil, errors.New("no pool or conn available in BlockWriter")
+		return nil, func() {}, errors.New("no pool or conn available in BlockWriter")
+	}
+	if err != nil {
+		return nil, func() {}, errors.Wrap(err, "failed to begin database transaction")
+	}
+
+	rollback := func() { //nolint:contextcheck // rollback must work even when ctx is cancelled
+		if rbErr := tx.Rollback(context.Background()); rbErr != nil && !errors.Is(rbErr, pgx.ErrTxClosed) {
+			logger.Warn("failed to rollback transaction: ", rbErr)
+		}
 	}
+	return tx, rollback, nil
 }
 
 // batchParams holds all the parameter slices for a block's batch inserts.
@@ -115,12 +121,18 @@ type batchParams struct {
 }
 
 // buildBatchParams flattens all parsed block data into per-table param slices.
-func buildBatchParams(data *types.ParsedBlockData) (*batchParams, error) {
+func buildBatchParams(blockNum uint64, data *types.ParsedBlockData) (*batchParams, error) {
 	p := &batchParams{
-		txParams: make([]dbsqlc.InsertTransactionParams, 0, len(data.Transactions)),
+		txParams:         make([]dbsqlc.InsertTransactionParams, 0, len(data.Transactions)),
+		nsParams:         make([]dbsqlc.InsertTxNamespaceParams, 0, len(data.Transactions)),
+		readOnlyParams:   make([]dbsqlc.InsertReadOnlyParams, 0, len(data.Transactions)),
+		readWriteParams:  make([]dbsqlc.InsertReadWriteParams, 0, len(data.Transactions)),
+		blindWriteParams: make([]dbsqlc.InsertBlindWriteParams, 0, len(data.Transactions)),
+		endorseParams:    make([]dbsqlc.InsertTxEndorsementParams, 0, len(data.Transactions)),
+		policyParams:     make([]dbsqlc.UpsertNamespacePolicyParams, 0, len(data.Policies)),
 	}
 	for _, txRec := range data.Transactions {
-		if err := p.appendTx(txRec); err != nil {
+		if err := p.appendTx(blockNum, txRec); err != nil {
 			return nil, err
 		}
 	}
@@ -137,25 +149,23 @@ func buildBatchParams(data *types.ParsedBlockData) (*batchParams, error) {
 	return p, nil
 }
 
-// appendTx adds a transaction and all its namespace data to the param slices.
-func (p *batchParams) appendTx(txRec types.TxRecord) error {
+func (p *batchParams) appendTx(blockNum uint64, txRec types.TxRecord) error {
 	txIDBytes, err := hex.DecodeString(txRec.TxID)
 	if err != nil {
 		return errors.Wrapf(err, "failed to decode tx_id %s", txRec.TxID)
 	}
 	p.txParams = append(p.txParams, dbsqlc.InsertTransactionParams{
-		BlockNum:       int64(txRec.BlockNum), //nolint:gosec // fits in int64
-		TxNum:          int64(txRec.TxNum),    //nolint:gosec // fits in int64
+		BlockNum:       int64(blockNum),    //nolint:gosec // fits in int64
+		TxNum:          int64(txRec.TxNum), //nolint:gosec // fits in int64
 		TxID:           txIDBytes,
-		ValidationCode: int64(txRec.ValidationCode),
+		ValidationCode: int16(txRec.ValidationCode), //nolint:gosec // max status value is 115, fits in int16
 	})
 	for _, ns := range txRec.Namespaces {
-		p.appendNamespace(txRec.BlockNum, txRec.TxNum, ns)
+		p.appendNamespace(blockNum, txRec.TxNum, ns)
 	}
 	return nil
 }
 
-// appendNamespace adds a single (tx, namespace) pair to the param slices.
 func (p *batchParams) appendNamespace(blockNum, txNum uint64, ns types.TxNamespaceRecord) {
 	bn := int64(blockNum) //nolint:gosec // fits in int64
 	tn := int64(txNum)    //nolint:gosec // fits in int64