Merge pull request #8568 from bhandras/native-sql-ensure-empty-invoicedb

lnd: ensure that LND won't start in native SQL mode if it has any KV invoices

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 {

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) 

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,
+	},
 }

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()))
+}

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")
 		}

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.