Merge changes from topic "protocol" into main

* changes:
  libstore client: remove remaining dead code
  libstore: refuse to serialise ancient protocols
  libstore client: remove support for <2.3 clients
  libstore daemon: remove very old protocol support (<2.3)
  Delete old ValidPathInfo test, fix UnkeyedValidPathInfo
  Set up minimum protocol version

src/libstore/daemon.cc

@@ -47,10 +47,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)
     {
@@ -129,12 +134,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);
@@ -142,7 +141,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);
@@ -150,7 +148,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);
@@ -267,14 +264,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) {
 
@@ -527,22 +518,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 = buildModeFromInteger(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();
@@ -746,13 +734,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();
@@ -822,15 +808,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;
@@ -904,12 +889,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();
 
@@ -1011,7 +991,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);
@@ -1027,13 +1007,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;
@@ -1044,10 +1024,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

@@ -75,17 +75,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();
@@ -126,24 +123,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);
@@ -207,20 +202,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);
 }
 
 
@@ -236,20 +224,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);
-    }
 }
 
 
@@ -259,45 +237,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);
     }
 }
 
@@ -314,10 +272,10 @@ std::shared_ptr<const ValidPathInfo> RemoteStore::queryPathInfoUncached(const St
             return nullptr;
         throw;
     }
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) {
+
-        bool valid; conn->from >> valid;
+    bool valid; conn->from >> valid;
-        if (!valid) return nullptr;
+    if (!valid) return nullptr;
-    }
+
     return std::make_shared<ValidPathInfo>(
         StorePath{path},
         WorkerProto::Serialise<UnkeyedValidPathInfo>::read(*this, *conn));
@@ -346,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());
@@ -358,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);
@@ -498,51 +456,21 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
 {
     auto conn(getConnection());
 
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 18) {
+    conn->to << WorkerProto::Op::AddToStoreNar
-        conn->to << WorkerProto::Op::ImportPaths;
+             << printStorePath(info.path)
+             << (info.deriver ? printStorePath(*info.deriver) : "")
+             << info.narHash.to_string(Base16, false);
+    WorkerProto::write(*this, *conn, info.references);
+    conn->to << info.registrationTime << info.narSize
+             << info.ultimate << info.sigs << renderContentAddress(info.ca)
+             << repair << !checkSigs;
 
-        auto source2 = sinkToSource([&](Sink & sink) {
+    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) {
-            sink << 1 // == path follows
+        conn.withFramedSink([&](Sink & sink) {
-                ;
             copyNAR(source, sink);
-            sink
-                << exportMagic
-                << printStorePath(info.path);
-            WorkerProto::WriteConn nested { .to = sink, .version = conn->daemonVersion };
-            WorkerProto::write(*this, nested, info.references);
-            sink
-                << (info.deriver ? printStorePath(*info.deriver) : "")
-                << 0 // == no legacy signature
-                << 0 // == no path follows
-                ;
         });
-
+    } else {
-        conn.processStderr(0, source2.get());
+        conn.processStderr(0, &source);
-
-        auto importedPaths = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        assert(importedPaths.size() <= 1);
-    }
-
-    else {
-        conn->to << WorkerProto::Op::AddToStoreNar
-                 << printStorePath(info.path)
-                 << (info.deriver ? printStorePath(*info.deriver) : "")
-                 << info.narHash.to_string(Base16, false);
-        WorkerProto::write(*this, *conn, info.references);
-        conn->to << info.registrationTime << info.narSize
-                 << info.ultimate << info.sigs << renderContentAddress(info.ca)
-                 << repair << !checkSigs;
-
-        if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) {
-            conn.withFramedSink([&](Sink & sink) {
-                copyNAR(source, sink);
-            });
-        } else if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21) {
-            conn.processStderr(0, &source);
-        } else {
-            copyNAR(source, conn->to);
-            conn.processStderr(0, nullptr);
-        }
     }
 }
 
@@ -553,14 +481,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);
         }
@@ -667,15 +594,8 @@ void RemoteStore::buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMod
 
     auto conn(getConnection());
     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);
 }
@@ -866,25 +786,14 @@ void RemoteStore::queryMissing(const std::vector<DerivedPath> & targets,
     StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
     uint64_t & downloadSize, uint64_t & narSize)
 {
-    {
+    auto conn(getConnection());
-        auto conn(getConnection());
+    conn->to << WorkerProto::Op::QueryMissing;
-        if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 19)
+    WorkerProto::write(*this, *conn, targets);
-            // Don't hold the connection handle in the fallback case
+    conn.processStderr();
-            // to prevent a deadlock.
+    willBuild = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-            goto fallback;
+    willSubstitute = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        conn->to << WorkerProto::Op::QueryMissing;
+    unknown = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        WorkerProto::write(*this, *conn, targets);
+    conn->from >> downloadSize >> narSize;
-        conn.processStderr();
-        willBuild = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        willSubstitute = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        unknown = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        conn->from >> downloadSize >> narSize;
-        return;
-    }
-
- fallback:
-    return Store::queryMissing(targets, willBuild, willSubstitute,
-        unknown, downloadSize, narSize);
 }
 
 

src/libstore/store-api.cc

@@ -368,15 +368,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/worker-protocol.cc

@@ -165,11 +165,11 @@ UnkeyedValidPathInfo WorkerProto::Serialise<UnkeyedValidPathInfo>::read(const St
     if (deriver != "") info.deriver = store.parseStorePath(deriver);
     info.references = WorkerProto::Serialise<StorePathSet>::read(store, conn);
     conn.from >> info.registrationTime >> info.narSize;
-    if (GET_PROTOCOL_MINOR(conn.version) >= 16) {
+
-        conn.from >> info.ultimate;
+    conn.from >> info.ultimate;
-        info.sigs = readStrings<StringSet>(conn.from);
+    info.sigs = readStrings<StringSet>(conn.from);
-        info.ca = ContentAddress::parseOpt(readString(conn.from));
+    info.ca = ContentAddress::parseOpt(readString(conn.from));
-    }
+
     return info;
 }
 
@@ -180,12 +180,11 @@ void WorkerProto::Serialise<UnkeyedValidPathInfo>::write(const Store & store, Wr
         << pathInfo.narHash.to_string(Base16, false);
     WorkerProto::write(store, conn, pathInfo.references);
     conn.to << pathInfo.registrationTime << pathInfo.narSize;
-    if (GET_PROTOCOL_MINOR(conn.version) >= 16) {
+
-        conn.to
+    conn.to
-            << pathInfo.ultimate
+        << pathInfo.ultimate
-            << pathInfo.sigs
+        << pathInfo.sigs
-            << renderContentAddress(pathInfo.ca);
+        << renderContentAddress(pathInfo.ca);
-    }
 }
 
 }

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 {