From 64606f0132490fe14da66d7d28f3cde27df80641 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sat, 4 Jan 2025 13:57:32 +0200 Subject: [PATCH 1/4] accounts: add new UpdateAccountBalanceAndExpiry Store method In this commit, we remove one call to the UpdateAccount store method and replace it with a call to a new UpdateAccountBalanceAndExpiry method which updates an accounts balance and/or expiry fields and finds the account via the given ID. This method signature is more appropriate for a SQL backend than the UpdateAccount method. --- accounts/interface.go | 7 ++++ accounts/service.go | 21 ++++++------ accounts/store_kvdb.go | 64 ++++++++++++++++++++++++++++++++++++ accounts/store_test.go | 74 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 11 deletions(-) diff --git a/accounts/interface.go b/accounts/interface.go index c9c1e78e0..a1f6378ae 100644 --- a/accounts/interface.go +++ b/accounts/interface.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" @@ -219,6 +220,12 @@ type Store interface { // Accounts retrieves all accounts from the store and un-marshals them. Accounts(ctx context.Context) ([]*OffChainBalanceAccount, error) + // UpdateAccountBalanceAndExpiry updates the balance and/or expiry of an + // account. + UpdateAccountBalanceAndExpiry(ctx context.Context, id AccountID, + newBalance fn.Option[lnwire.MilliSatoshi], + newExpiry fn.Option[time.Time]) error + // RemoveAccount finds an account by its ID and removes it from the¨ // store. RemoveAccount(ctx context.Context, id AccountID) error diff --git a/accounts/service.go b/accounts/service.go index 820dad23e..8720addd8 100644 --- a/accounts/service.go +++ b/accounts/service.go @@ -9,8 +9,8 @@ import ( "github.com/btcsuite/btcd/chaincfg" "github.com/lightninglabs/lndclient" - "github.com/lightninglabs/taproot-assets/fn" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/fn" invpkg "github.com/lightningnetwork/lnd/invoices" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntypes" @@ -313,36 +313,35 @@ func (s *InterceptorService) UpdateAccount(ctx context.Context, return nil, ErrAccountServiceDisabled } - account, err := s.store.Account(ctx, accountID) - if err != nil { - return nil, fmt.Errorf("error fetching account: %w", err) - } - // If the expiration date was set, parse it as a unix time stamp. A // value of -1 signals "don't update the expiration date". + var expiry fn.Option[time.Time] if expirationDate > 0 { - account.ExpirationDate = time.Unix(expirationDate, 0) + expiry = fn.Some(time.Unix(expirationDate, 0)) } else if expirationDate == 0 { // Setting the expiration to 0 means don't expire in which case // we use a zero time (zero unix time would still be 1970, so // that doesn't work for us). - account.ExpirationDate = time.Time{} + expiry = fn.Some(time.Time{}) } // 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] if accountBalance >= 0 { // Convert from satoshis to millisatoshis for storage. - account.CurrentBalance = int64(accountBalance) * 1000 + balance = fn.Some(lnwire.MilliSatoshi(accountBalance) * 1000) } // Create the actual account in the macaroon account store. - err = s.store.UpdateAccount(ctx, account) + err := s.store.UpdateAccountBalanceAndExpiry( + ctx, accountID, balance, expiry, + ) if err != nil { return nil, fmt.Errorf("unable to update account: %w", err) } - return account, nil + return s.store.Account(ctx, accountID) } // Account retrieves an account from the bolt DB and un-marshals it. If the diff --git a/accounts/store_kvdb.go b/accounts/store_kvdb.go index 84f22f891..7f57e8dc0 100644 --- a/accounts/store_kvdb.go +++ b/accounts/store_kvdb.go @@ -11,6 +11,7 @@ import ( "time" "github.com/btcsuite/btcwallet/walletdb" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnwire" "go.etcd.io/bbolt" @@ -194,6 +195,56 @@ func (s *BoltStore) UpdateAccount(_ context.Context, }, func() {}) } +// UpdateAccountBalanceAndExpiry updates the balance and/or expiry of an +// account. +// +// NOTE: This is part of the Store interface. +func (s *BoltStore) UpdateAccountBalanceAndExpiry(_ context.Context, + id AccountID, newBalance fn.Option[lnwire.MilliSatoshi], + newExpiry fn.Option[time.Time]) error { + + update := func(account *OffChainBalanceAccount) error { + newBalance.WhenSome(func(balance lnwire.MilliSatoshi) { + account.CurrentBalance = int64(balance) + }) + newExpiry.WhenSome(func(expiry time.Time) { + account.ExpirationDate = expiry + }) + + return nil + } + + return s.updateAccount(id, update) +} + +func (s *BoltStore) updateAccount(id AccountID, + updateFn func(*OffChainBalanceAccount) error) error { + + return s.db.Update(func(tx kvdb.RwTx) error { + bucket := tx.ReadWriteBucket(accountBucketName) + if bucket == nil { + return ErrAccountBucketNotFound + } + + account, err := getAccount(bucket, id) + if err != nil { + return fmt.Errorf("error fetching account, %w", err) + } + + err = updateFn(account) + if err != nil { + return fmt.Errorf("error updating account, %w", err) + } + + err = storeAccount(bucket, account) + if err != nil { + return fmt.Errorf("error storing account, %w", err) + } + + return nil + }, func() {}) +} + // storeAccount serializes and writes the given account to the given account // bucket. func storeAccount(accountBucket kvdb.RwBucket, @@ -209,6 +260,19 @@ func storeAccount(accountBucket kvdb.RwBucket, return accountBucket.Put(account.ID[:], accountBinary) } +// getAccount retrieves an account from the given account bucket and +// deserializes it. +func getAccount(accountBucket kvdb.RwBucket, id AccountID) ( + *OffChainBalanceAccount, error) { + + accountBinary := accountBucket.Get(id[:]) + if len(accountBinary) == 0 { + return nil, ErrAccNotFound + } + + return deserializeAccount(accountBinary) +} + // uniqueRandomAccountID generates a new random ID and makes sure it does not // yet exist in the DB. func uniqueRandomAccountID(accountBucket kvdb.RBucket) (AccountID, error) { diff --git a/accounts/store_test.go b/accounts/store_test.go index b7167c3cf..5f0ee6ef1 100644 --- a/accounts/store_test.go +++ b/accounts/store_test.go @@ -5,8 +5,10 @@ import ( "testing" "time" + "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" ) @@ -105,6 +107,78 @@ func assertEqualAccounts(t *testing.T, expected, actual.LastUpdate = actualUpdate } +// TestAccountUpdateMethods tests that all the Store methods that update an +// account work correctly. +func TestAccountUpdateMethods(t *testing.T) { + t.Parallel() + ctx := context.Background() + + t.Run("UpdateAccountBalanceAndExpiry", func(t *testing.T) { + store := NewTestDB(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](), + fn.None[time.Time](), + ) + require.ErrorIs(t, err, ErrAccNotFound) + + acct, err := store.NewAccount(ctx, 0, time.Time{}, "foo") + require.NoError(t, err) + + assertBalanceAndExpiry := func(balance lnwire.MilliSatoshi, + expiry time.Time) { + + dbAcct, err := store.Account(ctx, acct.ID) + require.NoError(t, err) + require.EqualValues(t, balance, dbAcct.CurrentBalance) + require.WithinDuration( + t, expiry, dbAcct.ExpirationDate, 0, + ) + } + + // Get the account from the store and check to see what its + // initial balance and expiry fields are set to. + assertBalanceAndExpiry(0, time.Time{}) + + // Now, update just the balance of the account. + newBalance := lnwire.MilliSatoshi(123) + err = store.UpdateAccountBalanceAndExpiry( + ctx, acct.ID, fn.Some(newBalance), fn.None[time.Time](), + ) + require.NoError(t, err) + assertBalanceAndExpiry(newBalance, time.Time{}) + + // Now update just the expiry of the account. + newExpiry := time.Now().Add(time.Hour) + err = store.UpdateAccountBalanceAndExpiry( + ctx, acct.ID, fn.None[lnwire.MilliSatoshi](), + fn.Some(newExpiry), + ) + require.NoError(t, err) + assertBalanceAndExpiry(newBalance, newExpiry) + + // Update both the balance and expiry of the account. + newBalance = 456 + newExpiry = time.Now().Add(2 * time.Hour) + err = store.UpdateAccountBalanceAndExpiry( + ctx, acct.ID, fn.Some(newBalance), fn.Some(newExpiry), + ) + require.NoError(t, err) + assertBalanceAndExpiry(newBalance, newExpiry) + + // 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](), + ) + require.NoError(t, err) + assertBalanceAndExpiry(newBalance, newExpiry) + }) +} + // TestLastInvoiceIndexes makes sure the last known invoice indexes can be // stored and retrieved correctly. func TestLastInvoiceIndexes(t *testing.T) { From 5a4b22f6e7d897796fede91628443b541d63ce35 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sat, 4 Jan 2025 14:22:59 +0200 Subject: [PATCH 2/4] accounts: add new AddAccountInvoice Store method And use it instead of UpdateAccount for the InterceptorService's AssociateInvoice method. --- accounts/interface.go | 4 ++++ accounts/service.go | 8 +++---- accounts/store_kvdb.go | 16 ++++++++++++++ accounts/store_test.go | 47 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 4 deletions(-) diff --git a/accounts/interface.go b/accounts/interface.go index a1f6378ae..82b546844 100644 --- a/accounts/interface.go +++ b/accounts/interface.go @@ -226,6 +226,10 @@ type Store interface { newBalance fn.Option[lnwire.MilliSatoshi], newExpiry fn.Option[time.Time]) error + // AddAccountInvoice adds an invoice hash to an account. + AddAccountInvoice(ctx context.Context, id AccountID, + hash lntypes.Hash) error + // RemoveAccount finds an account by its ID and removes it from the¨ // store. RemoveAccount(ctx context.Context, id AccountID) error diff --git a/accounts/service.go b/accounts/service.go index 8720addd8..2e1de007e 100644 --- a/accounts/service.go +++ b/accounts/service.go @@ -438,15 +438,15 @@ func (s *InterceptorService) AssociateInvoice(ctx context.Context, id AccountID, s.Lock() defer s.Unlock() - account, err := s.store.Account(ctx, id) + err := s.store.AddAccountInvoice(ctx, id, hash) if err != nil { - return err + return fmt.Errorf("error adding invoice to account: %w", err) } - account.Invoices[hash] = struct{}{} + // If the above was successful, then we update our in-memory map. s.invoiceToAccount[hash] = id - return s.store.UpdateAccount(ctx, account) + return nil } // PaymentErrored removes a pending payment from the account's registered diff --git a/accounts/store_kvdb.go b/accounts/store_kvdb.go index 7f57e8dc0..5e1fc82d6 100644 --- a/accounts/store_kvdb.go +++ b/accounts/store_kvdb.go @@ -13,6 +13,7 @@ import ( "github.com/btcsuite/btcwallet/walletdb" "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "go.etcd.io/bbolt" ) @@ -217,6 +218,21 @@ func (s *BoltStore) UpdateAccountBalanceAndExpiry(_ context.Context, return s.updateAccount(id, update) } +// AddAccountInvoice adds an invoice hash to the account with the given ID. +// +// NOTE: This is part of the Store interface. +func (s *BoltStore) AddAccountInvoice(_ context.Context, id AccountID, + hash lntypes.Hash) error { + + update := func(account *OffChainBalanceAccount) error { + account.Invoices[hash] = struct{}{} + + return nil + } + + return s.updateAccount(id, update) +} + func (s *BoltStore) updateAccount(id AccountID, updateFn func(*OffChainBalanceAccount) error) error { diff --git a/accounts/store_test.go b/accounts/store_test.go index 5f0ee6ef1..dfc550428 100644 --- a/accounts/store_test.go +++ b/accounts/store_test.go @@ -177,6 +177,53 @@ func TestAccountUpdateMethods(t *testing.T) { require.NoError(t, err) assertBalanceAndExpiry(newBalance, newExpiry) }) + + t.Run("AddAccountInvoice", func(t *testing.T) { + store := NewTestDB(t) + + acct, err := store.NewAccount(ctx, 0, time.Time{}, "foo") + require.NoError(t, err) + + assertInvoices := func(invoices ...lntypes.Hash) { + dbAcct, err := store.Account(ctx, acct.ID) + require.NoError(t, err) + + // First make sure the number of invoices match before + // de-duping the hashes. + require.Len(t, dbAcct.Invoices, len(invoices)) + + dbInvs := make([]lntypes.Hash, 0, len(dbAcct.Invoices)) + for hash := range dbAcct.Invoices { + dbInvs = append(dbInvs, hash) + } + + require.ElementsMatch(t, invoices, dbInvs) + } + + // The account initially has no invoices. + assertInvoices() + + // Add an invoice to the account. + hash1 := lntypes.Hash{1, 2, 3, 4} + err = store.AddAccountInvoice(ctx, acct.ID, hash1) + require.NoError(t, err) + + assertInvoices(hash1) + + // Assert that adding the same invoice again does not change the + // state. + err = store.AddAccountInvoice(ctx, acct.ID, hash1) + require.NoError(t, err) + + assertInvoices(hash1) + + // Now add a second invoice. + hash2 := lntypes.Hash{5, 6, 7, 8} + err = store.AddAccountInvoice(ctx, acct.ID, hash2) + require.NoError(t, err) + + assertInvoices(hash1, hash2) + }) } // TestLastInvoiceIndexes makes sure the last known invoice indexes can be From 0580d9acf08b3e44a889f3b56c62f2826ff41513 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sat, 4 Jan 2025 14:33:24 +0200 Subject: [PATCH 3/4] accounts: add new IncreaseAccountBalance Store method And use it instead of UpdateAccount in the InterceptorService's invoiceUpdate method. --- accounts/interface.go | 5 +++++ accounts/service.go | 16 ++++------------ accounts/store_kvdb.go | 22 ++++++++++++++++++++++ accounts/store_test.go | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 12 deletions(-) diff --git a/accounts/interface.go b/accounts/interface.go index 82b546844..2542fab38 100644 --- a/accounts/interface.go +++ b/accounts/interface.go @@ -230,6 +230,11 @@ type Store interface { AddAccountInvoice(ctx context.Context, id AccountID, hash lntypes.Hash) error + // IncreaseAccountBalance increases the balance of the account with the + // given ID by the given amount. + IncreaseAccountBalance(ctx context.Context, id AccountID, + amount lnwire.MilliSatoshi) error + // RemoveAccount finds an account by its ID and removes it from the¨ // store. RemoveAccount(ctx context.Context, id AccountID) error diff --git a/accounts/service.go b/accounts/service.go index 2e1de007e..6ef861ecd 100644 --- a/accounts/service.go +++ b/accounts/service.go @@ -598,21 +598,13 @@ func (s *InterceptorService) invoiceUpdate(ctx context.Context, return nil } - account, err := s.store.Account(ctx, acctID) - if err != nil { - return s.disableAndErrorfUnsafe( - "error fetching account: %w", err, - ) - } - // If we get here, the current account has the invoice associated with // it that was just paid. Credit the amount to the account and update it // in the DB. - account.CurrentBalance += int64(invoice.AmountPaid) - if err := s.store.UpdateAccount(ctx, account); err != nil { - return s.disableAndErrorfUnsafe( - "error updating account: %w", err, - ) + err := s.store.IncreaseAccountBalance(ctx, acctID, invoice.AmountPaid) + if err != nil { + return s.disableAndErrorfUnsafe("error increasing account "+ + "balance account: %w", err) } // We've now fully processed the invoice and don't need to keep it diff --git a/accounts/store_kvdb.go b/accounts/store_kvdb.go index 5e1fc82d6..7c482ac7e 100644 --- a/accounts/store_kvdb.go +++ b/accounts/store_kvdb.go @@ -7,6 +7,7 @@ import ( "encoding/binary" "encoding/hex" "fmt" + "math" "os" "time" @@ -233,6 +234,27 @@ func (s *BoltStore) AddAccountInvoice(_ context.Context, id AccountID, return s.updateAccount(id, update) } +// IncreaseAccountBalance increases the balance of the account with the given ID +// by the given amount. +// +// NOTE: This is part of the Store interface. +func (s *BoltStore) IncreaseAccountBalance(_ context.Context, id AccountID, + amount lnwire.MilliSatoshi) error { + + update := func(account *OffChainBalanceAccount) error { + if amount > math.MaxInt64 { + return fmt.Errorf("amount %d exceeds the maximum of %d", + amount, math.MaxInt64) + } + + account.CurrentBalance += int64(amount) + + return nil + } + + return s.updateAccount(id, update) +} + func (s *BoltStore) updateAccount(id AccountID, updateFn func(*OffChainBalanceAccount) error) error { diff --git a/accounts/store_test.go b/accounts/store_test.go index dfc550428..85543ba50 100644 --- a/accounts/store_test.go +++ b/accounts/store_test.go @@ -203,6 +203,11 @@ func TestAccountUpdateMethods(t *testing.T) { // The account initially has no invoices. assertInvoices() + // Adding an invoice to an account that doesnt exist yet should + // error out. + err = store.AddAccountInvoice(ctx, AccountID{}, lntypes.Hash{}) + require.ErrorIs(t, err, ErrAccNotFound) + // Add an invoice to the account. hash1 := lntypes.Hash{1, 2, 3, 4} err = store.AddAccountInvoice(ctx, acct.ID, hash1) @@ -224,6 +229,34 @@ func TestAccountUpdateMethods(t *testing.T) { assertInvoices(hash1, hash2) }) + + t.Run("IncreaseAccountBalance", func(t *testing.T) { + store := NewTestDB(t) + + // Increasing the balance of an account that doesn't exist + // should error out. + err := store.IncreaseAccountBalance(ctx, AccountID{}, 100) + require.ErrorIs(t, err, ErrAccNotFound) + + acct, err := store.NewAccount(ctx, 123, time.Time{}, "foo") + require.NoError(t, err) + + assertBalance := func(balance int64) { + dbAcct, err := store.Account(ctx, acct.ID) + require.NoError(t, err) + require.EqualValues(t, balance, dbAcct.CurrentBalance) + } + + // The account initially has a balance of 123. + assertBalance(123) + + // Increase the balance by 100 and assert that the new balance + // is 223. + err = store.IncreaseAccountBalance(ctx, acct.ID, 100) + require.NoError(t, err) + + assertBalance(223) + }) } // TestLastInvoiceIndexes makes sure the last known invoice indexes can be From 34d1f67dd0b690199d7386a608511fb7b665878e Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Fri, 17 Jan 2025 08:37:20 +0200 Subject: [PATCH 4/4] accounts: update the service's UpdateAccount method ... to take a more strict btcutil.Amount type for the account balance parameter. --- accounts/rpcserver.go | 3 ++- accounts/service.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/accounts/rpcserver.go b/accounts/rpcserver.go index 5c1314820..79e3e674b 100644 --- a/accounts/rpcserver.go +++ b/accounts/rpcserver.go @@ -122,7 +122,8 @@ func (s *RPCServer) UpdateAccount(ctx context.Context, // Ask the service to update the account. account, err := s.service.UpdateAccount( - ctx, accountID, req.AccountBalance, req.ExpirationDate, + ctx, accountID, btcutil.Amount(req.AccountBalance), + req.ExpirationDate, ) if err != nil { return nil, err diff --git a/accounts/service.go b/accounts/service.go index 6ef861ecd..9db0e4395 100644 --- a/accounts/service.go +++ b/accounts/service.go @@ -7,6 +7,7 @@ import ( "sync" "time" + "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/lightninglabs/lndclient" "github.com/lightningnetwork/lnd/channeldb" @@ -299,7 +300,7 @@ func (s *InterceptorService) NewAccount(ctx context.Context, // UpdateAccount writes an account to the database, overwriting the existing one // if it exists. func (s *InterceptorService) UpdateAccount(ctx context.Context, - accountID AccountID, accountBalance, + accountID AccountID, accountBalance btcutil.Amount, expirationDate int64) (*OffChainBalanceAccount, error) { s.Lock()