From 444524a7629dc40c98e8582b2374f6a528347b5c Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Thu, 23 Jan 2025 14:32:18 +0100 Subject: [PATCH 1/2] channeldb+lnd: set invoice bucket tombstone after migration This commit introduces the functionality to set a tombstone key in the invoice bucket after the migration to the native SQL database. The tombstone prevents the user from switching back to the KV invoice database, ensuring data consistency and avoiding potential issues like lingering invoices or partial state in KV tables. The tombstone is checked on startup to block any manual overrides that attempt to revert the migration. --- channeldb/invoice_test.go | 25 ++++++++++++++ channeldb/invoices.go | 53 +++++++++++++++++++++++++++++ config_builder.go | 35 ++++++++++++++++++- itest/lnd_invoice_migration_test.go | 10 ++++++ 4 files changed, 122 insertions(+), 1 deletion(-) diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index 1a4409f2d..c2d208d06 100644 --- a/channeldb/invoice_test.go +++ b/channeldb/invoice_test.go @@ -103,3 +103,28 @@ func TestEncodeDecodeAmpInvoiceState(t *testing.T) { // The two states should match. require.Equal(t, ampState, ampState2) } + +// TestInvoiceBucketTombstone tests the behavior of setting and checking the +// invoice bucket tombstone. It verifies that the tombstone can be set correctly +// and detected when present in the database. +func TestInvoiceBucketTombstone(t *testing.T) { + t.Parallel() + + // Initialize a test database. + db, err := MakeTestDB(t) + require.NoError(t, err, "unable to initialize db") + + // Ensure the tombstone doesn't exist initially. + tombstoneExists, err := db.GetInvoiceBucketTombstone() + require.NoError(t, err) + require.False(t, tombstoneExists) + + // Set the tombstone. + err = db.SetInvoiceBucketTombstone() + require.NoError(t, err) + + // Verify that the tombstone exists after setting it. + tombstoneExists, err = db.GetInvoiceBucketTombstone() + require.NoError(t, err) + require.True(t, tombstoneExists) +} diff --git a/channeldb/invoices.go b/channeldb/invoices.go index 669da1608..39775ac18 100644 --- a/channeldb/invoices.go +++ b/channeldb/invoices.go @@ -80,6 +80,13 @@ var ( // // settleIndexNo => invoiceKey settleIndexBucket = []byte("invoice-settle-index") + + // invoiceBucketTombstone is a special key that indicates the invoice + // bucket has been permanently closed. Its purpose is to prevent the + // invoice bucket from being reopened in the future. A key use case for + // the tombstone is to ensure users cannot switch back to the KV invoice + // database after migrating to the native SQL database. + invoiceBucketTombstone = []byte("invoice-tombstone") ) const ( @@ -2402,3 +2409,49 @@ func (d *DB) DeleteInvoice(_ context.Context, return err } + +// SetInvoiceBucketTombstone sets the tombstone key in the invoice bucket to +// mark the bucket as permanently closed. This prevents it from being reopened +// in the future. +func (d *DB) SetInvoiceBucketTombstone() error { + return kvdb.Update(d, func(tx kvdb.RwTx) error { + // Access the top-level invoice bucket. + invoices := tx.ReadWriteBucket(invoiceBucket) + if invoices == nil { + return fmt.Errorf("invoice bucket does not exist") + } + + // Add the tombstone key to the invoice bucket. + err := invoices.Put(invoiceBucketTombstone, []byte("1")) + if err != nil { + return fmt.Errorf("failed to set tombstone: %w", err) + } + + return nil + }, func() {}) +} + +// GetInvoiceBucketTombstone checks if the tombstone key exists in the invoice +// bucket. It returns true if the tombstone is present and false otherwise. +func (d *DB) GetInvoiceBucketTombstone() (bool, error) { + var tombstoneExists bool + + err := kvdb.View(d, func(tx kvdb.RTx) error { + // Access the top-level invoice bucket. + invoices := tx.ReadBucket(invoiceBucket) + if invoices == nil { + return fmt.Errorf("invoice bucket does not exist") + } + + // Check if the tombstone key exists. + tombstone := invoices.Get(invoiceBucketTombstone) + tombstoneExists = tombstone != nil + + return nil + }, func() {}) + if err != nil { + return false, err + } + + return tombstoneExists, nil +} diff --git a/config_builder.go b/config_builder.go index 45a923e05..43c9e4a68 100644 --- a/config_builder.go +++ b/config_builder.go @@ -1101,11 +1101,22 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( // regardless of the flag. if !d.cfg.DB.SkipSQLInvoiceMigration { migrationFn := func(tx *sqlc.Queries) error { - return invoices.MigrateInvoicesToSQL( + err := invoices.MigrateInvoicesToSQL( ctx, dbs.ChanStateDB.Backend, dbs.ChanStateDB, tx, invoiceMigrationBatchSize, ) + if err != nil { + return fmt.Errorf("failed to migrate "+ + "invoices to SQL: %w", err) + } + + // Set the invoice bucket tombstone to indicate + // that the migration has been completed. + d.logger.Debugf("Setting invoice bucket " + + "tombstone") + + return dbs.ChanStateDB.SetInvoiceBucketTombstone() //nolint:ll } // Make sure we attach the custom migration function to @@ -1147,6 +1158,28 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( dbs.InvoiceDB = sqlInvoiceDB } else { + // Check if the invoice bucket tombstone is set. If it is, we + // need to return and ask the user switch back to using the + // native SQL store. + ripInvoices, err := dbs.ChanStateDB.GetInvoiceBucketTombstone() + d.logger.Debugf("Invoice bucket tombstone set to: %v", + ripInvoices) + + if err != nil { + err = fmt.Errorf("unable to check invoice bucket "+ + "tombstone: %w", err) + d.logger.Error(err) + + return nil, nil, err + } + if ripInvoices { + err = fmt.Errorf("invoices bucket tombstoned, please " + + "switch back to native SQL") + d.logger.Error(err) + + return nil, nil, err + } + dbs.InvoiceDB = dbs.ChanStateDB } diff --git a/itest/lnd_invoice_migration_test.go b/itest/lnd_invoice_migration_test.go index fdebfdc86..637ec5992 100644 --- a/itest/lnd_invoice_migration_test.go +++ b/itest/lnd_invoice_migration_test.go @@ -298,6 +298,16 @@ func testInvoiceMigration(ht *lntest.HarnessTest) { } } + // Now restart Bob without the --db.use-native-sql flag so we can check + // that the KV tombstone was set and that Bob will fail to start. + require.NoError(ht, bob.Stop()) + bob.SetExtraArgs(nil) + + // Bob should now fail to start due to the tombstone being set. + require.NoError(ht, bob.StartLndCmd(ht.Context())) + require.Error(ht, bob.WaitForProcessExit()) + // Start Bob again so the test can complete. + bob.SetExtraArgs([]string{"--db.use-native-sql"}) require.NoError(ht, bob.Start(ht.Context())) } From 3f6f6c19c16bb4c59ffdc2edce944232ac182d80 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Thu, 23 Jan 2025 14:36:02 +0100 Subject: [PATCH 2/2] docs: update release-notes-0.19.0.md --- docs/release-notes/release-notes-0.19.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 2d4edf1e2..187e27c51 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -268,6 +268,9 @@ The underlying functionality between those two options remain the same. SQL](https://github.com/lightningnetwork/lnd/pull/8831) as part of a larger effort to support SQL databases natively in LND. +* [Set invoice bucket + ](https://github.com/lightningnetwork/lnd/pull/9438) tombstone after native + SQL migration. ## Code Health