sweepbatcher: harden batch txid loading

Return errors from batch row conversion instead of silently
accepting malformed batch txids while appending nil batches.

Author: starius

sweepbatcher/store.go

@@ -11,6 +11,7 @@ import (
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightninglabs/loop/loopdb"
 	"github.com/lightninglabs/loop/loopdb/sqlc"
+	"github.com/lightninglabs/loop/utils/chainhashutil"
 	"github.com/lightningnetwork/lnd/lntypes"
 )
 
@@ -91,15 +92,15 @@ func (s *SQLStore) FetchUnconfirmedSweepBatches(ctx context.Context) (
 	}
 
 	for _, dbBatch := range dbBatches {
-		batch := convertBatchRow(dbBatch)
+		batch, err := convertBatchRow(dbBatch)
 		if err != nil {
 			return nil, err
 		}
 
 		batches = append(batches, batch)
 	}
 
-	return batches, err
+	return batches, nil
 }
 
 // InsertSweepBatch inserts a batch into the database, returning the id of the
@@ -198,7 +199,7 @@ func (s *SQLStore) GetParentBatch(ctx context.Context, outpoint wire.OutPoint) (
 		return nil, err
 	}
 
-	return convertBatchRow(batch), nil
+	return convertBatchRow(batch)
 }
 
 // UpsertSweep inserts a sweep into the database, or updates an existing sweep
@@ -258,17 +259,24 @@ type dbSweep struct {
 }
 
 // convertBatchRow converts a batch row from db to a sweepbatcher.Batch struct.
-func convertBatchRow(row sqlc.SweepBatch) *dbBatch {
+func convertBatchRow(row sqlc.SweepBatch) (*dbBatch, error) {
 	batch := dbBatch{
 		ID:        row.ID,
 		Confirmed: row.Confirmed,
 	}
 
-	if row.BatchTxID.Valid {
-		err := chainhash.Decode(&batch.BatchTxid, row.BatchTxID.String)
+	// Loop never writes empty batch txids, but tolerate them on read so a
+	// malformed row does not prevent batcher recovery.
+	if row.BatchTxID.Valid && row.BatchTxID.String != "" {
+		hash, err := chainhashutil.NewHashFromStrExact(
+			row.BatchTxID.String,
+		)
 		if err != nil {
-			return nil
+			return nil, fmt.Errorf("invalid batch txid %q: %w",
+				row.BatchTxID.String, err)
 		}
+
+		batch.BatchTxid = hash
 	}
 
 	batch.BatchPkScript = row.BatchPkScript
@@ -283,7 +291,7 @@ func convertBatchRow(row sqlc.SweepBatch) *dbBatch {
 
 	batch.MaxTimeoutDistance = row.MaxTimeoutDistance
 
-	return &batch
+	return &batch, nil
 }
 
 // batchToInsertArgs converts a Batch struct to the arguments needed to insert

sweepbatcher/store_test.go

@@ -0,0 +1,147 @@
+package sweepbatcher
+
+import (
+	"context"
+	"database/sql"
+	"strings"
+	"testing"
+
+	"github.com/btcsuite/btcd/chaincfg"
+	"github.com/btcsuite/btcd/wire"
+	"github.com/lightninglabs/loop/loopdb"
+	"github.com/lightninglabs/loop/loopdb/sqlc"
+	"github.com/stretchr/testify/require"
+)
+
+// TestFetchUnconfirmedSweepBatchesRejectsInvalidBatchTxID verifies that
+// malformed persisted batch txids are rejected during batch loading.
+func TestFetchUnconfirmedSweepBatchesRejectsInvalidBatchTxID(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name   string
+		txid   string
+		errMsg string
+	}{
+		{
+			name:   "short",
+			txid:   "abcd",
+			errMsg: "invalid batch txid",
+		},
+		{
+			name:   "non-hex",
+			txid:   strings.Repeat("z", 64),
+			errMsg: "invalid batch txid",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctxb := t.Context()
+			testDb := loopdb.NewTestDB(t)
+			defer testDb.Close()
+
+			store := NewSQLStore(
+				loopdb.NewTypedStore[Querier](testDb),
+				&chaincfg.RegressionNetParams,
+			)
+
+			_, err := testDb.Queries.InsertBatch(
+				ctxb, sqlc.InsertBatchParams{
+					BatchTxID: sql.NullString{
+						String: test.txid,
+						Valid:  true,
+					},
+					BatchPkScript: []byte{0x00},
+					LastRbfHeight: sql.NullInt32{
+						Int32: 1,
+						Valid: true,
+					},
+					LastRbfSatPerKw: sql.NullInt32{
+						Int32: 1000,
+						Valid: true,
+					},
+					MaxTimeoutDistance: 1,
+				},
+			)
+			require.NoError(t, err)
+
+			_, err = store.FetchUnconfirmedSweepBatches(ctxb)
+			require.ErrorContains(t, err, test.errMsg)
+		})
+	}
+}
+
+// TestFetchUnconfirmedSweepBatchesAllowsEmptyBatchTxID verifies that empty
+// persisted batch txids are tolerated for recovery robustness.
+func TestFetchUnconfirmedSweepBatchesAllowsEmptyBatchTxID(t *testing.T) {
+	t.Parallel()
+
+	ctxb := t.Context()
+	testDb := loopdb.NewTestDB(t)
+	defer testDb.Close()
+
+	store := NewSQLStore(
+		loopdb.NewTypedStore[Querier](testDb),
+		&chaincfg.RegressionNetParams,
+	)
+
+	_, err := testDb.Queries.InsertBatch(
+		ctxb, sqlc.InsertBatchParams{
+			BatchTxID: sql.NullString{
+				String: "",
+				Valid:  true,
+			},
+			BatchPkScript: []byte{0x00},
+			LastRbfHeight: sql.NullInt32{
+				Int32: 1,
+				Valid: true,
+			},
+			LastRbfSatPerKw: sql.NullInt32{
+				Int32: 1000,
+				Valid: true,
+			},
+			MaxTimeoutDistance: 1,
+		},
+	)
+	require.NoError(t, err)
+
+	_, err = store.FetchUnconfirmedSweepBatches(ctxb)
+	require.NoError(t, err)
+}
+
+// parentBatchDB is a test double that overrides GetParentBatch while reusing
+// the rest of the SQL store interface from an embedded BaseDB.
+type parentBatchDB struct {
+	BaseDB
+
+	batch sqlc.SweepBatch
+	err   error
+}
+
+// GetParentBatch returns the preconfigured batch row for store tests.
+func (s parentBatchDB) GetParentBatch(ctx context.Context,
+	outpoint string) (sqlc.SweepBatch, error) {
+
+	return s.batch, s.err
+}
+
+// TestGetParentBatchRejectsInvalidBatchTxID verifies that malformed persisted
+// batch txids are rejected through the GetParentBatch path as well.
+func TestGetParentBatchRejectsInvalidBatchTxID(t *testing.T) {
+	t.Parallel()
+
+	store := NewSQLStore(parentBatchDB{
+		batch: sqlc.SweepBatch{
+			BatchTxID: sql.NullString{
+				String: "abcd",
+				Valid:  true,
+			},
+		},
+	}, &chaincfg.RegressionNetParams)
+
+	_, err := store.GetParentBatch(t.Context(), wire.OutPoint{})
+	require.ErrorContains(t, err, "invalid batch txid")
+}