feat: Hash parts on upload to detect silent bit-rot

[danim7]

When uploading a file to a peer, we could add an option to check the hash of that part before sending it.

We could use the SHA hash tree which hashes every 180KB part for this, which aligns better with the read parts algo in the upload thread. This feature could be toggled via a new preference option, defaulted to Off to stay aligned with the current behavior and not increasing CPU usage for the average user.

The performance cost breakdown would be:

• The file part needs to be read from disk for the upload anyway, so the main performance cost is already paid.
• We need to retrieve the SHA hashes for that part. I believe that during memory leaks analysis for the 3.0.0 release, it was said that the SHA hash tree is kept in memory, but I cannot find that thread now. If that is true, no extra IO/RAM cost. If not, there is a small extra cost to retrieve that information from the file.
• The remaining cost is CPU to calculate the hash. A modern CPU with SHA instructions is very fast (> 1GB/s), an old CPU could just keep the option off by default if they don't want to pay the cost.

The benefits of this feature are:

• Detect silent bit-rot in files. For part files this also allows an early detection, to re-ask for that part instead of waiting for the completion hash.
• Do not upload corrupted data to peers, and avoid being banned because of this.

When a hash failure is detected:

• At least, add a log line to inform the user about that error
• For part files, mark that part as uncompleted to re-ask for it
• For completed files, I'm unsure about the best course of action. At least, not sending that part to the peer, and maybe stop sharing the whole file? Maybe add the file to a corrupted file list for the user to act upon...Something else?

Opinions?

[got3nks]

Thanks for the writeup. Walked through the existing code to map overlap — here's the analysis.

What's already in place

• Download side: MD4 part hash on each part completion → mismatch marks Failed and re-requests that part. AICH recovery (180 KB granularity) kicks in when MD4 fails. Full-file hash check on completion.
• mtime/size watch on shared files: SharedFileList.cpp:781-789 — the watcher path compares disk mtime + size against the stored m_lastDateChanged / GetFileSize(). Any delta detaches the file and queues a fresh CHashingTask. The startup scan does the same comparison. So in-place edits / re-encodes / copy-overs automatically trigger a re-hash.
• Upload side: no hash check today. We open the file, seek, read, send.

AICH residency

One assumption worth correcting: AICH hashsets are NOT kept resident by default. SHAHashSet.cpp:487 / 529 shows LoadHashSet() reads from known2.met on demand and FreeHashSet() releases right after each operation. A naïve "check before sending each block" would do a known2.met read + free per upload chunk; a practical implementation would need to cache AICH for the lifetime of each active upload session per file (~80 KB / GB of file size, scoped to active uploads only to keep steady-state RAM small).

What's left after subtracting existing behaviour

Scenario	Already caught	By
In-place re-encode / edit	✅	mtime+size watcher → re-hash
File overwritten under same name	✅	mtime watcher
Bad MD4 part on download	✅	receive-side check
Bad partfile content while still downloading	⚠️	partial — handled in DL queue, not via SharedFileList
True silent bit-rot (mtime doesn't change)	❌	nothing — this is the unique add

Pros / cons

Pros

• Catches true silent bit-rot — undetected controller-level write errors, SSD cells where the write succeeded but the read drifts, cosmic-ray bit flips. Rare on consumer ext4 / NTFS, non-zero on long-running seeders.
• "Don't knowingly serve corrupted data" is a clean stance, even if the eD2k consequence isn't a formal ban — peers just re-request and downgrade us as a source over time.
• Opt-in / default-off keeps the cost zero for the average user.

Cons

• The remaining unique value-add is essentially "filesystem-level checksumming for users who aren't on ZFS / Btrfs / APFS-sealed". Small audience.
• Implementation effort is non-trivial: AICH caching, upload-path integration, pref + UX, failure-mode plumbing.
• Per-session memory overhead needs scoping carefully so heavy seeders don't pay a steady cost.

My take

Worth flagging but probably not a near-term priority. If the AICH-caching change is wanted for other reasons (e.g. faster CreatePartRecoveryData on busy seeders), the hash-on-upload check could ride along almost for free. Standalone it's hard to justify the work against the narrow remaining value-add.

Curious if there's a specific reproducible case that motivated this — would help calibrate whether the silent-bit-rot scenario actually bites users in practice.

[danim7]

Thanks for the answer.

The motivation is not based on a solid reproducible evidence, just on observational data.

I have suffered bit-rot in the past myself. And also, in the ed2k network, it is not rare to search for a file, and get multiple results with the exact name and size, but different hashes. I think those cases are probably explained by bit rot:

1. Someone shares a file that gets bit-rot.
2. For a time, this person shares corrupt data, even if other peers identify it and recover from that.
3. At some point, that person touches the file, copies it, reinstall his ed2k client, migrates to another computer...and it triggers a rehash. The file is then shared with the new hash, and we get 2 results in the search.

Normally, in those cases, the logical move is to download the file with more sources, but I remember once that all files just had a couple of sources, so I downloaded both, and diffed the result: just a few bytes were different.

I guess that for a single person, having bit-rot is rare. For a network with thousand of peers, it becomes less rare.

If the AICH-caching change is wanted for other reasons (e.g. faster CreatePartRecoveryData on busy seeders)

This makes me wonder: what is the Time Complexity to retrieve the hash set from known2 file when a peer asks for the AICH hashes of a given file? O(1), O(log N), O(N) being N the size of the library?

For this particular feature request: if you think this could be considered in the future or if you have a wishlist label, keep it open. If you prefer to have a clean backlog or you will never accept a PR about it, feel free to close it.

[Stoatwblr]

Disk ECC read-rot is more common than you think. It works out to about one silently corrupted sector per 5-8TB read

That's with 512e or 512n drives. one of the selling points of 4096n is that it throws more bits into ECC and improves this rate to something like 1 per PB read

On large and busy ZFS fileservers, silent ECC errors are a regular (but occasional) log feature

[Stoatwblr]

I get that there's a penalty but AICH scrub isn't a bad idea in any case.

Perhaps gated behind checkboxes for regular checking of the entire tree and "Per-file upon upload request"

For a very long time I've wondered about the idea of converting known files to database tables and then dealing with them via standardised SQL queries - it would make third party software access a lot easier, but in this instance it's one pass of the file for its hashes and a DB query to see if they match

Files like known.met or are good for many uses but like all things which work in small setups they don't necessarily scale out well

[danim7]

This makes me wonder: what is the Time Complexity to retrieve the hash set from known2 file when a peer asks for the AICH hashes of a given file? O(1), O(log N), O(N) being N the size of the library?

Files like known.met or are good for many uses but like all things which work in small setups they don't necessarily scale out well

If I'm reading the code correctly:

• When a peer asks for the AICH Hash set, it triggers a linear scan of know2 file (O(N) being N the library size) to retrieve the required hashes.
• The known2 file contains roughly 1 SHA1 hash (20 bytes long) for each 180 KB of shared data: ~120 MiB per 1 TiB of shared data.
• The treatment of this request is not gated through the CheckForAggressive() function, so an evil client could trigger our instance to repeatedly perform this O(N) scan. The larger the library, the known2 file gets bigger, the more vulnerable our client is.

Shall this request be somehow limited per client?
Could we store the offset where each KnownFile can find its AICH Hash set in known2 to improve the retrieval from O(N) to O(1)?

For a very long time I've wondered about the idea of converting known files to database tables and then dealing with them via standardised SQL queries - it would make third party software access a lot easier, but in this instance it's one pass of the file for its hashes and a DB query to see if they match

I guess we keep these files to be compatible with eMule, and to avoid having one extra SQL dependency to compile with. But yep, I also think that would be cool to run our queries for stats or other uses. All we have now is the FILEVIEW tool.

[got3nks]

Verified all three findings from your code walk:

1. O(N) scan confirmed. LoadHashSet walks known2.met sequentially: read root hash, compare, seek past on miss. Even the hit path traverses entries up to the match.

2. Ungated handler confirmed. OP_AICHREQUEST dispatch at ClientTCPSocket.cpp:1548-1553 goes straight to ProcessAICHRequest with no CheckForAggressive() call, unlike the file-request paths at :539 and ClientUDPSocket.cpp:194.

3. No offset map. The existing s_rootHashCache at SHAHashSet.cpp:50-53 is a dedup unordered_set<CAICHHash> used by SaveHashSet to skip duplicate writes — it's not a hash→offset map, so it doesn't help LoadHashSet.

So the DoS vector is real: 16-byte OP_AICHREQUEST from an attacker forces an O(N) disk-backed scan on the seeder per packet. Worse on big libraries — ~120 MiB / TiB of shared content per your estimate.

Both mitigations you proposed are sound and land independently of the original feature ask:

• Rate-limit AICH requests via CheckForAggressive(). Small, surgical, removes the amplification today. Should ship regardless of whether upload-hash-check ever happens. Worth its own PR.
• Offset cache for LoadHashSet. Extend s_rootHashCache from a set into an unordered_map<CAICHHash, uint64 file_offset>, populated by the same one-shot walk in LoadAICHCache. Converts LoadHashSet from O(N) to O(1) after the cache is warm, with negligible RAM cost (32 bytes per entry × number of files in library). Pays off for both legitimate AICH responses on busy seeders and for any future hash-on-upload feature.

These are two clean, tractable PRs. The original silent-bit-rot question is independent of both and can stay open as a backlog item with Stoatwblr's ECC data as a real-world motivator.

[got3nks]

Both side-issues from this thread (rate-limit on OP_AICHREQUEST and the O(N) LoadHashSet scan) are addressed in #186. Keeping this issue open for the original feature request — hash parts on upload to detect silent bit-rot.

[Stoatwblr]

Can we add "scrub (or preen) filestore" as an extension option to the check-on-upload functionality?

It's a "here be dragons" item in terms of the system impact(*) but for large sharesets I think it's worthwhile and could prevent/mitigate the issue of "identical files with different hashes"

(*) You can say the same thing about ZFS scrub or "smartctl -t long" but they still get invoked regularly

[danim7]

Both side-issues from this thread (rate-limit on OP_AICHREQUEST and the O(N) LoadHashSet scan) are addressed in #186. Keeping this issue open for the original feature request — hash parts on upload to detect silent bit-rot.

Happy this thread was already useful to detect and fix some other issues.

Can we add "scrub (or preen) filestore" as an extension option to the check-on-upload functionality?

It's a "here be dragons" item in terms of the system impact(*) but for large sharesets I think it's worthwhile and could prevent/mitigate the issue of "identical files with different hashes"

(*) You can say the same thing about ZFS scrub or "smartctl -t long" but they still get invoked regularly

Behind a check box with a big performance warning, why not?

However, I don't see how would it be triggered? User-initiated by clicking on a button? Periodic scrub X weeks/months after the last check?

[Stoatwblr]

"User-initiated by clicking on a button? Periodic scrub X weeks/months after the last check?"

Yes.

Both. The user initiated button up front and a "schedule scrubs" once it's been used. That way they get to experience the hit and decide if they want it scheduled or to stay manual.

The paralleism stuff I'm messing about with would be an ideal way. If it's run through the original single-threaded 1000ms GUI timer-based hasher the load is low but the process is loooong (which might be ideal for this purpose - a "quiet background" idle-level scan or "burn down my filestore" sprint)