diff --git a/config_builder.go b/config_builder.go index 5195777e1..83974344e 100644 --- a/config_builder.go +++ b/config_builder.go @@ -1022,6 +1022,34 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( // Instantiate a native SQL invoice store if the flag is set. if d.cfg.DB.UseNativeSQL { + // KV invoice db resides in the same database as the graph and + // channel state DB. Let's query the database to see if we have + // any invoices there. If we do, we won't allow the user to + // start lnd with native SQL enabled, as we don't currently + // migrate the invoices to the new database schema. + invoiceSlice, err := dbs.GraphDB.QueryInvoices( + ctx, invoices.InvoiceQuery{ + NumMaxInvoices: 1, + }, + ) + if err != nil { + cleanUp() + d.logger.Errorf("Unable to query KV invoice DB: %v", + err) + + return nil, nil, err + } + + if len(invoiceSlice.Invoices) > 0 { + cleanUp() + err := fmt.Errorf("found invoices in the KV invoice " + + "DB, migration to native SQL is not yet " + + "supported") + d.logger.Error(err) + + return nil, nil, err + } + executor := sqldb.NewTransactionExecutor( dbs.NativeSQLStore, func(tx *sql.Tx) sqldb.InvoiceQueries { diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md index 59f57d9ab..ff08ce23c 100644 --- a/docs/release-notes/release-notes-0.18.0.md +++ b/docs/release-notes/release-notes-0.18.0.md @@ -374,6 +374,10 @@ for SQL backends enabling new users to optionally use an experimental native SQL invoices database. +* [Ensure that LND won't + start](https://github.com/lightningnetwork/lnd/pull/8568) if native SQL is + enabled but the channeldb already has any KV invoices stored. + ## Code Health * [Remove database pointers](https://github.com/lightningnetwork/lnd/pull/8117) diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 1de36b962..7a618402f 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -578,4 +578,8 @@ var allTestCases = []*lntest.TestCase{ Name: "open channel locked balance", TestFunc: testOpenChannelLockedBalance, }, + { + Name: "nativesql no migration", + TestFunc: testNativeSQLNoMigration, + }, } diff --git a/itest/lnd_misc_test.go b/itest/lnd_misc_test.go index a88abb7b3..58a57ed29 100644 --- a/itest/lnd_misc_test.go +++ b/itest/lnd_misc_test.go @@ -1,6 +1,7 @@ package itest import ( + "context" "encoding/hex" "fmt" "io/ioutil" @@ -1242,3 +1243,41 @@ func testSignVerifyMessageWithAddr(ht *lntest.HarnessTest) { require.False(ht, respValid.Valid, "external signature did validate") } + +// testNativeSQLNoMigration tests that nodes that have invoices would not start +// up with native SQL enabled, as we don't currently support migration of KV +// invoices to the new SQL schema. +func testNativeSQLNoMigration(ht *lntest.HarnessTest) { + alice := ht.Alice + + // Make sure we run the test with SQLite or Postgres. + if alice.Cfg.DBBackend != node.BackendSqlite && + alice.Cfg.DBBackend != node.BackendPostgres { + + ht.Skip("node not running with SQLite or Postgres") + } + + // Skip the test if the node is already running with native SQL. + if alice.Cfg.NativeSQL { + ht.Skip("node already running with native SQL") + } + + alice.RPC.AddInvoice(&lnrpc.Invoice{ + Value: 10_000, + }) + + alice.SetExtraArgs([]string{"--db.use-native-sql"}) + + // Restart the node manually as we're really only interested in the + // startup error. + require.NoError(ht, alice.Stop()) + require.NoError(ht, alice.StartLndCmd(context.Background())) + + // We expect the node to fail to start up with native SQL enabled, as we + // have an invoice in the KV store. + require.Error(ht, alice.WaitForProcessExit()) + + // Reset the extra args and restart alice. + alice.SetExtraArgs(nil) + require.NoError(ht, alice.Start(ht.Context())) +} diff --git a/lncfg/db.go b/lncfg/db.go index 49f506bb1..4072a51fe 100644 --- a/lncfg/db.go +++ b/lncfg/db.go @@ -122,13 +122,24 @@ func DefaultDB() *DB { // Validate validates the DB config. func (db *DB) Validate() error { switch db.Backend { - case BoltBackend, SqliteBackend: + case BoltBackend: + if db.UseNativeSQL { + return fmt.Errorf("cannot use native SQL with bolt " + + "backend") + } + + case SqliteBackend: case PostgresBackend: if err := db.Postgres.Validate(); err != nil { return err } case EtcdBackend: + if db.UseNativeSQL { + return fmt.Errorf("cannot use native SQL with etcd " + + "backend") + } + if !db.Etcd.Embedded && db.Etcd.Host == "" { return fmt.Errorf("etcd host must be set") } diff --git a/lntest/node/harness_node.go b/lntest/node/harness_node.go index 71d55d9ea..fb0eea014 100644 --- a/lntest/node/harness_node.go +++ b/lntest/node/harness_node.go @@ -629,7 +629,7 @@ func (hn *HarnessNode) cleanup() error { // waitForProcessExit Launch a new goroutine which that bubbles up any // potential fatal process errors to the goroutine running the tests. -func (hn *HarnessNode) waitForProcessExit() error { +func (hn *HarnessNode) WaitForProcessExit() error { var err error errChan := make(chan error, 1) @@ -752,7 +752,7 @@ func (hn *HarnessNode) Stop() error { } // Wait for lnd process to exit in the end. - return hn.waitForProcessExit() + return hn.WaitForProcessExit() } // CloseConn closes the grpc connection.