Always prefer peer's snapshot

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Changed
 
 - The `submit_recovery_share.sh` script will no longer try to create a virtual environment and install the CCF Python package on every call. Instead it will return an error if the package is not installed (specifically if the `ccf_cose_sign1` tool it relies on cannot be found) (#7306)
+- When the `fetch_recent_snapshots` behaviour is enabled by the node config, the Joiner will now prefer the peer's snapshot over _any_ local snapshot, regardless of version.
 
 ### Removed
 

doc/host_config_schema/cchost_config.json

@@ -381,7 +381,7 @@
                   "fetch_recent_snapshot": {
                     "type": "boolean",
                     "default": true,
-                    "description": "Whether to ask the target for a newer snapshot before joining. The node will ask the target what their latest snapshot is, and if that is later than what the node has locally, will fetch it via RPC before launching. Should generally only be turned off for specific test cases"
+                    "description": "Whether to ask the target for a snapshot before joining. The node will ask the target what their latest snapshot is and will fetch it via RPC before launching. Should generally only be turned off for specific test cases"
                   }
                 },
                 "required": ["target_rpc_address"],

src/host/run.cpp

@@ -847,21 +847,15 @@ namespace ccf
         config.command.type == StartType::Join ||
         config.command.type == StartType::Recover)
       {
-        auto latest_local_snapshot = snapshots.find_latest_committed_snapshot();
-
         if (
           config.command.type == StartType::Join &&
           config.command.join.fetch_recent_snapshot)
         {
           // Try to fetch a recent snapshot from peer
-          const size_t latest_local_idx = latest_local_snapshot.has_value() ?
-            snapshots::get_snapshot_idx_from_file_name(
-              latest_local_snapshot->second) :
-            0;
           auto latest_peer_snapshot = snapshots::fetch_from_peer(
             config.command.join.target_rpc_address,
             config.command.service_certificate_file,
-            latest_local_idx);
+            std::nullopt);
 
           if (latest_peer_snapshot.has_value())
           {
@@ -876,11 +870,10 @@ namespace ccf
               fs::path(latest_peer_snapshot->snapshot_name);
             if (files::exists(dst_path))
             {
-              LOG_FATAL_FMT(
-                "Unable to write peer snapshot - already have a file at {}. "
-                "Exiting.",
+              LOG_FAIL_FMT(
+                "Overwriting existing snapshot at {} with data retrieved from "
+                "peer",
                 dst_path);
-              return static_cast<int>(CLI::ExitCodes::FileError);
             }
             files::dump(latest_peer_snapshot->snapshot_data, dst_path);
             startup_snapshot = latest_peer_snapshot->snapshot_data;
@@ -889,6 +882,9 @@ namespace ccf
 
         if (startup_snapshot.empty())
         {
+          auto latest_local_snapshot =
+            snapshots.find_latest_committed_snapshot();
+
           if (latest_local_snapshot.has_value())
           {
             auto& [snapshot_dir, snapshot_file] = latest_local_snapshot.value();

src/snapshots/fetch.h

@@ -42,7 +42,7 @@ namespace snapshots
   static std::optional<SnapshotResponse> fetch_from_peer(
     const std::string& peer_address,
     const std::string& path_to_peer_cert,
-    size_t latest_local_snapshot)
+    std::optional<size_t> since_seqno)
   {
     try
     {
@@ -53,10 +53,13 @@ namespace snapshots
         ccf::curl::UniqueCURL curl_easy;
         curl_easy.set_opt(CURLOPT_CAINFO, path_to_peer_cert.c_str());
 
-        auto initial_url = fmt::format(
-          "https://{}/node/snapshot?since={}",
-          peer_address,
-          latest_local_snapshot);
+        auto initial_url =
+          fmt::format("https://{}/node/snapshot", peer_address);
+
+        if (since_seqno.has_value())
+        {
+          initial_url += fmt::format("?since={}", since_seqno.value());
+        }
 
         ccf::curl::UniqueSlist headers;
 
@@ -83,8 +86,15 @@ namespace snapshots
         }
         if (status_code == HTTP_STATUS_NOT_FOUND)
         {
-          LOG_INFO_FMT(
-            "Peer has no snapshot newer than {}", latest_local_snapshot);
+          if (since_seqno.has_value())
+          {
+            LOG_INFO_FMT(
+              "Peer has no snapshot newer than {}", since_seqno.value());
+          }
+          else
+          {
+            LOG_INFO_FMT("Peer has no snapshot");
+          }
           return std::nullopt;
         }
         EXPECT_HTTP_RESPONSE_STATUS(

tests/e2e_operations.py

@@ -391,7 +391,14 @@ def test_empty_snapshot(network, args):
 
             # Create new node and join network
             new_node = network.create_node("local://localhost")
-            network.join_node(new_node, args.package, args, snapshots_dir=snapshots_dir)
+            network.join_node(
+                new_node,
+                args.package,
+                args,
+                snapshots_dir=snapshots_dir,
+                # Don't try to fetch a snapshot, look at the local files
+                fetch_recent_snapshot=False,
+            )
             new_node.stop()
 
             # Check that the empty snapshot is correctly skipped
@@ -428,6 +435,8 @@ def test_nulled_snapshot(network, args):
                 args.package,
                 args,
                 snapshots_dir=snapshots_dir,
+                # Don't try to fetch a snapshot, look at the local files
+                fetch_recent_snapshot=False,
             )
         except Exception as e:
             failed = True