Merge pull request #472 from vector-im/bwindels/fix-new-join-detect-one-write-phase

Fix missing new joins preventing key shares

src/matrix/SessionContainer.js

@@ -204,7 +204,7 @@ export class SessionContainer {
             reconnector: this._reconnector,
         });
         this._sessionId = sessionInfo.id;
-        this._storage = await this._platform.storageFactory.create(sessionInfo.id);
+        this._storage = await this._platform.storageFactory.create(sessionInfo.id, log);
         // no need to pass access token to session
         const filteredSessionInfo = {
             id: sessionInfo.id,

src/matrix/e2ee/DeviceTracker.js

@@ -19,6 +19,22 @@ import {verifyEd25519Signature, SIGNATURE_ALGORITHM} from "./common.js";
 const TRACKING_STATUS_OUTDATED = 0;
 const TRACKING_STATUS_UPTODATE = 1;
 
+export function addRoomToIdentity(identity, userId, roomId) {
+    if (!identity) {
+        identity = {
+            userId: userId,
+            roomIds: [roomId],
+            deviceTrackingStatus: TRACKING_STATUS_OUTDATED,
+        };
+        return identity;
+    } else {
+        if (!identity.roomIds.includes(roomId)) {
+            identity.roomIds.push(roomId);
+            return identity;
+        }
+    }
+}
+
 // map 1 device from /keys/query response to DeviceIdentity
 function deviceKeysAsDeviceIdentity(deviceSection) {
     const deviceId = deviceSection["device_id"];
@@ -107,17 +123,9 @@ export class DeviceTracker {
     async _writeMember(member, txn) {
         const {userIdentities} = txn;
         const identity = await userIdentities.get(member.userId);
-        if (!identity) {
+        const updatedIdentity = addRoomToIdentity(identity, member.userId, member.roomId);
-            userIdentities.set({
+        if (updatedIdentity) {
-                userId: member.userId,
+            userIdentities.set(updatedIdentity);
-                roomIds: [member.roomId],
-                deviceTrackingStatus: TRACKING_STATUS_OUTDATED,
-            });
-        } else {
-            if (!identity.roomIds.includes(member.roomId)) {
-                identity.roomIds.push(member.roomId);
-                userIdentities.set(identity);
-            }
         }
     }
 

src/matrix/room/Room.js

@@ -30,6 +30,7 @@ const EVENT_ENCRYPTED_TYPE = "m.room.encrypted";
 export class Room extends BaseRoom {
     constructor(options) {
         super(options);
+        // TODO: pass pendingEvents to start like pendingOperations?
         const {pendingEvents} = options;
         const relationWriter = new RelationWriter({
             roomId: this.id,
@@ -120,7 +121,8 @@ export class Room extends BaseRoom {
             txn.roomMembers.removeAllForRoom(this.id);
         }
         const {entries: newEntries, updatedEntries, newLiveKey, memberChanges} =
-            await log.wrap("syncWriter", log => this._syncWriter.writeSync(roomResponse, isRejoin, txn, log), log.level.Detail);
+            await log.wrap("syncWriter", log => this._syncWriter.writeSync(
+                roomResponse, isRejoin, summaryChanges.hasFetchedMembers, txn, log), log.level.Detail);
         if (decryptChanges) {
             const decryption = await log.wrap("decryptChanges", log => decryptChanges.write(txn, log));
             log.set("decryptionResults", decryption.results.size);

src/matrix/room/members/RoomMember.js

@@ -141,6 +141,16 @@ export class MemberChange {
         return this.previousMembership === "join" && this.membership !== "join";
     }
 
+    /** The result can be a false negative when all of these apply:
+     *  - the complete set of room members hasn't been fetched yet.
+     *  - the member event for this change was received in the
+     *    state section and wasn't present in the timeline section.
+     *  - the room response was limited, e.g. there was a gap.
+     * 
+     * This is because during sync, in this case it is not possible
+     * to distinguish between a new member that joined the room
+     * during a gap and a lazy-loading member.
+     * */
     get hasJoined() {
         return this.previousMembership !== "join" && this.membership === "join";
     }

src/matrix/room/timeline/persistence/MemberWriter.js

@@ -23,57 +23,27 @@ export class MemberWriter {
         this._cache = new LRUCache(5, member => member.userId);
     }
 
-    writeTimelineMemberEvent(event, txn) {
+    prepareMemberSync(stateEvents, timelineEvents, hasFetchedMembers) {
-        return this._writeMemberEvent(event, false, txn);
+        return new MemberSync(this, stateEvents, timelineEvents, hasFetchedMembers);
     }
 
-    writeStateMemberEvent(event, isLimited, txn) {
+    async _writeMember(member, txn) {
-        // member events in the state section when the room response
+        let existingMember = this._cache.get(member.userId);
-        // is not limited must always be lazy loaded members.
-        // If they are not, they will be repeated in the timeline anyway.
-        return this._writeMemberEvent(event, !isLimited, txn);
-    }
-
-    async _writeMemberEvent(event, isLazyLoadingMember, txn) {
-        const userId = event.state_key;
-        if (!userId) {
-            return;
-        }
-        const member = RoomMember.fromMemberEvent(this._roomId, event);
-        if (!member) {
-            return;
-        }
-
-        let existingMember = this._cache.get(userId);
         if (!existingMember) {
-            const memberData = await txn.roomMembers.get(this._roomId, userId);
+            const memberData = await txn.roomMembers.get(this._roomId, member.userId);
             if (memberData) {
                 existingMember = new RoomMember(memberData);
             }
         }
-
         // either never heard of the member, or something changed
         if (!existingMember || !existingMember.equals(member)) {
             txn.roomMembers.set(member.serialize());
             this._cache.set(member);
-            // we also return a member change for lazy loading members if something changed,
-            // so when the dupe timeline event comes and it doesn't see a diff
-            // with the cache, we already returned the event here.
-            // 
-            // it's just important that we don't consider the first LL event
-            // for a user we see as a membership change, or we'll share keys with
-            // them, etc...
-            if (isLazyLoadingMember && !existingMember) {
-                // we don't have a previous member, but we know this is not a
-                // membership change as it's a lazy loaded
-                // member so take the membership from the member
-                return new MemberChange(member, member.membership);
-            }
             return new MemberChange(member, existingMember?.membership);
         }
     }
 
-    async lookupMember(userId, event, timelineEvents, txn) {
+    async lookupMember(userId, txn) {
         let member = this._cache.get(userId);
         if (!member) {
             const memberData = await txn.roomMembers.get(this._roomId, userId);
@@ -82,61 +52,154 @@ export class MemberWriter {
                 this._cache.set(member);
             }
         }
-        if (!member) {
+        return member;
-            // sometimes the member event isn't included in state, but rather in the timeline,
+    }
-            // even if it is not the first event in the timeline. In this case, go look for
+}
-            // the last one before the event, or if none is found,
+
-            // the least recent matching member event in the timeline.
+class MemberSync {
-            // The latter is needed because of new joins picking up their own display name
+    constructor(memberWriter, stateEvents, timelineEvents, hasFetchedMembers) {
-            let foundEvent = false;
+        this._memberWriter = memberWriter;
-            let memberEventBefore;
+        this._timelineEvents = timelineEvents;
-            let firstMemberEvent;
+        this._hasFetchedMembers = hasFetchedMembers;
-            for (let i = timelineEvents.length - 1; i >= 0; i -= 1) {
+        this._newStateMembers = null;
+        if (stateEvents) {
+            this._newStateMembers = this._stateEventsToMembers(stateEvents);
+        }
+    }
+
+    get _roomId() {
+        return this._memberWriter._roomId;
+    }
+
+    _stateEventsToMembers(stateEvents) {
+        let members;
+        for (const event of stateEvents) {
+            if (event.type === MEMBER_EVENT_TYPE) {
+                const member = RoomMember.fromMemberEvent(this._roomId, event);
+                if (member) {
+                    if (!members) {
+                        members = new Map();
+                    }
+                    members.set(member.userId, member);
+                }
+            }
+        }
+        return members;
+    }
+
+    _timelineEventsToMembers(timelineEvents) {
+        let members;
+        // iterate backwards to only add the last member in the timeline
+        for (let i = timelineEvents.length - 1; i >= 0; i--) {
             const e = timelineEvents[i];
-                let matchingEvent;
+            const userId = e.state_key;
-                if (e.type === MEMBER_EVENT_TYPE && e.state_key === userId) {
+            if (e.type === MEMBER_EVENT_TYPE && !members?.has(userId)) {
-                    matchingEvent = e;
+                const member = RoomMember.fromMemberEvent(this._roomId, e);
-                    firstMemberEvent = matchingEvent;
+                if (member) {
+                    if (!members) {
+                        members = new Map();
                     }
-                if (!foundEvent) {
+                    members.set(member.userId, member);
+                }
+            }
+        }
+        return members;
+    }
+
+    async lookupMemberAtEvent(userId, event, txn) {
+        let member;
+        if (this._timelineEvents) {
+            member = this._findPrecedingMemberEventInTimeline(userId, event);
+            if (member) {
+                return member;
+            }
+        }
+        member = this._newStateMembers?.get(userId);
+        if (member) {
+            return member;
+        }
+        return await this._memberWriter.lookupMember(userId, txn);
+    }
+
+    async write(txn) {
+        const memberChanges = new Map();
+        let newTimelineMembers;
+        if (this._timelineEvents) {
+            newTimelineMembers = this._timelineEventsToMembers(this._timelineEvents);
+        }
+        if (this._newStateMembers) {
+            for (const member of this._newStateMembers.values()) {
+                if (!newTimelineMembers?.has(member.userId)) {
+                    const memberChange = await this._memberWriter._writeMember(member, txn);
+                    if (memberChange) {
+                        // if the member event appeared only in the state section,
+                        // AND we haven't heard about it AND we haven't fetched all members yet (to avoid #470),
+                        // this may be a lazy loading member (if it's not in a gap, we are certain
+                        // it is a ll member, in a gap, we can't tell), so we pass in our own membership as
+                        // as the previous one so we won't consider it a join to not have false positives (to avoid #192).
+                        // see also MemberChange.hasJoined
+                        const maybeLazyLoadingMember = !this._hasFetchedMembers && !memberChange.previousMembership;
+                        if (maybeLazyLoadingMember) {
+                            memberChange.previousMembership = member.membership;
+                        }
+                        memberChanges.set(memberChange.userId, memberChange);
+                    }
+                }
+            }
+        }
+        if (newTimelineMembers) {
+            for (const member of newTimelineMembers.values()) {
+                const memberChange = await this._memberWriter._writeMember(member, txn);
+                if (memberChange) {
+                    memberChanges.set(memberChange.userId, memberChange);
+                }
+            }
+        }
+        return memberChanges;
+    }
+
+    // try to find the first member event before the given event,
+    // so we respect historical display names within the chunk of timeline
+    _findPrecedingMemberEventInTimeline(userId, event) {
+        let eventIndex = -1;
+        for (let i = this._timelineEvents.length - 1; i >= 0; i--) {
+            const e = this._timelineEvents[i];
             if (e.event_id === event.event_id) {
-                        foundEvent = true;
+                eventIndex = i;
-                    } 
-                } else if (matchingEvent) {
-                    memberEventBefore = matchingEvent;
                 break;
             }
         }
-            // first see if we found a member event before the event we're looking up the sender for
+        for (let i = eventIndex - 1; i >= 0; i--) {
-            if (memberEventBefore) {
+            const e = this._timelineEvents[i];
-                member = RoomMember.fromMemberEvent(this._roomId, memberEventBefore);
+            if (e.type === MEMBER_EVENT_TYPE && e.state_key === userId) {
-            }
+                const member = RoomMember.fromMemberEvent(this._roomId, e);
-            // and only if we didn't, fall back to the first member event,
+                if (member) {
-            // regardless of where it is positioned relative to the lookup event
-            else if (firstMemberEvent) {
-                member = RoomMember.fromMemberEvent(this._roomId, firstMemberEvent);
-            }
-        }
                     return member;
                 }
+            }
+        }
+    }
 }
 
 export function tests() {
 
+    let idCounter = 0;
+
     function createMemberEvent(membership, userId, displayName, avatarUrl) {
+        idCounter += 1;
         return {
             content: {
                 membership,
                 "displayname": displayName,
                 "avatar_url": avatarUrl
             },
+            event_id: `$${idCounter}`,
             sender: userId,
             "state_key": userId,
             type: "m.room.member"
         };
     }
 
-
     function createStorage(initialMembers = []) {
         const members = new Map();
         for (const m of initialMembers) {
@@ -164,102 +227,195 @@ export function tests() {
     const avatar = "mxc://hs.tld/def";
 
     return {
-        "new join through state": async assert => {
+        "new join": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage();
-            const change = await writer.writeStateMemberEvent(createMemberEvent("join", alice), true, txn);
+            const memberSync = writer.prepareMemberSync([], [createMemberEvent("join", alice)], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
             assert(change.hasJoined);
             assert.equal(txn.members.get(alice).membership, "join");
         },
-        "accept invite through state": async assert => {
+        "accept invite": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("invite", alice)]);
-            const change = await writer.writeStateMemberEvent(createMemberEvent("join", alice), true, txn);
+            const memberSync = writer.prepareMemberSync([], [createMemberEvent("join", alice)], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
             assert.equal(change.previousMembership, "invite");
             assert(change.hasJoined);
             assert.equal(txn.members.get(alice).membership, "join");
         },
-        "change display name through timeline": async assert => {
+        "change display name": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("join", alice, "Alice")]);
-            const change = await writer.writeTimelineMemberEvent(createMemberEvent("join", alice, "Alies"), txn);
+            const memberSync = writer.prepareMemberSync([], [createMemberEvent("join", alice, "Alies")], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
             assert(!change.hasJoined);
             assert.equal(change.member.displayName, "Alies");
             assert.equal(txn.members.get(alice).displayName, "Alies");
         },
-        "set avatar through timeline": async assert => {
+        "set avatar": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("join", alice, "Alice")]);
-            const change = await writer.writeTimelineMemberEvent(createMemberEvent("join", alice, "Alice", avatar), txn);
+            const memberSync = writer.prepareMemberSync([], [createMemberEvent("join", alice, "Alice", avatar)], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
             assert(!change.hasJoined);
             assert.equal(change.member.avatarUrl, avatar);
             assert.equal(txn.members.get(alice).avatarUrl, avatar);
         },
-        "ignore redundant member event": async assert => {
+        "ignore redundant member event in timeline": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("join", alice, "Alice", avatar)]);
-            const change = await writer.writeTimelineMemberEvent(createMemberEvent("join", alice, "Alice", avatar), txn);
+            const memberSync = writer.prepareMemberSync([], [createMemberEvent("join", alice, "Alice", avatar)], false);
-            assert(!change);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 0);
+        },
+        "ignore redundant member event in state": async assert => {
+            const writer = new MemberWriter(roomId);
+            const txn = createStorage([member("join", alice, "Alice", avatar)]);
+            const memberSync = writer.prepareMemberSync([createMemberEvent("join", alice, "Alice", avatar)], [], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 0);
         },
         "leave": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("join", alice, "Alice")]);
-            const change = await writer.writeTimelineMemberEvent(createMemberEvent("leave", alice, "Alice"), txn);
+            const memberSync = writer.prepareMemberSync([], [createMemberEvent("leave", alice, "Alice")], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
             assert(change.hasLeft);
             assert(!change.hasJoined);
         },
         "ban": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("join", alice, "Alice")]);
-            const change = await writer.writeTimelineMemberEvent(createMemberEvent("ban", alice, "Alice"), txn);
+            const memberSync = writer.prepareMemberSync([], [createMemberEvent("ban", alice, "Alice")], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
             assert(change.hasLeft);
             assert(!change.hasJoined);
         },
         "reject invite": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("invite", alice, "Alice")]);
-            const change = await writer.writeTimelineMemberEvent(createMemberEvent("leave", alice, "Alice"), txn);
+            const memberSync = writer.prepareMemberSync([], [createMemberEvent("leave", alice, "Alice")], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
             assert(!change.hasLeft);
             assert(!change.hasJoined);
         },
         "lazy loaded member we already know about doens't return change": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("join", alice, "Alice")]);
-            const change = await writer.writeStateMemberEvent(createMemberEvent("join", alice, "Alice"), false, txn);
+            const memberSync = writer.prepareMemberSync([createMemberEvent("join", alice, "Alice")], [], false);
-            assert(!change);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 0);
         },
         "lazy loaded member we already know about changes display name": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage([member("join", alice, "Alice")]);
-            const change = await writer.writeStateMemberEvent(createMemberEvent("join", alice, "Alies"), false, txn);
+            const memberSync = writer.prepareMemberSync([createMemberEvent("join", alice, "Alies")], [], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
+            assert(!change.hasJoined);
             assert.equal(change.member.displayName, "Alies");
         },
-        "unknown lazy loaded member returns change, but not considered a membership change": async assert => {
+        "unknown lazy loaded member returns change, but not considered a join": async assert => {
             const writer = new MemberWriter(roomId);
             const txn = createStorage();
-            const change = await writer.writeStateMemberEvent(createMemberEvent("join", alice, "Alice"), false, txn);
+            const memberSync = writer.prepareMemberSync([createMemberEvent("join", alice, "Alice")], [], false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
             assert(!change.hasJoined);
             assert(!change.hasLeft);
             assert.equal(change.member.membership, "join");
             assert.equal(txn.members.get(alice).displayName, "Alice");
         },
-        "newly joined member causes a change with lookup done first": async assert => {
+        "new join through both timeline and state": async assert => {
-            const event = createMemberEvent("join", alice, "Alice");
             const writer = new MemberWriter(roomId);
             const txn = createStorage();
-            const member = await writer.lookupMember(event.sender, event, [event], txn);
+            const aliceJoin = createMemberEvent("join", alice, "Alice");
-            assert(member);
+            const memberSync = writer.prepareMemberSync([aliceJoin], [aliceJoin], false);
-            const change = await writer.writeTimelineMemberEvent(event, txn);
+            const changes = await memberSync.write(txn);
-            assert(change);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
+            assert(change.hasJoined);
+            assert(!change.hasLeft);
         },
-        "lookupMember returns closest member in the past": async assert => {
+        "change display name in timeline with lazy loaded member in state": async assert => {
+            const writer = new MemberWriter(roomId);
+            const txn = createStorage();
+            const memberSync = writer.prepareMemberSync(
+                [createMemberEvent("join", alice, "Alice")],
+                [createMemberEvent("join", alice, "Alies")],
+                false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
+            assert(change.hasJoined);
+            assert(!change.hasLeft);
+            assert.equal(change.member.displayName, "Alies");
+        },
+        "lookupMemberAtEvent returns closest member in the past": async assert => {
             const event1 = createMemberEvent("join", alice, "Alice");
             const event2 = createMemberEvent("join", alice, "Alies");
             const event3 = createMemberEvent("join", alice, "Alys");
+            const events = [event1, event2, event3];
+            // we write first because the MemberWriter assumes it is called before
+            // the SyncWriter does any lookups
             const writer = new MemberWriter(roomId);
             const txn = createStorage();
-            const member = await writer.lookupMember(event3.sender, event3, [event1, event2, event3], txn);
+            const memberSync = await writer.prepareMemberSync([], events, false);
+            let member = await memberSync.lookupMemberAtEvent(event1.sender, event1, txn);
+            assert.equal(member, undefined);
+            member = await memberSync.lookupMemberAtEvent(event2.sender, event2, txn);
+            assert.equal(member.displayName, "Alice");
+            member = await memberSync.lookupMemberAtEvent(event3.sender, event3, txn);
             assert.equal(member.displayName, "Alies");
+
+            assert.equal(txn.members.size, 0);
+            const changes = await memberSync.write(txn);
+            assert.equal(txn.members.size, 1);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
+            assert(change.hasJoined);
+        },
+        "lookupMemberAtEvent falls back on state event": async assert => {
+            const event1 = createMemberEvent("join", alice, "Alice");
+            const event2 = createMemberEvent("join", alice, "Alies");
+            // we write first because the MemberWriter assumes it is called before
+            // the SyncWriter does any lookups
+            const writer = new MemberWriter(roomId);
+            const txn = createStorage();
+            const memberSync = await writer.prepareMemberSync([event1], [event2], false);
+            const member = await memberSync.lookupMemberAtEvent(event2.sender, event2, txn);
+            assert.equal(member.displayName, "Alice");
+
+            assert.equal(txn.members.size, 0);
+            const changes = await memberSync.write(txn);
+            assert.equal(txn.members.size, 1);
+            assert.equal(changes.size, 1);
+            const change = changes.get(alice);
+            assert(change.hasJoined);
+        },
+        "write works without event arrays": async assert => {
+            const writer = new MemberWriter(roomId);
+            const txn = createStorage();
+            const memberSync = await writer.prepareMemberSync(undefined, undefined, false);
+            const changes = await memberSync.write(txn);
+            assert.equal(changes.size, 0);
         },
     };
 }

src/matrix/room/timeline/persistence/SyncWriter.js

@@ -133,38 +133,31 @@ export class SyncWriter {
         return currentKey;
     }
 
-    async _writeStateEvents(roomResponse, memberChanges, isLimited, txn, log) {
+    async _writeStateEvents(stateEvents, txn, log) {
-        // persist state
+        let nonMemberStateEvents = 0;
-        const {state} = roomResponse;
+        for (const event of stateEvents) {
-        if (Array.isArray(state?.events)) {
+            // member events are written prior by MemberWriter
-            log.set("stateEvents", state.events.length);
+            if (event.type !== MEMBER_EVENT_TYPE) {
-            for (const event of state.events) {
-                if (event.type === MEMBER_EVENT_TYPE) {
-                    const memberChange = await this._memberWriter.writeStateMemberEvent(event, isLimited, txn);
-                    if (memberChange) {
-                        memberChanges.set(memberChange.userId, memberChange);
-                    }
-                } else {
                 txn.roomState.set(this._roomId, event);
+                nonMemberStateEvents += 1;
             }
         }
-        }
+        log.set("stateEvents", nonMemberStateEvents);
     }
 
-    async _writeTimeline(timeline, currentKey, memberChanges, txn, log) {
+    async _writeTimeline(timelineEvents, timeline, memberSync, currentKey, txn, log) {
         const entries = [];
         const updatedEntries = [];
-        if (Array.isArray(timeline?.events) && timeline.events.length) {
+        if (timelineEvents?.length) {
             // only create a fragment when we will really write an event
             currentKey = await this._ensureLiveFragment(currentKey, entries, timeline, txn, log);
-            const events = deduplicateEvents(timeline.events);
+            log.set("timelineEvents", timelineEvents.length);
-            log.set("timelineEvents", events.length);
             let timelineStateEventCount = 0;
-            for(const event of events) {
+            for(const event of timelineEvents) {
                 // store event in timeline
                 currentKey = currentKey.nextKey();
                 const storageEntry = createEventEntry(currentKey, this._roomId, event);
-                let member = await this._memberWriter.lookupMember(event.sender, event, events, txn);
+                let member = await memberSync.lookupMemberAtEvent(event.sender, event, txn);
                 if (member) {
                     storageEntry.displayName = member.displayName;
                     storageEntry.avatarUrl = member.avatarUrl;
@@ -178,19 +171,13 @@ export class SyncWriter {
                 }
                 // update state events after writing event, so for a member event,
                 // we only update the member info after having written the member event
-                // to the timeline, as we want that event to have the old profile info
+                // to the timeline, as we want that event to have the old profile info.
-                if (typeof event.state_key === "string") {
+                // member events are written prior by MemberWriter.
+                if (typeof event.state_key === "string" && event.type !== MEMBER_EVENT_TYPE) {
                     timelineStateEventCount += 1;
-                    if (event.type === MEMBER_EVENT_TYPE) {
-                        const memberChange = await this._memberWriter.writeTimelineMemberEvent(event, txn);
-                        if (memberChange) {
-                            memberChanges.set(memberChange.userId, memberChange);
-                        }
-                    } else {
                     txn.roomState.set(this._roomId, event);
                 }
             }
-            }
             log.set("timelineStateEventCount", timelineStateEventCount);
         }
         return {currentKey, entries, updatedEntries};
@@ -224,14 +211,13 @@ export class SyncWriter {
      * @type {SyncWriterResult}
      * @property {Array<BaseEntry>} entries new timeline entries written
      * @property {EventKey} newLiveKey the advanced key to write events at
-     * @property {Map<string, MemberChange>} memberChanges member changes in the processed sync ny user id
      * 
      * @param  {Object}  roomResponse [description]
      * @param  {boolean}  isRejoin whether the room was rejoined in the sync being processed
      * @param  {Transaction}  txn     
      * @return {SyncWriterResult}
      */
-    async writeSync(roomResponse, isRejoin, txn, log) {
+    async writeSync(roomResponse, isRejoin, hasFetchedMembers, txn, log) {
         let {timeline} = roomResponse;
         // we have rejoined the room after having synced it before,
         // check for overlap with the last synced event
@@ -239,13 +225,22 @@ export class SyncWriter {
         if (isRejoin) {
             timeline = await this._handleRejoinOverlap(timeline, txn, log);
         }
-        const memberChanges = new Map();
+        let timelineEvents;
-        // important this happens before _writeTimeline so
+        if (Array.isArray(timeline?.events)) {
-        // members are available in the transaction
+            timelineEvents = deduplicateEvents(timeline.events);
-        await this._writeStateEvents(roomResponse, memberChanges, timeline?.limited, txn, log);
+        }
+        const {state} = roomResponse;
+        let stateEvents;
+        if (Array.isArray(state?.events)) {
+            stateEvents = state.events;
+        }
+        const memberSync = this._memberWriter.prepareMemberSync(stateEvents, timelineEvents, hasFetchedMembers);
+        if (stateEvents) {
+            await this._writeStateEvents(stateEvents, txn, log);
+        }
         const {currentKey, entries, updatedEntries} =
-            await this._writeTimeline(timeline, this._lastLiveKey, memberChanges, txn, log);
+            await this._writeTimeline(timelineEvents, timeline, memberSync, this._lastLiveKey, txn, log);
-        log.set("memberChanges", memberChanges.size);
+        const memberChanges = await memberSync.write(txn);
         return {entries, updatedEntries, newLiveKey: currentKey, memberChanges};
     }
 

src/matrix/storage/idb/StorageFactory.js

@@ -21,8 +21,9 @@ import { schema } from "./schema.js";
 import { detectWebkitEarlyCloseTxnBug } from "./quirks.js";
 
 const sessionName = sessionId => `hydrogen_session_${sessionId}`;
-const openDatabaseWithSessionId = function(sessionId, idbFactory) {
+const openDatabaseWithSessionId = function(sessionId, idbFactory, log) {
-    return openDatabase(sessionName(sessionId), createStores, schema.length, idbFactory);
+    const create = (db, txn, oldVersion, version) => createStores(db, txn, oldVersion, version, log);
+    return openDatabase(sessionName(sessionId), create, schema.length, idbFactory);
 }
 
 async function requestPersistedStorage() {
@@ -49,7 +50,7 @@ export class StorageFactory {
         this._IDBKeyRange = IDBKeyRange;
     }
 
-    async create(sessionId) {
+    async create(sessionId, log) {
         await this._serviceWorkerHandler?.preventConcurrentSessionAccess(sessionId);
         requestPersistedStorage().then(persisted => {
             // Firefox lies here though, and returns true even if the user denied the request
@@ -59,7 +60,7 @@ export class StorageFactory {
         });
 
         const hasWebkitEarlyCloseTxnBug = await detectWebkitEarlyCloseTxnBug(this._idbFactory);
-        const db = await openDatabaseWithSessionId(sessionId, this._idbFactory);
+        const db = await openDatabaseWithSessionId(sessionId, this._idbFactory, log);
         return new Storage(db, this._IDBKeyRange, hasWebkitEarlyCloseTxnBug);
     }
 
@@ -80,10 +81,11 @@ export class StorageFactory {
     }
 }
 
-async function createStores(db, txn, oldVersion, version) {
+async function createStores(db, txn, oldVersion, version, log) {
     const startIdx = oldVersion || 0;
-
+    return log.wrap({l: "storage migration", oldVersion, version}, async log => {
         for(let i = startIdx; i < version; ++i) {
-        await schema[i](db, txn);
+            await log.wrap(`v${i + 1}`, log => schema[i](db, txn, log));
         }
+    });
 }

src/matrix/storage/idb/schema.js

@@ -1,8 +1,10 @@
 import {iterateCursor, reqAsPromise} from "./utils.js";
 import {RoomMember, EVENT_TYPE as MEMBER_EVENT_TYPE} from "../../room/members/RoomMember.js";
+import {addRoomToIdentity} from "../../e2ee/DeviceTracker.js";
 import {RoomMemberStore} from "./stores/RoomMemberStore.js";
 import {SessionStore} from "./stores/SessionStore.js";
 import {encodeScopeTypeKey} from "./stores/OperationStore.js";
+import {MAX_UNICODE} from "./stores/common.js";
 
 // FUNCTIONS SHOULD ONLY BE APPENDED!!
 // the index in the array is the database version
@@ -17,6 +19,7 @@ export const schema = [
     createArchivedRoomSummaryStore,
     migrateOperationScopeIndex,
     createTimelineRelationsStore,
+    fixMissingRoomsInUserIdentities
 ];
 // TODO: how to deal with git merge conflicts of this array?
 
@@ -142,3 +145,47 @@ async function migrateOperationScopeIndex(db, txn) {
 function createTimelineRelationsStore(db) {
     db.createObjectStore("timelineRelations", {keyPath: "key"});
 }
+
+//v11 doesn't change the schema, but ensures all userIdentities have all the roomIds they should (see #470)
+async function fixMissingRoomsInUserIdentities(db, txn, log) {
+    const roomSummaryStore = txn.objectStore("roomSummary");
+    const trackedRoomIds = [];
+    await iterateCursor(roomSummaryStore.openCursor(), roomSummary => {
+        if (roomSummary.isTrackingMembers) {
+            trackedRoomIds.push(roomSummary.roomId);
+        }
+    });
+    const outboundGroupSessionsStore = txn.objectStore("outboundGroupSessions");
+    const userIdentitiesStore = txn.objectStore("userIdentities");
+    const roomMemberStore = txn.objectStore("roomMembers");
+    for (const roomId of trackedRoomIds) {
+        let foundMissing = false;
+        const joinedUserIds = [];
+        const memberRange = IDBKeyRange.bound(roomId, `${roomId}|${MAX_UNICODE}`, true, true);
+        await log.wrap({l: "room", id: roomId}, async log => {
+            await iterateCursor(roomMemberStore.openCursor(memberRange), member => {
+                if (member.membership === "join") {
+                    joinedUserIds.push(member.userId);
+                }
+            });
+            log.set("joinedUserIds", joinedUserIds.length);
+            for (const userId of joinedUserIds) {
+                const identity = await reqAsPromise(userIdentitiesStore.get(userId));
+                const originalRoomCount = identity?.roomIds?.length;
+                const updatedIdentity = addRoomToIdentity(identity, userId, roomId);
+                if (updatedIdentity) {
+                    log.log({l: `fixing up`, id: userId,
+                        roomsBefore: originalRoomCount, roomsAfter: updatedIdentity.roomIds.length});
+                    userIdentitiesStore.put(updatedIdentity);
+                    foundMissing = true;
+                }
+            }
+            log.set("foundMissing", foundMissing);
+            if (foundMissing) {
+                // clear outbound megolm session,
+                // so we'll create a new one on the next message that will be properly shared
+                outboundGroupSessionsStore.delete(roomId);
+            }
+        });
+    }
+}

src/matrix/storage/idb/utils.js

@@ -66,11 +66,18 @@ export function decodeUint32(str) {
 
 export function openDatabase(name, createObjectStore, version, idbFactory = window.indexedDB) {
     const req = idbFactory.open(name, version);
-    req.onupgradeneeded = (ev) => {
+    req.onupgradeneeded = async (ev) => {
         const db = ev.target.result;
         const txn = ev.target.transaction;
         const oldVersion = ev.oldVersion;
-        createObjectStore(db, txn, oldVersion, version);
+        try {
+            await createObjectStore(db, txn, oldVersion, version);
+        } catch (err) {
+            // try aborting on error, if that hasn't been done already
+            try {
+                txn.abort();
+            } catch (err) {}
+        }
     }; 
     return reqAsPromise(req);
 }

src/mocks/Storage.js

@@ -16,7 +16,8 @@ limitations under the License.
 
 import {FDBFactory, FDBKeyRange} from "../../lib/fake-indexeddb/index.js";
 import {StorageFactory} from "../matrix/storage/idb/StorageFactory.js";
+import {NullLogItem} from "../logging/NullLogger.js";
 
 export function createMockStorage() {
-    return new StorageFactory(null, new FDBFactory(), FDBKeyRange).create(1);
+    return new StorageFactory(null, new FDBFactory(), FDBKeyRange).create(1, new NullLogItem());
 }