Skip to content

Commit

Permalink
Try #6003:
Browse files Browse the repository at this point in the history
  • Loading branch information
spacemesh-bors[bot] authored Aug 13, 2024
2 parents f135361 + 1d8d310 commit 5c1d8aa
Show file tree
Hide file tree
Showing 194 changed files with 2,163 additions and 1,243 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,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.
Expand Down
76 changes: 76 additions & 0 deletions CODING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
```
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/).
Expand Down Expand Up @@ -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.
Expand All @@ -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).
Expand All @@ -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).
5 changes: 2 additions & 3 deletions activation/activation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -78,7 +77,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
Expand Down Expand Up @@ -174,7 +173,7 @@ func NewBuilder(
conf Config,
db sql.Executor,
atxsdata *atxsdata.Data,
localDB *localsql.Database,
localDB sql.LocalDatabase,
publisher pubsub.Publisher,
nipostBuilder nipostBuilder,
layerClock layerClock,
Expand Down
5 changes: 3 additions & 2 deletions activation/activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ==========
Expand All @@ -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
Expand All @@ -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")),

Expand Down
9 changes: 4 additions & 5 deletions activation/certifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
7 changes: 4 additions & 3 deletions activation/certifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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{
Expand All @@ -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())
Expand Down
4 changes: 2 additions & 2 deletions activation/e2e/activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions activation/e2e/atx_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()

Expand Down
4 changes: 2 additions & 2 deletions activation/e2e/builds_atx_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions activation/e2e/certifier_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions activation/e2e/checkpoint_merged_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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()

Expand Down Expand Up @@ -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()

Expand Down
6 changes: 3 additions & 3 deletions activation/e2e/checkpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down Expand Up @@ -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()

Expand Down
Loading

0 comments on commit 5c1d8aa

Please sign in to comment.