Make sure we don't send fast-forwarded state changes over federation, start with empty set when overwriting

roomserver/internal/input_latest_events.go

@@ -122,10 +122,20 @@ type latestEventsUpdater struct {
 
 func (u *latestEventsUpdater) doUpdateLatestEvents() error {
 	prevEvents := u.event.PrevEvents()
-	oldLatest := u.updater.LatestEvents()
 	u.lastEventIDSent = u.updater.LastEventIDSent()
 	u.oldStateNID = u.updater.CurrentStateSnapshotNID()
 
+	// If we are doing a regular event update then we will get the
+	// previous latest events to use as a part of the calculation. If
+	// we are overwriting the latest events because we have a complete
+	// state snapshot from somewhere else, e.g. a federated room join,
+	// then start with an empty set - none of the forward extremities
+	// that we knew about before matter anymore.
+	oldLatest := []types.StateAtEventAndReference{}
+	if !u.stateAtEvent.Overwrite {
+		oldLatest = u.updater.LatestEvents()
+	}
+
 	// If the event has already been written to the output log then we
 	// don't need to do anything, as we've handled it already.
 	hasBeenSent, err := u.updater.HasEventBeenSent(u.stateAtEvent.EventNID)
@@ -221,10 +231,11 @@ func (u *latestEventsUpdater) latestState() error {
 		return err
 	}
 
-	// If we are overwriting the state then we don't need to do anything
+	// If we are overwriting the state then we should make sure that we
-	// further here.
+	// don't send anything out over federation again, it will very likely
+	// be a repeat.
 	if u.stateAtEvent.Overwrite {
-		return nil
+		u.sendAsServer = ""
 	}
 
 	// Now that we have a new state snapshot based on the latest events,

roomserver/internal/input_membership.go

@@ -16,7 +16,6 @@ package internal
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	"github.com/matrix-org/dendrite/roomserver/api"
@@ -108,10 +107,10 @@ func updateMembership(
 	}
 
 	if add == nil {
-		// This shouldn't happen. Returning an error here is better than panicking
+		// This can happen when we have rejoined a room and suddenly we have a
-		// in the membership updater functions later on.
+		// divergence between the former state and the new one. We don't want to
-		// TODO: Why does this happen to begin with?
+		// act on removals and apparently there are no adds, so stop here.
-		return updates, errors.New("add should not be nil")
+		return updates, nil
 	}
 
 	mu, err := updater.MembershipUpdater(targetUserNID)