Harden imported artifact remote page fallback#1231
Conversation
| return { availability: 'unverified_page', remote, sourcePeerId }; | ||
| const result: AssertionArtifactRemoteResult = { availability: 'unverified_page', remote, sourcePeerId }; | ||
| if (opts.cache && !opts.explicitSourcePeerId) { | ||
| fallback ??= result; |
There was a problem hiding this comment.
🔴 Bug: fallback ??= result leaves an earlier unavailable/denied/hash-mismatch fallback in place even after a later peer returns a usable unverified page. With default cache promotion and no explicit sourcePeerId, a candidate list like [unavailable-peer, large-artifact-peer] now scans past the large artifact's unverified page, but returns the first unavailable response when no fully verified peer is found. This regresses remote reads for large or otherwise non-cache-promotable artifacts. Replace lower-priority unavailable fallbacks with the first unverified_page fallback while still continuing to look for a verified peer.
This keeps the first fallback even when it is only unavailable, so a later valid unverified_page can be discarded. With discovered peers, cache enabled, and no explicit source, a sequence like peer-down then peer-large-valid-but-unverified now returns the earlier unavailable result instead of the usable unverified page. Prefer the unverified result over an unavailable fallback, or track fallback priorities separately.
| })); | ||
| }); | ||
|
|
||
| it('rejects an oversized unverified remote page', async () => { |
There was a problem hiding this comment.
🟡 Issue: This only exercises the cache: false readRequestedPage path. The PR also added decoded-length guards in the cache-promotion path for the first page and subsequent pages, which is the path that can assemble verifiedBytes and promote bytes into the file store. A regression in those guards would not fail this suite. Add cache: true cases for an oversized first page and an oversized later page, asserting hashMismatch, bytesB64 cleared, and no verifiedBytes.
2 participants
Summary
Changes
Test Plan
pnpm test)pnpm build)dkg start(if applicable)Related Issues