From 92689c546cd8a6674c591dcc394c5fc186ea5396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Tigerstr=C3=B6m?= Date: Thu, 24 Apr 2025 15:48:10 +0700 Subject: [PATCH 1/4] accounts: update NewTestDB func to return Store In preparation for upcoming migration tests from a kvdb to an SQL store, this commit updates the NewTestDB function to return the Store interface rather than a concrete store implementation. This change ensures that migration tests can call NewTestDB under any build tag while receiving a consistent return type. --- accounts/test_kvdb.go | 4 ++-- accounts/test_postgres.go | 4 ++-- accounts/test_sqlite.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/accounts/test_kvdb.go b/accounts/test_kvdb.go index 99c3e2ae6..546c1eee7 100644 --- a/accounts/test_kvdb.go +++ b/accounts/test_kvdb.go @@ -15,14 +15,14 @@ import ( var ErrDBClosed = errors.New("database not open") // NewTestDB is a helper function that creates an BBolt database for testing. -func NewTestDB(t *testing.T, clock clock.Clock) *BoltStore { +func NewTestDB(t *testing.T, clock clock.Clock) Store { return NewTestDBFromPath(t, t.TempDir(), clock) } // NewTestDBFromPath is a helper function that creates a new BoltStore with a // connection to an existing BBolt database for testing. func NewTestDBFromPath(t *testing.T, dbPath string, - clock clock.Clock) *BoltStore { + clock clock.Clock) Store { store, err := NewBoltStore(dbPath, DBFilename, clock) require.NoError(t, err) diff --git a/accounts/test_postgres.go b/accounts/test_postgres.go index e4ae89bcf..609eeb608 100644 --- a/accounts/test_postgres.go +++ b/accounts/test_postgres.go @@ -15,14 +15,14 @@ import ( var ErrDBClosed = errors.New("database is closed") // NewTestDB is a helper function that creates an SQLStore database for testing. -func NewTestDB(t *testing.T, clock clock.Clock) *SQLStore { +func NewTestDB(t *testing.T, clock clock.Clock) Store { return NewSQLStore(db.NewTestPostgresDB(t).BaseDB, clock) } // NewTestDBFromPath is a helper function that creates a new SQLStore with a // connection to an existing postgres database for testing. func NewTestDBFromPath(t *testing.T, dbPath string, - clock clock.Clock) *SQLStore { + clock clock.Clock) Store { return NewSQLStore(db.NewTestPostgresDB(t).BaseDB, clock) } diff --git a/accounts/test_sqlite.go b/accounts/test_sqlite.go index 9e6916fb5..0dd042a28 100644 --- a/accounts/test_sqlite.go +++ b/accounts/test_sqlite.go @@ -15,14 +15,14 @@ import ( var ErrDBClosed = errors.New("database is closed") // NewTestDB is a helper function that creates an SQLStore database for testing. -func NewTestDB(t *testing.T, clock clock.Clock) *SQLStore { +func NewTestDB(t *testing.T, clock clock.Clock) Store { return NewSQLStore(db.NewTestSqliteDB(t).BaseDB, clock) } // NewTestDBFromPath is a helper function that creates a new SQLStore with a // connection to an existing SQL database for testing. func NewTestDBFromPath(t *testing.T, dbPath string, - clock clock.Clock) *SQLStore { + clock clock.Clock) Store { return NewSQLStore( db.NewTestSqliteDbHandleFromPath(t, dbPath).BaseDB, clock, From 6030f650fd71738757bffd639b746fb7d311dadf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Tigerstr=C3=B6m?= Date: Thu, 15 May 2025 17:03:22 +0200 Subject: [PATCH 2/4] db: order ListAllAccounts result by account id. In preparation for the migration from kvdb to SQL, we update the results of the ListAllAccounts query. After the migration has been implemented, we will test that the result off all accounts in the SQL database is the same as the result of the fetching all accounts in the kvdb. By ordering the SQL query's result by account id, we ensure that all accounts are returned in the same order as they are returned by the kvdb. --- db/sqlc/accounts.sql.go | 1 + db/sqlc/queries/accounts.sql | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/db/sqlc/accounts.sql.go b/db/sqlc/accounts.sql.go index 4deefdb88..f6b3fc815 100644 --- a/db/sqlc/accounts.sql.go +++ b/db/sqlc/accounts.sql.go @@ -261,6 +261,7 @@ func (q *Queries) ListAccountPayments(ctx context.Context, accountID int64) ([]A const listAllAccounts = `-- name: ListAllAccounts :many SELECT id, alias, label, type, initial_balance_msat, current_balance_msat, last_updated, expiration FROM accounts +ORDER BY id ` func (q *Queries) ListAllAccounts(ctx context.Context) ([]Account, error) { diff --git a/db/sqlc/queries/accounts.sql b/db/sqlc/queries/accounts.sql index 637a49727..9c23c8c4f 100644 --- a/db/sqlc/queries/accounts.sql +++ b/db/sqlc/queries/accounts.sql @@ -62,7 +62,8 @@ WHERE id = $1; -- name: ListAllAccounts :many SELECT * -FROM accounts; +FROM accounts +ORDER BY id; -- name: ListAccountPayments :many SELECT * From dbb5d730790563e1504286d3dc0d155149b092e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Tigerstr=C3=B6m?= Date: Wed, 16 Apr 2025 11:37:39 +0200 Subject: [PATCH 3/4] accounts: add migration code from kvdb to SQL This commit introduces the migration logic for transitioning the accounts store from kvdb to SQL. Note that as of this commit, the migration is not yet triggered by any production code, i.e. only tests execute the migration logic. --- accounts/sql_migration.go | 233 ++++++++++++++++++++++ accounts/sql_migration_test.go | 346 +++++++++++++++++++++++++++++++++ go.mod | 4 +- 3 files changed, 581 insertions(+), 2 deletions(-) create mode 100644 accounts/sql_migration.go create mode 100644 accounts/sql_migration_test.go diff --git a/accounts/sql_migration.go b/accounts/sql_migration.go new file mode 100644 index 000000000..befa62f15 --- /dev/null +++ b/accounts/sql_migration.go @@ -0,0 +1,233 @@ +package accounts + +import ( + "context" + "database/sql" + "errors" + "fmt" + "math" + "reflect" + "time" + + "github.com/davecgh/go-spew/spew" + "github.com/lightninglabs/lightning-terminal/db/sqlc" + "github.com/pmezard/go-difflib/difflib" +) + +var ( + // ErrMigrationMismatch is returned when the migrated account does not + // match the original account. + ErrMigrationMismatch = fmt.Errorf("migrated account does not match " + + "original account") +) + +// MigrateAccountStoreToSQL runs the migration of all accounts and indices from +// the KV database to the SQL database. The migration is done in a single +// transaction to ensure that all accounts are migrated or none at all. +func MigrateAccountStoreToSQL(ctx context.Context, kvStore *BoltStore, + tx SQLQueries) error { + + log.Infof("Starting migration of the KV accounts store to SQL") + + err := migrateAccountsToSQL(ctx, kvStore, tx) + if err != nil { + return fmt.Errorf("unsuccessful migration of accounts to "+ + "SQL: %w", err) + } + + err = migrateAccountsIndicesToSQL(ctx, kvStore, tx) + if err != nil { + return fmt.Errorf("unsuccessful migration of account indices "+ + "to SQL: %w", err) + } + + return nil +} + +// migrateAccountsToSQL runs the migration of all accounts from the KV database +// to the SQL database. The migration is done in a single transaction to ensure +// that all accounts are migrated or none at all. +func migrateAccountsToSQL(ctx context.Context, kvStore *BoltStore, + tx SQLQueries) error { + + log.Infof("Starting migration of accounts from KV to SQL") + + kvAccounts, err := kvStore.Accounts(ctx) + if err != nil { + return err + } + + for _, kvAccount := range kvAccounts { + migratedAccountID, err := migrateSingleAccountToSQL( + ctx, tx, kvAccount, + ) + if err != nil { + return fmt.Errorf("unable to migrate account(%v): %w", + kvAccount.ID, err) + } + + migratedAccount, err := getAndMarshalAccount( + ctx, tx, migratedAccountID, + ) + if err != nil { + return fmt.Errorf("unable to fetch migrated "+ + "account(%v): %w", kvAccount.ID, err) + } + + overrideAccountTimeZone(kvAccount) + overrideAccountTimeZone(migratedAccount) + + if !reflect.DeepEqual(kvAccount, migratedAccount) { + diff := difflib.UnifiedDiff{ + A: difflib.SplitLines( + spew.Sdump(kvAccount), + ), + B: difflib.SplitLines( + spew.Sdump(migratedAccount), + ), + FromFile: "Expected", + FromDate: "", + ToFile: "Actual", + ToDate: "", + Context: 3, + } + diffText, _ := difflib.GetUnifiedDiffString(diff) + + return fmt.Errorf("%w: %v.\n%v", ErrMigrationMismatch, + kvAccount.ID, diffText) + } + } + + log.Infof("All accounts migrated from KV to SQL. Total number of "+ + "accounts migrated: %d", len(kvAccounts)) + + return nil +} + +// migrateSingleAccountToSQL runs the migration for a single account from the +// KV database to the SQL database. +func migrateSingleAccountToSQL(ctx context.Context, + tx SQLQueries, account *OffChainBalanceAccount) (int64, error) { + + accountAlias, err := account.ID.ToInt64() + if err != nil { + return 0, err + } + + insertAccountParams := sqlc.InsertAccountParams{ + Type: int16(account.Type), + InitialBalanceMsat: int64(account.InitialBalance), + CurrentBalanceMsat: account.CurrentBalance, + LastUpdated: account.LastUpdate.UTC(), + Alias: accountAlias, + Expiration: account.ExpirationDate.UTC(), + Label: sql.NullString{ + String: account.Label, + Valid: len(account.Label) > 0, + }, + } + + sqlId, err := tx.InsertAccount(ctx, insertAccountParams) + if err != nil { + return 0, err + } + + for hash := range account.Invoices { + addInvoiceParams := sqlc.AddAccountInvoiceParams{ + AccountID: sqlId, + Hash: hash[:], + } + + err = tx.AddAccountInvoice(ctx, addInvoiceParams) + if err != nil { + return sqlId, err + } + } + + for hash, paymentEntry := range account.Payments { + upsertPaymentParams := sqlc.UpsertAccountPaymentParams{ + AccountID: sqlId, + Hash: hash[:], + Status: int16(paymentEntry.Status), + FullAmountMsat: int64(paymentEntry.FullAmount), + } + + err = tx.UpsertAccountPayment(ctx, upsertPaymentParams) + if err != nil { + return sqlId, err + } + } + + return sqlId, nil +} + +// migrateAccountsIndicesToSQL runs the migration for the account indices from +// the KV database to the SQL database. +func migrateAccountsIndicesToSQL(ctx context.Context, kvStore *BoltStore, + tx SQLQueries) error { + + log.Infof("Starting migration of accounts indices from KV to SQL") + + addIndex, settleIndex, err := kvStore.LastIndexes(ctx) + if errors.Is(err, ErrNoInvoiceIndexKnown) { + log.Infof("No indices found in KV store, skipping migration") + return nil + } else if err != nil { + return err + } + + if addIndex > math.MaxInt64 { + return fmt.Errorf("%s:%v is above max int64 value", + addIndexName, addIndex) + } + + if settleIndex > math.MaxInt64 { + return fmt.Errorf("%s:%v is above max int64 value", + settleIndexName, settleIndex) + } + + setAddIndexParams := sqlc.SetAccountIndexParams{ + Name: addIndexName, + Value: int64(addIndex), + } + + err = tx.SetAccountIndex(ctx, setAddIndexParams) + if err != nil { + return err + } + + setSettleIndexParams := sqlc.SetAccountIndexParams{ + Name: settleIndexName, + Value: int64(settleIndex), + } + + err = tx.SetAccountIndex(ctx, setSettleIndexParams) + if err != nil { + return err + } + + log.Infof("Successfully migratated accounts indices from KV to SQL") + + return nil +} + +// overrideAccountTimeZone overrides the time zone of the account to the local +// time zone and chops off the nanosecond part for comparison. This is needed +// because KV database stores times as-is which as an unwanted side effect would +// fail migration due to time comparison expecting both the original and +// migrated accounts to be in the same local time zone and in microsecond +// precision. Note that PostgresSQL stores times in microsecond precision while +// SQLite can store times in nanosecond precision if using TEXT storage class. +func overrideAccountTimeZone(account *OffChainBalanceAccount) { + fixTime := func(t time.Time) time.Time { + return t.In(time.Local).Truncate(time.Microsecond) + } + + if !account.ExpirationDate.IsZero() { + account.ExpirationDate = fixTime(account.ExpirationDate) + } + + if !account.LastUpdate.IsZero() { + account.LastUpdate = fixTime(account.LastUpdate) + } +} diff --git a/accounts/sql_migration_test.go b/accounts/sql_migration_test.go new file mode 100644 index 000000000..219d4692d --- /dev/null +++ b/accounts/sql_migration_test.go @@ -0,0 +1,346 @@ +package accounts + +import ( + "context" + "database/sql" + "testing" + "time" + + "github.com/lightninglabs/lightning-terminal/db" + "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/fn" + "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/sqldb" + "github.com/stretchr/testify/require" +) + +// TestAccountStoreMigration tests the migration of account store from a bolt +// backed to a SQL database. Note that this test does not attempt to be a +// complete migration test. +func TestAccountStoreMigration(t *testing.T) { + t.Parallel() + + ctx := context.Background() + clock := clock.NewTestClock(time.Now()) + + // When using build tags that creates a kvdb store for NewTestDB, we + // skip this test as it is only applicable for postgres and sqlite tags. + store := NewTestDB(t, clock) + if _, ok := store.(*BoltStore); ok { + t.Skipf("Skipping account store migration test for kvdb build") + } + + makeSQLDB := func(t *testing.T) (*SQLStore, + *db.TransactionExecutor[SQLQueries]) { + + testDBStore := NewTestDB(t, clock) + t.Cleanup(func() { + require.NoError(t, testDBStore.Close()) + }) + + store, ok := testDBStore.(*SQLStore) + require.True(t, ok) + + baseDB := store.BaseDB + + genericExecutor := db.NewTransactionExecutor( + baseDB, func(tx *sql.Tx) SQLQueries { + return baseDB.WithTx(tx) + }, + ) + + return store, genericExecutor + } + + assertMigrationResults := func(t *testing.T, sqlStore *SQLStore, + kvAccounts []*OffChainBalanceAccount, kvAddIndex uint64, + kvSettleIndex uint64, expectLastIndex bool) { + + // The migration function will check if the inserted accounts + // and indices equals the migrated ones, but as a sanity check + // we'll also fetch the accounts and indices from the sql store + // and compare them to the original. + // First we compare the migrated accounts to the original ones. + sqlAccounts, err := sqlStore.Accounts(ctx) + require.NoError(t, err) + require.Equal(t, len(kvAccounts), len(sqlAccounts)) + + for i := 0; i < len(kvAccounts); i++ { + assertEqualAccounts(t, kvAccounts[i], sqlAccounts[i]) + } + + // After that we compare the migrated indices. However, if we + // don't expect the last indexes to be set, we don't need to + // compare them. + if !expectLastIndex { + return + } + + sqlAddIndex, sqlSettleIndex, err := sqlStore.LastIndexes(ctx) + require.NoError(t, err) + + require.Equal(t, kvAddIndex, sqlAddIndex) + require.Equal(t, kvSettleIndex, sqlSettleIndex) + } + + tests := []struct { + name string + expectLastIndex bool + populateDB func(t *testing.T, kvStore *BoltStore) + }{ + { + name: "empty", + expectLastIndex: false, + populateDB: func(t *testing.T, kvStore *BoltStore) { + // Don't populate the DB. + }, + }, + { + name: "account no expiry", + expectLastIndex: false, + populateDB: func(t *testing.T, kvStore *BoltStore) { + // Create an account that does not expire. + acct1, err := kvStore.NewAccount( + ctx, 0, time.Time{}, "foo", + ) + require.NoError(t, err) + require.False(t, acct1.HasExpired()) + }, + }, + { + name: "account with expiry", + expectLastIndex: false, + populateDB: func(t *testing.T, kvStore *BoltStore) { + // Create an account that does expire. + acct1, err := kvStore.NewAccount( + ctx, 0, time.Now().Add(time.Hour), + "foo", + ) + require.NoError(t, err) + require.False(t, acct1.HasExpired()) + }, + }, + { + name: "account with set UpdatedAt", + expectLastIndex: false, + populateDB: func(t *testing.T, kvStore *BoltStore) { + // Create an account that does expire. + acct1, err := kvStore.NewAccount( + ctx, 0, time.Now().Add(time.Hour), + "foo", + ) + require.NoError(t, err) + require.False(t, acct1.HasExpired()) + + err = kvStore.UpdateAccountBalanceAndExpiry( + ctx, acct1.ID, fn.None[int64](), + fn.Some(time.Now().Add(time.Minute)), + ) + require.NoError(t, err) + }, + }, + { + name: "account with balance", + expectLastIndex: false, + populateDB: func(t *testing.T, kvStore *BoltStore) { + // Create an account with balance + acct1, err := kvStore.NewAccount( + ctx, 100000, time.Time{}, "foo", + ) + require.NoError(t, err) + require.False(t, acct1.HasExpired()) + }, + }, + { + name: "account with invoices", + expectLastIndex: true, + populateDB: func(t *testing.T, kvStore *BoltStore) { + // Create an account with balance + acct1, err := kvStore.NewAccount( + ctx, 0, time.Time{}, "foo", + ) + require.NoError(t, err) + require.False(t, acct1.HasExpired()) + + hash1 := lntypes.Hash{1, 2, 3, 4} + err = kvStore.AddAccountInvoice( + ctx, acct1.ID, hash1, + ) + require.NoError(t, err) + + err = kvStore.StoreLastIndexes(ctx, 1, 0) + require.NoError(t, err) + + hash2 := lntypes.Hash{1, 2, 3, 4, 5} + err = kvStore.AddAccountInvoice( + ctx, acct1.ID, hash2, + ) + require.NoError(t, err) + + err = kvStore.StoreLastIndexes(ctx, 2, 1) + require.NoError(t, err) + }, + }, + { + name: "account with payments", + expectLastIndex: false, + populateDB: func(t *testing.T, kvStore *BoltStore) { + // Create an account with balance + acct1, err := kvStore.NewAccount( + ctx, 0, time.Time{}, "foo", + ) + require.NoError(t, err) + require.False(t, acct1.HasExpired()) + + hash1 := lntypes.Hash{1, 1, 1, 1} + known, err := kvStore.UpsertAccountPayment( + ctx, acct1.ID, hash1, 100, + lnrpc.Payment_UNKNOWN, + ) + require.NoError(t, err) + require.False(t, known) + + hash2 := lntypes.Hash{2, 2, 2, 2} + known, err = kvStore.UpsertAccountPayment( + ctx, acct1.ID, hash2, 200, + lnrpc.Payment_IN_FLIGHT, + ) + require.NoError(t, err) + require.False(t, known) + + hash3 := lntypes.Hash{3, 3, 3, 3} + known, err = kvStore.UpsertAccountPayment( + ctx, acct1.ID, hash3, 200, + lnrpc.Payment_SUCCEEDED, + ) + require.NoError(t, err) + require.False(t, known) + + hash4 := lntypes.Hash{4, 4, 4, 4} + known, err = kvStore.UpsertAccountPayment( + ctx, acct1.ID, hash4, 200, + lnrpc.Payment_FAILED, + ) + require.NoError(t, err) + require.False(t, known) + }, + }, + { + name: "multiple accounts", + expectLastIndex: true, + populateDB: func(t *testing.T, kvStore *BoltStore) { + // Create two accounts with balance and that + // expires. + acct1, err := kvStore.NewAccount( + ctx, 100000, time.Now().Add(time.Hour), + "foo", + ) + require.NoError(t, err) + require.False(t, acct1.HasExpired()) + + acct2, err := kvStore.NewAccount( + ctx, 200000, time.Now().Add(time.Hour), + "bar", + ) + require.NoError(t, err) + require.False(t, acct2.HasExpired()) + + // Create invoices for both accounts. + hash1 := lntypes.Hash{1, 1, 1, 1} + err = kvStore.AddAccountInvoice( + ctx, acct1.ID, hash1, + ) + require.NoError(t, err) + + err = kvStore.StoreLastIndexes(ctx, 1, 0) + require.NoError(t, err) + + hash2 := lntypes.Hash{2, 2, 2, 2} + err = kvStore.AddAccountInvoice( + ctx, acct2.ID, hash2, + ) + require.NoError(t, err) + + err = kvStore.StoreLastIndexes(ctx, 2, 0) + require.NoError(t, err) + + // Create payments for both accounts. + hash3 := lntypes.Hash{3, 3, 3, 3} + known, err := kvStore.UpsertAccountPayment( + ctx, acct1.ID, hash3, 100, + lnrpc.Payment_SUCCEEDED, + ) + require.NoError(t, err) + require.False(t, known) + + hash4 := lntypes.Hash{4, 4, 4, 4} + known, err = kvStore.UpsertAccountPayment( + ctx, acct2.ID, hash4, 200, + lnrpc.Payment_IN_FLIGHT, + ) + require.NoError(t, err) + require.False(t, known) + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + // Create a new kvdb store to populate with test data. + kvStore, err := NewBoltStore( + t.TempDir(), DBFilename, clock, + ) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, kvStore.db.Close()) + }) + + // Populate the kv store. + test.populateDB(t, kvStore) + + // Create the SQL store that we will migrate the data + // to. + sqlStore, txEx := makeSQLDB(t) + + // We fetch the accounts and indices from the kvStore + // before migrating them to the SQL store, just to + // ensure that the migration doesn't affect the original + // data. + kvAccounts, err := kvStore.Accounts(ctx) + require.NoError(t, err) + + kvAddIndex, kvSettleIndex, err := kvStore.LastIndexes( + ctx, + ) + + if !test.expectLastIndex { + // If the test expects there to be no invoices + // indices, we also verify that the database + // contains none. + require.ErrorIs(t, err, ErrNoInvoiceIndexKnown) + } else { + require.NoError(t, err) + } + + // Perform the migration. + var opts sqldb.MigrationTxOptions + err = txEx.ExecTx(ctx, &opts, + func(tx SQLQueries) error { + return MigrateAccountStoreToSQL( + ctx, kvStore, tx, + ) + }, + ) + require.NoError(t, err) + + // Assert migration results. + assertMigrationResults( + t, sqlStore, kvAccounts, kvAddIndex, + kvSettleIndex, test.expectLastIndex, + ) + }) + } +} diff --git a/go.mod b/go.mod index c647825a7..2ee741358 100644 --- a/go.mod +++ b/go.mod @@ -35,11 +35,13 @@ require ( github.com/lightningnetwork/lnd/fn v1.2.3 github.com/lightningnetwork/lnd/fn/v2 v2.0.8 github.com/lightningnetwork/lnd/kvdb v1.4.16 + github.com/lightningnetwork/lnd/sqldb v1.0.9 github.com/lightningnetwork/lnd/tlv v1.3.0 github.com/lightningnetwork/lnd/tor v1.1.6 github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f github.com/mwitkow/grpc-proxy v0.0.0-20230212185441-f345521cb9c9 github.com/ory/dockertest/v3 v3.10.0 + github.com/pmezard/go-difflib v1.0.0 github.com/stretchr/testify v1.10.0 github.com/urfave/cli v1.22.14 go.etcd.io/bbolt v1.3.11 @@ -143,7 +145,6 @@ require ( github.com/lightningnetwork/lightning-onion v1.2.1-0.20240712235311-98bd56499dfb // indirect github.com/lightningnetwork/lnd/healthcheck v1.2.6 // indirect github.com/lightningnetwork/lnd/queue v1.1.1 // indirect - github.com/lightningnetwork/lnd/sqldb v1.0.9 // indirect github.com/lightningnetwork/lnd/ticker v1.1.1 // indirect github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796 // indirect github.com/mattn/go-isatty v0.0.20 // indirect @@ -161,7 +162,6 @@ require ( github.com/opencontainers/image-spec v1.1.0 // indirect github.com/opencontainers/runc v1.2.0 // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.14.0 // indirect github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/common v0.37.0 // indirect From 1e0e88b32f0081c5fb9c3938bf9914a746f9a4dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Tigerstr=C3=B6m?= Date: Thu, 24 Apr 2025 16:04:05 +0700 Subject: [PATCH 4/4] accounts: add randomized accounts migration tests --- .gitignore | 3 + accounts/sql_migration_test.go | 239 +++++++++++++++++++++++++++++++++ go.mod | 2 +- 3 files changed, 243 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 3958b0234..86ee46dd9 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,9 @@ itest/itest.test itest/.logs itest/*.log +# Failed rapid test runs +accounts/testdata/rapid/* + vendor *.idea *.run diff --git a/accounts/sql_migration_test.go b/accounts/sql_migration_test.go index 219d4692d..d1e331e42 100644 --- a/accounts/sql_migration_test.go +++ b/accounts/sql_migration_test.go @@ -3,6 +3,7 @@ package accounts import ( "context" "database/sql" + "fmt" "testing" "time" @@ -11,8 +12,11 @@ import ( "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/sqldb" "github.com/stretchr/testify/require" + "golang.org/x/exp/rand" + "pgregory.net/rapid" ) // TestAccountStoreMigration tests the migration of account store from a bolt @@ -283,6 +287,16 @@ func TestAccountStoreMigration(t *testing.T) { require.False(t, known) }, }, + { + name: "randomized accounts", + expectLastIndex: true, + populateDB: randomizeAccounts, + }, + { + name: "rapid randomized accounts", + expectLastIndex: true, + populateDB: rapidRandomizeAccounts, + }, } for _, test := range tests { @@ -344,3 +358,228 @@ func TestAccountStoreMigration(t *testing.T) { }) } } + +// randomizeAccounts adds 10 randomized accounts to the kvStore, each with +// 50-1000 invoices and payments. The accounts are randomized in terms of +// balance, expiry, number of invoices and payments, and payment status. +func randomizeAccounts(t *testing.T, kvStore *BoltStore) { + ctx := context.Background() + + var ( + // numberOfAccounts is set to 10 to add enough accounts to get + // enough variation between number of invoices and payments, but + // kept low enough for the test not take too long to run, as the + // test time increases drastically by the number of accounts we + // migrate. + numberOfAccounts = 10 + invoiceCounter uint64 = 0 + ) + + for i := 0; i < numberOfAccounts; i++ { + label := fmt.Sprintf("account%d", i) + + // Generate a random balance between 1,000 and 100,000,000. + balance := lnwire.MilliSatoshi( + rand.Int63n(100000000-1000) + 1000, + ) + + // Generate a random expiry between 10 and 10,000 minutes. + expiry := time.Now().Add( + time.Minute * time.Duration(rand.Intn(10000-10)+10), + ) + + acct, err := kvStore.NewAccount(ctx, balance, expiry, label) + require.NoError(t, err) + + // Add between 50 and 1000 invoices for the account. + numberOfInvoices := rand.Intn(1000-50) + 50 + for j := 0; j < numberOfInvoices; j++ { + invoiceCounter++ + + var rHash lntypes.Hash + _, err := rand.Read(rHash[:]) + require.NoError(t, err) + + err = kvStore.AddAccountInvoice(ctx, acct.ID, rHash) + require.NoError(t, err) + + err = kvStore.StoreLastIndexes(ctx, invoiceCounter, 0) + require.NoError(t, err) + } + + // Add between 50 and 1000 payments for the account. + numberOfPayments := rand.Intn(1000-50) + 50 + for j := 0; j < numberOfPayments; j++ { + var rHash lntypes.Hash + _, err := rand.Read(rHash[:]) + require.NoError(t, err) + + // Generate a random payment amount from 1,000 to + // 100,000,000. + amt := lnwire.MilliSatoshi( + rand.Int63n(100000000-1000) + 1000, + ) + + // Ensure that we get an almost equal amount of + // different payment statuses for the payments. + status := paymentStatus(j) + + known, err := kvStore.UpsertAccountPayment( + ctx, acct.ID, rHash, amt, status, + ) + require.NoError(t, err) + require.False(t, known) + } + } +} + +// rapidRandomizeAccounts is a rapid test that generates randomized +// accounts using rapid, invoices and payments, and inserts them into the +// kvStore. Each account is generated with a random balance, expiry, label, +// and a random number of 20-100 invoices and payments. The invoices and +// payments are also generated with random hashes and amounts. +func rapidRandomizeAccounts(t *testing.T, kvStore *BoltStore) { + invoiceCounter := uint64(0) + + ctx := context.Background() + + rapid.Check(t, func(t *rapid.T) { + // Generate the randomized account for this check run. + acct := makeAccountGen().Draw(t, "account") + + // Then proceed to insert the account with its invoices and + // payments into the db + newAcct, err := kvStore.NewAccount( + ctx, acct.balance, acct.expiry, acct.label, + ) + require.NoError(t, err) + + for _, invoiceHash := range acct.invoices { + invoiceCounter++ + + err := kvStore.AddAccountInvoice( + ctx, newAcct.ID, invoiceHash, + ) + require.NoError(t, err) + + err = kvStore.StoreLastIndexes(ctx, invoiceCounter, 0) + require.NoError(t, err) + } + + for _, pmt := range acct.payments { + // Note that as rapid can generate multiple payments + // of the same values, we cannot be sure that the + // payment is unknown. + _, err := kvStore.UpsertAccountPayment( + ctx, newAcct.ID, pmt.hash, pmt.amt, pmt.status, + ) + require.NoError(t, err) + } + }) +} + +// makeAccountGen returns a rapid generator that generates accounts, with +// random labels, balances, expiry times, and between 20-100 randomly generated +// invoices and payments. The invoices and payments are also generated with +// random hashes and amounts. +func makeAccountGen() *rapid.Generator[account] { + return rapid.Custom(func(t *rapid.T) account { + // As the store has a unique constraint for inserting labels, + // we don't use rapid to generate it, and instead use + // sufficiently large random number as the account suffix to + // avoid collisions. + label := fmt.Sprintf("account:%d", rand.Int63()) + + balance := lnwire.MilliSatoshi( + rapid.Int64Range(1000, 100000000).Draw( + t, fmt.Sprintf("balance_%s", label), + ), + ) + + expiry := time.Now().Add( + time.Duration( + rapid.IntRange(10, 10000).Draw( + t, fmt.Sprintf("expiry_%s", label), + ), + ) * time.Minute, + ) + + // Generate the random invoices + numInvoices := rapid.IntRange(20, 100).Draw( + t, fmt.Sprintf("numInvoices_%s", label), + ) + invoices := make([]lntypes.Hash, numInvoices) + for i := range invoices { + invoices[i] = randomHash( + t, fmt.Sprintf("invoiceHash_%s_%d", label, i), + ) + } + + // Generate the random payments + numPayments := rapid.IntRange(20, 100).Draw( + t, fmt.Sprintf("numPayments_%s", label), + ) + payments := make([]payment, numPayments) + for i := range payments { + hashName := fmt.Sprintf("paymentHash_%s_%d", label, i) + amtName := fmt.Sprintf("amt_%s_%d", label, i) + + payments[i] = payment{ + hash: randomHash(t, hashName), + amt: lnwire.MilliSatoshi( + rapid.Int64Range(1000, 100000000).Draw( + t, amtName, + ), + ), + status: paymentStatus(i), + } + } + + return account{ + label: label, + balance: balance, + expiry: expiry, + invoices: invoices, + payments: payments, + } + }) +} + +// randomHash generates a random hash of 32 bytes. It uses rapid to generate +// the random bytes, and then copies them into a lntypes.Hash struct. +func randomHash(t *rapid.T, name string) lntypes.Hash { + hashBytes := rapid.SliceOfN(rapid.Byte(), 32, 32).Draw(t, name) + var hash lntypes.Hash + copy(hash[:], hashBytes) + return hash +} + +// paymentStatus returns a payment status based on the given index by taking +// the index modulo 4. This ensures an approximately equal distribution of +// different payment statuses across payments. +func paymentStatus(i int) lnrpc.Payment_PaymentStatus { + switch i % 4 { + case 0: + return lnrpc.Payment_SUCCEEDED + case 1: + return lnrpc.Payment_IN_FLIGHT + case 2: + return lnrpc.Payment_UNKNOWN + default: + return lnrpc.Payment_FAILED + } +} + +type account struct { + label string + balance lnwire.MilliSatoshi + expiry time.Time + invoices []lntypes.Hash + payments []payment +} + +type payment struct { + hash lntypes.Hash + amt lnwire.MilliSatoshi + status lnrpc.Payment_PaymentStatus +} diff --git a/go.mod b/go.mod index 2ee741358..ce06dfb47 100644 --- a/go.mod +++ b/go.mod @@ -54,6 +54,7 @@ require ( gopkg.in/macaroon-bakery.v2 v2.1.0 gopkg.in/macaroon.v2 v2.1.0 modernc.org/sqlite v1.34.5 + pgregory.net/rapid v1.2.0 ) require ( @@ -222,7 +223,6 @@ require ( modernc.org/mathutil v1.6.0 // indirect modernc.org/memory v1.8.0 // indirect nhooyr.io/websocket v1.8.7 // indirect - pgregory.net/rapid v1.2.0 // indirect sigs.k8s.io/yaml v1.2.0 // indirect )