docs(comments): add 5 why-comments to the encryption code - #88
Merged
Conversation
Constructive commenting pass (/comment-run) over the on-disk encryption surface merged in #85 — the complement to the wrong-comment fixes in #87. Additive only: ADD/EXTEND, no existing comment text deleted or rewritten. Comments only; full test suite, clippy, and fmt stay green. Each captures a non-obvious "we do X because Y" a first-time reader could not infer from the code: - crypto/mod.rs: Argon2id's Version::V0x13 and 32-byte output are pinned to the on-disk format (like KEK_INFO for HKDF) — changing either silently breaks unwrap of every existing Argon2id slot. - superblock/mod.rs: the four bootstrap fields must stay cleartext because they ARE the AAD, and slot selection + AAD derivation run before any DEK is available (a sealed body would be a chicken-and-egg deadlock). - page_cache.rs: the encrypted cold-load DecryptionFailed is fatal/poisoning, a peer of ChecksumMismatch, not a retryable operational error. - page_io.rs: set_stride swallows a seek error to 0 because its signature is infallible (bootstrap call site) and 0 makes reads fail closed. - spillway.rs: spill writes are deliberately not fsynced — content never crosses a transaction boundary, so durability would be wasted I/O. The pass found 0 wrong comments (fixed in #87) and 0 bugs. Six of the eleven modules reviewed needed nothing added — the encryption code was already well-commented from its per-task review gates.
🚦 Bench results: PR vs main✅ No regressions detected
Per-scenario detail (4 metrics × cells)document-store
mutation-log
ycsb-a
ycsb-b
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A constructive commenting pass (
/comment-run) over the on-disk encryption code (#85) — the complement to the wrong-comment fixes in #87. Additive only: ADD/EXTEND per the standard, no existing comment text deleted or rewritten. Comments only, no behavior change.Each of the five explains a non-obvious why a first-time reader could not infer from the code:
crypto/mod.rsVersion::V0x13+ 32-byte output are format-critical — changing either silently breaks unwrap of every existing Argon2id slot (data loss on a careless library bump).superblock/mod.rspage_cache.rsDecryptionFailedis fatal/poisoning, a peer ofChecksumMismatch, not a retryable operational error.page_io.rsset_strideswallows a seek error to0because its signature is infallible (bootstrap call site) and0makes reads fail closed (InvalidPageId).spillway.rsMethod & findings
Reviewed the 11 encryption production modules one agent at a time, each self-applying a strict why-not-what filter (rejecting anything that merely restates the code). 6 of 11 modules needed nothing added — the encryption code was already well-commented from its per-task review gates. The pass found 0 wrong comments (those were fixed in #87) and 0 bugs.
Verification
Comment-only;
cargo build, fullcargo test,cargo clippy --workspace -- -D warnings, andcargo fmt --checkall clean.