Skip to content

Commit b4b36f6

Browse files
committed
session: handle removed linked account
If a user creates a session linked to an account, but later deletes that account using the `litcli accounts remove` command, the KVDB session will still contain the account ID of the removed account. During the KVDB to SQL migration, the accounts migration runs before the sessions migration. Since the deleted account no longer exists, it cannot be migrated. As a result, the migrated SQL session cannot link to the now non-existent account in the SQL store. In such cases, we will migrate the session and not link to any account in the SQL session.
1 parent ef3bf2b commit b4b36f6

File tree

2 files changed

+96
-1
lines changed

2 files changed

+96
-1
lines changed

session/sql_migration.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/davecgh/go-spew/spew"
1414
"github.com/lightninglabs/lightning-terminal/accounts"
1515
"github.com/lightninglabs/lightning-terminal/db/sqlc"
16+
"github.com/lightningnetwork/lnd/fn"
1617
"github.com/lightningnetwork/lnd/sqldb"
1718
"github.com/pmezard/go-difflib/difflib"
1819
"go.etcd.io/bbolt"
@@ -196,6 +197,7 @@ func migrateSessionsToSQLAndValidate(ctx context.Context,
196197
overrideSessionTimeZone(kvSession)
197198
overrideSessionTimeZone(migratedSession)
198199
overrideMacaroonRecipe(kvSession, migratedSession)
200+
overrideRemovedAccount(kvSession, migratedSession)
199201

200202
if !reflect.DeepEqual(kvSession, migratedSession) {
201203
diff := difflib.UnifiedDiff{
@@ -245,7 +247,17 @@ func migrateSingleSessionToSQL(ctx context.Context,
245247
var acctDBID int64
246248
acctDBID, err = tx.GetAccountIDByAlias(ctx, acctAlias)
247249
if errors.Is(err, sql.ErrNoRows) {
248-
err = accounts.ErrAccNotFound
250+
// If we can't find the account in the SQL store, it
251+
// most likely means that the user deleted the account
252+
// with the "litcli accounts remove" command after the
253+
// session was created. We therefore can't link the
254+
// SQL session to the account, and we therefore just
255+
// leave the acctID as sql.Null
256+
log.Warnf("Unable to find account %v in SQL store, "+
257+
"skipping linking session %x to account",
258+
acctAlias, session.ID)
259+
err = nil
260+
249261
return
250262
} else if err != nil {
251263
return
@@ -495,3 +507,26 @@ func overrideMacaroonRecipe(kvSession *Session, migratedSession *Session) {
495507
}
496508
}
497509
}
510+
511+
// overrideRemovedAccount modifies the kvSession to remove the account ID if the
512+
// migrated session does not have an account ID, while the kvSession has one.
513+
// This happens when the account was not found in the SQL store during the
514+
// migration, which can occur if the account was deleted after the session
515+
// was created.
516+
func overrideRemovedAccount(kvSession *Session, migratedSession *Session) {
517+
kvSession.AccountID = fn.ElimOption(
518+
migratedSession.AccountID,
519+
520+
// If the migrated session does not have a linked account, we
521+
// also remove it from the kv session.
522+
func() fn.Option[accounts.AccountID] {
523+
return fn.None[accounts.AccountID]()
524+
},
525+
526+
// If the migrated session has a linked account, we keep the
527+
// account on the kv session.
528+
func(id accounts.AccountID) fn.Option[accounts.AccountID] {
529+
return kvSession.AccountID
530+
},
531+
)
532+
}

session/sql_migration_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ func TestSessionsStoreMigration(t *testing.T) {
7676
// kvSession has such a recipe stored.
7777
overrideMacaroonRecipe(kvSession, sqlSession)
7878

79+
// We also need to override the account ID if the
80+
// kvSession has an account ID set, while the SQL
81+
// session doesn't.
82+
overrideRemovedAccount(kvSession, sqlSession)
83+
7984
assertEqualSessions(t, kvSession, sqlSession)
8085
}
8186

@@ -314,6 +319,51 @@ func TestSessionsStoreMigration(t *testing.T) {
314319
return getBoltStoreSessions(t, store)
315320
},
316321
},
322+
{
323+
name: "one session with a deleted linked account",
324+
populateDB: func(t *testing.T, store *BoltStore,
325+
acctStore accounts.Store) []*Session {
326+
327+
// Create an account with balance
328+
acct, err := acctStore.NewAccount(
329+
ctx, 1234, time.Now().Add(time.Hour),
330+
"",
331+
)
332+
require.NoError(t, err)
333+
require.False(t, acct.HasExpired())
334+
335+
// For now, we manually add the account caveat
336+
// for bbolt compatibility.
337+
accountCaveat := checkers.Condition(
338+
macaroons.CondLndCustom,
339+
fmt.Sprintf("%s %x",
340+
accounts.CondAccount,
341+
acct.ID[:],
342+
),
343+
)
344+
345+
sessCaveats := []macaroon.Caveat{
346+
{
347+
Id: []byte(accountCaveat),
348+
},
349+
}
350+
351+
_, err = store.NewSession(
352+
ctx, "test", TypeMacaroonAccount,
353+
time.Unix(1000, 0), "",
354+
WithAccount(acct.ID),
355+
WithMacaroonRecipe(sessCaveats, nil),
356+
)
357+
require.NoError(t, err)
358+
359+
// Now delete the account, which represents
360+
// that the user ran "litcli accounts remove".
361+
err = acctStore.RemoveAccount(ctx, acct.ID)
362+
require.NoError(t, err)
363+
364+
return getBoltStoreSessions(t, store)
365+
},
366+
},
317367
{
318368
name: "linked session",
319369
populateDB: func(t *testing.T, store *BoltStore,
@@ -664,6 +714,16 @@ func randomizedSessions(t *testing.T, kvStore *BoltStore,
664714
}
665715
}
666716

717+
// When an account is set, we also remove the account in 50% of
718+
// the cases. This simulates that the user ran "litcli accounts
719+
// remove" after creating the session.
720+
activeSess.AccountID.WhenSome(func(id accounts.AccountID) {
721+
if rand.Intn(2) == 0 {
722+
err = accountsStore.RemoveAccount(ctx, id)
723+
}
724+
})
725+
require.NoError(t, err)
726+
667727
// Finally, we shift the active session to a random state.
668728
// As the state we set may be a state that's no longer set
669729
// through the current code base, or be an illegal state

0 commit comments

Comments
 (0)