tapdb: check and error on dirty database version

Add a check during startup to ensure that the database version is not
dirty (i.e., the last migration did not complete successfully). If it is
dirty, the application will exit with an error.

This is done to resolve a bug that would occur when the **latest**
migration failed. In that scenario, the database version would correctly
be marked as dirty and exit with an error. However, under `sqlite`
database backends, when `tapd` was restarted, the dirty database version
would never be checked and `tapd` would start up normally, despite the
database being in a dirty state.
This is due to the check here: https://github.com/lightninglabs/taproot-assets/blob/0ddb08670f59e41e7edb2925a49df18d2124598e/tapdb/sqlite.go#L206-L213
In that scenario, since no migration is needed the migrate library is
never called to perform any potential migration. And since the dirty
check is normally done in the migrate library during a migration, it is
never performed in that scenario.

Author: ViktorT-11

tapdb/migrations.go

@@ -166,12 +166,20 @@ func applyMigrations(fs fs.FS, driver database.Driver, path, dbName string,
 		return err
 	}
 
-	migrationVersion, _, err := sqlMigrate.Version()
+	migrationVersion, dirty, err := sqlMigrate.Version()
 	if err != nil && !errors.Is(err, migrate.ErrNilVersion) {
 		return fmt.Errorf("unable to determine current migration "+
 			"version: %w", err)
 	}
 
+	// If the migration version is dirty, we should not proceed with further
+	// migrations, as this indicates that a previous migration did not
+	// complete successfully and requires manual intervention.
+	if dirty {
+		return fmt.Errorf("database is in a dirty state at version "+
+			"%v, manual intervention required", migrationVersion)
+	}
+
 	// As the down migrations may end up *dropping* data, we want to
 	// prevent that without explicit accounting.
 	latestVersion := opts.latestVersion.UnwrapOr(LatestMigrationVersion)

tapdb/migrations_test.go

@@ -3,6 +3,7 @@ package tapdb
 import (
 	"context"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -694,3 +695,80 @@ func TestMigration37(t *testing.T) {
 
 	require.Len(t, burns, 5)
 }
+
+// TestDirtySqliteVersion tests that if a migration fails and leaves an Sqlite
+// database backend in a dirty state, any attempts of re-executing migrations on
+// the db (i.e. restart tapd), will fail with an error indicating that the
+// database is in a dirty state. This is regardless of whether the failing
+// migration is the latest migration or an intermediate migration.
+func TestDirtySqliteVersion(t *testing.T) {
+	var (
+		err       error
+		testError = errors.New("test error")
+
+		// testPostMigrationChecks1 is a map that will trigger a
+		// migration callback for migration 2 which always returns an
+		// error. This is used to simulate an intermediate migration
+		// that fails and leaves the db in a dirty state.
+		testPostMigrationChecks1 = map[uint]postMigrationCheck{
+			2: func(ctx context.Context, q sqlc.Querier) error {
+				return testError
+			},
+		}
+
+		// testPostMigrationChecks2 is a map that will trigger a
+		// migration callback for the latest migration which always
+		// returns an error. This is used to simulate that the latest
+		// migration fails and leaves the db in a dirty state.
+		testPostMigrationChecks2 = map[uint]postMigrationCheck{
+			LatestMigrationVersion: func(ctx context.Context,
+				q sqlc.Querier) error {
+
+				return testError
+			},
+		}
+	)
+
+	// First, we'll test that intermediate migration that fails and leaves
+	// the db in a dirty state is detected on subsequent migration attempts.
+	// Note that this test only targets Sqlite database backends, and
+	// therefore we create a new Sqlite test db for this.
+	db1 := NewTestSqliteDBWithVersion(t, 1)
+
+	// As intend that the failing migration version should be an
+	// intermediate migration, we use the testPostMigrationChecks1 when
+	// executing the migration. Note that we use the `backupAndMigrate` func
+	// as the MigrationTarget, to simulate what is used in production for
+	// Sqlite database backends.
+	err = db1.ExecuteMigrations(db1.backupAndMigrate, WithPostStepCallbacks(
+		makePostStepCallbacks(db1, testPostMigrationChecks1),
+	))
+	require.ErrorIs(t, err, testError)
+
+	// If we now attempt to execute migrations again, it should fail with an
+	// error indicating that the db is in a dirty state.
+	err = db1.ExecuteMigrations(db1.backupAndMigrate, WithPostStepCallbacks(
+		makePostStepCallbacks(db1, testPostMigrationChecks1),
+	))
+	require.ErrorContains(t, err, "database is in a dirty state")
+
+	// Next, we'll test that if the **latest** migration fails and leaves
+	// the db in a dirty state, that this is also detected on subsequent
+	// migration attempts.
+	db2 := NewTestSqliteDBWithVersion(t, 1)
+
+	// As we intend that the failing migration version should be the latest
+	// migration, we now use the testPostMigrationChecks2 when executing
+	// the migration.
+	err = db2.ExecuteMigrations(db2.backupAndMigrate, WithPostStepCallbacks(
+		makePostStepCallbacks(db2, testPostMigrationChecks2),
+	))
+	require.ErrorIs(t, err, testError)
+
+	// If we now attempt to execute migrations again, it should fail with an
+	// error indicating that the db is in a dirty state.
+	err = db2.ExecuteMigrations(db2.backupAndMigrate, WithPostStepCallbacks(
+		makePostStepCallbacks(db2, testPostMigrationChecks2),
+	))
+	require.ErrorContains(t, err, "database is in a dirty state")
+}