[Merged by Bors] - sql: improve database schema handling

CHANGELOG.md

@@ -163,6 +163,9 @@ operating your own PoET and want to use certificate authentication please refer
   ATXs. This vulnerability allows an attacker to claim rewards for a full tick amount although they should not be
   eligible for them.
 
+* [#6003](https://github.com/spacemeshos/go-spacemesh/pull/6003) Improve database schema handling.
+  This includes schema drift detection which may happen after running unreleased versions.
+
 * [#6031](https://github.com/spacemeshos/go-spacemesh/pull/6031) Fixed an edge case where the storage units might have
   changed after the initial PoST was generated but before the first ATX has been emitted, invalidating the initial PoST.
   The node will now try to verify the initial PoST and regenerate it if necessary.

CODING.md

@@ -105,3 +105,79 @@ Some useful logging recommendations for cleaner output:
 ## Commit Messages
 
 For commit messages, follow [this guideline](https://www.conventionalcommits.org/en/v1.0.0/). Use reasonable length for the subject and body, ideally no longer than 72 characters. Use the imperative mood for subject lines.
+
+## Handling database schema changes
+
+go-spacemesh currently maintains 2 SQLite databases in the data folder: `state.sql` (state database) and `local.sql` (local database). It employs schema versioning for both databases, with a possibility to upgrade older versions of each database to the current schema version by means of running a series of migrations. Also, go-spacemesh tracks any schema drift (unexpected schema changes) in the databases.
+
+When a database is first created, the corresponding schema file embedded in go-spacemesh executable is used to initialize it:
+* `sql/statesql/schema/schema.sql` for `state.sql`
+* `sql/localsql/schema/schema.sql` for `local.sql`
+The schema file includes `PRAGMA user_version = ...` which sets the version of the database schema. The version of the schema is equal to the number of migrations defined for the corresponding database (`state.sql` or `local.sql`).
+
+For an existing database, the `PRAGMA user_version` is checked against the expected version number. If the database's schema version is too new, go-spacemesh fails right away as an older go-spacemesh version cannot be expected to work with a database from a newer version. If the database version number is older than the expected version, go-spacemesh runs the necessary migration scripts embedded in go-spacemesh executable and updates `PRAGMA user_version = ...`. The migration scripts are located in the following folders:
+* `sql/statesql/schema/migrations` for `state.sql`
+* `sql/localsql/schema/migrations` for `local.sql`
+
+Additionally, some migrations ("coded migrations") can be implemented in Go code, in which case they reside in `.go` files located in `sql/statesql` and `sql/localsql` packages, respectively. It is worth noting that old coded migrations can be removed at some point, rendering database versions that are *too* old unusable with newer go-spacemesh versions.
+
+After all the migrations are run, go-spacemesh compares the schema of each database to the embedded schema scripts and if they differ, fails with an error message:
+```
+Error: open sqlite db schema drift detected (uri file:data/state.sql):
+  (
+        """
+        ... // 82 identical lines
+            PRIMARY KEY (layer, block)
+        );
++       CREATE TABLE foo(id int);
+        CREATE TABLE identities
+        (
+        ... // 66 identical lines
+        """
+  )
+```
+
+In this case, a table named `foo` has somehow appeared in the database, causing go-spacemesh to fail due to the schema drift. The possible reasons for schema drift can be the following:
+* running an unreleased version of go-spacemesh using your data folder. The unreleased version may contain migrations that may be changed before the release happens
+* manual changes in the database
+* external SQLite tooling used on the database that adds some tables, indices etc.
+
+In case if you want to run go-spacemesh with schema drift anyway, you can set `main.db-allow-schema-drift` to true. In this case, a warning with schema diff will be logged instead of failing.
+
+The schema changes in go-spacemesh code should be always done by means of adding migrations. Let's for example create a new migration (use zero-padded N+1 instead of 0010 with N being the number of the last migration for the local db):
+
+```console
+$ echo 'CREATE TABLE foo(id int);' >sql/localsql/schema/migrations/0010_foo.sql
+```
+
+After that, we update the schema files
+```console
+$ make generate
+$ # alternative: cd sql/localsql && go generate
+$ git diff sql/localsql/schema/schema.sql
+diff --git a/sql/localsql/schema/schema.sql b/sql/localsql/schema/schema.sql
+index 02c44d3cc..ebcdf4278 100755
+--- a/sql/localsql/schema/schema.sql
++++ b/sql/localsql/schema/schema.sql
+@@ -1,4 +1,4 @@
+-PRAGMA user_version = 9;
++PRAGMA user_version = 10;
+ CREATE TABLE atx_sync_requests
+ (
+     epoch     INT NOT NULL,
+@@ -24,6 +24,7 @@ CREATE TABLE "challenge"
+     post_indices  VARCHAR,
+     post_pow      UNSIGNED LONG INT
+ , poet_proof_ref        CHAR(32), poet_proof_membership VARCHAR) WITHOUT ROWID;
++CREATE TABLE foo(id int);
+ CREATE TABLE malfeasance_sync_state
+ (
+   id INT NOT NULL PRIMARY KEY,
+```
+
+Note that the changes include both the new table and an updated `PRAGMA user_version` line.
+The changes in the schema file must be committed along with the migration we added.
+```console
+$ git add sql/localsql/schema/migrations/0010_foo.sql sql/localsql/schema.sql
+$ git commit -m "sql: add a test migration"
+```

CONTRIBUTING.md

@@ -2,7 +2,7 @@
 Thank you for considering to contribute to the go-spacemesh open source project. We welcome contributions large and small and we actively accept contributions.
 
 - go-spacemesh is part of [The Spacemesh open source project](https://spacemesh.io), and is MIT licensed free open source software.
-- Please make sure to scan the [open issues](https://github.com/spacemeshos/go-spacemesh/issues). 
+- Please make sure to scan the [open issues](https://github.com/spacemeshos/go-spacemesh/issues).
 - Search the closed ones before reporting things, and help us with the open ones.
 - Make sure that you are able to contribute to MIT licensed free open software (no legal issues please).
 - Introduce yourself, ask questions about issues or talk about things on our [discord server](https://chat.spacemesh.io/).
@@ -39,7 +39,7 @@ Thank you for considering to contribute to the go-spacemesh open source project.
 # Code Guidelines
 Please follow these guidelines for your PR to be reviewed and be considered for merging into the project.
 
-1. Document all methods and functions using [go commentary](https://golang.org/doc/effective_go.html#commentary).  
+1. Document all methods and functions using [go commentary](https://golang.org/doc/effective_go.html#commentary).
 2. Provide at least one unit test for each function and method.
 3. Provide at least one integration test for each feature with a flow which involves more than one function call. Your tests should reflect the main ways that your code should be used.
 4. Run `go mod tidy`, `go fmt ./...` and `make lint` to format and lint your code before submitting your PR.
@@ -49,7 +49,7 @@ Please follow these guidelines for your PR to be reviewed and be considered for
 - Check for existing 3rd-party packages in the vendor folder `./vendor` before adding a new dependency.
 - Use [govendor](https://github.com/kardianos/govendor) to add a new dependency.
 
-# Working on a funded issue 
+# Working on a funded issue
 
 ## Step 1 - Discover :sunrise_over_mountains:
 - Browse the [open funded issues](https://github.com/spacemeshos/go-spacemesh/labels/funded%20%3Amoneybag%3A) in our github repo, or on our [gitcoin.io funded issues page](https://gitcoin.co/profile/spacemeshos).
@@ -68,6 +68,6 @@ Please follow these guidelines for your PR to be reviewed and be considered for
 ## Step 3 - Get paid :moneybag:
 - When ready, submit your PR for review and go through the code review process with one of our maintainers.
 - Expect a review process that ensures that you have followed our code guidelines at that your design and implementation are solid. You are expected to revision your code based on reviewers comments.
-- You should receive your bounty as soon as your PR is approved and merged by one of our maintainers. 
+- You should receive your bounty as soon as your PR is approved and merged by one of our maintainers.
 
 Please review our funded issues program [legal notes](https://github.com/spacemeshos/go-spacemesh/blob/master/legal.md).

activation/activation.go

@@ -28,7 +28,6 @@ import (
 	"github.com/spacemeshos/go-spacemesh/signing"
 	"github.com/spacemeshos/go-spacemesh/sql"
 	"github.com/spacemeshos/go-spacemesh/sql/atxs"
-	"github.com/spacemeshos/go-spacemesh/sql/localsql"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql/nipost"
 )
 
@@ -85,7 +84,7 @@ type Builder struct {
 	conf              Config
 	db                sql.Executor
 	atxsdata          *atxsdata.Data
-	localDB           *localsql.Database
+	localDB           sql.LocalDatabase
 	publisher         pubsub.Publisher
 	nipostBuilder     nipostBuilder
 	validator         nipostValidator
@@ -181,7 +180,7 @@ func NewBuilder(
 	conf Config,
 	db sql.Executor,
 	atxsdata *atxsdata.Data,
-	localDB *localsql.Database,
+	localDB sql.LocalDatabase,
 	publisher pubsub.Publisher,
 	nipostBuilder nipostBuilder,
 	layerClock layerClock,

activation/activation_test.go

@@ -33,6 +33,7 @@ import (
 	"github.com/spacemeshos/go-spacemesh/sql/localsql"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql/nipost"
 	sqlmocks "github.com/spacemeshos/go-spacemesh/sql/mocks"
+	"github.com/spacemeshos/go-spacemesh/sql/statesql"
 )
 
 // ========== Vars / Consts ==========
@@ -54,7 +55,7 @@ func TestMain(m *testing.M) {
 type testAtxBuilder struct {
 	*Builder
 	db          sql.Executor
-	localDb     *localsql.Database
+	localDb     sql.LocalDatabase
 	goldenATXID types.ATXID
 
 	observedLogs *observer.ObservedLogs
@@ -77,7 +78,7 @@ func newTestBuilder(tb testing.TB, numSigners int, opts ...BuilderOption) *testA
 
 	ctrl := gomock.NewController(tb)
 	tab := &testAtxBuilder{
-		db:          sql.InMemory(),
+		db:          statesql.InMemory(),
 		localDb:     localsql.InMemory(sql.WithConnections(numSigners)),
 		goldenATXID: types.ATXID(types.HexToHash32("77777")),
 

activation/certifier.go

@@ -21,7 +21,6 @@ import (
 	"github.com/spacemeshos/go-spacemesh/common/types"
 	"github.com/spacemeshos/go-spacemesh/sql"
 	"github.com/spacemeshos/go-spacemesh/sql/atxs"
-	"github.com/spacemeshos/go-spacemesh/sql/localsql"
 	certifierdb "github.com/spacemeshos/go-spacemesh/sql/localsql/certifier"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql/nipost"
 )
@@ -80,14 +79,14 @@ type CertifyResponse struct {
 
 type Certifier struct {
 	logger *zap.Logger
-	db     *localsql.Database
+	db     sql.LocalDatabase
 	client certifierClient
 
 	certifications singleflight.Group
 }
 
 func NewCertifier(
-	db *localsql.Database,
+	db sql.LocalDatabase,
 	logger *zap.Logger,
 	client certifierClient,
 ) *Certifier {
@@ -145,7 +144,7 @@ type CertifierClient struct {
 	client  *retryablehttp.Client
 	logger  *zap.Logger
 	db      sql.Executor
-	localDb *localsql.Database
+	localDb sql.LocalDatabase
 }
 
 type certifierClientOpts func(*CertifierClient)
@@ -160,7 +159,7 @@ func WithCertifierClientConfig(cfg CertifierClientConfig) certifierClientOpts {
 
 func NewCertifierClient(
 	db sql.Executor,
-	localDb *localsql.Database,
+	localDb sql.LocalDatabase,
 	logger *zap.Logger,
 	opts ...certifierClientOpts,
 ) *CertifierClient {

activation/certifier_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/spacemeshos/go-spacemesh/sql/localsql"
 	certdb "github.com/spacemeshos/go-spacemesh/sql/localsql/certifier"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql/nipost"
+	"github.com/spacemeshos/go-spacemesh/sql/statesql"
 )
 
 func TestPersistsCerts(t *testing.T) {
@@ -113,15 +114,15 @@ func TestObtainingPost(t *testing.T) {
 	id := types.RandomNodeID()
 
 	t.Run("no POST or ATX", func(t *testing.T) {
-		db := sql.InMemory()
+		db := statesql.InMemory()
 		localDb := localsql.InMemory()
 
 		certifier := NewCertifierClient(db, localDb, zaptest.NewLogger(t))
 		_, err := certifier.obtainPost(context.Background(), id)
 		require.ErrorContains(t, err, "PoST not found")
 	})
 	t.Run("initial POST available", func(t *testing.T) {
-		db := sql.InMemory()
+		db := statesql.InMemory()
 		localDb := localsql.InMemory()
 
 		post := nipost.Post{
@@ -142,7 +143,7 @@ func TestObtainingPost(t *testing.T) {
 		require.Equal(t, post, *got)
 	})
 	t.Run("initial POST unavailable but ATX exists", func(t *testing.T) {
-		db := sql.InMemory()
+		db := statesql.InMemory()
 		localDb := localsql.InMemory()
 
 		atx := newInitialATXv1(t, types.RandomATXID())

activation/e2e/activation_test.go

@@ -27,9 +27,9 @@ import (
 	"github.com/spacemeshos/go-spacemesh/p2p/pubsub"
 	"github.com/spacemeshos/go-spacemesh/p2p/pubsub/mocks"
 	"github.com/spacemeshos/go-spacemesh/signing"
-	"github.com/spacemeshos/go-spacemesh/sql"
 	"github.com/spacemeshos/go-spacemesh/sql/atxs"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql"
+	"github.com/spacemeshos/go-spacemesh/sql/statesql"
 	"github.com/spacemeshos/go-spacemesh/timesync"
 )
 
@@ -61,7 +61,7 @@ func Test_BuilderWithMultipleClients(t *testing.T) {
 	logger := zaptest.NewLogger(t)
 	goldenATX := types.ATXID{2, 3, 4}
 	cfg := testPostConfig()
-	db := sql.InMemory()
+	db := statesql.InMemory()
 	localDB := localsql.InMemory()
 
 	svc := grpcserver.NewPostService(logger, grpcserver.PostServiceQueryInterval(100*time.Millisecond))

activation/e2e/atx_merge_test.go

@@ -27,11 +27,11 @@ import (
 	"github.com/spacemeshos/go-spacemesh/datastore"
 	"github.com/spacemeshos/go-spacemesh/p2p/pubsub/mocks"
 	"github.com/spacemeshos/go-spacemesh/signing"
-	"github.com/spacemeshos/go-spacemesh/sql"
 	"github.com/spacemeshos/go-spacemesh/sql/atxs"
 	"github.com/spacemeshos/go-spacemesh/sql/identities"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql/nipost"
+	"github.com/spacemeshos/go-spacemesh/sql/statesql"
 	"github.com/spacemeshos/go-spacemesh/system"
 	smocks "github.com/spacemeshos/go-spacemesh/system/mocks"
 	"github.com/spacemeshos/go-spacemesh/timesync"
@@ -205,7 +205,7 @@ func Test_MarryAndMerge(t *testing.T) {
 	logger := zaptest.NewLogger(t)
 	goldenATX := types.ATXID{2, 3, 4}
 	cfg := testPostConfig()
-	db := sql.InMemory()
+	db := statesql.InMemory()
 	cdb := datastore.NewCachedDB(db, logger)
 	localDB := localsql.InMemory()
 

activation/e2e/builds_atx_v2_test.go

@@ -25,9 +25,9 @@ import (
 	"github.com/spacemeshos/go-spacemesh/p2p/pubsub"
 	"github.com/spacemeshos/go-spacemesh/p2p/pubsub/mocks"
 	"github.com/spacemeshos/go-spacemesh/signing"
-	"github.com/spacemeshos/go-spacemesh/sql"
 	"github.com/spacemeshos/go-spacemesh/sql/atxs"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql"
+	"github.com/spacemeshos/go-spacemesh/sql/statesql"
 	smocks "github.com/spacemeshos/go-spacemesh/system/mocks"
 	"github.com/spacemeshos/go-spacemesh/timesync"
 )
@@ -53,7 +53,7 @@ func TestBuilder_SwitchesToBuildV2(t *testing.T) {
 	require.NoError(t, err)
 
 	cfg := testPostConfig()
-	db := sql.InMemory()
+	db := statesql.InMemory()
 	cdb := datastore.NewCachedDB(db, logger)
 
 	opts := testPostSetupOpts(t)

activation/e2e/certifier_client_test.go

@@ -23,9 +23,9 @@ import (
 	"github.com/spacemeshos/go-spacemesh/api/grpcserver"
 	"github.com/spacemeshos/go-spacemesh/common/types"
 	"github.com/spacemeshos/go-spacemesh/signing"
-	"github.com/spacemeshos/go-spacemesh/sql"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql/nipost"
+	"github.com/spacemeshos/go-spacemesh/sql/statesql"
 )
 
 func TestCertification(t *testing.T) {
@@ -34,7 +34,7 @@ func TestCertification(t *testing.T) {
 	require.NoError(t, err)
 
 	cfg := testPostConfig()
-	db := sql.InMemory()
+	db := statesql.InMemory()
 	localDb := localsql.InMemory()
 
 	opts := testPostSetupOpts(t)

activation/e2e/checkpoint_merged_test.go

@@ -25,11 +25,11 @@ import (
 	"github.com/spacemeshos/go-spacemesh/datastore"
 	"github.com/spacemeshos/go-spacemesh/p2p/pubsub/mocks"
 	"github.com/spacemeshos/go-spacemesh/signing"
-	"github.com/spacemeshos/go-spacemesh/sql"
 	"github.com/spacemeshos/go-spacemesh/sql/accounts"
 	"github.com/spacemeshos/go-spacemesh/sql/atxs"
 	"github.com/spacemeshos/go-spacemesh/sql/identities"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql"
+	"github.com/spacemeshos/go-spacemesh/sql/statesql"
 	smocks "github.com/spacemeshos/go-spacemesh/system/mocks"
 	"github.com/spacemeshos/go-spacemesh/timesync"
 )
@@ -43,7 +43,7 @@ func Test_CheckpointAfterMerge(t *testing.T) {
 	logger := zaptest.NewLogger(t)
 	goldenATX := types.ATXID{2, 3, 4}
 	cfg := testPostConfig()
-	db := sql.InMemory()
+	db := statesql.InMemory()
 	cdb := datastore.NewCachedDB(db, logger)
 	localDB := localsql.InMemory()
 
@@ -261,7 +261,7 @@ func Test_CheckpointAfterMerge(t *testing.T) {
 	require.NoError(t, err)
 	require.Nil(t, data)
 
-	newDB, err := sql.Open("file:" + recoveryCfg.DbPath())
+	newDB, err := statesql.Open("file:" + recoveryCfg.DbPath())
 	require.NoError(t, err)
 	defer newDB.Close()
 

activation/e2e/checkpoint_test.go

@@ -25,9 +25,9 @@ import (
 	"github.com/spacemeshos/go-spacemesh/p2p/pubsub"
 	"github.com/spacemeshos/go-spacemesh/p2p/pubsub/mocks"
 	"github.com/spacemeshos/go-spacemesh/signing"
-	"github.com/spacemeshos/go-spacemesh/sql"
 	"github.com/spacemeshos/go-spacemesh/sql/accounts"
 	"github.com/spacemeshos/go-spacemesh/sql/localsql"
+	"github.com/spacemeshos/go-spacemesh/sql/statesql"
 	smocks "github.com/spacemeshos/go-spacemesh/system/mocks"
 	"github.com/spacemeshos/go-spacemesh/timesync"
 )
@@ -46,7 +46,7 @@ func TestCheckpoint_PublishingSoloATXs(t *testing.T) {
 	require.NoError(t, err)
 
 	cfg := testPostConfig()
-	db := sql.InMemory()
+	db := statesql.InMemory()
 	cdb := datastore.NewCachedDB(db, logger)
 
 	opts := testPostSetupOpts(t)
@@ -181,7 +181,7 @@ func TestCheckpoint_PublishingSoloATXs(t *testing.T) {
 	require.NoError(t, err)
 	require.Nil(t, data)
 
-	newDB, err := sql.Open("file:" + recoveryCfg.DbPath())
+	newDB, err := statesql.Open("file:" + recoveryCfg.DbPath())
 	require.NoError(t, err)
 	defer newDB.Close()