Skip to content

Commit 1196e77

Browse files
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 f71840e commit 1196e77

File tree

2 files changed

+88
-1
lines changed

2 files changed

+88
-1
lines changed

session/sql_migration.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/davecgh/go-spew/spew"
1313
"github.com/lightninglabs/lightning-terminal/accounts"
1414
"github.com/lightninglabs/lightning-terminal/db/sqlc"
15+
"github.com/lightningnetwork/lnd/fn"
1516
"github.com/lightningnetwork/lnd/sqldb"
1617
"github.com/pmezard/go-difflib/difflib"
1718
"go.etcd.io/bbolt"
@@ -147,6 +148,7 @@ func migrateSessionsToSQLAndValidate(ctx context.Context,
147148
overrideSessionTimeZone(kvSession)
148149
overrideSessionTimeZone(migratedSession)
149150
overrideMacaroonRecipe(kvSession, migratedSession)
151+
overrideRemovedAccount(kvSession, migratedSession)
150152

151153
if !reflect.DeepEqual(kvSession, migratedSession) {
152154
diff := difflib.UnifiedDiff{
@@ -196,7 +198,17 @@ func migrateSingleSessionToSQL(ctx context.Context,
196198
var acctDBID int64
197199
acctDBID, err = tx.GetAccountIDByAlias(ctx, acctAlias)
198200
if errors.Is(err, sql.ErrNoRows) {
199-
err = accounts.ErrAccNotFound
201+
// If we can't find the account in the SQL store, it
202+
// most likely means that the user deleted the account
203+
// with the "litcli accounts remove" command after the
204+
// session was created. We therefore can't link the
205+
// SQL session to the account, and we therefore just
206+
// leave the acctID as sql.Null
207+
log.Warnf("Unable to find account %v in SQL store, "+
208+
"skipping linking session %v to account",
209+
acctAlias, session.ID)
210+
err = nil
211+
200212
return
201213
} else if err != nil {
202214
return
@@ -394,3 +406,21 @@ func overrideMacaroonRecipe(kvSession *Session, migratedSession *Session) {
394406
}
395407
}
396408
}
409+
410+
// overrideRemovedAccount modifies the kvSession to remove the account ID if the
411+
// migrated session does not have an account ID, while the kvSession has one.
412+
// This happens when the account was not found in the SQL store during the
413+
// migration, which can occur if the account was deleted after the session
414+
// was created.
415+
func overrideRemovedAccount(kvSession *Session, migratedSession *Session) {
416+
if migratedSession.AccountID.IsSome() {
417+
return
418+
}
419+
420+
// There was a linked account in the kv session, but not in the
421+
// migrated session. We therefore remove the account ID from the
422+
// kvSession.
423+
kvSession.AccountID.WhenSome(func(alias accounts.AccountID) {
424+
kvSession.AccountID = fn.None[accounts.AccountID]()
425+
})
426+
}

session/sql_migration_test.go

Lines changed: 57 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

@@ -291,6 +296,48 @@ func TestSessionsStoreMigration(t *testing.T) {
291296
require.NoError(t, err)
292297
},
293298
},
299+
{
300+
name: "one session with a deleted linked account",
301+
populateDB: func(t *testing.T, store *BoltStore,
302+
acctStore accounts.Store) {
303+
304+
// Create an account with balance
305+
acct, err := acctStore.NewAccount(
306+
ctx, 1234, time.Now().Add(time.Hour),
307+
"",
308+
)
309+
require.NoError(t, err)
310+
require.False(t, acct.HasExpired())
311+
312+
// For now, we manually add the account caveat
313+
// for bbolt compatibility.
314+
accountCaveat := checkers.Condition(
315+
macaroons.CondLndCustom,
316+
fmt.Sprintf("%s %x",
317+
accounts.CondAccount,
318+
acct.ID[:],
319+
),
320+
)
321+
322+
sessCaveats := []macaroon.Caveat{
323+
{
324+
Id: []byte(accountCaveat),
325+
},
326+
}
327+
328+
_, err = store.NewSession(
329+
ctx, "test", TypeMacaroonAccount,
330+
time.Unix(1000, 0), "",
331+
WithAccount(acct.ID),
332+
WithMacaroonRecipe(sessCaveats, nil),
333+
)
334+
require.NoError(t, err)
335+
336+
// Now delete the account, which represents
337+
// that the user ran "litcli accounts remove".
338+
err = acctStore.RemoveAccount(ctx, acct.ID)
339+
},
340+
},
294341
{
295342
name: "linked session",
296343
populateDB: func(t *testing.T, store *BoltStore,
@@ -538,6 +585,16 @@ func randomizedSessions(t *testing.T, kvStore *BoltStore,
538585
}
539586
}
540587

588+
// When an account is set, we also remove the account in 50% of
589+
// the cases. This simulates that the user ran "litcli accounts
590+
// remove" after creating the session.
591+
activeSess.AccountID.WhenSome(func(id accounts.AccountID) {
592+
if rand.Intn(2) == 0 {
593+
err = accountsStore.RemoveAccount(ctx, id)
594+
}
595+
})
596+
require.NoError(t, err)
597+
541598
// Finally, we shift the active session to a random state.
542599
// As the state we set may be a state that's no longer set
543600
// through the current code base, or be an illegal state

0 commit comments

Comments
 (0)