tapdb: error on dirty database versions

[ViktorT-11]

This PR fixes 2 issues in the current handling of database migrations in tapdb:

1. The main issue resolved is that we 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 now always exit with an error.
   This is done to resolve a bug that would occur when the migration with the latest migration version 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:

   taproot-assets/tapdb/sqlite.go

   Lines 206 to 213 in 0ddb086

   	versionUpgradePending := currentDbVersion < int(maxMigrationVersion)
   	if !versionUpgradePending {
   	log.Infof("Current database version is up-to-date, skipping "+
   	"migration attempt and backup creation "+
   	"(current_db_version=%v, max_migration_version=%v)",
   	currentDbVersion, maxMigrationVersion)
   	return nil
   	}

   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.

2. Secondly, we add checking of an error that was previously unchecked.

[gemini-code-assist]

Summary of Changes

Hello @ViktorT-11, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the reliability of database operations within tapdb by introducing a robust mechanism to detect and prevent the use of a database that is in a "dirty" state following an incomplete migration. By forcing the application to exit when such a state is detected, it safeguards against potential data corruption and ensures that manual intervention is required to resolve inconsistencies, thereby enhancing overall system stability.

Highlights

• Database Dirty State Check: Implemented a startup check in tapdb to ensure the database version is not dirty. If it is, the application now exits with an error, preventing tapd from operating on an incomplete migration, particularly for SQLite backends where this check was previously bypassed.
• Error Handling Improvement: Added a check for an error that was previously unhandled during migration version retrieval.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a crucial safety check to prevent the application from starting with a dirty database, which could happen if a migration fails. The change correctly identifies a dirty state by checking the flag returned by the migration library and exits with an informative error. The implementation is sound. I've added one minor comment regarding string formatting to improve readability and align more closely with the style guide's examples.

[ffranr]

Thank you for pointing this out and fixing it!

Might be worth adding a regression test to tapdb/migrations_test.go if not too expensive.

[ViktorT-11]

Thanks for the review @ffranr! I added an TestDirtySqliteVersion unit test. If you add that test to main, and remove the db1 part of the test, you'll see that the test errors with "no error" after the second ExecuteMigrations call.
Unfortunately though, this test won't be compatible with lightninglabs/migrate#3 and will need to be removed after that migrate version is used. This is due to that the test uses post migration callbacks to trigger the failing migrations which sets the db to a dirty state.
I don't think there's any easy way to achieve the same outcome using purely SQL migrations. Please let me know if you have any suggestion of how we'd achieve the same easily using that though :).

I do think this added test adds value and proves the need for this PR until lightninglabs/migrate#3 is used though!

[GeorgeTsagk]

Looks good, consider adding a release note for this

[GeorgeTsagk]

Also an extra note: We're adding this to the v0.7.0-rc2 scope so it's ok to merge into main 👍