From f602f029ce38291c2e92f64a65380081060badf2 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Thu, 25 Sep 2025 14:17:29 +0000 Subject: [PATCH 1/5] Always prefer peer's snapshot --- CHANGELOG.md | 1 + doc/host_config_schema/cchost_config.json | 2 +- src/host/run.cpp | 11 ++++------- src/snapshots/fetch.h | 24 ++++++++++++++++------- 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45e818fa68c7..41a9a8238634 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/doc/host_config_schema/cchost_config.json b/doc/host_config_schema/cchost_config.json index 033b094e2233..cfc3dcbb8ff6 100644 --- a/doc/host_config_schema/cchost_config.json +++ b/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"], diff --git a/src/host/run.cpp b/src/host/run.cpp index 18cab128d531..d9622c344fd3 100644 --- a/src/host/run.cpp +++ b/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()) { @@ -889,6 +883,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(); diff --git a/src/snapshots/fetch.h b/src/snapshots/fetch.h index d63e5a88f0c9..ae7e32ef9c83 100644 --- a/src/snapshots/fetch.h +++ b/src/snapshots/fetch.h @@ -42,7 +42,7 @@ namespace snapshots static std::optional fetch_from_peer( const std::string& peer_address, const std::string& path_to_peer_cert, - size_t latest_local_snapshot) + std::optional 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( From d73e522330ca48bfc35f425412432c3b93eb1dc8 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Thu, 25 Sep 2025 15:22:04 +0000 Subject: [PATCH 2/5] Fix operations tests --- tests/e2e_operations.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index c2c2b91cad1e..b60d32e71477 100644 --- a/tests/e2e_operations.py +++ b/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 From f47f3dad47b2533eebd709af1a5c70c6fc44224c Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Fri, 26 Sep 2025 08:44:17 +0000 Subject: [PATCH 3/5] Overwrite existing snapshot if necessary --- src/host/run.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/host/run.cpp b/src/host/run.cpp index d9622c344fd3..18fd72f82d8a 100644 --- a/src/host/run.cpp +++ b/src/host/run.cpp @@ -870,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(CLI::ExitCodes::FileError); } files::dump(latest_peer_snapshot->snapshot_data, dst_path); startup_snapshot = latest_peer_snapshot->snapshot_data; From 9ebbd4717e6d7a5af1846e0174c3a758995436cd Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Tue, 30 Sep 2025 16:22:46 +0000 Subject: [PATCH 4/5] fixes --- CHANGELOG.md | 9 ++++++++- src/snapshots/fetch.h | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1391b7a7ed23..6930566c47e3 100644 --- a/CHANGELOG.md +++ b/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 @@ -25,7 +33,6 @@ 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 diff --git a/src/snapshots/fetch.h b/src/snapshots/fetch.h index f75b77a0ba78..fe9634e47408 100644 --- a/src/snapshots/fetch.h +++ b/src/snapshots/fetch.h @@ -273,7 +273,7 @@ namespace snapshots static std::optional fetch_from_peer( const std::string& peer_address, const std::string& path_to_peer_cert, - size_t latest_local_snapshot, + std::optional latest_local_snapshot, size_t max_attempts, size_t retry_delay_ms) { From 83acedacbc8effad4b413b1481acfc0f2bbe36e6 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Tue, 30 Sep 2025 16:23:28 +0000 Subject: [PATCH 5/5] pyproject --- python/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/pyproject.toml b/python/pyproject.toml index baeacc68c37e..57d7962a485c 100644 --- a/python/pyproject.toml +++ b/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="CCF-Sec@microsoft.com" }, ]