remember joined chans

[martinetd]

some clients don't reconnect joined rooms on reconnect, remember "currently active rooms" for them to join it back on reconnect

[martinetd]

This came up from @aiobofh a bit ago, so brainstorming here a bit...

I think we need to dump quite a bit of state to disk:

• channel names <-> matrix id, because on name collision we're adding a suffix to the names and that could change from one reconnect to another and that's just horrible UX
• for the same reason, users joined in a chan also have duplicates, so foo_2 and foo could interchange between two reconnects!
• as pointed out a user might prefer having some direct conversations as chan depending on their client, we could remember that easily -- other way around should also work as we prefix messages in queries when it's not obvious who it come from.
• Need to decide on an interface but would also allow to rename some chans or nicks in chans

Obviously dumping it all to json and writing to disk everytime would be possible but possibly a bit slow; since we already depend on sqlite for the matrix rust sdk I'm thinking of making that a sqlite db and just updating on changes which should be much more efficient.

I don't think there's anything sensitive here, but might as well encrypt the data on disk -- that can perhaps come later -- need to check how the matrix sdk does its encryption but we can add a new random key to the initial session and keep it in memory as low stake secret (we've got all the state there anyway!)

So:

• check rust sqlite bindings
• dump & load stuff
• profit!

[martinetd]

So while I said encryption is optional I;d rather not leak channel names so I had a quick look at matrix-rust-sdk code

Unfortunately, matrix-sdk-store-encryption is rather low level (readme suggests not to use it directly with some recommendations about rekeying I'm not confident I understand given I don't see matrix-rust-sdk do it...), while the matrix-sdk-sqlite crate bakes in the matrix client schema so it is not appropriate to use either.

With that in mind I'm not sure whether it makes much sense to roll our own or look for alternatives e.g. rustqlcipher?
On the flip side "rolling our own" doesn't have to be too bad -- if we give up on sqlite/a db interface, it's easy enough to serialize some kind of state to json and re-encrypt it in the state dir everytime we change it (with a random nonce...), because we'd only be keeping in joined channels list it's not something that'd change often.
However it has a few downsides:

• it's a bad idea to keep the password around in memory while a client is connected (the current code ought to forget it once matrix has logged in)
• it'll be one more obstacle to changing password, we shouldn't use the irc client password for encryption everywhere.

Both of these problems can be solved by having an intermediate passphrase derived from the original password (which is my understanding of what the matrix-sdk encryption stuff does), but it's starting to look like a PITA and I'm way too tired right now to think straight let alone think about encryption 😁

So, yeah, v0 might be in plain text for people who trust/run their own server (which I expect to be 100% of matrirc users at this point), with the caveat that we might want to make this optional in that case?

Once that's decided, the feature itself (keeping list of joined channels) ought to be really simple, so it's really just a matter of deciding how we want to store it.

Opinions welcome if anyone cares

[noctux]

Hi, I'm probably "anyone" in this case :) (Unless I'm simply holding matrirc the wrong way and my problem can be fixed in another way)

My problem description: When reconnecting to my matrirc instance with weechat, I get the following output:

           │18:51:16 matrirc │ Welcome to matrirc. Restoring session to https://example.org
           │18:51:21 matrirc │ okay
           │18:51:23 matrirc │ Finished initial room sync

However, in contrast to my initial connection to matrirc, my buffers do not reopen. While reopening only open buffers as discussed above can be difficult, an intermediate solution that works with minimal to no state would be to simply reopen all buffers. Below is my proof of concept (beware, it disregards all sorts of visibility constraints and module boundaries). One problem that I have with it that the detection of queries vs channels is not quite reliable yet (sometimes, we appear to be in a joining state without membership information; unfortunately, as channel-names cannot be changed on the IRC side, this seems like a smaller impedement as we cannot do the upgrade and add an # to the name later on).

What do you think about something like this as an intermediate solution? Concerning state we only have to store a singe boolean to make the join configureable per user... Is it worth to invest into such an solution?

Best regards,
Simon

P.S.: Thanks btw for matrirc in the first place, it seems to fit nicely with my need for a weechat-compatible matrix client otherwise, especially given that weechat-matrix is languishing nowadays.

Here's my current patch:

diff --git a/src/ircd/mod.rs b/src/ircd/mod.rs
index f99af5f..40766c6 100644
--- a/src/ircd/mod.rs
+++ b/src/ircd/mod.rs
@@ -90,6 +90,9 @@ async fn handle_client(mut stream: Framed<TcpStream, IrcCodec>) -> Result<()> {
         .irc()
         .send_privmsg("matrirc", &matrirc.irc().nick, "okay")
         .await?;
+
+    proto::join_channels(&matrirc).await?;
+
     if let Err(e) = proto::ircd_sync_read(reader_stream, reader_matrirc).await {
         info!("irc read task failed: {:?}", e);
     }
diff --git a/src/ircd/proto.rs b/src/ircd/proto.rs
index 0c4c2ad..d203e0b 100644
--- a/src/ircd/proto.rs
+++ b/src/ircd/proto.rs
@@ -10,7 +10,8 @@ use tokio::net::TcpStream;
 use tokio::sync::mpsc;
 use tokio_util::codec::Framed;
 
-use crate::{matrirc::Matrirc, matrix::MatrixMessageType};
+use crate::{matrirc::Matrirc, matrix::MatrixMessageType, matrix::room_mappings::RoomTargetType};
+use matrix_sdk::RoomMemberships;
 
 /// it's a bit of a pain to redo the work twice for notice/privmsg,
 /// so these types wrap it around a bit
@@ -162,6 +163,29 @@ pub async fn ircd_sync_write(
     Ok(())
 }
 
+pub async fn join_channels(
+    matrirc: &Matrirc
+) -> Result<()> {
+    let irc = matrirc.irc();
+    let matrix = matrirc.matrix();
+    let mapping = matrirc.mappings();
+
+    for joined in matrix.joined_rooms() {
+        if joined.is_tombstoned() {
+            trace!(
+                "Skipping tombstoned {}",
+                joined
+                    .name()
+                    .unwrap_or_else(|| joined.room_id().to_string())
+            );
+            continue;
+        }
+        let roomtarget = mapping.try_room_target(&joined).await?;
+        roomtarget.join_chan(&irc).await;
+    }
+    Ok(())
+}
+
 pub async fn ircd_sync_read(
     mut reader: SplitStream<Framed<TcpStream, IrcCodec>>,
     matrirc: Matrirc,
diff --git a/src/matrix/room_mappings.rs b/src/matrix/room_mappings.rs
index ee17e42..9375551 100644
--- a/src/matrix/room_mappings.rs
+++ b/src/matrix/room_mappings.rs
@@ -56,8 +56,8 @@ pub struct RoomTarget {
     inner: Arc<RwLock<RoomTargetInner>>,
 }
 
-#[derive(Debug, PartialEq)]
-enum RoomTargetType {
+#[derive(Debug, PartialEq, Clone)]
+pub enum RoomTargetType {
     /// room maps to a query e.g. single other member (or alone!)
     Query,
     /// room maps to a chan, and irc side has it joined
@@ -213,18 +213,21 @@ impl RoomTarget {
     pub async fn target(&self) -> String {
         self.inner.read().await.target.clone()
     }
+    pub async fn target_type(&self) -> RoomTargetType {
+        self.inner.read().await.target_type.clone()
+    }
 
-    async fn join_chan(&self, irc: &IrcClient) -> bool {
+    pub async fn join_chan(&self, irc: &IrcClient) -> bool {
         let mut lock = self.inner.write().await;
-        match &lock.target_type {
-            RoomTargetType::LeftChan => (),
-            RoomTargetType::Query => (),
+        let prefix = match &lock.target_type {
+            RoomTargetType::LeftChan => "",
+            RoomTargetType::Query => "#",
             // got raced or already joined
             RoomTargetType::JoiningChan => return false,
             RoomTargetType::Chan => return false,
         };
         lock.target_type = RoomTargetType::JoiningChan;
-        let chan = format!("#{}", lock.target);
+        let chan = format!("{}{}", prefix, lock.target);
         drop(lock);
 
         // we need to initate the join before getting members in another task
@@ -458,7 +461,7 @@ impl Mappings {
     // long enough to check for deduplicate and it's a bit of a mess; it could be done
     // with a more generic 'insert_free_target' that takes a couple of callbacks but
     // it's just not worth it
-    async fn try_room_target(&self, room: &Room) -> Result<RoomTarget> {
+    pub async fn try_room_target(&self, room: &Room) -> Result<RoomTarget> {
         // happy case first
         if let Some(target) = self.inner.read().await.rooms.get(room.room_id()) {
             return Ok(target.clone());

[martinetd]

Thank you!

I have the same worry as in the other issue with queries (I don't think a JOIN makes sense for queries, regular irc servers certainly don't issue these), but otherwise I agree it can be a good stop-gap.

Please send this as a PR and I'll test on my client, if it's not too disturbing for me let's get this in until we can properly store state (at which point we'll be able to rename channels etc and it'll be much nicer)
It's been a while since I last had looked at encrypted sqlite so perhaps it's gotten easier now..