Merge kernel's "can't remove damoose fixes" and more! #1001

kernelkind (9):
      appease clippy
      use `NwcError` instead of nwc::Error
      make `UserAccount` cloneable
      allow removal of Damoose account
      expose `AccountCache::falback`
      move select account logic to own method
      bugfix: properly sub to new selected acc after removal of selected
      bugfix: unsubscribe from timelines on deck deletion
      bugfix: unsubscribe all decks when log out account

crates/notedeck/src/account/accounts.rs

@@ -97,16 +97,31 @@ impl Accounts {
         }
     }
 
-    pub fn remove_account(&mut self, pk: &Pubkey) {
+    pub fn remove_account(
-        let Some(removed) = self.cache.remove(pk) else {
+        &mut self,
-            return;
+        pk: &Pubkey,
+        ndb: &mut Ndb,
+        pool: &mut RelayPool,
+        ctx: &egui::Context,
+    ) -> bool {
+        let Some(resp) = self.cache.remove(pk) else {
+            return false;
         };
 
-        if let Some(key_store) = &self.storage_writer {
+        if pk != self.cache.fallback() {
-            if let Err(e) = key_store.remove_key(&removed.key) {
+            if let Some(key_store) = &self.storage_writer {
-                tracing::error!("Could not remove account {pk}: {e}");
+                if let Err(e) = key_store.remove_key(&resp.deleted) {
+                    tracing::error!("Could not remove account {pk}: {e}");
+                }
             }
         }
+
+        if let Some(swap_to) = resp.swap_to {
+            let txn = Transaction::new(ndb).expect("txn");
+            self.select_account_internal(&swap_to, ndb, &txn, pool, ctx);
+        }
+
+        true
     }
 
     pub fn contains_full_kp(&self, pubkey: &enostr::Pubkey) -> bool {
@@ -212,6 +227,18 @@ impl Accounts {
             return;
         }
 
+        self.select_account_internal(pk_to_select, ndb, txn, pool, ctx);
+    }
+
+    /// Have already selected in `AccountCache`, updating other things
+    fn select_account_internal(
+        &mut self,
+        pk_to_select: &Pubkey,
+        ndb: &mut Ndb,
+        txn: &Transaction,
+        pool: &mut RelayPool,
+        ctx: &egui::Context,
+    ) {
         if let Some(key_store) = &self.storage_writer {
             if let Err(e) = key_store.select_key(Some(*pk_to_select)) {
                 tracing::error!("Could not select key {:?}: {e}", pk_to_select);
@@ -375,6 +402,7 @@ fn get_acc_from_storage(user_account_serializable: UserAccountSerializable) -> O
     })
 }
 
+#[derive(Clone)]
 pub struct AccountData {
     pub(crate) relay: AccountRelayData,
     pub(crate) muted: AccountMutedData,

crates/notedeck/src/account/cache.rs

@@ -6,6 +6,7 @@ use crate::{SingleUnkIdAction, UserAccount};
 pub struct AccountCache {
     selected: Pubkey,
     fallback: Pubkey,
+    fallback_account: UserAccount,
 
     // never empty at rest
     accounts: HashMap<Pubkey, UserAccount>,
@@ -16,12 +17,13 @@ impl AccountCache {
         let mut accounts = HashMap::with_capacity(1);
 
         let pk = fallback.key.pubkey;
-        accounts.insert(pk, fallback);
+        accounts.insert(pk, fallback.clone());
 
         (
             Self {
                 selected: pk,
                 fallback: pk,
+                fallback_account: fallback,
                 accounts,
             },
             SingleUnkIdAction::pubkey(pk),
@@ -48,15 +50,20 @@ impl AccountCache {
         self.accounts.entry(pk).insert(account)
     }
 
-    pub(super) fn remove(&mut self, pk: &Pubkey) -> Option<UserAccount> {
+    pub(super) fn remove(&mut self, pk: &Pubkey) -> Option<AccountDeletionResponse> {
-        // fallback account should never be removed
+        if *pk == self.fallback && self.accounts.len() == 1 {
-        if *pk == self.fallback {
+            // no point in removing it since it'll just get re-added anyway
             return None;
         }
 
-        let removed = self.accounts.remove(pk);
+        let removed = self.accounts.remove(pk)?;
 
-        if removed.is_some() && self.selected == *pk {
+        if self.accounts.is_empty() {
+            self.accounts
+                .insert(self.fallback, self.fallback_account.clone());
+        }
+
+        if self.selected == *pk {
             // TODO(kernelkind): choose next better
             let (next, _) = self
                 .accounts
@@ -64,9 +71,17 @@ impl AccountCache {
                 .next()
                 .expect("accounts can never be empty");
             self.selected = *next;
+
+            return Some(AccountDeletionResponse {
+                deleted: removed.key,
+                swap_to: Some(*next),
+            });
         }
 
-        removed
+        Some(AccountDeletionResponse {
+            deleted: removed.key,
+            swap_to: None,
+        })
     }
 
     /// guarenteed that all selected exist in accounts
@@ -90,6 +105,10 @@ impl AccountCache {
             .get_mut(&self.selected)
             .expect("guarenteed that selected exists in accounts")
     }
+
+    pub fn fallback(&self) -> &Pubkey {
+        &self.fallback
+    }
 }
 
 impl<'a> IntoIterator for &'a AccountCache {
@@ -100,3 +119,8 @@ impl<'a> IntoIterator for &'a AccountCache {
         self.accounts.iter()
     }
 }
+
+pub struct AccountDeletionResponse {
+    pub deleted: enostr::Keypair,
+    pub swap_to: Option<Pubkey>,
+}

crates/notedeck/src/account/contacts.rs

@@ -3,11 +3,13 @@ use std::collections::HashSet;
 use enostr::Pubkey;
 use nostrdb::{Filter, Ndb, Note, NoteKey, Subscription, Transaction};
 
+#[derive(Clone)]
 pub struct Contacts {
     pub filter: Filter,
     pub(super) state: ContactState,
 }
 
+#[derive(Clone)]
 pub enum ContactState {
     Unreceived,
     Received {

crates/notedeck/src/account/mute.rs

@@ -5,6 +5,7 @@ use tracing::{debug, error};
 
 use crate::Muted;
 
+#[derive(Clone)]
 pub(crate) struct AccountMutedData {
     pub filter: Filter,
     pub muted: Arc<Muted>,

crates/notedeck/src/account/relay.rs

@@ -7,6 +7,7 @@ use url::Url;
 
 use crate::{AccountData, RelaySpec};
 
+#[derive(Clone)]
 pub(crate) struct AccountRelayData {
     pub filter: Filter,
     pub local: BTreeSet<RelaySpec>, // used locally but not advertised

crates/notedeck/src/user_account.rs

@@ -6,6 +6,7 @@ use crate::{
     AccountData, IsFollowing,
 };
 
+#[derive(Clone)]
 pub struct UserAccount {
     pub key: Keypair,
     pub wallet: Option<ZapWallet>,

crates/notedeck/src/wallet.rs

@@ -1,4 +1,4 @@
-use std::sync::Arc;
+use std::{fmt::Display, sync::Arc};
 
 use nwc::{
     nostr::nips::nip47::{NostrWalletConnectURI, PayInvoiceRequest, PayInvoiceResponse},
@@ -57,7 +57,17 @@ pub enum WalletError {
 pub struct Wallet {
     pub uri: String,
     wallet: Arc<RwLock<NWC>>,
-    balance: Option<Promise<Result<u64, nwc::Error>>>,
+    balance: Option<Promise<Result<u64, NwcError>>>,
+}
+
+impl Clone for Wallet {
+    fn clone(&self) -> Self {
+        Self {
+            uri: self.uri.clone(),
+            wallet: self.wallet.clone(),
+            balance: None,
+        }
+    }
 }
 
 #[derive(Clone)]
@@ -95,7 +105,7 @@ impl Wallet {
         })
     }
 
-    pub fn get_balance(&mut self) -> Option<&Result<u64, nwc::Error>> {
+    pub fn get_balance(&mut self) -> Option<&Result<u64, NwcError>> {
         if self.balance.is_none() {
             self.balance = Some(get_balance(self.wallet.clone()));
             return None;
@@ -117,11 +127,51 @@ impl Wallet {
     }
 }
 
-fn get_balance(nwc: Arc<RwLock<NWC>>) -> Promise<Result<u64, nwc::Error>> {
+#[derive(Clone)]
+pub enum NwcError {
+    /// NIP47 error
+    NIP47(String),
+    /// Relay
+    Relay(String),
+    /// Premature exit
+    PrematureExit,
+    /// Request timeout
+    Timeout,
+}
+
+impl From<nwc::Error> for NwcError {
+    fn from(value: nwc::Error) -> Self {
+        match value {
+            nwc::error::Error::NIP47(error) => NwcError::NIP47(error.to_string()),
+            nwc::error::Error::Relay(error) => NwcError::Relay(error.to_string()),
+            nwc::error::Error::PrematureExit => NwcError::PrematureExit,
+            nwc::error::Error::Timeout => NwcError::Timeout,
+        }
+    }
+}
+
+impl Display for NwcError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            NwcError::NIP47(err) => write!(f, "NIP47 error: {}", err),
+            NwcError::Relay(err) => write!(f, "Relay error: {}", err),
+            NwcError::PrematureExit => write!(f, "Premature exit"),
+            NwcError::Timeout => write!(f, "Request timed out"),
+        }
+    }
+}
+
+fn get_balance(nwc: Arc<RwLock<NWC>>) -> Promise<Result<u64, NwcError>> {
     let (sender, promise) = Promise::new();
 
     tokio::spawn(async move {
-        sender.send(nwc.read().await.get_balance().await);
+        sender.send(
+            nwc.read()
+                .await
+                .get_balance()
+                .await
+                .map_err(nwc::Error::into),
+        );
     });
 
     promise
@@ -196,7 +246,7 @@ fn construct_global_wallet(wallet_handler: &TokenHandler) -> Option<ZapWallet> {
     Some(wallet)
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct ZapWallet {
     pub wallet: Wallet,
     pub default_zap: DefaultZapMsats,

crates/notedeck/src/zaps/default_zap.rs

@@ -4,7 +4,7 @@ use crate::get_current_wallet;
 
 const DEFAULT_ZAP_MSATS: u64 = 10_000;
 
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Clone)]
 pub struct DefaultZapMsats {
     pub msats: Option<u64>,
     pub pending: PendingDefaultZapState,
@@ -83,7 +83,7 @@ impl TokenSerializable for UserZapMsats {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct PendingDefaultZapState {
     pub amount_sats: String,
     pub error_message: Option<DefaultZapError>,
@@ -110,7 +110,7 @@ fn msats_to_sats_string(msats: u64) -> String {
     (msats / 1000).to_string()
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub enum DefaultZapError {
     InvalidUserInput,
 }

crates/notedeck_columns/src/column.rs

@@ -124,7 +124,7 @@ impl Columns {
                 IntermediaryRoute::Timeline(mut timeline) => {
                     let route = Route::timeline(timeline.kind.clone());
                     timeline.subscription.increment();
-                    timeline_cache.insert(timeline.kind.clone(), timeline);
+                    timeline_cache.insert(timeline.kind.clone(), *timeline);
                     route
                 }
                 IntermediaryRoute::Route(route) => route,
@@ -247,7 +247,7 @@ impl Columns {
 }
 
 pub enum IntermediaryRoute {
-    Timeline(Timeline),
+    Timeline(Box<Timeline>),
     Route(Route),
 }
 

crates/notedeck_columns/src/decks.rs

@@ -1,6 +1,6 @@
 use std::collections::{hash_map::ValuesMut, HashMap};
 
-use enostr::Pubkey;
+use enostr::{Pubkey, RelayPool};
 use nostrdb::Transaction;
 use notedeck::{AppContext, FALLBACK_PUBKEY};
 use tracing::{error, info};
@@ -155,9 +155,24 @@ impl DecksCache {
         }
     }
 
-    pub fn remove_for(&mut self, key: &Pubkey) {
+    pub fn remove(
+        &mut self,
+        key: &Pubkey,
+        timeline_cache: &mut TimelineCache,
+        ndb: &mut nostrdb::Ndb,
+        pool: &mut RelayPool,
+    ) {
+        let Some(decks) = self.account_to_decks.remove(key) else {
+            return;
+        };
         info!("Removing decks for {:?}", key);
-        self.account_to_decks.remove(key);
+
+        decks.unsubscribe_all(timeline_cache, ndb, pool);
+
+        if !self.account_to_decks.contains_key(&self.fallback_pubkey) {
+            self.account_to_decks
+                .insert(self.fallback_pubkey, Decks::default());
+        }
     }
 
     pub fn get_fallback_pubkey(&self) -> &Pubkey {
@@ -265,10 +280,25 @@ impl Decks {
         }
     }
 
-    pub fn remove_deck(&mut self, index: usize) {
+    pub fn remove_deck(
+        &mut self,
+        index: usize,
+        timeline_cache: &mut TimelineCache,
+        ndb: &mut nostrdb::Ndb,
+        pool: &mut enostr::RelayPool,
+    ) {
+        let Some(deck) = self.remove_deck_internal(index) else {
+            return;
+        };
+
+        delete_deck(deck, timeline_cache, ndb, pool);
+    }
+
+    fn remove_deck_internal(&mut self, index: usize) -> Option<Deck> {
+        let mut res = None;
         if index < self.decks.len() {
             if self.decks.len() > 1 {
-                self.decks.remove(index);
+                res = Some(self.decks.remove(index));
 
                 let info_prefix = format!("Removed deck at index {index}");
                 match index.cmp(&self.active_deck) {
@@ -311,6 +341,37 @@ impl Decks {
         } else {
             error!("index was out of bounds");
         }
+        res
+    }
+
+    pub fn unsubscribe_all(
+        self,
+        timeline_cache: &mut TimelineCache,
+        ndb: &mut nostrdb::Ndb,
+        pool: &mut enostr::RelayPool,
+    ) {
+        for deck in self.decks {
+            delete_deck(deck, timeline_cache, ndb, pool);
+        }
+    }
+}
+
+fn delete_deck(
+    mut deck: Deck,
+    timeline_cache: &mut TimelineCache,
+    ndb: &mut nostrdb::Ndb,
+    pool: &mut enostr::RelayPool,
+) {
+    let cols = deck.columns_mut();
+    let num_cols = cols.num_columns();
+    for i in (0..num_cols).rev() {
+        let kinds_to_pop = cols.delete_column(i);
+
+        for kind in &kinds_to_pop {
+            if let Err(err) = timeline_cache.pop(kind, ndb, pool) {
+                error!("error popping timeline: {err}");
+            }
+        }
     }
 }
 

crates/notedeck_columns/src/nav.rs

@@ -94,7 +94,16 @@ impl SwitchingAction {
                         .router_mut()
                         .go_back();
                 }
-                AccountsAction::Remove(to_remove) => ctx.accounts.remove_account(to_remove),
+                AccountsAction::Remove(to_remove) => 's: {
+                    if !ctx
+                        .accounts
+                        .remove_account(to_remove, ctx.ndb, ctx.pool, ui_ctx)
+                    {
+                        break 's;
+                    }
+
+                    decks_cache.remove(to_remove, timeline_cache, ctx.ndb, ctx.pool);
+                }
             },
             SwitchingAction::Columns(columns_action) => match *columns_action {
                 ColumnsAction::Remove(index) => {
@@ -116,7 +125,12 @@ impl SwitchingAction {
                     get_decks_mut(ctx.accounts, decks_cache).set_active(index)
                 }
                 DecksAction::Removing(index) => {
-                    get_decks_mut(ctx.accounts, decks_cache).remove_deck(index)
+                    get_decks_mut(ctx.accounts, decks_cache).remove_deck(
+                        index,
+                        timeline_cache,
+                        ctx.ndb,
+                        ctx.pool,
+                    );
                 }
             },
         }

crates/notedeck_columns/src/storage/decks.rs

@@ -351,9 +351,9 @@ impl CleanIntermediaryRoute {
         match self {
             CleanIntermediaryRoute::ToTimeline(timeline_kind) => {
                 let txn = Transaction::new(ndb).unwrap();
-                Some(IntermediaryRoute::Timeline(
+                Some(IntermediaryRoute::Timeline(Box::new(
                     timeline_kind.into_timeline(&txn, ndb)?,
-                ))
+                )))
             }
             CleanIntermediaryRoute::ToRoute(route) => Some(IntermediaryRoute::Route(route)),
         }