fix(db): version vector serialisation :( (#10050)

ffs

internal/db/sqlite/db_global_test.go

@@ -268,7 +268,7 @@ func TestDontNeedIgnored(t *testing.T) {
 	}
 }
 
-func TestRemoveDontNeedLocalIgnored(t *testing.T) {
+func TestRemoteDontNeedLocalIgnored(t *testing.T) {
 	t.Parallel()
 
 	db, err := OpenTemp()
@@ -392,6 +392,32 @@ func TestRemoteDontNeedDeletedMissing(t *testing.T) {
 		t.Log(names)
 		t.Error("need no files")
 	}
+
+	// Another remote has announced it, but has set the invalid bit,
+	// presumably it's being ignored.
+	file = genFile("test1", 1, 103)
+	file.SetIgnored()
+	err = db.Update(folderID, protocol.DeviceID{43}, []protocol.FileInfo{file})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// They don't need it, either
+	s, err = db.CountNeed(folderID, protocol.DeviceID{43})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if s.Bytes != 0 || s.Files != 0 || s.Deleted != 0 {
+		t.Log(s)
+		t.Error("bad need")
+	}
+
+	// It shouldn't show up in their need list
+	names = mustCollect[protocol.FileInfo](t)(db.AllNeededGlobalFiles(folderID, protocol.DeviceID{42}, config.PullOrderAlphabetic, 0, 0))
+	if len(names) != 0 {
+		t.Log(names)
+		t.Error("need no files")
+	}
 }
 
 func TestNeedRemoteSymlinkAndDir(t *testing.T) {

internal/db/sqlite/db_test.go

@@ -1109,6 +1109,54 @@ func TestErrorWrap(t *testing.T) {
 	}
 }
 
+func TestStrangeDeletedGlobalBug(t *testing.T) {
+	// This exercises an edge case with serialisation and ordering of
+	// version vectors. It does not need to make sense, it just needs to
+	// pass.
+
+	t.Parallel()
+
+	sdb, err := OpenTemp()
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		if err := sdb.Close(); err != nil {
+			t.Fatal(err)
+		}
+	})
+
+	// One remote device announces the original version of the file
+
+	file := genFile("test", 1, 1)
+	file.Version = protocol.Vector{Counters: []protocol.Counter{{ID: 35494436325452, Value: 1742900373}}}
+	t.Log("orig", file.Version)
+	sdb.Update(folderID, protocol.DeviceID{42}, []protocol.FileInfo{file})
+
+	// Another one announces a newer one that is deleted
+
+	del := file
+	del.SetDeleted(43)
+	del.Version = protocol.Vector{Counters: []protocol.Counter{{ID: 55445057455644, Value: 1742918457}, {ID: 35494436325452, Value: 1742900373}}}
+	t.Log("del", del.Version)
+	sdb.Update(folderID, protocol.DeviceID{43}, []protocol.FileInfo{del})
+
+	// We have an instance of the original file
+
+	sdb.Update(folderID, protocol.LocalDeviceID, []protocol.FileInfo{file})
+
+	// Which one is the global? It should be the deleted one, clearly.
+
+	g, _, err := sdb.GetGlobalFile(folderID, "test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !g.Deleted {
+		t.Log(g)
+		t.Fatal("should be deleted")
+	}
+}
+
 func mustCollect[T any](t *testing.T) func(it iter.Seq[T], errFn func() error) []T {
 	t.Helper()
 	return func(it iter.Seq[T], errFn func() error) []T {

internal/db/sqlite/folderdb_counts.go

@@ -97,7 +97,7 @@ func (s *folderDB) needSizeRemote(device protocol.DeviceID) (db.Counts, error) {
 		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND g.deleted AND NOT g.invalid AND EXISTS (
 			SELECT 1 FROM FILES f
 			INNER JOIN devices d ON d.idx = f.device_idx
-			WHERE f.name = g.name AND d.device_id = ? AND NOT f.deleted
+			WHERE f.name = g.name AND d.device_id = ? AND NOT f.deleted AND NOT f.invalid
 		)
 		GROUP BY g.type, g.local_flags, g.deleted
 	`).Select(&res, device.String(),

internal/db/sqlite/folderdb_global.go

@@ -148,11 +148,11 @@ func (s *folderDB) neededGlobalFilesLocal(selectOpts string) (iter.Seq[protocol.
 func (s *folderDB) neededGlobalFilesRemote(device protocol.DeviceID, selectOpts string) (iter.Seq[protocol.FileInfo], func() error) {
 	// Select:
 	//
-	// - all the valid, non-deleted global files that don't have a corresponding
-	//   remote file with the same version.
+	// - all the valid, non-deleted global files that don't have a
+	//   corresponding remote file with the same version.
 	//
-	// - all the valid, deleted global files that have a corresponding non-deleted
-	//   remote file (of any version)
+	// - all the valid, deleted global files that have a corresponding
+	//   non-deleted and valid remote file (of any version)
 
 	it, errFn := iterStructs[indirectFI](s.stmt(`
 		SELECT fi.fiprotobuf, bl.blprotobuf, g.name, g.size, g.modified FROM fileinfos fi
@@ -172,7 +172,7 @@ func (s *folderDB) neededGlobalFilesRemote(device protocol.DeviceID, selectOpts
 		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND g.deleted AND NOT g.invalid AND EXISTS (
 			SELECT 1 FROM FILES f
 			INNER JOIN devices d ON d.idx = f.device_idx
-			WHERE f.name = g.name AND d.device_id = ? AND NOT f.deleted
+			WHERE f.name = g.name AND d.device_id = ? AND NOT f.deleted AND NOT f.invalid
 		)
 	`+selectOpts).Queryx(
 		device.String(),

internal/db/sqlite/util.go

@@ -7,9 +7,11 @@
 package sqlite
 
 import (
+	"cmp"
 	"database/sql/driver"
 	"errors"
 	"iter"
+	"slices"
 
 	"github.com/jmoiron/sqlx"
 	"github.com/syncthing/syncthing/internal/gen/bep"
@@ -71,6 +73,11 @@ func (v *dbVector) Scan(value any) error {
 	if err != nil {
 		return wrap(err)
 	}
+
+	// This is only necessary because I messed up counter serialisation and
+	// thereby ordering in 2.0.0 betas, and can be removed in the future.
+	slices.SortFunc(vec.Counters, func(a, b protocol.Counter) int { return cmp.Compare(a.ID, b.ID) })
+
 	v.Vector = vec
 
 	return nil

internal/db/sqlite/util_test.go

@@ -0,0 +1,33 @@
+// Copyright (C) 2025 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+package sqlite
+
+import (
+	"testing"
+
+	"github.com/syncthing/syncthing/lib/protocol"
+)
+
+func TestDbvector(t *testing.T) {
+	vec := protocol.Vector{Counters: []protocol.Counter{{ID: 42, Value: 7}, {ID: 123456789, Value: 42424242}}}
+	dbVec := dbVector{vec}
+	val, err := dbVec.Value()
+	if err != nil {
+		t.Fatal(val)
+	}
+
+	var dbVec2 dbVector
+	if err := dbVec2.Scan(val); err != nil {
+		t.Fatal(err)
+	}
+
+	if !dbVec2.Vector.Equal(vec) {
+		t.Log(vec)
+		t.Log(dbVec2.Vector)
+		t.Fatal("should match")
+	}
+}

lib/protocol/vector.go

@@ -31,7 +31,9 @@ func (v *Vector) String() string {
 		if i > 0 {
 			buf.WriteRune(',')
 		}
-		fmt.Fprintf(&buf, "%x:%d", c.ID, c.Value)
+		var idbs [8]byte
+		binary.BigEndian.PutUint64(idbs[:], uint64(c.ID))
+		fmt.Fprintf(&buf, "%x:%d", idbs, c.Value)
 	}
 	return buf.String()
 }