Always prefer peer's snapshot

CHANGELOG.md

@@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [7.0.0-dev4]
+
+[7.0.0-dev4]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.0-dev
+
+### Changed
+
+- 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.
+
 ## [7.0.0-dev3]
 
 [7.0.0-dev3]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.0-dev3

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"
                   },
                   "fetch_snapshot_max_attempts": {
                     "type": "integer",

python/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "ccf"
-version = "7.0.0-dev3"
+version = "7.0.0-dev4"
 authors = [
   { name="CCF Team", email="[email protected]" },
 ]

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,
             config.command.join.fetch_snapshot_max_attempts,
             config.command.join.fetch_snapshot_retry_interval.count_ms());
 
@@ -878,11 +872,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;
@@ -891,6 +884,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> try_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(
@@ -263,7 +273,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> latest_local_snapshot,
     size_t max_attempts,
     size_t retry_delay_ms)
   {

tests/e2e_operations.py

@@ -392,7 +392,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
@@ -429,6 +436,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