Skip to content

Fix snapshot download since parameter to use closed/inclusive semantics#7775

Merged
achamayou merged 6 commits intomainfrom
copilot/fix-issue-7742
Mar 26, 2026
Merged

Fix snapshot download since parameter to use closed/inclusive semantics#7775
achamayou merged 6 commits intomainfrom
copilot/fix-issue-7742

Conversation

Copy link
Contributor

Copilot AI commented Mar 26, 2026

  • Understand issue We have different semantics for "since" in ledger chunk download (closed) vs snapshot download (open) #7742: snapshot download uses open/exclusive since semantics, while ledger chunk uses closed/inclusive; align snapshot to closed/inclusive
  • Change src/snapshots/filenames.h: filter condition idx <= minimum_idxidx < minimum_idx
  • Remove - 1 /* YIKES */ hack in src/node/node_state.h
  • Update test expectations in tests/e2e_operations.py (since=snapshot_index now returns snapshot)
  • Update CHANGELOG.md
  • Fix C++ formatting issues in pre-existing files (cose_auth.cpp, historical.cpp, kv.cpp, quote_endorsements_client.h)

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Copilot AI requested a review from achamayou March 26, 2026 11:28
@achamayou achamayou added the run-long-test Run Long Test job label Mar 26, 2026
@achamayou achamayou marked this pull request as ready for review March 26, 2026 11:33
@achamayou achamayou requested a review from a team as a code owner March 26, 2026 11:33
Copilot AI review requested due to automatic review settings March 26, 2026 11:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns the since query parameter semantics on GET /node/snapshot with GET /node/ledger_chunk by making snapshot filtering closed/inclusive, and removes the related off-by-one workaround in backup snapshot fetching.

Changes:

  • Make snapshot discovery treat since=N as inclusive (return snapshots with index >= N).
  • Remove the - 1 workaround when scheduling backup snapshot fetches based on snapshot evidence.
  • Update E2E expectations and document the behavior change in the changelog.

Reviewed changes

Copilot reviewed 4 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/snapshots/filenames.h Adjusts minimum snapshot index filtering to be inclusive (< instead of <=).
src/node/node_state.h Removes the off-by-one adjustment when scheduling BackupSnapshotFetch tasks.
tests/e2e_operations.py Updates snapshot endpoint since parameter expectations to match inclusive behavior.
CHANGELOG.md Documents the updated since semantics for GET /node/snapshot.
src/node/quote_endorsements_client.h Contains an unrelated constructor formatting-only change.
src/js/extensions/ccf/kv.cpp Contains an unrelated constructor formatting-only change.
src/js/extensions/ccf/historical.cpp Contains an unrelated constructor formatting-only change.
src/endpoints/authentication/cose_auth.cpp Contains an unrelated constructor formatting-only change.

@achamayou
Copy link
Member

@copilot fix the formatting!

Copy link
Contributor Author

Copilot AI commented Mar 26, 2026

@copilot fix the formatting!

Fixed in 0e5adc3 — removed trailing semicolons after constructor body braces in the 4 pre-existing files that clang-format 18 flags.

Copy link
Contributor Author

Copilot AI commented Mar 26, 2026

@eddyashton I've opened a new pull request, #7777, to work on those changes. Once the pull request is ready, I'll request review from you.

eddyashton and others added 3 commits March 26, 2026 12:07
Co-authored-by: Eddy Ashton <ashton.eddy@gmail.com>
Co-authored-by: Eddy Ashton <ashton.eddy@gmail.com>
Copilot AI requested a review from eddyashton March 26, 2026 13:17
@eddyashton eddyashton added this to the 7.0.0-rc0 milestone Mar 26, 2026
@achamayou achamayou enabled auto-merge (squash) March 26, 2026 13:44
@achamayou achamayou merged commit 219c4ee into main Mar 26, 2026
17 of 18 checks passed
@achamayou achamayou deleted the copilot/fix-issue-7742 branch March 26, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-long-test Run Long Test job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants