Snapshot fixes

[svlachakis]

Changes

• Fix 1 — SnapshotDownloader.cs: HTTP 200 on resume now skips bytes instead of overwriting
• Fix 2 — InitDatabaseSnapshot.cs: disk-space check uses the correct mount

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Manual test of Arbitrum Snapshot is in progress.

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

[claude]

Claude finished @svlachakis's task in 2m 43s —— View job

---

PR Review: Snapshot fixes

• Gather context and read changed files
• Read relevant rules files
• Review SnapshotDownloader.cs changes
• Review InitDatabaseSnapshot.cs changes
• Post inline comments for specific issues
• Write summary

---

Summary

Both fixes are correct and address real production bugs. No blocking issues found.

Fix 1 — SnapshotDownloader.cs: HTTP 200 resume

The old code treated any HTTP 200 response as "no range support → discard partial file and restart." The new code recognises that HTTP 200 + existing partial file means the server sent the full content from byte 0, and correctly skips the already-downloaded prefix via SkipBytesAsync before appending the remainder. The approach is sound:

• ResolveCopyStrategy switch arms are in the right order; the when existingSize > 0 guard correctly shadows the plain OK arm.
• SkipBytesAsync correctly throws IOException when the stream ends short, which propagates to DownloadWithRetryAsync for a clean retry.
• The (int)Math.Min(buffer.Length, remaining) cast is safe: the result is always ≤ BufferSize (65 536).
• Nullable arithmetic on existingSize + contentLength propagates null correctly → indeterminate progress when the server omits Content-Length.

Fix 2 — InitDatabaseSnapshot.cs: disk-space check mount

The old code used Path.GetPathRoot(dbPath) ?? "/" which always resolves to "/" on Linux, causing the check to examine root partition free space rather than the partition that will actually hold the database. The new code delegates to the well-tested IFileSystem.GetDriveInfos(dbPath) extension (longest-prefix mount match, handles symlinks, safely no-ops on macOS). The multiplier increase from 2× to 2.5× is a minor improvement in safety margin.

---

Findings

#	Severity	File	Description
1	Low	SnapshotDownloader.cs:91	existingSize + contentLength nullable arithmetic is correct but non-obvious without a comment
2	Low	SnapshotDownloader.cs:72	Progress display freezes during SkipBytesAsync — cosmetic UX issue for large partial downloads on non-range servers
3	Low	InitDatabaseSnapshot.cs:194	new FileInfo(snapshotPath).Length bypasses api.FileSystem abstraction (pre-existing, consistency note only)

No Critical, High, or Medium findings. This PR is mergeable.

[svlachakis]

@claude review again, fixed one comment - the other two doesn't seem to be worth the complexity

[claude]

Claude finished @svlachakis's task in 2m 5s —— View job

---

PR Re-Review: Snapshot fixes

• Read changed files
• Compare against previous review comments
• Post updated findings

---

What changed since last review

Three Low findings were raised previously:

#	Previous finding	Status
1	existingSize + contentLength nullable arithmetic — suggest inline comment	Not addressed (author deferred — acceptable)
2	Progress display freezes during SkipBytesAsync skip phase	Not addressed (author deferred — acceptable)
3	new FileInfo(snapshotPath).Length bypasses api.FileSystem abstraction	Fixed — now api.FileSystem.FileInfo.New(snapshotPath).Length ✓

Fresh read of both files

SnapshotDownloader.cs — no new issues.

• ResolveCopyStrategy 3-tuple is correct across all three arms (206 / 200+partial / 200 fresh).
• SkipBytesAsync is clean: ArrayPool use is balanced, (int)Math.Min(buffer.Length, remaining) is a safe downcast (result always ≤ BufferSize = 65 536), throws IOException on premature EOF to trigger the existing retry path.
• initialProgress = existingSize on FileMode.Append is correct for both the 206 and 200+partial paths; the progress tracker shows work already done, then CopyWithProgressAsync accumulates the remainder.

InitDatabaseSnapshot.cs — no new issues.

• GetDriveInfos(dbPath) correctly resolves the mount that will hold the extracted database, fixing the original Linux root-partition bug.
• Early return on drives.Length == 0 (macOS no-op) is correct.
• snapshotSize is only accessed after the early return, so no unnecessary I/O when the check is skipped.
• The foreach over drives is conservative (each drive individually checked against required) but safe; the typical case is a single drive.

DI comment (damian-orzechowski) — author's explanation is sound: the keyed IDriveInfo[] registration lives in HealthChecksPlugin, so pulling it into Nethermind.Init.Snapshot would create a cross-plugin dependency. api.FileSystem.GetDriveInfos(dbPath) keeps this self-contained. Agreed with deferring the registration lift to a follow-up.

---

Findings

No new findings. Previous Lows #1 and #2 are cosmetic and acknowledged. Previous Low #3 is fixed.

No Critical, High, or Medium findings. PR is mergeable.

[claude]

Claude finished @svlachakis's task in 1m 48s —— View job

---

PR Review: Snapshot fixes

• Gather context
• Read changed files
• Read relevant rules files
• Review all changes
• Post findings

---

What's in this PR

Three functional changes across two files, plus a CODEOWNERS update:

1. SnapshotDownloader.cs — HTTP 200 on resume now skips the already-downloaded prefix via SkipBytesAsync instead of overwriting from scratch
2. InitDatabaseSnapshot.cs — CheckDiskSpace uses api.FileSystem.GetDriveInfos(dbPath) (correct mount) instead of Path.GetPathRoot(dbPath) (always / on Linux)
3. InitDatabaseSnapshot.cs — Retry backoff now resets to InitialRetryDelaySeconds if the file grew since the last attempt (new since previous review)

---

Fix 1 — SnapshotDownloader.cs

The ResolveCopyStrategy 3-tuple is correct across all arms:

Status	existingSize	FileMode	bytesToSkip	totalSize
206 Partial Content	any	Append	0	existingSize + contentLength
200 OK	> 0	Append	existingSize	contentLength
200 OK	0	Create	0	contentLength

SkipBytesAsync is clean: ArrayPool use is balanced, the (int)Math.Min(buffer.Length, remaining) downcast is safe (result always ≤ BufferSize = 65 536), and the IOException on premature EOF propagates to the retry path correctly.

initialProgress = existingSize on FileMode.Append is correct for both the 206 and 200+partial paths.

No issues.

---

Fix 2 — InitDatabaseSnapshot.cs: disk-space check

api.FileSystem.GetDriveInfos(dbPath) correctly resolves the mount that will hold the extracted database, fixing the Linux root-partition bug. The early return on drives.Length == 0 (macOS no-op) is correct. snapshotSize is only accessed after the guard, so no unnecessary I/O when the check is skipped. The foreach loop is safe (typical case is a single drive).

(long)(snapshotSize * 2.5) — snapshotSize is long, multiply by double, explicit cast back to long. Correct; no overflow risk for realistic snapshot sizes.

No issues.

---

Fix 3 — Retry backoff reset (new since last review)

long currentSize = GetFileSize(destinationPath);
if (currentSize > lastSize)
    retryDelay = TimeSpan.FromSeconds(InitialRetryDelaySeconds);
lastSize = currentSize;

This is a sound heuristic: if the download was making progress before the error, reset the backoff rather than continuing to double it. GetFileSize uses api.FileSystem.FileInfo.New(path) — consistent with the rest of the method.

No issues.

---

CODEOWNERS

PR author adds themselves to /src/Nethermind/Nethermind.Init ownership. Fine if approved by existing owners (@LukaszRozmej, @asdacap).

---

Findings

#	Severity	Description
1	Low	Progress display freezes during SkipBytesAsync skip phase — acknowledged and deferred by author
2	Low	existingSize + contentLength nullable arithmetic — acknowledged and deferred by author

No Critical, High, or Medium findings. PR is mergeable.