libstore: refuse to serialise ancient protocols

We don't want to deal with these at all, let's stop doing so.

(marking this one as the fix commit since its immediate predecessors
aren't the complete fix)
Fixes: lix-project/lix#325

Change-Id: Ieea1b0b8ac0f903d1e24e5b3e63cfe12eeec119d

src/libstore/daemon.cc

@@ -45,10 +45,15 @@ struct TunnelLogger : public Logger
 
     Sync<State> state_;
 
-    WorkerProto::Version clientVersion;
+    /**
+     * Worker protocol version of the other side. May be newer than this daemon.
+     */
+    const WorkerProto::Version clientVersion;
 
     TunnelLogger(FdSink & to, WorkerProto::Version clientVersion)
-        : to(to), clientVersion(clientVersion) { }
+        : to(to), clientVersion(clientVersion) {
+        assert(clientVersion >= MIN_SUPPORTED_WORKER_PROTO_VERSION);
+    }
 
     void enqueueMsg(const std::string & s)
     {
@@ -127,12 +132,6 @@ struct TunnelLogger : public Logger
     void startActivity(ActivityId act, Verbosity lvl, ActivityType type,
         const std::string & s, const Fields & fields, ActivityId parent) override
     {
-        if (GET_PROTOCOL_MINOR(clientVersion) < 20) {
-            if (!s.empty())
-                log(lvl, s + "...");
-            return;
-        }
-
         StringSink buf;
         buf << STDERR_START_ACTIVITY << act << lvl << type << s << fields << parent;
         enqueueMsg(buf.s);
@@ -140,7 +139,6 @@ struct TunnelLogger : public Logger
 
     void stopActivity(ActivityId act) override
     {
-        if (GET_PROTOCOL_MINOR(clientVersion) < 20) return;
         StringSink buf;
         buf << STDERR_STOP_ACTIVITY << act;
         enqueueMsg(buf.s);
@@ -148,7 +146,6 @@ struct TunnelLogger : public Logger
 
     void result(ActivityId act, ResultType type, const Fields & fields) override
     {
-        if (GET_PROTOCOL_MINOR(clientVersion) < 20) return;
         StringSink buf;
         buf << STDERR_RESULT << act << type << fields;
         enqueueMsg(buf.s);
@@ -265,14 +262,8 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
     TrustedFlag trusted, RecursiveFlag recursive, WorkerProto::Version clientVersion,
     Source & from, BufferedSink & to, WorkerProto::Op op)
 {
-    WorkerProto::ReadConn rconn {
+    WorkerProto::ReadConn rconn{from, clientVersion};
-        .from = from,
+    WorkerProto::WriteConn wconn{to, clientVersion};
-        .version = clientVersion,
-    };
-    WorkerProto::WriteConn wconn {
-        .to = to,
-        .version = clientVersion,
-    };
 
     switch (op) {
 
@@ -525,22 +516,19 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
 
     case WorkerProto::Op::BuildPaths: {
         auto drvs = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
-        BuildMode mode = bmNormal;
+        BuildMode mode = buildModeFromInteger(readInt(from));
-        if (GET_PROTOCOL_MINOR(clientVersion) >= 15) {
-            mode = (BuildMode) readInt(from);
 
-            /* Repairing is not atomic, so disallowed for "untrusted"
+        /* Repairing is not atomic, so disallowed for "untrusted"
-               clients.
+           clients.
 
-               FIXME: layer violation in this message: the daemon code (i.e.
+           FIXME: layer violation in this message: the daemon code (i.e.
-               this file) knows whether a client/connection is trusted, but it
+           this file) knows whether a client/connection is trusted, but it
-               does not how how the client was authenticated. The mechanism
+           does not how how the client was authenticated. The mechanism
-               need not be getting the UID of the other end of a Unix Domain
+           need not be getting the UID of the other end of a Unix Domain
-               Socket.
+           Socket.
-              */
+          */
-            if (mode == bmRepair && !trusted)
+        if (mode == bmRepair && !trusted)
-                throw Error("repairing is not allowed because you are not in 'trusted-users'");
+            throw Error("repairing is not allowed because you are not in 'trusted-users'");
-        }
         logger->startWork();
         store->buildPaths(drvs, mode);
         logger->stopWork();
@@ -551,7 +539,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
     case WorkerProto::Op::BuildPathsWithResults: {
         auto drvs = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
         BuildMode mode = bmNormal;
-        mode = (BuildMode) readInt(from);
+        mode = buildModeFromInteger(readInt(from));
 
         /* Repairing is not atomic, so disallowed for "untrusted"
            clients.
@@ -582,7 +570,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
          * correctly.
          */
         readDerivation(from, *store, drv, Derivation::nameFromPath(drvPath));
-        BuildMode buildMode = (BuildMode) readInt(from);
+        BuildMode buildMode = buildModeFromInteger(readInt(from));
         logger->startWork();
 
         auto drvType = drv.type();
@@ -744,13 +732,11 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         clientSettings.buildCores = readInt(from);
         clientSettings.useSubstitutes = readInt(from);
 
-        if (GET_PROTOCOL_MINOR(clientVersion) >= 12) {
+        unsigned int n = readInt(from);
-            unsigned int n = readInt(from);
+        for (unsigned int i = 0; i < n; i++) {
-            for (unsigned int i = 0; i < n; i++) {
+            auto name = readString(from);
-                auto name = readString(from);
+            auto value = readString(from);
-                auto value = readString(from);
+            clientSettings.overrides.emplace(name, value);
-                clientSettings.overrides.emplace(name, value);
-            }
         }
 
         logger->startWork();
@@ -820,15 +806,14 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         try {
             info = store->queryPathInfo(path);
         } catch (InvalidPath &) {
-            if (GET_PROTOCOL_MINOR(clientVersion) < 17) throw;
+            // The path being invalid isn't fatal here since it will just be
+            // sent as not present.
         }
         logger->stopWork();
         if (info) {
-            if (GET_PROTOCOL_MINOR(clientVersion) >= 17)
+            to << 1;
-                to << 1;
             WorkerProto::write(*store, wconn, static_cast<const UnkeyedValidPathInfo &>(*info));
         } else {
-            assert(GET_PROTOCOL_MINOR(clientVersion) >= 17);
             to << 0;
         }
         break;
@@ -902,12 +887,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         else {
             std::unique_ptr<Source> source;
             StringSink saved;
-            if (GET_PROTOCOL_MINOR(clientVersion) >= 21)
+            source = std::make_unique<TunnelSource>(from, to);
-                source = std::make_unique<TunnelSource>(from, to);
-            else {
-                copyNAR(from, saved);
-                source = std::make_unique<StringSource>(saved.s);
-            }
 
             logger->startWork();
 
@@ -1009,7 +989,7 @@ void processConnection(
     to.flush();
     WorkerProto::Version clientVersion = readInt(from);
 
-    if (clientVersion < 0x10a)
+    if (clientVersion < MIN_SUPPORTED_WORKER_PROTO_VERSION)
         throw Error("the Nix client version is too old");
 
     auto tunnelLogger = new TunnelLogger(to, clientVersion);
@@ -1025,13 +1005,13 @@ void processConnection(
         printMsgUsing(prevLogger, lvlDebug, "%d operations", opCount);
     });
 
-    if (GET_PROTOCOL_MINOR(clientVersion) >= 14 && readInt(from)) {
+    // FIXME: what is *supposed* to be in this even?
+    if (readInt(from)) {
         // Obsolete CPU affinity.
         readInt(from);
     }
 
-    if (GET_PROTOCOL_MINOR(clientVersion) >= 11)
+    readInt(from); // obsolete reserveSpace
-        readInt(from); // obsolete reserveSpace
 
     if (GET_PROTOCOL_MINOR(clientVersion) >= 33)
         to << nixVersion;
@@ -1042,10 +1022,7 @@ void processConnection(
         auto temp = trusted
             ? store->isTrustedClient()
             : std::optional { NotTrusted };
-        WorkerProto::WriteConn wconn {
+        WorkerProto::WriteConn wconn {to, clientVersion};
-            .to = to,
-            .version = clientVersion,
-        };
         WorkerProto::write(*store, wconn, temp);
     }
 

src/libstore/remote-store-connection.hh

@@ -27,11 +27,8 @@ struct RemoteStore::Connection
     FdSource from;
 
     /**
-     * Worker protocol version used for the connection.
+     * The worker protocol version of the connected daemon. This may be newer
-     *
+     * than this Lix supports.
-     * Despite its name, I think it is actually the maximum version both
-     * sides support. (If the maximum doesn't exist, we would fail to
-     * establish a connection and produce a value of this type.)
      */
     WorkerProto::Version daemonVersion;
 
@@ -71,10 +68,7 @@ struct RemoteStore::Connection
      */
     operator WorkerProto::ReadConn ()
     {
-        return WorkerProto::ReadConn {
+        return WorkerProto::ReadConn {from, daemonVersion};
-            .from = from,
-            .version = daemonVersion,
-        };
     }
 
     /**
@@ -87,10 +81,7 @@ struct RemoteStore::Connection
      */
     operator WorkerProto::WriteConn ()
     {
-        return WorkerProto::WriteConn {
+        return WorkerProto::WriteConn {to, daemonVersion};
-            .to = to,
-            .version = daemonVersion,
-        };
     }
 
     virtual ~Connection();

src/libstore/remote-store.cc

@@ -76,17 +76,14 @@ void RemoteStore::initConnection(Connection & conn)
         conn.from >> conn.daemonVersion;
         if (GET_PROTOCOL_MAJOR(conn.daemonVersion) != GET_PROTOCOL_MAJOR(PROTOCOL_VERSION))
             throw Error("Nix daemon protocol version not supported");
-        if (GET_PROTOCOL_MINOR(conn.daemonVersion) < 10)
+        if (GET_PROTOCOL_MINOR(conn.daemonVersion) < MIN_SUPPORTED_MINOR_WORKER_PROTO_VERSION)
             throw Error("the Nix daemon version is too old");
         conn.to << PROTOCOL_VERSION;
 
-        if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 14) {
+        // Obsolete CPU affinity.
-            // Obsolete CPU affinity.
+        conn.to << 0;
-            conn.to << 0;
-        }
 
-        if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 11)
+        conn.to << false; // obsolete reserveSpace
-            conn.to << false; // obsolete reserveSpace
 
         if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 33) {
             conn.to.flush();
@@ -127,24 +124,22 @@ void RemoteStore::setOptions(Connection & conn)
        << settings.buildCores
        << settings.useSubstitutes;
 
-    if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 12) {
+    std::map<std::string, Config::SettingInfo> overrides;
-        std::map<std::string, Config::SettingInfo> overrides;
+    settings.getSettings(overrides, true); // libstore settings
-        settings.getSettings(overrides, true); // libstore settings
+    fileTransferSettings.getSettings(overrides, true);
-        fileTransferSettings.getSettings(overrides, true);
+    overrides.erase(settings.keepFailed.name);
-        overrides.erase(settings.keepFailed.name);
+    overrides.erase(settings.keepGoing.name);
-        overrides.erase(settings.keepGoing.name);
+    overrides.erase(settings.tryFallback.name);
-        overrides.erase(settings.tryFallback.name);
+    overrides.erase(settings.maxBuildJobs.name);
-        overrides.erase(settings.maxBuildJobs.name);
+    overrides.erase(settings.maxSilentTime.name);
-        overrides.erase(settings.maxSilentTime.name);
+    overrides.erase(settings.buildCores.name);
-        overrides.erase(settings.buildCores.name);
+    overrides.erase(settings.useSubstitutes.name);
-        overrides.erase(settings.useSubstitutes.name);
+    overrides.erase(loggerSettings.showTrace.name);
-        overrides.erase(loggerSettings.showTrace.name);
+    overrides.erase(experimentalFeatureSettings.experimentalFeatures.name);
-        overrides.erase(experimentalFeatureSettings.experimentalFeatures.name);
+    overrides.erase(settings.pluginFiles.name);
-        overrides.erase(settings.pluginFiles.name);
+    conn.to << overrides.size();
-        conn.to << overrides.size();
+    for (auto & i : overrides)
-        for (auto & i : overrides)
+        conn.to << i.first << i.second.value;
-            conn.to << i.first << i.second.value;
-    }
 
     auto ex = conn.processStderr();
     if (ex) std::rethrow_exception(ex);
@@ -208,20 +203,13 @@ bool RemoteStore::isValidPathUncached(const StorePath & path)
 StorePathSet RemoteStore::queryValidPaths(const StorePathSet & paths, SubstituteFlag maybeSubstitute)
 {
     auto conn(getConnection());
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) {
+    conn->to << WorkerProto::Op::QueryValidPaths;
-        StorePathSet res;
+    WorkerProto::write(*this, *conn, paths);
-        for (auto & i : paths)
+    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 27) {
-            if (isValidPath(i)) res.insert(i);
+        conn->to << maybeSubstitute;
-        return res;
-    } else {
-        conn->to << WorkerProto::Op::QueryValidPaths;
-        WorkerProto::write(*this, *conn, paths);
-        if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 27) {
-            conn->to << maybeSubstitute;
-        }
-        conn.processStderr();
-        return WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
     }
+    conn.processStderr();
+    return WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
 }
 
 
@@ -237,20 +225,10 @@ StorePathSet RemoteStore::queryAllValidPaths()
 StorePathSet RemoteStore::querySubstitutablePaths(const StorePathSet & paths)
 {
     auto conn(getConnection());
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) {
+    conn->to << WorkerProto::Op::QuerySubstitutablePaths;
-        StorePathSet res;
+    WorkerProto::write(*this, *conn, paths);
-        for (auto & i : paths) {
+    conn.processStderr();
-            conn->to << WorkerProto::Op::HasSubstitutes << printStorePath(i);
+    return WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-            conn.processStderr();
-            if (readInt(conn->from)) res.insert(i);
-        }
-        return res;
-    } else {
-        conn->to << WorkerProto::Op::QuerySubstitutablePaths;
-        WorkerProto::write(*this, *conn, paths);
-        conn.processStderr();
-        return WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-    }
 }
 
 
@@ -260,45 +238,25 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S
 
     auto conn(getConnection());
 
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) {
-
-        for (auto & i : pathsMap) {
-            SubstitutablePathInfo info;
-            conn->to << WorkerProto::Op::QuerySubstitutablePathInfo << printStorePath(i.first);
-            conn.processStderr();
-            unsigned int reply = readInt(conn->from);
-            if (reply == 0) continue;
-            auto deriver = readString(conn->from);
-            if (deriver != "")
-                info.deriver = parseStorePath(deriver);
-            info.references = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-            info.downloadSize = readLongLong(conn->from);
-            info.narSize = readLongLong(conn->from);
-            infos.insert_or_assign(i.first, std::move(info));
-        }
-
-    } else {
-
-        conn->to << WorkerProto::Op::QuerySubstitutablePathInfos;
-        if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 22) {
-            StorePathSet paths;
-            for (auto & path : pathsMap)
-                paths.insert(path.first);
-            WorkerProto::write(*this, *conn, paths);
-        } else
-            WorkerProto::write(*this, *conn, pathsMap);
-        conn.processStderr();
-        size_t count = readNum<size_t>(conn->from);
-        for (size_t n = 0; n < count; n++) {
-            SubstitutablePathInfo & info(infos[parseStorePath(readString(conn->from))]);
-            auto deriver = readString(conn->from);
-            if (deriver != "")
-                info.deriver = parseStorePath(deriver);
-            info.references = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-            info.downloadSize = readLongLong(conn->from);
-            info.narSize = readLongLong(conn->from);
-        }
 
+    conn->to << WorkerProto::Op::QuerySubstitutablePathInfos;
+    if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 22) {
+        StorePathSet paths;
+        for (auto & path : pathsMap)
+            paths.insert(path.first);
+        WorkerProto::write(*this, *conn, paths);
+    } else
+        WorkerProto::write(*this, *conn, pathsMap);
+    conn.processStderr();
+    size_t count = readNum<size_t>(conn->from);
+    for (size_t n = 0; n < count; n++) {
+        SubstitutablePathInfo & info(infos[parseStorePath(readString(conn->from))]);
+        auto deriver = readString(conn->from);
+        if (deriver != "")
+            info.deriver = parseStorePath(deriver);
+        info.references = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
+        info.downloadSize = readLongLong(conn->from);
+        info.narSize = readLongLong(conn->from);
     }
 }
 
@@ -315,10 +273,9 @@ std::shared_ptr<const ValidPathInfo> RemoteStore::queryPathInfoUncached(const St
             throw InvalidPath(std::move(e.info()));
         throw;
     }
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) {
+    bool valid; conn->from >> valid;
-        bool valid; conn->from >> valid;
+    if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path));
-        if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path));
+
-    }
     return std::make_shared<ValidPathInfo>(
         StorePath{path},
         WorkerProto::Serialise<UnkeyedValidPathInfo>::read(*this, *conn));
@@ -347,7 +304,7 @@ StorePathSet RemoteStore::queryValidDerivers(const StorePath & path)
 
 StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path)
 {
-    if (GET_PROTOCOL_MINOR(getProtocol()) >= 0x16) {
+    if (GET_PROTOCOL_MINOR(getProtocol()) >= 22) {
         return Store::queryDerivationOutputs(path);
     }
     auto conn(getConnection());
@@ -359,7 +316,7 @@ StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path)
 
 std::map<std::string, std::optional<StorePath>> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_)
 {
-    if (GET_PROTOCOL_MINOR(getProtocol()) >= 0x16) {
+    if (GET_PROTOCOL_MINOR(getProtocol()) >= 22) {
         if (!evalStore_) {
             auto conn(getConnection());
             conn->to << WorkerProto::Op::QueryDerivationOutputMap << printStorePath(path);
@@ -509,7 +466,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
             sink
                 << exportMagic
                 << printStorePath(info.path);
-            WorkerProto::WriteConn nested { .to = sink, .version = conn->daemonVersion };
+            WorkerProto::WriteConn nested { sink, conn->daemonVersion };
             WorkerProto::write(*this, nested, info.references);
             sink
                 << (info.deriver ? printStorePath(*info.deriver) : "")
@@ -554,14 +511,13 @@ void RemoteStore::addMultipleToStore(
     RepairFlag repair,
     CheckSigsFlag checkSigs)
 {
+    auto remoteVersion = getProtocol();
+
     auto source = sinkToSource([&](Sink & sink) {
         sink << pathsToCopy.size();
         for (auto & [pathInfo, pathSource] : pathsToCopy) {
             WorkerProto::Serialise<ValidPathInfo>::write(*this,
-                 WorkerProto::WriteConn {
+                 WorkerProto::WriteConn {sink, remoteVersion},
-                     .to = sink,
-                     .version = 16,
-                 },
                  pathInfo);
             pathSource->drainInto(sink);
         }
@@ -670,13 +626,7 @@ void RemoteStore::buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMod
     conn->to << WorkerProto::Op::BuildPaths;
     assert(GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13);
     WorkerProto::write(*this, *conn, drvPaths);
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15)
+    conn->to << buildMode;
-        conn->to << buildMode;
-    else
-        /* Old daemons did not take a 'buildMode' parameter, so we
-           need to validate it here on the client side.  */
-        if (buildMode != bmNormal)
-            throw Error("repairing or checking is not supported when building through the Nix daemon");
     conn.processStderr();
     readInt(conn->from);
 }

src/libstore/store-api.cc

@@ -22,6 +22,14 @@ using json = nlohmann::json;
 
 namespace nix {
 
+BuildMode buildModeFromInteger(int raw) {
+    switch (raw) {
+    case bmNormal: return bmNormal;
+    case bmRepair: return bmRepair;
+    case bmCheck: return bmCheck;
+    default: throw Error("Invalid BuildMode");
+    }
+}
 
 bool Store::isInStore(PathView path) const
 {
@@ -361,15 +369,14 @@ void Store::addMultipleToStore(
     RepairFlag repair,
     CheckSigsFlag checkSigs)
 {
+    auto remoteVersion = getProtocol();
+
     auto expected = readNum<uint64_t>(source);
     for (uint64_t i = 0; i < expected; ++i) {
-        // FIXME we should not be using the worker protocol here, let
+        // FIXME we should not be using the worker protocol here at all!
-        // alone the worker protocol with a hard-coded version!
         auto info = WorkerProto::Serialise<ValidPathInfo>::read(*this,
-            WorkerProto::ReadConn {
+            WorkerProto::ReadConn {source, remoteVersion}
-                .from = source,
+        );
-                .version = 16,
-            });
         info.ultimate = false;
         addToStore(info, source, repair, checkSigs);
     }

src/libstore/store-api.hh

@@ -90,6 +90,9 @@ const uint32_t exportMagic = 0x4558494e;
 
 
 enum BuildMode { bmNormal, bmRepair, bmCheck };
+/** Checks that a build mode is a valid one, then returns it */
+BuildMode buildModeFromInteger(int);
+
 enum TrustedFlag : bool { NotTrusted = false, Trusted = true };
 
 struct BuildResult;

src/libstore/worker-protocol.hh

@@ -9,7 +9,16 @@ namespace nix {
 #define WORKER_MAGIC_1 0x6e697863
 #define WORKER_MAGIC_2 0x6478696f
 
+// This must remain 1.35 (Nix 2.18) forever in Lix, since the protocol has
+// diverged in CppNix such that we cannot assign newer versions ourselves, the
+// protocol is bad in design and implementation and Lix intends to replace it
+// entirely.
 #define PROTOCOL_VERSION (1 << 8 | 35)
+// Nix 2.3 is protocol 1.21 (see RemoteStore::initConnection for client,
+// processConnection for server).
+#define MIN_SUPPORTED_MINOR_WORKER_PROTO_VERSION 21
+#define MIN_SUPPORTED_WORKER_PROTO_VERSION (1 << 8 | MIN_SUPPORTED_MINOR_WORKER_PROTO_VERSION)
+
 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 
@@ -63,6 +72,10 @@ struct WorkerProto
     struct ReadConn {
         Source & from;
         Version version;
+
+        ReadConn(Source & from, Version version) : from(from), version(version) {
+            assert(version >= MIN_SUPPORTED_WORKER_PROTO_VERSION);
+        }
     };
 
     /**
@@ -72,6 +85,10 @@ struct WorkerProto
     struct WriteConn {
         Sink & to;
         Version version;
+
+        WriteConn(Sink & to, Version version) : to(to), version(version) {
+            assert(version >= MIN_SUPPORTED_WORKER_PROTO_VERSION);
+        }
     };
 
     /**

tests/unit/libstore/protocol.hh

@@ -39,10 +39,8 @@ public:
                 StringSource from { expected };
                 Proto::template Serialise<T>::read(
                     *LibStoreTest::store,
-                    typename Proto::ReadConn {
+                    typename Proto::ReadConn {from, version}
-                        .from = from,
+                );
-                        .version = version,
-                    });
             });
 
             ASSERT_EQ(got, value);
@@ -60,10 +58,7 @@ public:
         StringSink to;
         Proto::write(
             *LibStoreTest::store,
-            typename Proto::WriteConn {
+            typename Proto::WriteConn {to, version},
-                .to = to,
-                .version = version,
-            },
             value);
 
         if (testAccept())

tests/unit/libstore/worker-protocol.cc

@@ -18,9 +18,9 @@ struct WorkerProtoTest : VersionedProtoTest<WorkerProto, workerProtoDir>
 {
     /**
      * For serializers that don't care about the minimum version, we
-     * used the oldest one: 1.0.
+     * have to use the minimum supported to not throw an assert.
      */
-    WorkerProto::Version defaultVersion = 1 << 8 | 0;
+    WorkerProto::Version defaultVersion = MIN_SUPPORTED_WORKER_PROTO_VERSION;
 };
 
 
@@ -331,9 +331,9 @@ VERSIONED_CHARACTERIZATION_TEST(
 
 VERSIONED_CHARACTERIZATION_TEST(
     WorkerProtoTest,
-    unkeyedValidPathInfo_1_15,
+    unkeyedValidPathInfo,
-    "unkeyed-valid-path-info-1.15",
+    "unkeyed-valid-path-info",
-    1 << 8 | 15,
+    defaultVersion,
     (std::tuple<UnkeyedValidPathInfo, UnkeyedValidPathInfo> {
         ({
             UnkeyedValidPathInfo info {
@@ -363,56 +363,9 @@ VERSIONED_CHARACTERIZATION_TEST(
 
 VERSIONED_CHARACTERIZATION_TEST(
     WorkerProtoTest,
-    validPathInfo_1_15,
+    validPathInfo,
-    "valid-path-info-1.15",
+    "valid-path-info",
-    1 << 8 | 15,
+    defaultVersion,
-    (std::tuple<ValidPathInfo, ValidPathInfo> {
-        ({
-            ValidPathInfo info {
-                StorePath {
-                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
-                },
-                UnkeyedValidPathInfo {
-                    Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
-                },
-            };
-            info.registrationTime = 23423;
-            info.narSize = 34878;
-            info;
-        }),
-        ({
-            ValidPathInfo info {
-                StorePath {
-                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
-                },
-                UnkeyedValidPathInfo {
-                    Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
-                },
-            };
-            info.deriver = StorePath {
-                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
-            };
-            info.references = {
-                // other reference
-                StorePath {
-                    "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo",
-                },
-                // self reference
-                StorePath {
-                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
-                },
-            };
-            info.registrationTime = 23423;
-            info.narSize = 34878;
-            info;
-        }),
-    }))
-
-VERSIONED_CHARACTERIZATION_TEST(
-    WorkerProtoTest,
-    validPathInfo_1_16,
-    "valid-path-info-1.16",
-    1 << 8 | 16,
     (std::tuple<ValidPathInfo, ValidPathInfo, ValidPathInfo> {
         ({
             ValidPathInfo info {