Repository navigation
feat(wasm)!: support large-file downloads and disk streaming - #214
Conversation
d444c5c to
837d625
Compare
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Code Review — Hermes Agent
Verdict: No material blockers.
Verified against head 1bc1931:
64-bit positions. FileIndex.offsets is Vec<u64>; chunk_span and chunks_overlapping operate on u64 throughout. wasm32 truncation at 4 GiB is eliminated. readRange now validates inputs as nonnegative safe integers and rejects fractional/negative/imprecise arguments.
Memory bound. STREAM_BUFFER_BYTES = 32 MiB. ordered_pipeline admits items while outstanding + cost(next) <= budget; outstanding covers in-flight + fetched + being-written. A blocked write stops fetching at the bound. Test pipeline_writes_in_order_within_its_budget covers this.
KDF. Decryption delegates to self_encryption::decrypt_chunk(chunk, encrypted, &src_hashes, 0) with original indices and neighbouring source hashes — same canonical KDF as the native path.
Manifest validation. Chunk list capped at 262 144 entries (~1 TiB). Reconstructed size must match declared size. src_size == 0 rejected per chunk. MAX_SAFE_JS_INTEGER bounds file size for JS.
AbortSignal. Cancellation::run selects against the abort channel; an aborted operation is dropped without running again. AbortListener detaches its listener on Drop.
Writer cleanup. Writer releases its lock on Drop. On pipeTo failure, abort is called and the promise is spawned (not awaited) so a stalled write does not block the rejection.
Test evidence (reported by author, CI-confirmed): 212 WASM tests including a 4,303,347,835-byte file through pipeTo with matching BLAKE3; 765 native tests; fmt + clippy green.
Caveats (non-blocking)
- "Real Chromium and local nodes" CI job fails on every PR (Rust 1.99 / pinned node; fixed by #217). Pre-existing, not introduced here.
- Production compatibility smoke and real-browser/live-network runs not yet done (noted in PR body).
- Root-map/index memory still grows with chunk count — documented as a residual limit in the ADR.
close()no longer clears the shared chunk cache — documented behaviour change; cache is bounded at 32 MiB LRU.
ADR
ADR-0004 amended (2026-10-01). Status remains Proposed. The new "Large-file download destinations" section accurately describes the implementation. Validate ADRs and immutable Accepted status CI check passes.
Complete downloads fetched one 4 MiB range at a time, ignoring the requested concurrency and restarting retry rounds per range. Reads are now chunk-aligned: a shared engine primitive fetches a set of chunks in one deferred retry pass and hands each decrypted chunk on as it completes. Complete downloads use one pass over the whole file and copy chunks into the JS output out of order, hashing in file order. pipeTo fetches 32 MiB windows of whole chunks, so no chunk is decrypted twice. - Restore "Downloaded chunk n/total" progress used by startup probes. - Abort the destination for every pipeTo failure, including invalid options and a closed reader; do not abort after the final write. - Add an AbortSignal option for per-operation cancellation; closing a reader cancels in-flight fetches and retry waits at once and evicts only its own records from the shared cache. - Take download and pipeTo options as objects (legacy positional download arguments still work) and reject unknown option names. - Accept DataMaps whose chunks exceed this build's MAX_CHUNK_SIZE, and cap manifest chunk lists at 2^18 entries. - Build the file index once per resolution, keep no hex chunk list in readers, and remove the unused progress plumbing and expected hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
self_encryption::decrypt_chunk decompresses without an output limit, so a crafted public DataMap could make a one-byte read expand a record to gigabytes and abort the WASM instance. Chunks are now decrypted with the same KDF, pad, cipher and Brotli steps, stopping one byte past the declared size, and DataMaps may declare chunks of at most 16 MiB. A test pins the output to decrypt_chunk. pipeTo replaces per-window retry passes with an ordered pipeline: chunks are fetched concurrently and written in order, fetching continues during writes up to a 32 MiB buffer, and each missing record retries on its own schedule so waits overlap. - Poll cancellation before the operation so a closed reader or aborted signal never lets a dropped read finish or refill the cache. - Keep download options optional in the generated typings and restore `>>> 0` coercion for the legacy numeric concurrency argument. - Accept an AbortSignal for complete downloads, covering resolution. - Hash out-of-order chunks between event-loop turns, copy chunks into the output without an intermediate buffer, and pass settings as a struct. - Evict a closed reader's records by visiting only cached entries. - Share one MAX_SAFE_JS_INTEGER, rename the safe-integer helper, keep a single owner for the empty chunk list, and fix a stale doc comment. - Make the backpressure and memory-budget tests able to fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nested DataMap records were still decrypted by self_encryption without an output limit, so a crafted public file could exhaust WASM memory while resolving. Resolution now decrypts each level through the bounded per-chunk path, and browsers cap each decoded level at 64 MiB. The 16 MiB chunk cap applies to browser reads only, so native range reads accept any chunk size again. - Do not await the destination's abort, so a stalled write cannot keep pipeTo from rejecting; let an AbortSignal cancel while the destination closes, but not closing the reader once every byte is written. - Count open readers per file: the last close evicts the file's content and DataMap records, looked up by address instead of scanning. - Content records are authenticated by decryption and their plaintext hash, so the engine no longer hashes them a second time; the per-chunk fetch reuses the shared deferred retry rounds. - Decrypt in place, and yield read-back hashing on a time budget. - Correct the docs' decompression claims. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ction Nested DataMap resolution reserved and zero-filled the full size a level declared before fetching anything, so a crafted map declaring terabyte chunks could exhaust a native client's memory. Each chunk is now kept once decrypted and the level is joined after every chunk verifies, and chunk decompression grows with its actual output. Closing a reader no longer evicts records from the shared cache. The per-file reader count missed readers dropped without close() and evicted records still used by readers of the same content opened another way; the cache is bounded at 32 MiB and its LRU policy now releases records. - Return a complete download's own output error (such as a JS RangeError) instead of reporting it as an encryption error. - Derive Deserialize for the download result. - Correct the manifest cap and test README descriptions, and cover the legacy null concurrency argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Reject zero-size nested chunks and saturate the decompression bound, so a crafted usize::MAX size cannot overflow in debug builds. - Stop re-hashing records the fetch layer verified: decryption authenticates content and nested records, and self_encryption matches whole-file records by content hash. Correct the comment that said otherwise. - Decode nested DataMap levels straight from their chunks with DataMap's bincode options, instead of joining them into a second copy. - Fetch a single pipeTo chunk through the shared chunk pass rather than a duplicate, and keep MIN_FILE_CHUNKS with the KDF that needs it. - Narrow the README/ADR claim: the legacy decryptPublicFile binding still decompresses without a limit. Fix the data_download_range doc. - Name the fixture generator's literals and close test readers in finally blocks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Read-ahead from main planned its windows over a usize RecordLayout, which wraps at 4 GiB on wasm32. It now shares the reader's FileIndex, so windows, anchors and recent read ends use 64-bit positions, and the reader moved to download.rs takes reads through it again: readRange borrows records read-ahead holds and stops read-ahead when records fail to decrypt, and closing or dropping the reader releases them. openPublicFile/openPrivateFile keep their streaming option. A DataMap whose range reads reject, such as one with non-contiguous indices, now fails to open instead of opening with read-ahead off, because the reader validates its index when it opens; the test from main is updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A review for over-engineering found about a third of the branch was
machinery beyond the ticket. This removes it:
- Decrypt chunks with self_encryption::decrypt_chunk again and resolve
nested DataMaps with main's resolver, dropping the bounded decryption
copy, the bincode resolver, the browser chunk and nested-map caps, and
the bincode, chacha20poly1305 and brotli-decompressor dependencies.
Bounding crafted-record decompression is left to self_encryption.
- Run complete downloads and pipeTo through one ordered stream: buffered
fetches handed to the writer over a one-slot channel, holding at most
32 MiB. This replaces ordered_pipeline, OrderedHasher and its read-back.
- Restore downloadPublicFile(file, concurrency?, onProgress?) with a
trailing { maxMemoryBytes?, signal? }, removing the dual-type argument,
its ToUint32 emulation and the unknown-option checks.
- Cancel only through an AbortSignal; close() makes later calls fail.
- Validate pipeTo options before taking the writer, so invalid calls
leave the destination untouched, as ReadableStream.pipeTo does.
- Keep data_download_range's u64 path internal and make the complete
download a free function that registers no read-ahead.
- Restore main's record verification and retry helpers unchanged.
The WASM mock re-entered WASM before copying an outgoing message, so a
large record store could grow memory and detach it; copy it first.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The buffered(n) stream that replaced ordered_pipeline stopped polling its fetches whenever a chunk waited for the writer, and sized its concurrency from declared chunk sizes alone, so a map of tiny chunks could start tens of thousands of fetches. Restore ordered_pipeline, which polls fetches while a write is pending, follows the client's adaptive fetch cap and counts the chunk being written against the 32 MiB budget. - The backpressure test now delays GETs and requires new ones while the first write is blocked, so a stalled stream fails it. - Document that an abort during the destination's close may come after the commit, parse reader options like the other options objects, and share the download progress format. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The backpressure test now requires exactly the 32 MiB buffer of chunks to be fetched behind a blocked write, which the stalled buffered stream missed (3 of 8), and fails instead of hanging if pipeTo ends before its first write. A unit test checks that ordered_pipeline keeps no more than its cap in flight. Docs note that a record waiting to retry keeps its place under the cap, and that a download's concurrency also limits it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1bc1931 to
16cb3b6
Compare
…mium large-file.mjs uploads the large-file fixture's plaintext natively to the local devnet, under a throwaway home so it stays out of the upload history, and checks it lands on the fixture's address. Real Chromium then reads across 4 GiB and pipes the whole file into the origin private file system, checking the hash, stored size and bytes, progress and the memory-budget error. Not run in CI: it needs about 5 GB of disk and a release ant binary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Range reads, read-ahead, pipeTo and complete downloads index the root DataMap with self_encryption's DataMapIndex (0.36.2) instead of an ant-core copy of its chunk-index and key-derivation contract. The index uses 64-bit positions on every platform, checks each decrypted chunk against the size and source hash the map declares, and stops decompression at the declared size. Complete downloads no longer run through pipeTo's in-order stream. They fetch every chunk as a native complete download does, within the adaptive fetch cap and the caller's concurrency, retrying missing records together in deferred rounds, and copy each decrypted chunk straight into its place in the output as it arrives. The stream's 32 MiB bound had capped them at about eight fetches, and its per-chunk retries added the waits of each missing record one after another. The file is hashed afterwards, and progress again counts chunks as they land. A complete download also checks its AbortSignal before allocating the output. Signals and destinations use the typed web-sys AbortSignal and WritableStream bindings, so an EventTarget that is not an AbortSignal is rejected. Safe-integer validation uses Number.isSafeInteger, and the large-file fixture generator addresses its DataMap record with compute_address. The workspace temporarily patches self_encryption until 0.36.2 is published. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Hermes paired review — provisional; hold merge gates
Reviewed #214 at 773ea5dac9b7c68efcf1f9c0a73a298e9fa46d3c with WithAutonomi/self_encryption#447 at e8aa39ee1337c11f52628356d97fa86ac0d1007d.
Code verdict: No new blocking code defect found in the inspected changes. I traced 64-bit range positions through the browser reader and read-ahead, chunk-index and source-integrity checks, ordered streaming with backpressure, shared-round complete-download retries, and AbortSignal/writer cleanup. This is not a claim that every browser or destination supports a large in-memory allocation.
Pre-merge gate: Publish self_encryption 0.36.2 from the reviewed companion change; remove the root temporary [patch.crates-io]; regenerate Cargo.lock against the registry release; rerun the native and generated-WASM checks. The dependency requirement is already 0.36.2, but the current lockfile resolves it from the Git patch. The crates.io version endpoint returned HTTP 404 during this review. This gate is acknowledged in the PR body, not a newly discovered code bug.
Checks I ran: cargo test --locked -p ant-core --lib passed all 819 tests. cargo fmt --all -- --check passed. The production browser-WASM build with browser-wasm,test-utils succeeded; the generated-WASM Node suite passed all 221 tests. The companion's full release suite passed 97 tests, with 1 ignored.
CI is not fully green: Security Audit failed on five quinn-proto vulnerabilities. I read the job log and compared every Cargo.lock package block against main: only self_encryption changed; quinn-proto remains 0.11.15 on both base and head. This is inherited security debt, not introduced by this PR. It still needs resolution or an explicit authorised risk decision; do not describe the check as passing. At the last head check, Ubuntu E2E Tests and Ubuntu Merkle E2E were still in progress.
Evidence limit: I did not independently run browser-tests/large-file.mjs, the opt-in real-Chromium/local-node upload and disk-streaming check above 4 GiB. Generated-WASM tests are not proof of that full path. The companion SDK and upload limits also remain outside this PR's stated scope.
Review-team participation: 1/3 completed. Qwen 3.8 (qwen38-27b) completed an independent paired review and found no new code blocker. GLM 5.3 (huggingface, zai-org/GLM-5.3) failed with HTTP 402/no credits. DS4 (deepseek-v4-flash) timed out. An earlier oversized panel was stopped without completed local opinions. No substitution or full consensus is claimed. This neutral review does not approve, merge or waive any gate.
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Clean re-review — provisional, not an approval
Reviewed head 773ea5dac9b7c68efcf1f9c0a73a298e9fa46d3c against main / merge base 9a2b6353a88207dd4d6ed99a5158c370531a7e09 in a detached, clean worktree. The paired self_encryption #447 review used head e8aa39ee1337c11f52628356d97fa86ac0d1007d.
Source verdict: no blocking defect introduced by this diff was confirmed. This is a COMMENT, not an APPROVE, and is not a merge-ready verdict.
Actual review participation and dissent
GLM 5.3 via OpenRouter (z-ai/glm-5.3) and local Qwen 3.8 (qwen38-27b) each completed independent correctness and adversarial reviews. DS4 (deepseek-v4-flash) timed out in both roles, including both bounded retries. Participation is 2/3 models, 4/6 seats. Reconciliation answers are not extra votes; full consensus is unavailable.
Qwen retained a possible AbortListener lifetime caution, while its reconciled verdict was PASS with caution. GLM requested inspection of manifest handling for shrunk DataMaps. I checked those concerns against source and executable tests below. These checks do not count as another independent review seat.
Source adjudication
- Prefetch failure/retry concern: a failed held record goes into
read_chunksandretry_data_fetch; it does not permanently prevent the normal on-demand retries. The existing WASM recovery regression passed. - Shrunk-map metadata concern:
src/browser.rs:169–193resolves the original/root DataMap before constructingchunks.src/browser/manifest.rs:173–203validates the original plaintext chunk sizes and their total. The native shrunk-map download/range-read regression passed. The reviewed metadata path does not confuse child-map byte counts with whole-file byte counts. - Chunk-index panic candidate: reviewed internal callers derive chunk indices from the validated index/range. I did not establish an out-of-bounds input reaching this code through those paths. Optional Rust API hardening is separate from a confirmed browser-path defect.
- Cancellation lifetime caution:
AbortListener::dropremoves the DOM listener before dropping its Rust closure on the JS thread. I added and ran three review-only WASM tests, each repeated six times: completed download followed by abort; abort from progress; abort during fetch. All 18 lifecycle operations left no registered abort listener; all 3 tests passed. I could not reproduce the claimed dropped-closure problem. This is bounded evidence, not proof of every real-browser event schedule.
Verification I ran
cargo test --locked -p ant-core --lib: 819 passed, 0 failed.- Fresh
wasm-pack build --target web --release --no-default-features --features browser-wasm,test-utils: passed. - All repository generated-WASM Node tests: 221 passed, 0 failed, 0 skipped.
- Additional review-only cancellation checks: 3 passed, 0 failed; counted separately from the 221 repository tests.
cargo fmt --check: passed.- Latest head check: unchanged; 21 CI checks succeeded and Security Audit failed. There are no pending checks in that snapshot.
Outstanding integration gates
- Published dependency: the crates.io
self_encryption/0.36.2endpoint still returns 404. After #447 lands and that version is published, remove this PR's temporary root[patch.crates-io]override and regenerate/verifyCargo.lockagainst the published release. The present local/CI results use the pinned companion revision, not an independently available crates.io release. - Security Audit: CI reports five vulnerabilities in
quinn-proto 0.11.15: RUSTSEC-2026-0338, RUSTSEC-2026-0339, RUSTSEC-2026-0340, RUSTSEC-2026-0341 and RUSTSEC-2026-0342. The fullquinn-protolockfile entry is identical on the merge base and head, so these are inherited, not introduced here. They still need resolution through the normal security/merge process; this review does not waive the failed check. Audit output. - Real-network evidence: I did not independently run a production-node compatibility smoke or a real-browser/live-node download-to-disk test above 4 GiB. Passing generated-WASM/mock transport tests do not establish those results.
No merge, dependency edit, deployment or release action was taken. This is Hermes's provisional technical assessment, posted through the authenticated GitHub account, not a human sign-off.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in #214, #218, #227 and #181. Two things needed a hand: - Cargo.toml: main removed its temporary self_encryption [patch.crates-io] entry when 0.36.2 was published; this branch's ant-protocol pin in the same section stays (rev 718f6068, unchanged). - ant-core/src/data/error.rs: #218 added Error::duplicate(), an exhaustive match; arms for this branch's PointerFinal and PointerForked are added, cloning the message like every other String variant. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
3 participants
Files larger than 1 GB are currently rejected even when a browser reads them in bounded ranges. Removing that check alone would still leave 32-bit file offsets on wasm32. This change uses 64-bit file positions throughout browser reads, including the read-ahead from main, and adds
BrowserFileReader.pipeTo()for public and private files, with backpressure,AbortSignalcancellation and writer cleanup.Complete downloads fetch as on main: within the adaptive fetch limit (capped by
concurrency), with missing records retried together in shared rounds. Each chunk is copied into its place in one JavaScriptUint8Arrayas it arrives. A complete download accepts an optional output-memory budget and anAbortSignal, and reports allocation failures with the streaming alternative.pipeTofetches chunks concurrently, retries each on its own, and writes them in file order while holding at most 32 MiB of plaintext. Chunk positions, range lookup and per-chunk decryption come fromself_encryption::DataMapIndex(WithAutonomi/self_encryption#447), which uses 64-bit positions on every platform and stops decompression at each chunk's declared size. Manifest validation accepts larger files and up to 262,144 chunks. The companion SDK and upload limits are outside this PR.Before merge: publish self_encryption 0.36.2 from WithAutonomi/self_encryption#447 and remove the temporary
[patch.crates-io]from the rootCargo.toml.Linear issue
Closes V2-1398
Risk tier
Proposed for reviewer confirmation: client download buffering, offset validation and destination handling; no wire, node, storage-format or payment changes.
Compatibility
self_encryption::DataMapIndex::decrypt_chunk, which uses the same KDF asself_encryption::decrypt_chunkand also rejects a chunk whose plaintext differs from its declared size, whichencryptnever produces.PublicFileDescriptor.sizechanges fromusizetou64, and affected WASM-export signatures change. Nativedata_download_rangekeeps its signature.downloadPublicFile/downloadPrivateFilegain a trailing{ maxMemoryBytes?, signal? }argument afterconcurrencyandonProgress.pipeTo(writable, { start?, end?, signal?, onProgress? })is additive. Progress text staysDownloaded chunk n/total. Sizes and offsets remain numbers but are validated as nonnegative safe integers:readRangerejects fractional, negative, missing or imprecise arguments that wasm-bindgen previously coerced (an omitted length used to read nothing, and1.5read from byte 1).BrowserFileReader.close()releases the records its read-ahead holds and makes later calls fail, but no longer clears the client's shared chunk cache (bounded at 32 MiB, LRU). A DataMap whose range reads reject, such as one with non-contiguous indices, now fails atopenPublicFile/openPrivateFileinstead of opening with read-ahead off.Semver impact
Breaking is proposed for Rust browser API consumers; existing JavaScript call forms remain supported.
Test evidence
pipeTowith a matching BLAKE3 and writes of at most 4 MiB, and fetching behind a blocked write stopping at the 32 MiB bound. They also coverAbortSignalcancellation ofpipeToand complete downloads (during retry waits, a stalled destination write and the destination's close), invalidpipeTocalls that leave the destination untouched (including asignalthat is not anAbortSignal), writer, allocation and output errors, memory budgets, progress, public/private seeks across 4 GiB, and main's read-ahead suite.DataMapIndex.--all-targets --all-features) and fmt pass, and its Criterion decrypt benchmarks are unchanged against master.cargo fmt --check, native clippy (--all-targets --all-features) and the wasm32browser-wasmclippy, with and withouttest-utils, pass.pipeTowrote the whole file into the origin private file system (a realFileSystemWritableFileStream) in 100 s (147 s on an earlier run), with 1,028 progress calls, a matching BLAKE3, and the right stored size and bytes around 4 GiB and at the end. A complete download over a 64 MiB budget failed before any transfer and pointed topipeTo. Chromium could not allocate one 4.3 GB buffer (RangeError: Array buffer allocation failed), so streaming is the only whole-file path at this size there. The file is one repeated byte and its records compress to about 67 KB, so this checks correctness, not network throughput. It is repeatable with the opt-inant-core/browser-tests/large-file.mjs, which is not run in CI.quinn-protoadvisories (RUSTSEC-2026-0339 to 0342); this PR'sCargo.lockdiffers from main's only inself_encryption.ant-core/wasm-tests/large-download.test.mjsandant-core/examples/generate-browser-large-file.rs.New dependency
None.
self_encryptionmoves to 0.36.2 forDataMapIndex; until that is published, a temporary[patch.crates-io]pins WithAutonomi/self_encryption#447 at869b2f94.ADR
ADR-0004: large-file download destinations. The existing Proposed ADR is amended and remains Proposed.
Mitigation / rollback
Revert this PR and regenerate consuming WASM artifacts; self_encryption 0.36.2 is additive and can stay. Applications can keep using bounded
readRangeand set an explicit output-memory budget. Root-map/index memory, destination capacity and JavaScript's safe-integer range (just under 8 PiB) remain practical limits; disk output requires a caller-provided writable destination. Follow-ups: cancellation for native range reads, and bypassing the shared chunk cache when streaming.🤖 Generated with Claude Code