diff --git a/e2e/deletion_record_test.go b/e2e/deletion_record_test.go index 6e93a11e..8a0dd9d1 100644 --- a/e2e/deletion_record_test.go +++ b/e2e/deletion_record_test.go @@ -203,3 +203,65 @@ func TestDeletion_SurvivesALostReport(t *testing.T) { t.Errorf("the deletion never reached B: %q", got) } } + +// The same with the proof from the other direction: B pulled what A served it +// and holds it still, while the report that records the files both hold is +// lost. A banks the served state as agreed; the record of shared files is +// repaired with it, so a file A deletes before its watcher notices is a +// deletion for B, not a file to fetch back (#66). +func TestDeletion_SurvivesALostReportWhenServed(t *testing.T) { + a, b, gameID := pairAndTrack(t, "Lost Report Served Deletion", map[string]string{"seed.sav": "seed"}) + testutil.SettleSync(t, gameID, a, b) + st := a.Daemon.Store + var oldBase string + if !testutil.WaitFor(30*time.Second, func() bool { + oldBase = st.GetAgreedHash(gameID, b.NodeID()) + return oldBase != "" + }) { + t.Fatal("setup: the devices never recorded that they agree") + } + + a.WriteSave("auto.sav", "v1") + time.Sleep(syncSettleWindow) + a.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) + if !testutil.WaitFor(30*time.Second, func() bool { return b.ReadSave("auto.sav") == "v1" }) { + t.Fatal("setup: auto.sav never reached B") + } + testutil.SettleSync(t, gameID, a, b) + + handedOver, err := delta.BuildManifest(a.SaveDir) + if err != nil { + t.Fatal(err) + } + if !testutil.WaitFor(30*time.Second, func() bool { + return st.GetAgreedHash(gameID, b.NodeID()) == handedOver.ManifestHash() + }) { + t.Fatal("setup: the report never arrived") + } + // Undone as a lost report leaves it, with only the served state to go + // by: the old base (which clears the push record), the old record of + // shared files, and what A served B under that base. + if err := st.SetAgreedHash(gameID, b.NodeID(), oldBase); err != nil { + t.Fatal(err) + } + if st.GetPushedHash(gameID, b.NodeID()) != "" { + t.Fatal("setup: a push is still on record") + } + if err := st.SetSyncState(gameID, b.NodeID(), []string{"seed.sav"}, nil); err != nil { + t.Fatal(err) + } + a.Daemon.P2P.Sync.NoteServed(gameID, b.NodeID(), handedOver) + + if err := os.Remove(filepath.Join(a.SaveDir, "auto.sav")); err != nil { + t.Fatal(err) + } + a.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) // before the watcher records the deletion + time.Sleep(5 * time.Second) + + if got := a.ReadSave("auto.sav"); got != "" { + t.Errorf("the deleted file came back: %q", got) + } + if got := b.ReadSave("auto.sav"); got != "" { + t.Errorf("the deletion never reached B: %q", got) + } +} diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 3d732f86..a053ccc8 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -1081,6 +1081,7 @@ func (d *Daemon) UntrackGame(gameID string) error { // same ID and would inherit all of it — and a stale lineage reads a file // that went missing while untracked as a deletion to propagate. _ = d.Store.ForgetGameSyncState(gameID) + d.P2P.Sync.ForgetServed(gameID) d.P2P.ClearPendingResync(gameID) d.P2P.NotifyUntrack(gameID) return nil @@ -1238,6 +1239,7 @@ func (d *Daemon) untrackFromPeer(gameID string) { // And everything it agreed with any peer, for the reason given in // UntrackGame: the ID comes back, and this must not come back with it. _ = d.Store.ForgetGameSyncState(gameID) + d.P2P.Sync.ForgetServed(gameID) d.P2P.ClearPendingResync(gameID) d.Log.Log("info", fmt.Sprintf("game %q untracked on a paired device", gameID)) } diff --git a/internal/p2p/syncengine/engine.go b/internal/p2p/syncengine/engine.go index 6c1c965b..7df1eda5 100644 --- a/internal/p2p/syncengine/engine.go +++ b/internal/p2p/syncengine/engine.go @@ -605,8 +605,19 @@ func (e *Engine) SyncWithPeer(ctx context.Context, gameID string, peer Peer) (Re // Matched on the peer's save as served, before this side's exclusion rules // filtered it, which is how it was recorded; banked in today's filtered // terms. Only against the base it was served under (served.go). + // + // The record of which files both hold is repaired with it, as the push + // proof above does and for the same reason: without it, a file the peer + // took from what was served and this device deleted before the watcher + // noted it read as one the peer had and this device never did, and was + // fetched back. And not while the game is held as emptied, for the same + // reason as above. Safe since untracking forgets what was served + // (ForgetServed): a re-tracked game no longer banks a state served before. if remoteHash := remoteData.Manifest.ManifestHash(); agreedHash != remoteHash && e.servedUnderBase(gameID, peer.ID, unfilteredRemote.ManifestHash(), storedBase) { + if _, held, err := e.Store.GetDeletionHold(gameID); err == nil && !held { + e.persistLineage(gameID, peer.ID, remoteData.Manifest, remoteData.Manifest) + } _ = e.Store.SetAgreedHash(gameID, peer.ID, remoteHash) agreedHash = remoteHash } diff --git a/internal/p2p/syncengine/served.go b/internal/p2p/syncengine/served.go index 43a0003c..a732eb20 100644 --- a/internal/p2p/syncengine/served.go +++ b/internal/p2p/syncengine/served.go @@ -1,6 +1,7 @@ package syncengine import ( + "strings" "time" "github.com/opensave/opensave/internal/delta" @@ -71,6 +72,23 @@ func (e *Engine) NoteServed(gameID, peerID string, m delta.Manifest) { e.served[key] = list } +// ForgetServed drops every served state recorded for a game, with any peer. +// Called when the game is untracked, with the rest of what it agreed with its +// peers (store.ForgetGameSyncState). The records live in memory, so they +// outlived the untrack: a game tracked again within the hour could bank a +// state served before it was untracked as agreed, against a base that the +// untrack had wiped. +func (e *Engine) ForgetServed(gameID string) { + prefix := servedKey(gameID, "") + e.servedMu.Lock() + defer e.servedMu.Unlock() + for key := range e.served { + if strings.HasPrefix(key, prefix) { + delete(e.served, key) + } + } +} + // servedUnderBase reports whether this device served hash to the peer while // base was the agreed state — and so whether a peer holding hash holds a state // this device held since that agreement. diff --git a/internal/p2p/syncengine/served_forget_test.go b/internal/p2p/syncengine/served_forget_test.go new file mode 100644 index 00000000..7fb7543a --- /dev/null +++ b/internal/p2p/syncengine/served_forget_test.go @@ -0,0 +1,34 @@ +package syncengine + +import ( + "testing" + + "github.com/opensave/opensave/internal/delta" +) + +// Untracking a game forgets what this device served for it, so a re-track +// cannot bank a state served before the untrack; other games keep theirs. +func TestForgetServed_ForgetsOnlyThatGame(t *testing.T) { + env := setupEngine(t) + write(t, env.localDir, "save.dat", "v1") + m, err := delta.BuildManifest(env.localDir) + if err != nil { + t.Fatal(err) + } + e := env.engine + e.NoteServed("game1", env.peer.ID, m) + e.NoteServed("game10", env.peer.ID, m) + base := e.Store.GetAgreedHash("game1", env.peer.ID) + if !e.servedUnderBase("game1", env.peer.ID, m.ManifestHash(), base) { + t.Fatal("setup: the served state was not recorded") + } + + e.ForgetServed("game1") + + if e.servedUnderBase("game1", env.peer.ID, m.ManifestHash(), base) { + t.Error("a state served before the untrack is still remembered") + } + if !e.servedUnderBase("game10", env.peer.ID, m.ManifestHash(), e.Store.GetAgreedHash("game10", env.peer.ID)) { + t.Error("forgetting game1 also forgot game10") + } +}