From 8b1c9a2893a8f771ece96ea796d1f0764a8920aa Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sun, 19 Jan 2025 19:07:08 +0200 Subject: [PATCH 1/4] accounts: revert UpdateAccountBalanceAndExpiry balance type to be int64 In our mission to replace UpdateAccount, we need UpdatAccountBalanceAndExpiry to take a negative balance so that we can use it to replace the behaviour of UpdateAccount as it stands today. --- accounts/interface.go | 2 +- accounts/service.go | 4 ++-- accounts/store_kvdb.go | 6 +++--- accounts/store_test.go | 13 +++++-------- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/accounts/interface.go b/accounts/interface.go index da3960891..c9154191b 100644 --- a/accounts/interface.go +++ b/accounts/interface.go @@ -223,7 +223,7 @@ type Store interface { // UpdateAccountBalanceAndExpiry updates the balance and/or expiry of an // account. UpdateAccountBalanceAndExpiry(ctx context.Context, id AccountID, - newBalance fn.Option[lnwire.MilliSatoshi], + newBalance fn.Option[int64], newExpiry fn.Option[time.Time]) error // AddAccountInvoice adds an invoice hash to an account. diff --git a/accounts/service.go b/accounts/service.go index 8023b5586..f84f12500 100644 --- a/accounts/service.go +++ b/accounts/service.go @@ -328,10 +328,10 @@ func (s *InterceptorService) UpdateAccount(ctx context.Context, // If the new account balance was set, parse it as millisatoshis. A // value of -1 signals "don't update the balance". - var balance fn.Option[lnwire.MilliSatoshi] + var balance fn.Option[int64] if accountBalance >= 0 { // Convert from satoshis to millisatoshis for storage. - balance = fn.Some(lnwire.MilliSatoshi(accountBalance) * 1000) + balance = fn.Some(int64(accountBalance) * 1000) } // Create the actual account in the macaroon account store. diff --git a/accounts/store_kvdb.go b/accounts/store_kvdb.go index e6b63b2c8..80ced6e82 100644 --- a/accounts/store_kvdb.go +++ b/accounts/store_kvdb.go @@ -208,12 +208,12 @@ func (s *BoltStore) UpdateAccount(_ context.Context, // // NOTE: This is part of the Store interface. func (s *BoltStore) UpdateAccountBalanceAndExpiry(_ context.Context, - id AccountID, newBalance fn.Option[lnwire.MilliSatoshi], + id AccountID, newBalance fn.Option[int64], newExpiry fn.Option[time.Time]) error { update := func(account *OffChainBalanceAccount) error { - newBalance.WhenSome(func(balance lnwire.MilliSatoshi) { - account.CurrentBalance = int64(balance) + newBalance.WhenSome(func(balance int64) { + account.CurrentBalance = balance }) newExpiry.WhenSome(func(expiry time.Time) { account.ExpirationDate = expiry diff --git a/accounts/store_test.go b/accounts/store_test.go index 544e688f6..bc7faf6da 100644 --- a/accounts/store_test.go +++ b/accounts/store_test.go @@ -9,7 +9,6 @@ import ( "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntypes" - "github.com/lightningnetwork/lnd/lnwire" "github.com/stretchr/testify/require" ) @@ -122,7 +121,7 @@ func TestAccountUpdateMethods(t *testing.T) { // Ensure that the function errors out if we try update an // account that does not exist. err := store.UpdateAccountBalanceAndExpiry( - ctx, AccountID{}, fn.None[lnwire.MilliSatoshi](), + ctx, AccountID{}, fn.None[int64](), fn.None[time.Time](), ) require.ErrorIs(t, err, ErrAccNotFound) @@ -130,7 +129,7 @@ func TestAccountUpdateMethods(t *testing.T) { acct, err := store.NewAccount(ctx, 0, time.Time{}, "foo") require.NoError(t, err) - assertBalanceAndExpiry := func(balance lnwire.MilliSatoshi, + assertBalanceAndExpiry := func(balance int64, expiry time.Time) { dbAcct, err := store.Account(ctx, acct.ID) @@ -146,7 +145,7 @@ func TestAccountUpdateMethods(t *testing.T) { assertBalanceAndExpiry(0, time.Time{}) // Now, update just the balance of the account. - newBalance := lnwire.MilliSatoshi(123) + newBalance := int64(123) err = store.UpdateAccountBalanceAndExpiry( ctx, acct.ID, fn.Some(newBalance), fn.None[time.Time](), ) @@ -156,8 +155,7 @@ func TestAccountUpdateMethods(t *testing.T) { // Now update just the expiry of the account. newExpiry := clock.Now().Add(time.Hour) err = store.UpdateAccountBalanceAndExpiry( - ctx, acct.ID, fn.None[lnwire.MilliSatoshi](), - fn.Some(newExpiry), + ctx, acct.ID, fn.None[int64](), fn.Some(newExpiry), ) require.NoError(t, err) assertBalanceAndExpiry(newBalance, newExpiry) @@ -174,8 +172,7 @@ func TestAccountUpdateMethods(t *testing.T) { // Finally, test an update that has no net changes to the // balance or expiry. err = store.UpdateAccountBalanceAndExpiry( - ctx, acct.ID, fn.None[lnwire.MilliSatoshi](), - fn.None[time.Time](), + ctx, acct.ID, fn.None[int64](), fn.None[time.Time](), ) require.NoError(t, err) assertBalanceAndExpiry(newBalance, newExpiry) From e965f4f6af1cb49630c07f2b3e5f3d0527375ac4 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sat, 4 Jan 2025 16:23:31 +0200 Subject: [PATCH 2/4] accounts: remove UpdateAccount calls from store_test Remove the call to UpdateAccount from the TestAccountStore test and instead replace it with all the other calls we have added. --- accounts/store_test.go | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/accounts/store_test.go b/accounts/store_test.go index bc7faf6da..3d44df3e7 100644 --- a/accounts/store_test.go +++ b/accounts/store_test.go @@ -38,7 +38,41 @@ func TestAccountStore(t *testing.T) { _, err = store.NewAccount(ctx, 123, time.Time{}, "0011223344556677") require.ErrorContains(t, err, "is not allowed as it can be mistaken") + now := clock.Now() + // Update all values of the account that we can modify. + // + // Update the balance and expiry. + err = store.UpdateAccountBalanceAndExpiry( + ctx, acct1.ID, fn.Some(int64(-500)), fn.Some(now), + ) + require.NoError(t, err) + + // Add 2 payments. + _, err = store.UpsertAccountPayment( + ctx, acct1.ID, lntypes.Hash{12, 34, 56, 78}, 123456, + lnrpc.Payment_FAILED, + ) + require.NoError(t, err) + + _, err = store.UpsertAccountPayment( + ctx, acct1.ID, lntypes.Hash{34, 56, 78, 90}, 789456123789, + lnrpc.Payment_SUCCEEDED, + ) + require.NoError(t, err) + + // Add 2 invoices. + err = store.AddAccountInvoice( + ctx, acct1.ID, lntypes.Hash{12, 34, 56, 78}, + ) + require.NoError(t, err) + err = store.AddAccountInvoice( + ctx, acct1.ID, lntypes.Hash{34, 56, 78, 90}, + ) + require.NoError(t, err) + + // Update the in-memory account so that we can compare it with the + // account we get from the store. acct1.CurrentBalance = -500 acct1.ExpirationDate = clock.Now() acct1.Payments[lntypes.Hash{12, 34, 56, 78}] = &PaymentEntry{ @@ -51,8 +85,6 @@ func TestAccountStore(t *testing.T) { } acct1.Invoices[lntypes.Hash{12, 34, 56, 78}] = struct{}{} acct1.Invoices[lntypes.Hash{34, 56, 78, 90}] = struct{}{} - err = store.UpdateAccount(ctx, acct1) - require.NoError(t, err) dbAccount, err = store.Account(ctx, acct1.ID) require.NoError(t, err) From 957c9a3a520eb3cf55b7128f54dc1281d8f5e569 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sat, 4 Jan 2025 16:33:21 +0200 Subject: [PATCH 3/4] accounts: remove UpdateAccount and last usages We remove the last few calls to UpdateAccount from the TestAccountService test. Previously, the UpdateAccount call was used in this test to also _insert_ new accounts and to force the account ID to be set to specific values. So here we need to instead call the store's NewAccount method to insert a new account, then we get the AccountID from the returned value and we need to then use this returned ID (which is no longer forceable) in the remainder of the tests. --- accounts/interface.go | 5 - accounts/service_test.go | 375 +++++++++++++++++++-------------------- accounts/store_kvdb.go | 17 -- 3 files changed, 185 insertions(+), 212 deletions(-) diff --git a/accounts/interface.go b/accounts/interface.go index c9154191b..11d3efe93 100644 --- a/accounts/interface.go +++ b/accounts/interface.go @@ -207,11 +207,6 @@ type Store interface { expirationDate time.Time, label string) ( *OffChainBalanceAccount, error) - // UpdateAccount writes an account to the database, overwriting the - // existing one if it exists. - UpdateAccount(ctx context.Context, - account *OffChainBalanceAccount) error - // Account retrieves an account from the Store and un-marshals it. If // the account cannot be found, then ErrAccNotFound is returned. Account(ctx context.Context, id AccountID) (*OffChainBalanceAccount, diff --git a/accounts/service_test.go b/accounts/service_test.go index bdf044df0..d6a79aeab 100644 --- a/accounts/service_test.go +++ b/accounts/service_test.go @@ -22,8 +22,6 @@ var ( testExpiration = time.Now().Add(24 * time.Hour) testTimeout = time.Millisecond * 500 testInterval = time.Millisecond * 20 - - testID2 = AccountID{22, 22, 22} ) type mockLnd struct { @@ -203,20 +201,22 @@ func TestAccountService(t *testing.T) { testCases := []struct { name string setup func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) + s *InterceptorService) []AccountID startupErr string validate func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) + ids []AccountID, s *InterceptorService) }{{ name: "startup err on invoice subscription", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + s *InterceptorService) []AccountID { lnd.invoiceSubscriptionErr = testErr + + return nil }, startupErr: testErr.Error(), validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { lnd.assertNoInvoiceRequest(t) require.False(t, s.IsRunning()) @@ -224,22 +224,20 @@ func TestAccountService(t *testing.T) { }, { name: "err on invoice update", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { - - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 1234, - Invoices: AccountInvoices{ - testHash: {}, - }, - } + s *InterceptorService) []AccountID { - err := s.store.UpdateAccount(ctx, acct) + acct, err := s.store.NewAccount( + ctx, 1234, testExpiration, "", + ) require.NoError(t, err) + + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash) + require.NoError(t, err) + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { // Start by closing the store. This should cause an // error once we make an invoice update, as the service @@ -269,22 +267,21 @@ func TestAccountService(t *testing.T) { }, { name: "err in invoice err channel", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { - - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 1234, - Invoices: AccountInvoices{ - testHash: {}, - }, - } + s *InterceptorService) []AccountID { - err := s.store.UpdateAccount(ctx, acct) + acct, err := s.store.NewAccount( + ctx, 1234, testExpiration, "", + ) require.NoError(t, err) + + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash) + require.NoError(t, err) + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { + // Ensure that the service was started successfully. require.True(t, s.IsRunning()) @@ -302,25 +299,22 @@ func TestAccountService(t *testing.T) { }, { name: "goroutine err sent on main err chan", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { - - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 1234, - Invoices: AccountInvoices{ - testHash: {}, - }, - Payments: make(AccountPayments), - } + s *InterceptorService) []AccountID { + + acct, err := s.store.NewAccount( + ctx, 1234, testExpiration, "", + ) + require.NoError(t, err) - err := s.store.UpdateAccount(ctx, acct) + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash) require.NoError(t, err) s.mainErrCallback(testErr) + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { lnd.assertInvoiceRequest(t, 0, 0) lnd.assertMainErrContains(t, testErr.Error()) @@ -328,24 +322,26 @@ func TestAccountService(t *testing.T) { }, { name: "startup do not track completed payments", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + s *InterceptorService) []AccountID { acct, err := s.store.NewAccount( ctx, 1234, testExpiration, "", ) require.NoError(t, err) - acct.Invoices[testHash] = struct{}{} - acct.Payments[testHash] = &PaymentEntry{ - Status: lnrpc.Payment_SUCCEEDED, - FullAmount: 1234, - } + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash) + require.NoError(t, err) - err = s.store.UpdateAccount(ctx, acct) + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash, 1234, + lnrpc.Payment_SUCCEEDED, + ) require.NoError(t, err) + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { require.Contains(t, s.invoiceToAccount, testHash) r.assertNoPaymentRequest(t) @@ -356,30 +352,28 @@ func TestAccountService(t *testing.T) { }, { name: "startup err on payment tracking", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { - - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 1234, - Invoices: AccountInvoices{ - testHash: {}, - }, - Payments: AccountPayments{ - testHash: { - Status: lnrpc.Payment_IN_FLIGHT, - FullAmount: 1234, - }, - }, - } + s *InterceptorService) []AccountID { - err := s.store.UpdateAccount(ctx, acct) + acct, err := s.store.NewAccount( + ctx, 1234, testExpiration, "", + ) + require.NoError(t, err) + + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash) + require.NoError(t, err) + + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash, 1234, + lnrpc.Payment_IN_FLIGHT, + ) require.NoError(t, err) r.trackPaymentErr = testErr + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { // Assert that the invoice subscription succeeded. require.Contains(t, s.invoiceToAccount, testHash) @@ -397,25 +391,23 @@ func TestAccountService(t *testing.T) { }, { name: "err on payment update", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { - - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 1234, - Payments: AccountPayments{ - testHash: { - Status: lnrpc.Payment_IN_FLIGHT, - FullAmount: 1234, - }, - }, - } + s *InterceptorService) []AccountID { + + acct, err := s.store.NewAccount( + ctx, 1234, testExpiration, "", + ) + require.NoError(t, err) - err := s.store.UpdateAccount(ctx, acct) + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash, 1234, + lnrpc.Payment_IN_FLIGHT, + ) require.NoError(t, err) + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { // Ensure that the service was started successfully, // and lnd contains the payment request. @@ -451,25 +443,23 @@ func TestAccountService(t *testing.T) { }, { name: "err in payment update chan", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { - - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 1234, - Payments: AccountPayments{ - testHash: { - Status: lnrpc.Payment_IN_FLIGHT, - FullAmount: 1234, - }, - }, - } + s *InterceptorService) []AccountID { - err := s.store.UpdateAccount(ctx, acct) + acct, err := s.store.NewAccount( + ctx, 1234, testExpiration, "", + ) require.NoError(t, err) + + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash, 1234, + lnrpc.Payment_IN_FLIGHT, + ) + require.NoError(t, err) + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { // Ensure that the service was started successfully, // and lnd contains the payment request. @@ -492,36 +482,40 @@ func TestAccountService(t *testing.T) { }, { name: "startup track in-flight payments", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { - - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 5000, - Invoices: AccountInvoices{ - testHash: {}, - }, - Payments: AccountPayments{ - testHash: { - Status: lnrpc.Payment_IN_FLIGHT, - FullAmount: 2000, - }, - testHash2: { - Status: lnrpc.Payment_UNKNOWN, - FullAmount: 1000, - }, - testHash3: { - Status: lnrpc.Payment_UNKNOWN, - FullAmount: 2000, - }, - }, - } + s *InterceptorService) []AccountID { - err := s.store.UpdateAccount(ctx, acct) + acct, err := s.store.NewAccount( + ctx, 5000, testExpiration, "", + ) + require.NoError(t, err) + + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash, 2000, + lnrpc.Payment_IN_FLIGHT, + ) + require.NoError(t, err) + + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash2, 1000, + lnrpc.Payment_UNKNOWN, + ) require.NoError(t, err) + + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash3, 2000, + lnrpc.Payment_UNKNOWN, + ) + require.NoError(t, err) + + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash) + require.NoError(t, err) + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { + + testID := ids[0] require.Contains(t, s.invoiceToAccount, testHash) r.assertPaymentRequests(t, map[lntypes.Hash]struct{}{ @@ -625,13 +619,15 @@ func TestAccountService(t *testing.T) { }, { name: "keep track of invoice indexes", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + s *InterceptorService) []AccountID { err := s.store.StoreLastIndexes(ctx, 987_654, 555_555) require.NoError(t, err) + + return nil }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { // We expect the initial subscription to start at the // indexes we stored in the DB. @@ -680,24 +676,21 @@ func TestAccountService(t *testing.T) { }, { name: "credit account", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { - - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 0, - Invoices: AccountInvoices{ - testHash: {}, - testHash2: {}, - }, - Payments: make(AccountPayments), - } + s *InterceptorService) []AccountID { - err := s.store.UpdateAccount(ctx, acct) + acct, err := s.store.NewAccount(ctx, 0, time.Time{}, "") require.NoError(t, err) + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash) + require.NoError(t, err) + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash2) + require.NoError(t, err) + + return []AccountID{acct.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { + + testID := ids[0] lnd.assertInvoiceRequest(t, 0, 0) lnd.invoiceChan <- &lndclient.Invoice{ @@ -737,56 +730,55 @@ func TestAccountService(t *testing.T) { }, { name: "in-flight payments", setup: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + s *InterceptorService) []AccountID { // We set up two accounts with a balance of 5k msats. // The first account has two in-flight payments, one of // 2k msats and one of 1k msats, totaling 3k msats. - acct := &OffChainBalanceAccount{ - ID: testID, - Type: TypeInitialBalance, - CurrentBalance: 5000, - Invoices: AccountInvoices{ - testHash: {}, - }, - Payments: AccountPayments{ - testHash: { - Status: lnrpc.Payment_IN_FLIGHT, - FullAmount: 2000, - }, - testHash2: { - Status: lnrpc.Payment_IN_FLIGHT, - FullAmount: 1000, - }, - }, - } + acct, err := s.store.NewAccount( + ctx, 5000, time.Time{}, "", + ) + require.NoError(t, err) + + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash, 2000, + lnrpc.Payment_IN_FLIGHT, + ) + require.NoError(t, err) + + _, err = s.store.UpsertAccountPayment( + ctx, acct.ID, testHash2, 1000, + lnrpc.Payment_IN_FLIGHT, + ) + require.NoError(t, err) - err := s.store.UpdateAccount(ctx, acct) + err = s.store.AddAccountInvoice(ctx, acct.ID, testHash) require.NoError(t, err) // The second account has one in-flight payment of 4k // msats. - acct2 := &OffChainBalanceAccount{ - ID: testID2, - Type: TypeInitialBalance, - CurrentBalance: 5000, - Invoices: AccountInvoices{ - testHash: {}, - }, - Payments: AccountPayments{ - testHash3: { - Status: lnrpc.Payment_IN_FLIGHT, - FullAmount: 4000, - }, - }, - } + acct2, err := s.store.NewAccount( + ctx, 5000, time.Time{}, "", + ) + require.NoError(t, err) + + _, err = s.store.UpsertAccountPayment( + ctx, acct2.ID, testHash3, 4000, + lnrpc.Payment_IN_FLIGHT, + ) + require.NoError(t, err) - err = s.store.UpdateAccount(ctx, acct2) + err = s.store.AddAccountInvoice(ctx, acct2.ID, testHash) require.NoError(t, err) + + return []AccountID{acct.ID, acct2.ID} }, validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, - s *InterceptorService) { + ids []AccountID, s *InterceptorService) { + + testID := ids[0] + testID2 := ids[1] // The first should be able to initiate another payment // with an amount smaller or equal to 2k msats. This @@ -823,10 +815,8 @@ func TestAccountService(t *testing.T) { }} for _, tc := range testCases { - tc := tc - - t.Run(tc.name, func(tt *testing.T) { - tt.Parallel() + t.Run(tc.name, func(t *testing.T) { + t.Parallel() lndMock := newMockLnd() routerMock := newMockRouter() @@ -837,10 +827,13 @@ func TestAccountService(t *testing.T) { service, err := NewService(store, errFunc) require.NoError(t, err) - // Is a setup call required to initialize initial - // conditions? + // Is a setup call required to initialize + // initial conditions? + var accountIDs []AccountID if tc.setup != nil { - tc.setup(t, lndMock, routerMock, service) + accountIDs = tc.setup( + t, lndMock, routerMock, service, + ) } // Any errors during startup expected? @@ -848,14 +841,13 @@ func TestAccountService(t *testing.T) { ctx, lndMock, routerMock, chainParams, ) if tc.startupErr != "" { - require.ErrorContains(tt, err, tc.startupErr) - + require.ErrorContains(t, err, tc.startupErr) lndMock.assertNoMainErr(t) if tc.validate != nil { tc.validate( - tt, lndMock, routerMock, - service, + t, lndMock, routerMock, + accountIDs, service, ) } @@ -864,11 +856,14 @@ func TestAccountService(t *testing.T) { // Any post execution validation that we need to run? if tc.validate != nil { - tc.validate(tt, lndMock, routerMock, service) + tc.validate( + t, lndMock, routerMock, + accountIDs, service, + ) } err = service.Stop() - require.NoError(tt, err) + require.NoError(t, err) lndMock.assertNoMainErr(t) }) } diff --git a/accounts/store_kvdb.go b/accounts/store_kvdb.go index 80ced6e82..355b41795 100644 --- a/accounts/store_kvdb.go +++ b/accounts/store_kvdb.go @@ -186,23 +186,6 @@ func (s *BoltStore) NewAccount(ctx context.Context, balance lnwire.MilliSatoshi, return account, nil } -// UpdateAccount writes an account to the database, overwriting the existing one -// if it exists. -// -// NOTE: This is part of the Store interface. -func (s *BoltStore) UpdateAccount(_ context.Context, - account *OffChainBalanceAccount) error { - - return s.db.Update(func(tx kvdb.RwTx) error { - bucket := tx.ReadWriteBucket(accountBucketName) - if bucket == nil { - return ErrAccountBucketNotFound - } - - return s.storeAccount(bucket, account) - }, func() {}) -} - // UpdateAccountBalanceAndExpiry updates the balance and/or expiry of an // account. // From ef78bc4d9d9b06b5133939911c76ab82fb608257 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 23 Jan 2025 09:07:58 +0200 Subject: [PATCH 4/4] accounts: assert no startup error in tests In the case where no startup error is expected, we should assert that no error is returned from Start. --- accounts/service_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/accounts/service_test.go b/accounts/service_test.go index d6a79aeab..1d4388664 100644 --- a/accounts/service_test.go +++ b/accounts/service_test.go @@ -372,6 +372,7 @@ func TestAccountService(t *testing.T) { return []AccountID{acct.ID} }, + startupErr: testErr.Error(), validate: func(t *testing.T, lnd *mockLnd, r *mockRouter, ids []AccountID, s *InterceptorService) { @@ -852,6 +853,8 @@ func TestAccountService(t *testing.T) { } return + } else { + require.NoError(t, err) } // Any post execution validation that we need to run?