feat(ipfs): multi-protocol ArNS + read-only IPFS gateway (Phases 1–2) - #793
feat(ipfs): multi-protocol ArNS + read-only IPFS gateway (Phases 1–2)#793vilenarios wants to merge 52 commits into
Conversation
Add opt-in IPFS content serving to AR.IO Gateway. Operators enable
IPFS_ENABLED=true and start a Kubo Docker sidecar (--profile ipfs) to
serve IPFS content via path-based (/ipfs/{CID}) and subdomain-based
({CID}.{gateway-host}) access patterns.
Features:
- Kubo HTTP gateway integration with connection + stall timeouts
- LRU bounded filesystem cache (streams to disk, not memory)
- File-based CID blocklist with hot-reload
- Separate rate limiter pool for IPFS traffic
- x402 payment protection (same as Arweave data endpoints)
- HTTPSIG response signing for verifiable IPFS responses
- CIDv0 to CIDv1 base32 redirect (DNS-safe, works with wildcard certs)
- Cross-CID redirect for directory listing navigation
- Path traversal protection
- Docker Compose profile with TCP+UDP swarm ports
Default off (IPFS_ENABLED=false). Zero runtime impact when disabled.
Designed as foundation for Phase 2 ArNS-to-CID resolution.
Fix strict-boolean-expressions (explicit nullish checks) and prettier formatting issues caught by CI eslint.
Add OpenTelemetry span tracing to IPFS request lifecycle: - IpfsService.getContent span (cache check, blocklist, delegation) - KuboDataSource.getContent span (HTTP fetch with latency attributes) - Spans record cache hit/miss, content size, errors, and content type Also fix strict-boolean-expressions and prettier formatting for CI.
- Fix prettier line-length formatting (5 errors from CI) - Add IPFS cache volume mount to docker-compose (data persists) - Add IPFS field to /ar-io/info endpoint (network discovery) - Add OTEL span tracing to IpfsService and KuboDataSource
Remove custom text-file blocklist in favor of the existing PUT /ar-io/admin/block-data API. Operators block IPFS CIDs the same way they block Arweave TX IDs — unified moderation, single API. Removed: IpfsBlocklist class, IPFS_BLOCKLIST_PATH config, blocklist volume mount. IpfsService now uses DataBlockListValidator (SQLite).
…or moderation (PE-9067)
- IPFS rate limiter defaults now match Arweave (100K IP tokens, 20/s refill,
1M resource tokens, 100/s refill)
- Remove custom text-file blocklist — use existing PUT /ar-io/admin/block-data
API for CID moderation (unified with Arweave content moderation)
- Remove IPFS_BLOCKLIST_PATH config and volume mount
- Add IPFS cache volume mount to docker-compose for persistence
…it defaults (PE-9067)
- Add IPFS Grafana dashboard example (requests/sec, cache hit rate,
latency percentiles, content size, blocked requests, route type)
- Update OpenAPI spec: block-data endpoint accepts IPFS CIDs
- Update ipfs-integration.md: admin API for moderation (not text file)
- Match IPFS rate limiter defaults to Arweave (100K IP, 1M resource)
- Enforce IPFS_MAX_RESPONSE_SIZE_BYTES (reject with 413 when Content-Length exceeds limit) - Destroy response stream on 404/408/504 from Kubo (prevents socket leak) - Fix docs/envs.md rate limiter defaults to match config.ts - Add Grafana dashboard example for IPFS metrics - Update OpenAPI spec for CID content moderation
- Use positiveIntOrDefault for all IPFS numeric configs (prevents NaN) - URL-encode IPFS path segments (prevents request injection) - Guard contentLength parse against NaN (fallback to 0) - Use SANDBOX_PROTOCOL for redirects (correct behind TLS termination) - Enforce size limit during streaming (catches chunked responses) - Set failed=true before cleanup in stream end handler (race fix) - Use conservative size estimate for x402 when Content-Length unknown - Increment cache hit/miss counters in route handler - Fix env var name: IPFS_CACHE_CLEANUP_THRESHOLD_SECONDS - Guard Grafana cache hit rate expr against division by zero - Destroy response stream on 404/408/504 from Kubo (socket leak)
IPFS cached content now triggers the same data-cached event as Arweave, feeding into the existing webhook system. No new configuration needed — operators' existing webhook consumers see IPFS content automatically.
…IPFS cache (PE-9067)
- IPFS blocked response now matches Arweave format (plain text with ID)
- Emit data-cached webhook event when IPFS content is cached (same event
as Arweave, feeds into content scanner pipeline automatically)
Bring the IPFS (Kubo sidecar) feature branch up to date with ~296 commits of develop drift. Conflicts were all additive and resolved as unions: - src/lib/httpsig.ts — CO_SIGNABLE_HEADERS keeps IPFS's x-cache/etag alongside develop's x-ar-io-root-* hint headers. - src/metrics.ts — keep IPFS metrics block + develop's chunk-anchor, hint, httpsig-digest, and optimistic-tx metrics. - docs/envs.md — relocate the IPFS section after the extended ClickHouse table + streaming-pipeline section so the table stays intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The merge took develop's lockfile (no multiformats entry); the IPFS feature depends on it. yarn install re-resolved it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
streamToCache created its write stream inside an async mkdir().then(), so a small or fast Kubo response could emit 'end' before the mkdir resolved — leaving writeStream null, hitting the silent-discard branch, and dropping the cache entry. Large files won the race and cached; small files never did (every request re-fetched from Kubo, X-Cache always MISS). Create the temp dir eagerly in the IpfsFsCache constructor and open the write stream synchronously in streamToCache, removing the race and the now-unneeded pending-chunk buffering. Verified in an isolated Kubo + gateway run: small object now MISS then HIT; large object still HIT. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ANT records now carry a targetProtocol (0=Arweave, 1=IPFS) and the record
target can be an IPFS CID instead of an Arweave TX ID (@ar.io/sdk 4.0.0).
Previously the on-demand resolver read only transactionId and validated it
as a 43-char Arweave ID, so a CID-targeted name failed to resolve.
- on-demand resolver: read targetProtocol; validate the target as a CID
when protocol is IPFS, else as an Arweave ID; surface protocol on the
resolution (cached transparently as part of NameResolution).
- NameResolution: optional protocol field ('arweave' | 'ipfs'); undefined
treated as 'arweave' for backward compat (e.g. trusted-gateway hops).
- arns middleware: when a name resolves to an IPFS CID and IPFS serving is
enabled, hand off to the IPFS handler (sets ipfsCid/ipfsPath, mirroring
the IPFS subdomain middleware) instead of the Arweave data handler.
Completes the 'ArNS -> IPFS CID' phase the IPFS PR was foundation for.
Verified: typecheck + lint clean, resolver unit tests pass, and live
on-demand Solana resolution of existing names still serves via Arweave.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Final-review hardening of the ArNS->IPFS feature:
- fix(cache-control): ArNS->IPFS responses no longer send immutable/1-year.
The IPFS handler only sets `immutable` for direct /ipfs/{CID} (and {CID}.host)
requests; when reached via an ArNS name (mutable name->CID binding) it keeps
the ArNS-TTL Cache-Control the ArNS middleware set, so a record repoint isn't
pinned in caches for ~a year (cf. PE-9072).
- feat(headers): emit signed `X-ArNS-Protocol: arweave|ipfs` on resolutions and
add `protocol` (+ `resolvedId`) to the /ar-io/resolver/:name JSON, so clients
know whether X-ArNS-Resolved-Id is a TX ID or a CID. Added x-arns-protocol to
TRIGGER_HEADERS so it's part of the signature.
- feat(httpsig): body-bind IPFS responses with RFC 9530 Content-Digest. The
SHA-256 is computed at cache-write time and emitted on cache hits (in
CO_SIGNABLE_HEADERS, so HTTPSIG signs it). Misses stream without it; the
signed ETag=CID still attests identity.
- test(ipfs): cache digest round-trip + legacy (digest-less) entry coverage.
- docs: rewrite the ipfs-integration.md Phase 2 section to the shipped
targetProtocol design (was speculative), glossary Target Protocol entry,
CLAUDE.md note. Documented the trusted-gateway-resolver protocol limitation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #793 +/- ##
===========================================
- Coverage 79.43% 78.85% -0.59%
===========================================
Files 138 147 +9
Lines 53401 56168 +2767
Branches 4087 4275 +188
===========================================
+ Hits 42420 44290 +1870
- Misses 10928 11821 +893
- Partials 53 57 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds opt-in IPFS serving through Kubo. It supports direct CID routes, ArNS protocol routing, caching, moderation, rate limiting, trustless formats, optional pinning, metrics, deployment configuration, and operational documentation. ChangesIPFS gateway integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant IpfsMiddleware
participant ArNSMiddleware
participant IpfsRoutes
participant IpfsService
participant KuboDataSource
participant Kubo
Client->>IpfsMiddleware: Request CID path or subdomain
IpfsMiddleware->>IpfsRoutes: Attach CID and path context
IpfsRoutes->>IpfsService: Request content
IpfsService->>IpfsService: Check blocklist and caches
IpfsService->>KuboDataSource: Request uncached content
KuboDataSource->>Kubo: Stream CID with range or format options
Kubo-->>KuboDataSource: Return content stream and metadata
KuboDataSource-->>IpfsService: Return stream
IpfsService-->>IpfsRoutes: Return response data
IpfsRoutes-->>Client: Stream content with headers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The ArNS->IPFS routing decision hinges on classifying an ANT record's target as arweave vs ipfs and validating the id accordingly. Extracted that logic from OnDemandArNSResolver into a pure, SDK-free helper (classifyResolvedTarget) and unit-tested it: arweave/ipfs by targetProtocol, undefined+unknown protocol -> arweave (fail-closed), CIDv0/v1 acceptance, and cross-format rejection (CID under arweave, TX id under ipfs, garbage). The ArNS middleware routing itself can't be unit-tested in isolation (it imports system.ts, booting the DI graph — no middleware has unit tests for this reason); it stays covered by live e2e. Also documented the three root/apex cases in ipfs-integration.md: a name's @ record and apex-via-APEX_ARNS_NAME route to IPFS; apex-via-APEX_TX_ID is Arweave-only (bypasses protocol routing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # src/config.ts
The trusted-gateway resolver read the ArNS envelope headers but never read X-ArNS-Protocol, and its isValidDataId gate rejected any non-43-char id — so an upstream IPFS resolution (a CID) was discarded as "invalid data ID" and the protocol classification was lost across a gateway hop. With the default ARNS_RESOLVER_PRIORITY_ORDER of `gateway,on-demand`, that meant a stock gateway silently misrouted IPFS-targeted names to the Arweave path. Read X-ArNS-Protocol from the upstream response and validate resolvedId with the shared classifyResolvedTarget (CID for ipfs, 43-char id for arweave), threading `protocol` into the returned NameResolution. An absent header defaults to arweave, so older peers are unaffected. Multi-protocol resolution now survives a trusted-gateway hop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
A path-style GET /ipfs/{CID} served active content (HTML/JS) on the shared
gateway origin — only CIDv0 was redirected (to convert to v1). That let one
CID's content run in the gateway's origin, the same XSS/same-origin risk the
Arweave data path avoids by forcing /{txid} to a sandbox subdomain.
Generalize the redirect: any path-style CID (v0 or v1) is redirected to its
per-CID sandbox subdomain {CIDv1base32}.{host}, reusing sandbox.ts's own
getRequestSandbox() to skip the redirect when the request already arrived on
that origin (no loop). ArNS-served IPFS is already isolated on the name's own
origin and reaches the handler directly, so it's unaffected.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
Bring three IPFS-path behaviors to Arweave-path parity: - HEAD /ipfs/:cid (and the subdomain/ArNS routes) now return the full header set with no body — for metadata probes and media players that HEAD before ranging. The shared handler skips the pipe, releases the upstream/cache stream, and bills zero egress for the HEAD. - Content-hash moderation: on a cache hit (where the base64url SHA-256 of the served bytes is known) the service now also checks isHashBlocked, matching the Arweave path — so a block-by-content-hash entry stops IPFS-served bytes, not only a block-by-CID. - 404s now carry Cache-Control (CACHE_NOT_FOUND_MAX_AGE, must-revalidate) like the Arweave sendNotFound, so absent CIDs aren't re-fetched from Kubo on every retry through upstream/edge caches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
An absent or unpinned CID re-hit Kubo on every request (latency and DoS amplification), unlike the Arweave path which short-circuits repeat misses via NegativeDataCache. Reuse that same cache in IpfsService: check isNegatively cached before the Kubo fetch and recordMiss on IpfsNotFoundError. Trips only after repeated misses (count + duration thresholds), so transient blips don't poison a CID that later pins. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
The IPFS path served full bodies only — no Accept-Ranges, no 206, no 416 — so media seeking failed and the observer's ranged sampling of >1MiB content downloaded the whole object per sample. Kubo's gateway already supports Range, so forward a client Range header to Kubo and relay its partial response: - kubo-data-source: forward `Range`, accept 206 (capture Content-Range), map a Kubo 416 to IpfsRangeNotSatisfiableError; surface statusCode/contentRange. - ipfs-service: range requests bypass the positive cache (never serve a partial from a full cached object, never cache a partial body) and stream straight from Kubo. - route: always advertise Accept-Ranges: bytes; relay 206 + Content-Range; map IpfsRangeNotSatisfiableError to 416. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
|
@coderabbitai review |
✅ Action performedReview finished.
|
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 53 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
test-ipfs.sh-1-6 (1)
1-6: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winHarden the script preamble and remove the dead parameters.
Four points:
- The script has no
set -uo pipefail. Do not add-e, because((pass++))returns a non-zero status when the counter is0./tmp/ipfs-test-bodyis a fixed path. A local user can pre-create a symlink there and redirect the write. Usemktempand remove the file on exit.extra_argsis never supplied by any caller, and its unquoted expansion at line 23 triggers globbing (SC2086).status=$?at line 43 is never read (SC2034).🛡️ Proposed fix
#!/bin/bash # End-to-end IPFS integration test script # Run this after starting the gateway with IPFS_ENABLED=true and Kubo running +set -uo pipefail + +BODY_FILE="$(mktemp)" +trap 'rm -f "$BODY_FILE"' EXITtest_case() { local name="$1" local expected_status="$2" local url="$3" - local extra_args="${4:-}" local result - result=$(curl -s -o /tmp/ipfs-test-body -w "%{http_code}" --max-time 30 $extra_args "$url" 2>&1) + result=$(curl -s -o "$BODY_FILE" -w "%{http_code}" --max-time 30 "$url" 2>&1) if [ "$result" = "$expected_status" ]; then echo -e " ${GREEN}PASS${NC} $name (HTTP $result)" ((pass++)) else echo -e " ${RED}FAIL${NC} $name — expected $expected_status, got $result" echo " URL: $url" - echo " Body: $(head -c 200 /tmp/ipfs-test-body)" + echo " Body: $(head -c 200 "$BODY_FILE")" ((fail++)) fi }Also applies to: 20-23, 43-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test-ipfs.sh` around lines 1 - 6, Harden the test script preamble by adding set -uo pipefail without -e, replace the fixed /tmp/ipfs-test-body path with a mktemp-created file and clean it up on exit, remove the unused extra_args parameter and its unquoted expansion, and delete the unread status assignment. Keep the existing pass counter behavior intact.Source: Linters/SAST tools
src/routes/arns.ts-96-100 (1)
96-100: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate
docs/openapi.yamlfor the resolver response changes.
/ar-io/resolver/:namenow documents resolver-specific fields, butdocs/openapi.yamlneeds the newresolvedId/protocolfields and the updatedtxIdmeaning across the response schema and examples. Add the change to the same pull request since this is a documented API contract change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/arns.ts` around lines 96 - 100, Update the resolver response schema and examples in docs/openapi.yaml to include resolvedId and protocol, and revise txId documentation to state that it remains for backward compatibility while representing a CID for IPFS records and preferring resolvedId plus protocol. Keep the OpenAPI contract aligned with the response returned by the resolver route.Source: Coding guidelines
src/middleware/ipfs.ts-67-68 (1)
67-68: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDefine one encoding contract for
req.ipfsPath.
req.ipfsPathis set percent-encoded by the CID middleware and decoded by the ArNS middleware before both values reach the same Kubo path consumer. SetipfsPathto one format, document that contract on theipfsCid/ipfsPathrequest interface, and make both middleware producers use the same value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/middleware/ipfs.ts` around lines 67 - 68, Standardize the encoding contract for request ipfsPath across the CID and ArNS middleware producers before it reaches the Kubo path consumer. Update the ipfsCid/ipfsPath request interface documentation to state the chosen format, then ensure both middleware paths assign that same format instead of one percent-encoding and the other decoding.src/middleware/ipfs.ts-76-93 (1)
76-93: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the query string on the cross-CID redirect, and simplify the no-op assignment.
Two points in this block:
- The 302 at line 85 drops the original query string.
src/routes/ipfs.tslines 163-168 preserves it. A request such as/ipfs/{otherCid}/x?format=carlosesformat=carand is then served as a UnixFS proxy response instead of trustless bytes.- Line 78
reqPath = remainder !== undefined ? remainder : undefinedreturnsremainderin both branches.🐛 Proposed fix
if (pathCid === cidLabel) { // Same CID — strip the redundant prefix - reqPath = remainder !== undefined ? remainder : undefined; + reqPath = remainder; } else if (isValidCid(pathCid)) { // Different CID — redirect to that CID's subdomain try { const targetCid = cidToV1Base32(pathCid); const rootHost = matchedEntry.host; const pathSuffix = remainder !== undefined ? `/${remainder}` : '/'; + const queryString = url.parse(req.originalUrl).query; res.redirect( 302, - `${config.SANDBOX_PROTOCOL ?? req.protocol}://${targetCid}.${rootHost}${pathSuffix}`, + `${config.SANDBOX_PROTOCOL ?? req.protocol}://${targetCid}.${rootHost}${pathSuffix}${ + queryString !== null && queryString !== '' ? `?${queryString}` : '' + }`, ); return;Add the import:
import url from 'node:url';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/middleware/ipfs.ts` around lines 76 - 93, Fix two issues in the CID-handling block: First, simplify the redundant ternary on the reqPath assignment to just assign remainder directly, since both branches return the same value. Second, preserve the query string when constructing the 302 redirect URL in the isValidCid branch by appending the original request's query string to the constructed target URL, similar to how src/routes/ipfs.ts handles query preservation, so that requests like /ipfs/{otherCid}/x?format=car maintain their query parameters instead of losing them during the cross-CID redirect.src/routes/ipfs.ts-100-102 (1)
100-102: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winOther (CWE-20): Improper Input Validation
Reachability: External
Reachability path
● Entry src/app.ts:152 arnsRouter │ ▼ ● Hop src/routes/arns.ts:27 createIpfsHandler │ ▼ ● Sink src/routes/ipfs.tsValidate
ipfsCidincreateIpfsHandler.
createIpfsPathHandlerrejects malformed CIDs before fetching content;createIpfsHandlerdoes not. On the ArNS path,ipfsCidcomes from ANT resolution and can contain a malformedresolvedId. Reject non-string values with400 Invalid CIDand invalid CID strings with502 Invalid IPFS CID in ArNS recordbefore callingipfsService.getContent({ cidString }).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/ipfs.ts` around lines 100 - 102, Add validation in createIpfsHandler before calling ipfsService.getContent to ensure ipfsCid is a valid string. First check that cidString is a non-empty string and reject with a 400 response if not, then validate the CID format itself (following the pattern used in createIpfsPathHandler) and reject with a 502 response if the CID string is malformed, ensuring both checks occur before the ipfsService.getContent call proceeds.docs/glossary.md-288-294 (1)
288-294: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the missing
IPFS CIDglossary entry.Line 291 links to
#ipfs-cid. markdownlint reports the fragment as invalid, and the neighbouring#item-idlinks do not trigger the same warning. That indicates noIPFS CIDterm exists in this file. Add the entry so the link resolves and so the new IPFS vocabulary is defined in the glossary.#!/bin/bash # Confirm whether an IPFS CID glossary entry exists. rg -n -i 'ipfs cid|ipfs-cid' docs/glossary.mdAs per coding guidelines: "add new terms to
docs/glossary.md".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/glossary.md` around lines 288 - 294, Add an “IPFS CID” glossary entry to docs/glossary.md so the existing `#ipfs-cid` link from the Target Protocol (ANT record) definition resolves. Define the term consistently with the surrounding glossary entries and preserve the existing target-routing description.src/ipfs/ipfs-cache.ts-189-229 (1)
189-229: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
putleaks its temp file and stores no digest; confirm it is still used.Two problems in
put:
- If
pipelineor eitherrename/writeFilethrows, the catch block logs the error and returns. It never unlinkstempPath.putFromFiledoes clean up on failure (Line 280). Apply the same cleanup here.- The metadata written at Line 211 has no
digestfield.IpfsServiceuses the stored digest to enforce block-by-hash on cache hits and on range requests. An entry written throughputtherefore bypasses hash-based moderation permanently.
streamToCacheinsrc/ipfs/ipfs-service.tscallsputFromFile, notput. Ifputhas no remaining caller, remove it rather than leaving a second write path with weaker moderation guarantees.#!/bin/bash # Find every caller of IpfsFsCache.put versus putFromFile. rg -n --type=ts -C3 '\.putFromFile\s*\(|(?<!putFrom)\bcache\.put\s*\(' -P🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ipfs/ipfs-cache.ts` around lines 189 - 229, Update the IPFS cache write path by checking callers of IpfsFsCache.put, including streamToCache and putFromFile; if put has no remaining callers, remove it. Otherwise, retain it only after matching putFromFile’s failure cleanup by unlinking tempPath, and populate CacheEntry metadata with the content digest required by IpfsService for block-by-hash validation.src/ipfs/ipfs-service.ts-280-291 (1)
280-291: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRange responses bypass every mid-stream size guard.
guardSizeis applied only whenformat !== undefined.streamToCache, which carries the other mid-stream size check, is skipped whenrange !== undefined. So a range response has only the up-front check at Line 241, and that check depends onresult.sizebeing known.A request with
Range: bytes=0-returns the whole object as a 206. If Kubo answers without a usableContent-Length,result.sizeis0, the Line 241 check is skipped, andIPFS_MAX_RESPONSE_SIZE_BYTESis not enforced for that response at all.Guard the range path as well.
🔒 Proposed fix
stream: - format !== undefined ? this.guardSize(result.stream) : result.stream, + format !== undefined || range !== undefined + ? this.guardSize(result.stream) + : result.stream,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ipfs/ipfs-service.ts` around lines 280 - 291, Update the response stream selection in the return block to apply guardSize whenever the response bypasses streamToCache, including range responses. Preserve the existing unguarded path only for responses that are handled by streamToCache, and retain the current trustless-format behavior.
🧹 Nitpick comments (5)
test-ipfs.sh (1)
93-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the Kubo container name configurable.
The script hardcodes
ar-io-node-kubo-1. That name depends on the Compose project directory name, so the script fails on any checkout with a different directory name or a custom project name. Read it from an environment variable with the current value as the default.♻️ Proposed change
+KUBO_CONTAINER="${KUBO_CONTAINER:-ar-io-node-kubo-1}" + -FILE_CID=$(echo "Hello from AR.IO IPFS integration test!" | docker exec -i ar-io-node-kubo-1 ipfs add -q 2>&1) +FILE_CID=$(echo "Hello from AR.IO IPFS integration test!" | docker exec -i "$KUBO_CONTAINER" ipfs add -q 2>&1) echo " File CID: $FILE_CID" -DIR_CID=$(docker exec ar-io-node-kubo-1 sh -c ' +DIR_CID=$(docker exec "$KUBO_CONTAINER" sh -c '🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test-ipfs.sh` around lines 93 - 101, Define a configurable Kubo container name in test-ipfs.sh using an environment variable and default it to ar-io-node-kubo-1. Update both docker exec invocations in the FILE_CID and DIR_CID setup flows to reuse this variable instead of hardcoding the container name.src/ipfs/ipfs-service.ts (1)
139-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the inner
span.end()calls beforethrow.Lines 147, 160, and 187 end the span, then throw. The outer catch at Line 292 calls
span.recordExceptionandspan.end()again on the same span. The OpenTelemetry API ignores the secondend()and the post-endrecordException, and typically logs a warning, so these throw paths record no exception and emit a diagnostic on every blocked or negatively-cached request.Let the catch block own span termination on all throw paths. Keep
span.end()only on the successfulreturnat Line 203.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ipfs/ipfs-service.ts` around lines 139 - 152, Remove the inner span.end() calls from the blocked and negatively cached throw paths in the method, including the branches around the cache digest and hash validation checks. Let the outer catch handle span.recordException and span.end() for all errors, retaining span.end() only immediately before the successful return path.src/ipfs/ipfs-rate-limiter.ts (1)
23-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
maxBucketsto a named constant or configuration value.Every other limiter parameter here comes from
src/config.ts.maxBuckets: 50000is an inline literal that directly bounds memory for the IP and resource bucket maps. An operator cannot tune it. Either export it fromsrc/constants.tsor add anIPFS_RATE_LIMITER_MAX_BUCKETSvariable insrc/config.tswith matching entries indocs/envs.mdanddocker-compose.yaml.As per coding guidelines: "Parse all environment variables in
src/config.tsand export typed constants; when adding or removing variables, keepdocs/envs.mdanddocker-compose.yamlsynchronized."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ipfs/ipfs-rate-limiter.ts` around lines 23 - 24, Replace the inline maxBuckets value in the limiter configuration with a typed, parsed configuration value from src/config.ts named for IPFS rate limiting. Add the corresponding IPFS_RATE_LIMITER_MAX_BUCKETS environment variable documentation and docker-compose.yaml entry, preserving the current default of 50000.Source: Coding guidelines
src/ipfs/ipfs-cache.test.ts (1)
27-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the index-rebuild-from-disk path.
get,has, andgetDigestall rebuild an index entry from the.metafile when the in-memory index has no key. That branch runs after every restart, and no test covers it. Construct a secondIpfsFsCacheover the samebaseDirafterputFromFile, then assert thatgetstill returns the bytes, the size, the content type, and the digest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ipfs/ipfs-cache.test.ts` around lines 27 - 43, Add a test in the IpfsFsCache suite that writes an entry with putFromFile, creates a second IpfsFsCache using the same baseDir to simulate a restart, and verifies get returns the original bytes, size, content type, and digest through the disk index-rebuild path.src/ipfs/kubo-data-source.test.ts (1)
36-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThese two tests cannot fail; assert the constructed URL.
Both tests abort the signal, then swallow the rejection and assert only
assert.ok(error)insidecatch. IfgetContentever resolved, thecatchblock would not run and the test would pass with no assertion. Neither test checks the URL, despite the names.The
range requestsandtrustless formatblocks already show the working pattern: install a request interceptor, captureconfig.url, and assert it. Reuse that pattern here so the URL construction, including the per-segmentencodeURIComponent, is actually covered.💚 Suggested structure
- it('constructs correct URL for CID with path', async () => { - const controller = new AbortController(); - controller.abort(); - - try { - await kuboDataSource.getContent({ - cidString: - 'bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi', - path: 'images/logo.png', - signal: controller.signal, - }); - } catch (error: any) { - assert.ok(error); - } - }); + it('constructs correct URL for CID with path', async () => { + let captured: any; + const id = axios.interceptors.request.use((config) => { + captured = config; + config.adapter = () => + Promise.resolve({ + status: 200, + statusText: 'OK', + headers: { 'content-length': '1' }, + config, + data: Readable.from([Buffer.alloc(1)]), + }); + return config; + }); + try { + const result = await kuboDataSource.getContent({ + cidString: + 'bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi', + path: 'images/logo.png', + }); + result.stream.destroy(); + assert.equal( + captured.url, + 'http://localhost:8080/ipfs/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi/images/logo.png', + ); + } finally { + axios.interceptors.request.eject(id); + } + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ipfs/kubo-data-source.test.ts` around lines 36 - 68, Update the tests “constructs correct URL for bare CID” and “constructs correct URL for CID with path” to intercept the outgoing request, capture its URL, and assert the expected bare-CID and path URLs, including per-segment encoding. Reuse the request-interceptor pattern from the existing “range requests” and “trustless format” tests, and remove the abort-and-swallow logic so each test fails when URL construction is incorrect.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/ar-io-gateway-operator/SKILL.md:
- Around line 116-118: Update the “IPFS serving (opt-in Kubo sidecar)” section
to explicitly require both setting IPFS_ENABLED=true for core and starting the
Compose ipfs profile, or supplying an equivalent override that runs kubo.
Clarify that enabling IPFS_ENABLED alone does not start kubo and can result in
502 or 504 responses.
In `@docker-compose.yaml`:
- Line 57: Update the IPFS cache volume entry to use the same environment
variable as the runtime configuration in src/config.ts, namely IPFS_CACHE_PATH,
while preserving the existing default host path and container mount target;
remove the separate IPFS_CACHE_DATA_PATH reference so configuration and
docker-compose.yaml remain synchronized.
In `@docs/envs.md`:
- Around line 645-648: Verify the canonical IPFS rate-limit defaults in
src/config.ts, then update docs/envs.md lines 645-648 and
docs/ipfs-integration.md lines 463-466 so both documentation tables use those
exact values for the IP and resource bucket capacities and refill rates. Keep
the environment variable names and documentation synchronized with the
configuration.
In `@docs/openapi.yaml`:
- Around line 2998-3004: Update the OpenAPI schema for the block endpoint to
complete the contract described in the summary and description. First, locate
the id parameter/field schema that currently states "TX ID" and expand its
description to clarify it accepts transaction IDs, data-item IDs, IPFS CIDs (as
CIDv1 base32 strings), and SHA-256 content hashes. Second, add HTTP 451 to the
responses section of this endpoint to document that blocked CIDs return this
status code, ensuring generated clients and API documentation reflect the actual
contract.
In `@src/ipfs/ipfs-cache.ts`:
- Around line 56-67: Update the IPFS cache initialization around the LRU index
construction to reconcile persisted files under the cache data directory with
the in-memory index, ensuring disk usage remains bounded across restarts by
seeding valid entries or removing untracked files. Also clean up stale files in
the temporary directory, including artifacts left by streamToCache and put after
crashes; use the existing dataPath, metaPath, and cache lifecycle symbols.
- Around line 57-58: Update the lru-cache sizeCalculation callback in the cache
configuration to clamp each entry’s size to at least 1 using Math.max(1,
entry.size), while preserving maxSize and the existing cache behavior.
In `@src/ipfs/ipfs-pinner.ts`:
- Around line 67-72: Update the eviction loop in the pinner’s pin-tracking flow
so each oldest entry remains in this.pinned until rpc('pin/rm', oldest)
succeeds; handle rejection without suppressing it, and retry or reconcile the
failed unpin so active pins remain tracked and the collection can eventually
return to max.
In `@src/ipfs/ipfs-service.ts`:
- Around line 393-415: Update the stream data handling around the cache writer
to honor writeStream.write backpressure by pausing the source when it returns
false and resuming on drain, while preserving cleanup and size-limit behavior.
Add a concise comment documenting the dependency on attachStallTimeout leaving
the stream paused until the client pipes it, and add a test that awaits between
getContent and piping the response to verify the client still receives the
complete body.
- Around line 154-164: Update the negative-cache key usage in the fetch flow so
it includes both normalizedCid and path, matching the content-cache identity.
Apply this composite key consistently to isNegativelyCached, both evict calls,
and recordMiss, preserving the existing negative-cache thresholds and error
behavior.
In `@src/ipfs/kubo-data-source.ts`:
- Around line 200-201: Update the Kubo data-source constructor and configuration
flow to define a bounded maximum request duration from a new IPFS_* setting,
document it in docs/envs.md and docker-compose.yaml, and store it alongside the
existing timeout fields. Pass that value as the maxRequestMs argument to
attachStallTimeout in the stream setup, preserving the existing stall-timeout
behavior.
- Around line 234-241: Update the catch block in the request method around the
visible release, timeout cleanup, and error handling to destroy the rejected
Axios response stream before returning or propagating the error. Use
error.response.data when it is a readable stream, preserving the existing in-try
stream cleanup behavior for 404, 408, 504, 416, and other unexpected statuses.
In `@src/resolution/resolved-target.ts`:
- Around line 29-40: Reject unsupported explicit protocols instead of silently
treating them as Arweave: in classifyResolvedTarget, accept undefined or 0 as
Arweave, 1 as IPFS, and throw for all other values. In
src/resolution/trusted-gateway-arns-resolver.ts lines 87-98, default only for an
absent header and reject explicit values other than arweave or ipfs. In
src/resolution/resolved-target.test.ts lines 26-28, replace the fallback
expectation with an assertion that unsupported protocols are rejected.
In `@src/routes/ipfs.ts`:
- Line 276: Update the ETag construction in the request handler around
cidToV1Base32 so it incorporates the requested sub-path, response format, and
range, producing distinct validators for each representation. Preserve the
existing quoted ETag format and ensure the same inputs continue to generate the
same validator for cache revalidation.
In `@test-ipfs.sh`:
- Around line 105-118: Update test_body_contains in test-ipfs.sh so its curl
request follows redirects, allowing Tests 1–3 to validate content when
createIpfsPathHandler redirects /ipfs/{CID} requests. Document in the relevant
test header that ARNS_ROOT_HOSTS must be configured and wildcard redirect hosts
require DNS or curl --resolve support.
- Around line 136-142: Update test_header in test-ipfs.sh so an empty
expected_value performs a non-empty header presence check instead of passing
grep -qi "". Preserve the existing case-insensitive value matching for non-empty
expectations, ensuring the “X-Cache header present” and “Content-Type is set”
assertions fail when the header is absent.
---
Minor comments:
In `@docs/glossary.md`:
- Around line 288-294: Add an “IPFS CID” glossary entry to docs/glossary.md so
the existing `#ipfs-cid` link from the Target Protocol (ANT record) definition
resolves. Define the term consistently with the surrounding glossary entries and
preserve the existing target-routing description.
In `@src/ipfs/ipfs-cache.ts`:
- Around line 189-229: Update the IPFS cache write path by checking callers of
IpfsFsCache.put, including streamToCache and putFromFile; if put has no
remaining callers, remove it. Otherwise, retain it only after matching
putFromFile’s failure cleanup by unlinking tempPath, and populate CacheEntry
metadata with the content digest required by IpfsService for block-by-hash
validation.
In `@src/ipfs/ipfs-service.ts`:
- Around line 280-291: Update the response stream selection in the return block
to apply guardSize whenever the response bypasses streamToCache, including range
responses. Preserve the existing unguarded path only for responses that are
handled by streamToCache, and retain the current trustless-format behavior.
In `@src/middleware/ipfs.ts`:
- Around line 67-68: Standardize the encoding contract for request ipfsPath
across the CID and ArNS middleware producers before it reaches the Kubo path
consumer. Update the ipfsCid/ipfsPath request interface documentation to state
the chosen format, then ensure both middleware paths assign that same format
instead of one percent-encoding and the other decoding.
- Around line 76-93: Fix two issues in the CID-handling block: First, simplify
the redundant ternary on the reqPath assignment to just assign remainder
directly, since both branches return the same value. Second, preserve the query
string when constructing the 302 redirect URL in the isValidCid branch by
appending the original request's query string to the constructed target URL,
similar to how src/routes/ipfs.ts handles query preservation, so that requests
like /ipfs/{otherCid}/x?format=car maintain their query parameters instead of
losing them during the cross-CID redirect.
In `@src/routes/arns.ts`:
- Around line 96-100: Update the resolver response schema and examples in
docs/openapi.yaml to include resolvedId and protocol, and revise txId
documentation to state that it remains for backward compatibility while
representing a CID for IPFS records and preferring resolvedId plus protocol.
Keep the OpenAPI contract aligned with the response returned by the resolver
route.
In `@src/routes/ipfs.ts`:
- Around line 100-102: Add validation in createIpfsHandler before calling
ipfsService.getContent to ensure ipfsCid is a valid string. First check that
cidString is a non-empty string and reject with a 400 response if not, then
validate the CID format itself (following the pattern used in
createIpfsPathHandler) and reject with a 502 response if the CID string is
malformed, ensuring both checks occur before the ipfsService.getContent call
proceeds.
In `@test-ipfs.sh`:
- Around line 1-6: Harden the test script preamble by adding set -uo pipefail
without -e, replace the fixed /tmp/ipfs-test-body path with a mktemp-created
file and clean it up on exit, remove the unused extra_args parameter and its
unquoted expansion, and delete the unread status assignment. Keep the existing
pass counter behavior intact.
---
Nitpick comments:
In `@src/ipfs/ipfs-cache.test.ts`:
- Around line 27-43: Add a test in the IpfsFsCache suite that writes an entry
with putFromFile, creates a second IpfsFsCache using the same baseDir to
simulate a restart, and verifies get returns the original bytes, size, content
type, and digest through the disk index-rebuild path.
In `@src/ipfs/ipfs-rate-limiter.ts`:
- Around line 23-24: Replace the inline maxBuckets value in the limiter
configuration with a typed, parsed configuration value from src/config.ts named
for IPFS rate limiting. Add the corresponding IPFS_RATE_LIMITER_MAX_BUCKETS
environment variable documentation and docker-compose.yaml entry, preserving the
current default of 50000.
In `@src/ipfs/ipfs-service.ts`:
- Around line 139-152: Remove the inner span.end() calls from the blocked and
negatively cached throw paths in the method, including the branches around the
cache digest and hash validation checks. Let the outer catch handle
span.recordException and span.end() for all errors, retaining span.end() only
immediately before the successful return path.
In `@src/ipfs/kubo-data-source.test.ts`:
- Around line 36-68: Update the tests “constructs correct URL for bare CID” and
“constructs correct URL for CID with path” to intercept the outgoing request,
capture its URL, and assert the expected bare-CID and path URLs, including
per-segment encoding. Reuse the request-interceptor pattern from the existing
“range requests” and “trustless format” tests, and remove the abort-and-swallow
logic so each test fails when URL construction is incorrect.
In `@test-ipfs.sh`:
- Around line 93-101: Define a configurable Kubo container name in test-ipfs.sh
using an environment variable and default it to ar-io-node-kubo-1. Update both
docker exec invocations in the FILE_CID and DIR_CID setup flows to reuse this
variable instead of hardcoding the container name.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aa26504b-785b-46df-aa65-04c25585b34e
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
.claude/skills/ar-io-gateway-operator/SKILL.md.dockerignoreCLAUDE.mddocker-compose.yamldocs/INDEX.mddocs/drafts/davids-brain-alignment.mddocs/drafts/ipfs-observation-incentive-analysis.mddocs/envs.mddocs/glossary.mddocs/ipfs-integration.mddocs/openapi.yamlmonitoring/grafana/dashboards/examples/ipfs-example.jsonpackage.jsonsrc/app.tssrc/config.tssrc/constants.tssrc/init/resolvers.tssrc/ipfs/ipfs-cache.test.tssrc/ipfs/ipfs-cache.tssrc/ipfs/ipfs-cid.test.tssrc/ipfs/ipfs-pinner.test.tssrc/ipfs/ipfs-pinner.tssrc/ipfs/ipfs-rate-limiter.tssrc/ipfs/ipfs-service.tssrc/ipfs/kubo-data-source.test.tssrc/ipfs/kubo-data-source.tssrc/lib/httpsig.tssrc/lib/ipfs-cid.tssrc/metrics.tssrc/middleware/arns.tssrc/middleware/ipfs.tssrc/middleware/sandbox.tssrc/resolution/on-demand-arns-resolver.tssrc/resolution/resolved-target.test.tssrc/resolution/resolved-target.tssrc/resolution/trusted-gateway-arns-resolver.test.tssrc/resolution/trusted-gateway-arns-resolver.tssrc/routes/ar-io-info-builder.tssrc/routes/ar-io.tssrc/routes/arns.tssrc/routes/ipfs.tssrc/system.tssrc/types.d.tstest-ipfs.sh
| - ${HEADERS_DATA_PATH:-./data/headers}:/app/data/headers | ||
| - ${SQLITE_DATA_PATH:-./data/sqlite}:/app/data/sqlite | ||
| - ${DUCKDB_DATA_PATH:-./data/duckdb}:/app/data/duckdb | ||
| - ${IPFS_CACHE_DATA_PATH:-./data/ipfs-cache}:/app/data/ipfs-cache |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Align the cache volume with the documented runtime path.
The host path uses IPFS_CACHE_DATA_PATH, but the core process uses IPFS_CACHE_PATH. The documentation exposes only IPFS_CACHE_PATH. If an operator changes that variable without changing the bind-source variable, the cache can be written outside the mounted volume and disappear when the container is replaced. Use one variable or document and validate the pair.
As per coding guidelines, keep environment variables and docker-compose.yaml synchronized with src/config.ts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.yaml` at line 57, Update the IPFS cache volume entry to use
the same environment variable as the runtime configuration in src/config.ts,
namely IPFS_CACHE_PATH, while preserving the existing default host path and
container mount target; remove the separate IPFS_CACHE_DATA_PATH reference so
configuration and docker-compose.yaml remain synchronized.
Source: Coding guidelines
| | IPFS_RATE_LIMITER_IP_TOKENS_PER_BUCKET | Number | 100000 | IPFS rate limiter: max tokens per IP bucket | | ||
| | IPFS_RATE_LIMITER_IP_REFILL_PER_SEC | Number | 20 | IPFS rate limiter: token refill rate per second (IP bucket) | | ||
| | IPFS_RATE_LIMITER_RESOURCE_TOKENS_PER_BUCKET | Number | 1000000 | IPFS rate limiter: max tokens per resource bucket | | ||
| | IPFS_RATE_LIMITER_RESOURCE_REFILL_PER_SEC | Number | 100 | IPFS rate limiter: token refill rate per second (resource bucket) | |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use one canonical IPFS rate-limit default.
The two documentation tables define different IP and resource bucket capacities and refill rates. Verify src/config.ts, then update both sites to the same values.
docs/envs.md#L645-L648: make the canonical environment reference matchsrc/config.ts.docs/ipfs-integration.md#L463-L466: update the integration guide to the same defaults.
As per coding guidelines, keep environment variables and their documentation synchronized with src/config.ts.
#!/bin/bash
set -euo pipefail
rg -n -C 3 'IPFS_RATE_LIMITER_(IP|RESOURCE)_(TOKENS_PER_BUCKET|REFILL_PER_SEC)' \
src/config.ts docs/envs.md docs/ipfs-integration.md docker-compose.yaml📍 Affects 2 files
docs/envs.md#L645-L648(this comment)docs/ipfs-integration.md#L463-L466
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/envs.md` around lines 645 - 648, Verify the canonical IPFS rate-limit
defaults in src/config.ts, then update docs/envs.md lines 645-648 and
docs/ipfs-integration.md lines 463-466 so both documentation tables use those
exact values for the IP and resource bucket capacities and refill rates. Keep
the environment variable names and documentation synchronized with the
configuration.
| summary: Blocks transactions, data-items, or IPFS CIDs so your AR.IO Gateway will not serve them. | ||
| description: | | ||
| Submits a TX ID/data-item ID or sha-256 content hash for content you do not want your AR.IO Gateway to serve. Once submitted, your Gateway will not respond to requests for these transactions or data-items. | ||
| Submits a TX ID/data-item ID, IPFS CID, or sha-256 content hash for content you do not want your AR.IO Gateway to serve. Once submitted, your Gateway will not respond to requests for these transactions, data-items, or IPFS CIDs. | ||
|
|
||
| For IPFS content, pass the CIDv1 base32 string (e.g. bafkreigbk3hjz6oyiywqf7eknthwc2osvt5xi6b6igwljn2qrxkthqgrp4) as the id field. The gateway returns HTTP 451 for blocked CIDs. | ||
|
|
||
| WARNING - Testing a TX ID here WILL result in that data being blocked by your Gateway. | ||
| WARNING - Testing an ID here WILL result in that data being blocked by your Gateway. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Complete the OpenAPI contract for IPFS block responses.
The new description accepts transaction IDs, data-item IDs, IPFS CIDs, and hashes, and states that blocked CIDs return 451. The id schema still says “TX ID”, and the response list omits 451. Update both parts so generated clients and API users see the actual contract.
Proposed schema alignment
id:
type: string
- description: TX ID for a transaction you want to block.
+ description: Transaction ID, data-item ID, IPFS CID, or SHA-256 content hash.
...
responses:
'200':
description: Successful operation.
+ '451':
+ description: Content blocked by the gateway.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/openapi.yaml` around lines 2998 - 3004, Update the OpenAPI schema for
the block endpoint to complete the contract described in the summary and
description. First, locate the id parameter/field schema that currently states
"TX ID" and expand its description to clarify it accepts transaction IDs,
data-item IDs, IPFS CIDs (as CIDv1 base32 strings), and SHA-256 content hashes.
Second, add HTTP 451 to the responses section of this endpoint to document that
blocked CIDs return this status code, ensuring generated clients and API
documentation reflect the actual contract.
| this.index = new LRUCache<string, CacheEntry>({ | ||
| maxSize: maxSizeBytes, | ||
| sizeCalculation: (entry) => entry.size, | ||
| dispose: (_entry, key) => { | ||
| // Delete file from disk when evicted from LRU | ||
| const dataPath = this.dataPath(key); | ||
| const metaPath = this.metaPath(key); | ||
| fs.promises.unlink(dataPath).catch(() => {}); | ||
| fs.promises.unlink(metaPath).catch(() => {}); | ||
| this.log.debug('Evicted IPFS cache entry', { key }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Cached bytes on disk are not bounded across restarts.
maxSizeBytes is enforced only through the in-memory index. The index starts empty on every process start. has, get, and getDigest re-add an entry lazily, but only for keys that are requested again. Files that are never requested again stay on disk forever and are never counted toward maxSize.
The result: over repeated restarts, the directory at IPFS_CACHE_PATH grows without an upper bound, while the LRU believes it is well under budget. On a gateway with a modest cache volume this ends in disk exhaustion.
Reconcile the index with the disk at construction, or add a periodic sweeper that walks data/ and either seeds the index or removes files that the index does not track. The tmp/ directory needs the same treatment: streamToCache and put leave temp files behind on a hard crash, and nothing removes them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ipfs/ipfs-cache.ts` around lines 56 - 67, Update the IPFS cache
initialization around the LRU index construction to reconcile persisted files
under the cache data directory with the in-memory index, ensuring disk usage
remains bounded across restarts by seeding valid entries or removing untracked
files. Also clean up stale files in the temporary directory, including artifacts
left by streamToCache and put after crashes; use the existing dataPath,
metaPath, and cache lifecycle symbols.
| if (result.size > 0) { | ||
| res.setHeader('Content-Length', result.size); | ||
| } | ||
| res.setHeader('ETag', `"${cidToV1Base32(cidString)}"`); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The ETag does not vary by path, format, or range.
The validator is derived only from the CID. The same ETag is returned for:
- different sub-paths of one CID, for example
/a.txtand/b.txton{cid}.{host}; ?format=raw,?format=car, and the default UnixFS response;- a full 200 body and a 206 partial body.
Every one of those responses also carries Cache-Control: public, max-age=29030400, immutable. A shared cache or a client that revalidates with If-None-Match can therefore match one entry and serve the wrong body for a different sub-path or format. Include the path, the format, and the range in the validator.
🐛 Proposed fix
- res.setHeader('ETag', `"${cidToV1Base32(cidString)}"`);
+ // Vary the validator by everything that changes the bytes: the resolved
+ // sub-path and the response format. Range responses (206) are excluded
+ // from strong-validator reuse below.
+ const etagSuffix = `${path !== undefined ? `/${path}` : ''}${
+ format !== undefined ? `#${format}` : ''
+ }`;
+ res.setHeader('ETag', `"${cidToV1Base32(cidString)}${etagSuffix}"`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/ipfs.ts` at line 276, Update the ETag construction in the request
handler around cidToV1Base32 so it incorporates the requested sub-path, response
format, and range, producing distinct validators for each representation.
Preserve the existing quoted ETag format and ensure the same inputs continue to
generate the same validator for cache revalidation.
- Negative cache keyed by CID+path (was CID-only): one missing sub-path no longer blackholes the whole CID/site for the negative-cache TTL. - ETag varies by sub-path and format (raw/car), so a shared cache can't serve the wrong body under `immutable`. (Range still shares the full entity's ETag.) - Pinner keeps a CID tracked until Kubo confirms pin/rm, and stops on failure instead of silently drifting the real pin count above max. - KuboDataSource destroys the response stream when axios rejects on 5xx (was a socket/fd leak), and forwards a wall-clock cap (IPFS_KUBO_MAX_REQUEST_MS) to attachStallTimeout so a backpressure-pause-then-upstream-stall can't hold a concurrency slot forever. - classifyResolvedTarget rejects explicit unsupported targetProtocol values (and the trusted-gateway resolver rejects unknown X-ArNS-Protocol) instead of mis-serving them as Arweave; test updated. - streamToCache honors write backpressure (pause/resume on drain) and documents the load-bearing upstream pause() dependency. - Docs: align IPFS rate-limit defaults in ipfs-integration.md with config.ts; clarify that IPFS_ENABLED needs the kubo profile (else 502/504); note the IPFS_CACHE_PATH/volume coupling; broaden the block-data `id` OpenAPI schema; document IPFS_KUBO_MAX_REQUEST_MS (envs + compose). - test-ipfs.sh: follow redirects in body checks; treat an empty header expectation as a presence check (was always passing). Not changed: IpfsFsCache disk usage isn't reconciled with its in-memory index across restarts (pre-existing #682 design) — a disk sweeper / index rebuild is a separate change, tracked as a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
|
Thanks @coderabbitai — addressed the review in e03da75. Summary: Correctness / security (code):
Docs / config:
Deferred (with reason):
|
|
Tip For best results, initiate chat on the files or code changes.
The deferred 🐇 ✅ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ipfs/ipfs-pinner.ts`:
- Around line 71-82: Update the capacity flow around the eviction loop and
pin/add handling so every new pin waits for successful eviction until
pinned.size is at most max. When pin/rm fails, defer the requested CID instead
of issuing pin/add, and retry the eviction with backoff before allowing new
pins; preserve the existing retry behavior and logging context.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: af7d1e81-49d1-4ce8-83f9-1eeb02098b1b
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.claude/skills/ar-io-gateway-operator/SKILL.mddocker-compose.yamldocs/envs.mddocs/ipfs-integration.mddocs/openapi.yamlsrc/config.tssrc/ipfs/ipfs-pinner.tssrc/ipfs/ipfs-service.tssrc/ipfs/kubo-data-source.tssrc/resolution/resolved-target.test.tssrc/resolution/resolved-target.tssrc/resolution/trusted-gateway-arns-resolver.tssrc/routes/ipfs.tssrc/system.tstest-ipfs.sh
🚧 Files skipped from review as they are similar to previous changes (12)
- src/resolution/resolved-target.test.ts
- docs/envs.md
- src/system.ts
- docs/openapi.yaml
- docs/ipfs-integration.md
- src/ipfs/kubo-data-source.ts
- src/resolution/trusted-gateway-arns-resolver.ts
- src/routes/ipfs.ts
- test-ipfs.sh
- src/ipfs/ipfs-service.ts
- docker-compose.yaml
- src/config.ts
| while (this.pinned.size > this.max) { | ||
| const oldest = this.pinned.values().next().value; | ||
| if (oldest === undefined) break; | ||
| try { | ||
| await this.rpc('pin/rm', oldest); | ||
| this.pinned.delete(oldest); | ||
| } catch (error: any) { | ||
| this.log.warn('Failed to unpin evicted CID; will retry later', { | ||
| cid: oldest, | ||
| message: error?.message, | ||
| }); | ||
| break; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Enforce max before more pin/add requests.
When pin/rm fails, pinned stays above max. Later distinct CIDs still execute pin/add and stop at the same failed eviction. This makes IPFS_PIN_MAX unbounded during a persistent unpin failure and can exhaust Kubo storage.
Serialize capacity checks. Evict before pin/add. If eviction fails, defer new pins and retry eviction with backoff.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ipfs/ipfs-pinner.ts` around lines 71 - 82, Update the capacity flow
around the eviction loop and pin/add handling so every new pin waits for
successful eviction until pinned.size is at most max. When pin/rm fails, defer
the requested CID instead of issuing pin/add, and retry the eviction with
backoff before allowing new pins; preserve the existing retry behavior and
logging context.
…-limit)
A multi-agent adversarial review surfaced 10 confirmed defects clustered in the
stream lifecycle and cache-control logic. All fixed here with tests.
High:
- Client disconnect mid-download now tears down the upstream Kubo stream
(res.on('close') -> stream.destroy), releasing the concurrency slot, socket,
and cache temp-fd immediately instead of at the ~20-min wall-clock cap.
Repeated aborts could otherwise pin all IPFS_KUBO_MAX_CONCURRENT_REQUESTS
slots and 502 legitimate traffic.
- The trustless format=raw|car branch only sets immutable Cache-Control for
direct-CID requests (req.arns === undefined), mirroring the UnixFS branch, so
an ArNS name repoint is no longer masked by an ~11-month immutable entry.
- The rate limiter is now charged the bytes actually streamed (a passthrough
counter), not the fixed 256 KB unknown-size reserve; a 1 GB CAR previously
cost ~256 tokens. Accounting also fires on 'close' (aborted transfers), not
only 'finish'.
Medium:
- HEAD/rate-limit teardown of a format response destroys the underlying Kubo
source, not just the guardSize wrapper (guard 'close' -> source.destroy).
- IPFS uses its own NegativeDataCache instance, so its health window no longer
gates Arweave negative-cache promotions (and vice-versa).
- Vary: Accept is set on all IPFS responses (representation is negotiated on
Accept: application/vnd.ipld.raw|car).
- getContent spans end on 'close' too, so destroy-without-error paths (HEAD,
rate-limited, guard abort) don't leak unended spans.
Low:
- Cache hit/miss counters increment once (in the service), not also in the route.
- The cross-CID subdomain redirect preserves the query string (e.g. ?format=raw).
- ArNS-root ipfsPath is normalized '' -> undefined to match the subdomain
middleware's cache/ETag key.
Tests: new ipfs-service, routes/ipfs, and routes/ipfs-rate-accounting suites plus
a kubo-data-source slot-release case cover H1/H2/H3/M1/M4/L1 deterministically
(client-abort teardown via a real socket; byte-accounting via an enabled limiter;
guard/source teardown; negative-cache path key). Full suite 2148 pass / 0 fail.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/system.ts (1)
1916-1929: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPartition or aggregate the negative-cache gauges.
NegativeDataCache.updateGauges()writes the shared unlabelled metrics, butsrc/system.tscreates separate Arweave and IPFS instances. Add a protocol/source label tonegative_cache_size,miss_tracker_size, andpromotion_history_size, or expose aggregate gauges for the two caches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/system.ts` around lines 1916 - 1929, Update NegativeDataCache gauge reporting and the Arweave/IPFS cache setup so negative_cache_size, miss_tracker_size, and promotion_history_size cannot collide across instances. Prefer adding a protocol/source label and passing distinct values for each cache, ensuring updateGauges() supplies that label consistently for both Arweave and IPFS instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/system.ts`:
- Around line 1916-1929: Update NegativeDataCache gauge reporting and the
Arweave/IPFS cache setup so negative_cache_size, miss_tracker_size, and
promotion_history_size cannot collide across instances. Prefer adding a
protocol/source label and passing distinct values for each cache, ensuring
updateGauges() supplies that label consistently for both Arweave and IPFS
instances.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e24164e0-de91-4479-8a31-4a7b04d42cf2
📒 Files selected for processing (10)
src/ipfs/ipfs-service.test.tssrc/ipfs/ipfs-service.tssrc/ipfs/kubo-data-source.test.tssrc/ipfs/kubo-data-source.tssrc/middleware/arns.tssrc/middleware/ipfs.tssrc/routes/ipfs-rate-accounting.test.tssrc/routes/ipfs.test.tssrc/routes/ipfs.tssrc/system.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/middleware/arns.ts
- src/ipfs/ipfs-service.ts
The M2 fix gave IPFS its own NegativeDataCache instance, but the three size gauges (negative_cache_size, miss_tracker_size, promotion_history_size) were unlabelled, so the Arweave and IPFS instances' updateGauges() calls clobbered each other. Add a `source` label (default 'arweave', 'ipfs' for the IPFS cache) so each instance reports a distinct series. Addresses CodeRabbit nitpick. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
|
Addressed the negative-cache gauge nitpick in e5bea62 — added a |
Add observer-ipfs-adjustments-spec.md — a verified, observer-only plan to make named IPFS data a first-class citizen with zero smart-contract changes (protocol awareness + capability ramp, trustless CID verification, neutral scoring, block sampling). Every current-state claim is cited to live source. Reconcile the two companion drafts to the latest thinking: - ipfs-observation-incentive-analysis.md (Fable): add a dated reconciliation note — no on-chain capability bit is needed (/ar-io/info already advertises ipfs.enabled and the observer already reads it), and the framing is first-class with an adoption ramp rather than mandatory-vs-optional. Body preserved. - davids-brain-alignment.md: cross-reference the new spec; note the OIP §5 "verify, don't trust" win is now specced with zero contract changes. Working drafts; to be cleaned up later. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
…spec Add §9 — cost/ops for first-class IPFS: observer changes are near-zero; the only real cost is running an IPFS node, satisfiable per gateway by a bundled Kubo sidecar or by pointing IPFS_KUBO_URL/IPFS_KUBO_API_URL at a shared/third-party node. Disk (10 GB cache, GC), bandwidth (DHT profile is a dial), pinning off by default; persistence is the deferred future lever. Grounded in verified config. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
An ArNS name pointing at unretrievable IPFS content (dead link, unpinned+GC'd, or offline) makes Kubo search the DHT and find no provider — Kubo never returns 404 for absent network content, it times out. Previously only 404s were negatively cached, so every repeat request for a dead CID re-ran the full IPFS_KUBO_REQUEST_TIMEOUT_MS (30s) search and held a Kubo concurrency slot; under load a single unresolvable CID could pin all IPFS_KUBO_MAX_CONCURRENT_REQUESTS slots and 502 healthy traffic. - Record a negative-cache miss on IpfsTimeoutError too, so a repeatedly-dead CID trips the (threshold-gated) negative cache and short-circuits to a fast, CDN-dampened 404 instead of re-searching. Transient cold-DHT slowness won't trip it (needs repeated misses over a window). IpfsUnavailableError / ECONNREFUSED stay uncached — those mean Kubo itself is down, not the content. - Add short Cache-Control (CACHE_NOT_FOUND_MAX_AGE, 60s) to the 504 so edges/CDNs dampen retry storms in front of the gateway. Tests: timeout records a miss; unavailable does not; 504 carries Cache-Control. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
Make explicit that neutral scoring is enforced at the observer level, not the contract (the chain only tallies a per-gateway pass/fail bitmap by >1/2-observer majority). Neutral therefore protects a non-IPFS gateway only once a majority of observers run the updated code; the majority rule is the backstop for an uneven rollout. It's a software release adopted by the observer fleet, not a contract deploy, and requires no action from gateway operators. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
…arial #2) The no-provider defaults commit fed IpfsTimeoutError into recordMiss, but the negative cache has a single-miss re-promotion fast path (priorPromotions>0 => effectiveCount 1, duration 0). So one transient cold-DHT timeout could instantly re-blackhole a previously-promoted-but-recovering CID for hours (exponential TTL) — contradicting the "needs repeated misses" safety claim. - NegativeDataCache.recordMiss gains an opts.softMiss flag: a soft miss never uses the single-miss fast path, always requiring the full miss count/duration threshold. Backward-compatible — Arweave callers pass nothing (hard miss). - ipfs-service records IpfsTimeoutError as a soft miss; a 404 (IpfsNotFoundError) remains a definitive hard miss. Tests: a single hard miss re-promotes after a prior promotion; a single soft miss does not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
…(adversarial #4) Negatively caching IpfsTimeoutError (soft miss) still blackholed recovering-but- slow content once tripped: it used the normal absent-content TTL (escalating toward maxTtlMs, hours), so a cold-DHT CID that recovered stayed cached-out with no self-healing until TTL expiry. - NegativeDataCache: a soft-miss promotion now uses a short, fixed softMissTtlMs and does NOT build the escalation/re-promotion history; hard (404) promotions keep the exponential-backoff TTL and history. Backward-compatible: softMissTtlMs defaults to ttlMs, so Arweave callers (which never pass softMiss) are unchanged. - IPFS negative cache wires softMissTtlMs = IPFS_TIMEOUT_NEGATIVE_CACHE_TTL_MS (default 60s), so a timing-out CID self-heals within ~a minute. - Corrected the ipfs-service comment to state the actual protection. Tests: a soft-miss promotion self-heals after the short TTL and builds no escalation history. negative-data-cache suite 31 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
…pec to as-shipped - Add IPFS_TIMEOUT_NEGATIVE_CACHE_TTL_MS to docs/envs.md and docker-compose.yaml (the short, self-healing negative-cache TTL for IPFS retrieval timeouts). - observer-ipfs-adjustments-spec.md: prepend a "§0 As shipped" section that is now the source of truth — the design evolved through 5 adversarial passes from the original phased proposal. Captures the final model: shared assessIpfsNameTrustless on the live GatewayAssessor path, CID-based routing (not the protocol header), the PASS/FAIL/NEUTRAL scoring rules (fail only on proven-wrong bytes; availability is neutral; behavioral capability), neutral excluded from every aggregate, the IPFS_ASSESSMENT_TIMEOUT_MS deadline, and the recommended self-reference + on-demand topology. Phase 3 (leaf/DAG sampling) remains deferred. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/drafts/davids-brain-alignment.md (1)
116-120: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not document
ETagas the CID.The route contract varies ETags by sub-path and response format. A CID-only ETag description can cause clients to treat distinct representations as equivalent. Document the ETag as representation-specific instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/drafts/davids-brain-alignment.md` around lines 116 - 120, Update the “Trustless retrieval added” documentation to describe ETag as representation-specific and varying by sub-path and response format, rather than equating it with the CID. Keep the existing trustless response details and CID verification description unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ipfs/ipfs-service.test.ts`:
- Around line 147-150: Extend the timeout test’s assertions for recordMiss in
the negative-cache flow to verify that its options argument includes softMiss:
true, while preserving the existing call-count and CID assertions. Use the
existing recordMiss mock captured in the test.
---
Outside diff comments:
In `@docs/drafts/davids-brain-alignment.md`:
- Around line 116-120: Update the “Trustless retrieval added” documentation to
describe ETag as representation-specific and varying by sub-path and response
format, rather than equating it with the CID. Keep the existing trustless
response details and CID verification description unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 51a15c56-7e7c-4359-ac4d-0680b9cc9294
📒 Files selected for processing (14)
docker-compose.yamldocs/drafts/davids-brain-alignment.mddocs/drafts/ipfs-observation-incentive-analysis.mddocs/drafts/observer-ipfs-adjustments-spec.mddocs/envs.mdsrc/config.tssrc/data/negative-data-cache.test.tssrc/data/negative-data-cache.tssrc/ipfs/ipfs-service.test.tssrc/ipfs/ipfs-service.tssrc/metrics.tssrc/routes/ipfs.test.tssrc/routes/ipfs.tssrc/system.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/envs.md
- src/system.ts
- docker-compose.yaml
- src/config.ts
- src/ipfs/ipfs-service.ts
- src/routes/ipfs.ts
…rt soft-miss (CodeRabbit)
- docker-compose.yaml: pass IPFS_ASSESSMENT_TIMEOUT_MS through to the bundled
observer service (the observer's trustless ?format=raw fetch deadline).
- ipfs-service.test.ts: assert the timeout miss is recorded with { softMiss: true }
so a regression to a hard miss (restoring the multi-hour blackhole) is caught.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
…ion shipped Record that two of the three original call-outs are resolved (trustless ?format=raw gateway; observer verifies CID→bytes instead of trusting a reference) and the third (CAR→Arweave storage/composite-source/chain-anchored proofs) is the uploading axis, out of scope by design for read-only IPFS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
…ed IPFS Design sketch for gateway-to-gateway verified IPFS fetch: a gateway fetches a CID from a peer AR.IO gateway as a CAR and imports it into Kubo (which verifies blocks against the CID on import) instead of hitting public IPFS. Turns the fleet into a durable, trustless serving layer for named IPFS without Arweave storage. Key unlock: the "local-only" serve mode that powers peer-fetch also gives the observer a trustless, un-gameable PINNING signal (a 200 + verifying bytes in local-only mode proves the gateway holds the content) — closing the "X-Cache is self-asserted, can't measure pinning" gap and enabling the persistence incentive. Composes with (and steps toward) David's CAR-to-Arweave phase 2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
…ro-contract incentive + 1.5a plan - Add the end-to-end lifecycle (chain binding → peer replication → trustless serve → observer holding-probe → reward) as one self-reinforcing flywheel. - Incentive integration: the WHOLE layer ships with zero contract changes. Gateways are already rewarded for serving what ArNS points to; rewarding HOLDING folds into the existing name assessment via a local-only observer probe (no new on-chain field). Serving→holding is a ramped policy choice, not a contract choice; only a dedicated holding-weight-beyond-sampled-names touches the contract. - Content routing in depth (DHT-filtered, named-holdings announce, deterministic assignment) and a concrete, no-contract 1.5a implementation plan (local-only serve mode, IpfsPeerDataSource, IPFS composite, CAR+Kubo-import verification). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
9d37c0a to
53c7ba6
Compare
Implementation-ready plan grounded in the current code: local-only serve mode, IpfsPeerDataSource (CAR + Kubo dag/import verify), SequentialIpfsSource composite, GAR-subset peer selection, config, and a testcontainers multi-node integration harness (2-3 gateways pulling verified content from each other, tamper rejection, public-IPFS-independent durability). 1.5c dropped: holding measurement rides on the local-only primitive with zero contract change. No smart-contract changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPmZYvBxMyHWxrTL85xFr7
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/drafts/davids-brain-alignment.md`:
- Around line 141-160: Update the “Update 2” section to use the same call-out
numbering as the earlier five-item list. Either explicitly map the three grouped
axes to the original call-outs, including `#4` and `#5`, or list the status of all
five call-outs so every item has an auditable disposition.
In `@docs/drafts/ipfs-peer-durability-layer.md`:
- Around line 81-83: Update both fenced code blocks containing the diagrams in
this document to specify the text language identifier, including the blocks
around the local cache flow and the second diagram, while preserving their
contents unchanged.
- Around line 49-64: Update the CAR import flow described in the durability
design to bind the imported content to the requested CID X: require the CAR’s
declared root to equal X, then traverse and validate the requested DAG/path
before serving or pinning it. Reject mismatched or incomplete CARs and continue
to the next peer, preserving pinning only after successful root and traversal
validation.
- Around line 68-73: The durability verification flow must not treat any
successful local-only cache response as proof of durable holding. Update the
observer logic associated with the local-only peer probe to accept only
pin-backed evidence, or require successful repeated probes throughout a defined
retention window before marking the peer as holding content.
- Around line 288-307: Update the IpfsPeerDataSource flow so peer-served CAR
content is moderated before being imported into Kubo, or quarantine the imported
blocks until moderation succeeds. Preserve CID verification, size/deadline
limits, and skip peers when moderation rejects the content; ensure blocked
content never crosses into trusted Kubo storage before the existing
content-control gate.
In `@docs/drafts/ipfs-peer-fetch-1.5a-implementation-plan.md`:
- Around line 56-60: Update the fenced ASCII diagram blocks in the serving-order
section and the referenced lines 403–409 to declare the text language. Add the
text identifier to each affected opening fence while preserving the diagram
contents and closing fences.
- Around line 44-51: Update the Objective section’s availability claim to
reflect that phase 1.5a queries only the selected peerCount peers: state that
content remains available when a selected peer holds it, or explicitly make
holder discovery a prerequisite before retaining the broader “any participating
gateway” guarantee.
- Around line 22-28: The implementation plan must stop characterizing the
local-only probe as trustless proof of local storage or as sufficient evidence
for holding rewards. Update the claims around the local-only observer probe and
its related holding-measurement sections to state only that it measures behavior
under honest server-side enforcement, or define an independent proof mechanism
before connecting the result to rewards.
- Around line 143-146: The request-controlled localOnly flag must not bypass
payment enforcement. Update the IPFS route and related getContent flow around
localOnly, X-Ar-Io-Local-Only, and paymentProcessor so public requests still
receive the existing 402 response; only separately authenticated peer requests
may avoid payment, or retain payment checks regardless of localOnly.
- Line 17: Update the heading “Why 1.5c collapses (and this plan is a and b
only)” to clearly state that the plan covers 1.5a and 1.5b only.
- Around line 261-275: Update the ipfsCompositeSource definition so the first
source wraps kuboDataSource with a LocalOnlyKubo-style adapter that always
forces localOnly: true, while the third source remains the raw kuboDataSource.
Keep the peer-enabled source order unchanged and ensure the wrapper is defined
within or alongside the composite construction.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5aee00b6-783c-44dc-adc8-c883150bdda0
📒 Files selected for processing (5)
docker-compose.yamldocs/drafts/davids-brain-alignment.mddocs/drafts/ipfs-peer-durability-layer.mddocs/drafts/ipfs-peer-fetch-1.5a-implementation-plan.mdsrc/ipfs/ipfs-service.test.ts
| ## Update 2 — the §5 win is now IMPLEMENTED (2026-08-05) | ||
|
|
||
| Two of the three original call-outs are resolved; the third is out of scope by | ||
| design (it's the uploading axis): | ||
|
|
||
| - **Call-out #1 (trustless gateway) — DONE.** `?format=raw|car` returns | ||
| client-verifiable bytes marked `X-Ar-Io-Trustless: true`; the UnixFS proxy path | ||
| is honestly marked `X-Ar-Io-Trustless: false`. This is David's Trustless-Gateway | ||
| shape and his §3 "don't imply verified." | ||
| - **§5 observer verify-don't-trust — DONE (ar-io-observer PR #112).** The observer | ||
| no longer trusts a reference gateway's bytes for IPFS names: it fetches | ||
| `?format=raw` and verifies the block against the CID's multihash. FAIL only on a | ||
| *proven-wrong* answer; availability is neutral; the gateway's self-reported | ||
| `resolvedId` is never trusted. On the IPFS axis this is now **more trustless than | ||
| the Arweave name check** (which still uses reference-digest comparison). | ||
| - **"The gateway is not a trust root" for RESOLUTION — DONE (ar-io-node PR #836).** | ||
| The default resolver is now `on-demand,gateway`: read the ANT binding from | ||
| **chain** first, hop to a trusted gateway only as a fallback. The bundled | ||
| observer references its own on-demand gateway, so the name→CID binding is | ||
| chain-derived, not oracle-trusted. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use one call-out numbering scheme in Update 2.
The earlier section lists five numbered call-outs. Update 2 says “two of the three original call-outs” and then names only #1 and #2/#3. It does not map the earlier #4 and #5.
State the three grouped axes explicitly, or list the status of all five call-outs. This keeps the implementation status auditable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/davids-brain-alignment.md` around lines 141 - 160, Update the
“Update 2” section to use the same call-out numbering as the earlier five-item
list. Either explicitly map the three grouped axes to the original call-outs,
including `#4` and `#5`, or list the status of all five call-outs so every item has
an auditable disposition.
| 2. B asks one or more **peer AR.IO gateways** for the content as a CAR: | ||
| `GET https://{peer}/ipfs/{X}?format=car` with a **local-only** hint (below). | ||
| 3. A peer **A** that *holds* `X` returns the CAR (the full DAG). We already serve | ||
| `?format=car` (`routes/ipfs.ts` → `KuboDataSource` `format: 'car'`). | ||
| 4. B imports the CAR into its own Kubo via the RPC we already use | ||
| (`{IPFS_KUBO_API_URL}/api/v0/dag/import`, same path pattern as `pin/add`). | ||
| **Kubo verifies every block against its CID on import** — so a lying peer's CAR | ||
| fails to import and B moves to the next peer. No bespoke DAG-verify code needed; | ||
| Kubo is the verifier. | ||
| 5. B now holds `X` (verified, in Kubo), serves the user, and — if | ||
| `IPFS_PIN_ARNS_CONTENT` — pins it. The content has replicated one more time. | ||
|
|
||
| For a raw single-block CID this is trivially one block; for a UnixFS/dag-pb DAG the | ||
| CAR carries the whole graph, and Kubo's import verifies the block links. That also | ||
| sidesteps the multi-block trust gap (the reassembled UnixFS bytes don't hash to the | ||
| CID, but the CAR's blocks do). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Bind every imported CAR to the requested CID.
Line 55 describes per-block verification, but the design does not require the CAR root to equal X or prove that all blocks for the requested path are present. A peer can return a valid CAR for another root or a truncated CAR. Require an explicit root match and DAG/path traversal before serving or pinning the result.
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 'dag/import|format=car|CAR|root|travers|pin' src docs docker-compose.yaml package.jsonAlso applies to: 288-292
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-durability-layer.md` around lines 49 - 64, Update the
CAR import flow described in the durability design to bind the imported content
to the requested CID X: require the CAR’s declared root to equal X, then
traverse and validate the requested DAG/path before serving or pinning it.
Reject mismatched or incomplete CARs and continue to the next peer, preserving
pinning only after successful root and traversal validation.
| A peer request must **not** recurse — if B asks A and A doesn't have it, A must NOT | ||
| turn around and hit public IPFS or its own peers (latency + loops + amplification). | ||
| So peer requests carry a header, e.g. `X-Ar-Io-Local-Only: true` (or a dedicated | ||
| peer endpoint), and A serves **only from its local cache / Kubo pin**, returning | ||
| 404 fast if it doesn't hold the content. This bounds the work and prevents fetch | ||
| loops across the fleet. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not treat a cache hit as durable holding.
local-only serves from the “local cache / Kubo pin”, but the observer treats any verifying 200 as proof of holding. A gateway can populate its cache through normal public-IPFS serving, pass the probe, and evict the content later without pinning. Use pin-only evidence or require repeated probes over a defined retention window.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-durability-layer.md` around lines 68 - 73, The
durability verification flow must not treat any successful local-only cache
response as proof of durable holding. Update the observer logic associated with
the local-only peer probe to accept only pin-backed evidence, or require
successful repeated probes throughout a defined retention window before marking
the peer as holding content.
| ``` | ||
| local cache → peer AR.IO gateways (verified CAR import) → local Kubo → public IPFS | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to both fenced code blocks.
markdownlint-cli2 reports MD040 at Line 81 and Line 173. Use text for both diagrams.
Proposed fix
-```
+```text
...
-```
+```textAlso applies to: 173-185
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 81-81: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-durability-layer.md` around lines 81 - 83, Update both
fenced code blocks containing the diagrams in this document to specify the text
language identifier, including the blocks around the local cache flow and the
second diagram, while preserving their contents unchanged.
Source: Linters/SAST tools
| - **`IpfsPeerDataSource`** (new, mirrors `KuboDataSource`'s interface): given a CID + | ||
| a peer list, `GET https://{peer}/ipfs/{CID}?format=car` local-only from a bounded, | ||
| short-deadline subset; on the first 200, `POST {IPFS_KUBO_API_URL}/api/v0/dag/import` | ||
| (same RPC path as `pin/add`) — **Kubo verifies blocks against the CID on import**; | ||
| reject+next-peer on import/verify failure. Returns the now-local content. | ||
| - **IPFS composite source.** Wrap `[localCache, IpfsPeerDataSource, KuboDataSource]` | ||
| in a sequential source (mirroring the Arweave `SequentialDataSource`); `IpfsService` | ||
| consumes the composite instead of `KuboDataSource` directly. | ||
| - **Peer set:** the GAR (already read by the node) → a bounded random/weighted subset; | ||
| DHT-filtered discovery is a follow-up (routing §1). | ||
|
|
||
| **Config (new):** | ||
| - `IPFS_PEER_FETCH_ENABLED` (default false initially, then true once proven). | ||
| - `IPFS_PEER_FETCH_COUNT` (peers to try, e.g. 3), `IPFS_PEER_FETCH_TIMEOUT_MS` | ||
| (short), `IPFS_PEER_FETCH_MAX_CAR_BYTES` (cap; fall back to public IPFS above it). | ||
|
|
||
| **Safety (reuses hardening we already did):** the peer fetch is bounded (deadline + | ||
| size cap), never recurses (local-only), verified (Kubo import), and rate-limited on | ||
| the serve side; a failing/lying peer is skipped, not trusted. Content-blocking/ | ||
| moderation applies to imported content exactly as to Kubo-fetched content. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files matching drafts/dog =="
git ls-files | rg 'docs/drafts/ipfs-peer-durability-layer\.md|docker-compose\.yaml|docs/envs\.md|\b(en|moderator|moderation|block|content|content-blocking|dag/import)\b' -n || true
echo
echo "== target file around snippet =="
sed -n '260,320p' docs/drafts/ipfs-peer-durability-layer.md 2>/dev/null || true
echo
echo "== search moderation/content-blocking definitions =="
rg -n 'moderation|content-blocking|block.*content|ContentBlocking|Moderation|FLAG|KUBO|dag/import|pin/add|serve|import.*moderation|moderation.*import|import' docs docker-compose.yaml -S || true
echo
echo "== search service/interface context =="
rg -n 'SequentialDataSource|KuboDataSource|IpfsService|DagImport|dag/import|pin/add|ContentBlocking|moderation' . -S --glob '!node_modules' --glob '!dist' --glob '!build' -n | head -200 || trueRepository: ar-io/ar-io-node
Length of output: 1921
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git status =="
git status --short
echo
echo "== target file around snippet =="
sed -n '260,320p' docs/drafts/ipfs-peer-durability-layer.md
echo
echo "== moderation/content-blocking references =="
rg -n --hidden '\b(moderation|content-blocking|content blocking|ContentBlocking|moderate|flag|block|import.*local|dag/import|pin/add)\b' docs docker-compose.yaml .github src packages || true
echo
echo "== IPFS service/datasource references =="
rg -n --hidden '\b(IpfsService|IpfsPeerDataSource|KuboDataSource|SequentialDataSource|DagImport|ipfs|IPFS_KUBO)\b' . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200 || trueRepository: ar-io/ar-io-node
Length of output: 1921
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git status =="
git status --short
echo
echo "== target file around snippet =="
sed -n '260,320p' docs/drafts/ipfs-peer-durability-layer.md
echo
echo "== moderation/content-blocking references =="
rg -n --hidden '\b(moderation|content-blocking|content blocking|ContentBlocking|moderate|flag|block|import.*local|dag/import|pin/add)\b' docs docker-compose.yaml .github src packages || true
echo
echo "== IPFS service/datasource references =="
rg -n --hidden '\b(IpfsService|IpfsPeerDataSource|KuboDataSource|SequentialDataSource|DagImport|ipfs|IPFS_KUBO)\b' . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200 || trueRepository: ar-io/ar-io-node
Length of output: 1921
Content-modération-policy (CWE-693)
Reachability: External · Exploitability: Moderate
Apply moderation before importing peer-served CARs into Kubo.
The peer source imports the CAR to Kubo before moderation runs, so blocked content can cross the storage boundary before the documented content-control gate. Add pre-import moderation or quarantine imported blocks until moderation passes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-durability-layer.md` around lines 288 - 307, Update the
IpfsPeerDataSource flow so peer-served CAR content is moderated before being
imported into Kubo, or quarantine the imported blocks until moderation succeeds.
Preserve CID verification, size/deadline limits, and skip peers when moderation
rejects the content; ensure blocked content never crosses into trusted Kubo
storage before the existing content-control gate.
| - **Measuring holding needs no contract and no new phase.** The load-bearing | ||
| primitive in 1.5a — the **local-only serve mode** — *is* the trustless | ||
| holding-measurement. The observer already fetches `?format=raw` and verifies bytes | ||
| against the CID (shipped, PR #112). Adding `X-Ar-Io-Local-Only: true` to that | ||
| existing probe means a 200+verifying response *proves the gateway holds the content | ||
| locally* (a proxy 404s in local-only mode). That is a ~5-line observer change that | ||
| **rides on 1.5a**, not a separate contract phase. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not describe the local-only probe as a trustless proof of local storage.
A valid CID response proves content identity, not storage locality. A gateway can ignore X-Ar-Io-Local-Only, fetch the block before responding, or return valid bytes from another process. A response marker is also self-reported.
Narrow the claim to honest server-side enforcement, or define an independent proof before using this result for holding rewards.
Also applies to: 149-150, 472-479
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-fetch-1.5a-implementation-plan.md` around lines 22 -
28, The implementation plan must stop characterizing the local-only probe as
trustless proof of local storage or as sufficient evidence for holding rewards.
Update the claims around the local-only observer probe and its related
holding-measurement sections to state only that it measures behavior under
honest server-side enforcement, or define an independent proof mechanism before
connecting the result to rewards.
| ## 1. Objective | ||
|
|
||
| Turn the gateway from a read-only proxy to public IPFS into a **verifiable fleet | ||
| serving layer**: when a gateway needs a CID it doesn't hold, it fetches it from a peer | ||
| AR.IO gateway that *does* hold it (as a CAR, over HTTP), and Kubo verifies every block | ||
| against the CID on import. Named IPFS content then stays available as long as **any** | ||
| participating gateway holds it — independent of public-IPFS provider health — and is | ||
| served fast and trustlessly. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Qualify the 1.5a availability guarantee.
IpfsPeerDataSource tries only peerCount selected peers. A holder outside that subset is never queried. Therefore, “as long as any participating gateway holds it” is not guaranteed in phase 1.5a.
Change the claim to “as long as a selected peer holds it,” or make holder discovery a prerequisite for the broader guarantee.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-fetch-1.5a-implementation-plan.md` around lines 44 -
51, Update the Objective section’s availability claim to reflect that phase 1.5a
queries only the selected peerCount peers: state that content remains available
when a selected peer holds it, or explicitly make holder discovery a
prerequisite before retaining the broader “any participating gateway” guarantee.
| ``` | ||
| 1. local Kubo (offline) — do I already hold it? fast, no network | ||
| 2. peer AR.IO gateways — does a fleet peer hold it? bounded, CAR+verify import | ||
| 3. Kubo (public IPFS) — public DHT fallback existing behavior | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to the fenced blocks.
Markdownlint MD040 flags the serving-order and topology fences. Use text for these ASCII diagrams.
Proposed fix
-```
+```text
...
-```
+```textAlso applies to: 403-409
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-fetch-1.5a-implementation-plan.md` around lines 56 -
60, Update the fenced ASCII diagram blocks in the serving-order section and the
referenced lines 403–409 to declare the text language. Add the text identifier
to each affected opening fence while preserving the diagram contents and closing
fences.
Source: Linters/SAST tools
| - Parse the request header: `const localOnly = req.headers['x-ar-io-local-only'] === | ||
| 'true';` (also accept `?local=1` for convenience/testing). | ||
| - Thread it into the call: `ipfsService.getContent({ cidString, path, signal: | ||
| req.signal, range: format ? undefined : rangeForKubo, format, localOnly })`. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Authorization Bypass (CWE-862): Missing Authorization
Reachability: External
Do not make localOnly a payment bypass.
The route accepts X-Ar-Io-Local-Only and local=1 from the caller. If payment is skipped when !localOnly, any external caller can set the flag and avoid the 402 response.
Authenticate peer requests separately, or keep payment enforcement on the public route. Do not treat a client-controlled routing flag as authorization.
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'localOnly|X-Ar-Io-Local-Only|paymentProcessor|402' \
src/routes/ipfs.ts src/ipfs/ipfs-service.tsAlso applies to: 298-307
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-fetch-1.5a-implementation-plan.md` around lines 143 -
146, The request-controlled localOnly flag must not bypass payment enforcement.
Update the IPFS route and related getContent flow around localOnly,
X-Ar-Io-Local-Only, and paymentProcessor so public requests still receive the
existing 402 response; only separately authenticated peer requests may avoid
payment, or retain payment checks regardless of localOnly.
| const ipfsCompositeSource = new SequentialIpfsSource({ | ||
| log, | ||
| sources: config.IPFS_PEER_FETCH_ENABLED | ||
| ? [kuboDataSource /*tier1 offline via localOnly*/, ipfsPeerDataSource, kuboDataSource /*tier3 public*/] | ||
| : [kuboDataSource], | ||
| }); | ||
| ``` | ||
| > Tier 1 and tier 3 are the *same* `KuboDataSource` instance; tier 1 is reached with | ||
| > `localOnly:true` and tier 3 without. Because `SequentialIpfsSource` under | ||
| > `localOnly` runs only `sources[0]`, and under normal mode runs all three, the | ||
| > cleanest encoding is a small wrapper that fixes `localOnly` per tier — e.g. | ||
| > `new LocalOnlyKubo(kuboDataSource)` for tier 1 (forces `localOnly:true`) and the | ||
| > raw `kuboDataSource` for tier 3. Add that 10-line wrapper in | ||
| > `sequential-ipfs-source.ts` or inline. This avoids the composite having to know | ||
| > which index is "the offline one." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make the local-only tier wrapper part of the composite definition.
With peer fetching enabled, the first and third sources are the same kuboDataSource. A normal request has localOnly === false, so the first source can reach public Kubo and succeed before the peer source runs.
Use a wrapper that always forces localOnly: true for tier 1. Keep the raw Kubo source only as tier 3.
Proposed fix
const ipfsCompositeSource = new SequentialIpfsSource({
log,
- sources: config.IPFS_PEER_FETCH_ENABLED
- ? [kuboDataSource /*tier1 offline via localOnly*/, ipfsPeerDataSource, kuboDataSource /*tier3 public*/]
- : [kuboDataSource],
+ sources: config.IPFS_PEER_FETCH_ENABLED
+ ? [
+ new LocalOnlyKubo(kuboDataSource),
+ ipfsPeerDataSource,
+ kuboDataSource,
+ ]
+ : [kuboDataSource],
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/drafts/ipfs-peer-fetch-1.5a-implementation-plan.md` around lines 261 -
275, Update the ipfsCompositeSource definition so the first source wraps
kuboDataSource with a LocalOnlyKubo-style adapter that always forces localOnly:
true, while the third source remains the raw kuboDataSource. Keep the
peer-enabled source order unchanged and ensure the wrapper is defined within or
alongside the composite construction.
Summary
Multi-protocol ArNS + a read-only IPFS gateway for
ar-io-node, nowtargeting
developand consolidating the whole IPFS stack into one PR:/ipfs/{CID}+{CID}.{host}subdomains).CID (
targetProtocol: ipfs) resolve and serve through the gateway.adversarial-review pass, and full docs added on top.
#682is superseded — its commits are included here. Review the diff againstdevelop.What it does
Multi-protocol ArNS resolution
protocol(arweave|ipfs) is a first-class field onNameResolution,derived from the ANT record's
targetProtocol, and propagated across atrusted-gateway hop via the signed
X-ArNS-Protocolheader (the resolverreads it and validates the resolved id per protocol). Previously protocol was
set only by the on-demand resolver and dropped across a gateway hop — so a
stock
gateway,on-demanddeployment silently misrouted IPFS names to Arweave.Read-only IPFS mode (this is a proxy to public IPFS, not Arweave storage)
/ipfs/{CID},{CID}.{host}, ArNS→CID): Kubo reassembles;the gateway caches, moderates, rate-limits, and signs. Marked
X-Ar-Io-Trustless: false— gateway-attested, not a content proof.?format=raw(single verifiable block) and?format=car(verifiable DAG), or the equivalent IPLDAccepttypes, areforwarded to Kubo and relayed for the client to verify against the CID
(
X-Ar-Io-Trustless: true,Content-Disposition: attachment,ETag=CID).IPFS_PIN_ARNS_CONTENT): pin the CIDs ArNS namesresolve to (via the Kubo RPC API) so named content stays retrievable despite
Kubo GC — the substrate an "incentivize serving named IPFS data" layer builds
on. Best-effort, bounded FIFO (
IPFS_PIN_MAX).Arweave-path parity
/ipfs/{CID}redirects to a per-CID sandbox subdomain(all CIDs, not just CIDv0), like Arweave
/{txid}.isIdBlocked(CID)pre-serve, plusisHashBlockedon served bytes; hash-blocked bytes are never persisted, and aCID found hash-blocked is remembered and blocked pre-serve thereafter.
206+Accept-Ranges+416(single-range),negative cache for absent/unpinned CIDs, cache-controlled
404s.X-ArNS-Protocolis a trigger header;Content-Rangeis bound on206.Security / DoS hardening (from an adversarial review)
'close'cleanup instreamToCache).IPFS_KUBO_MAX_CONCURRENT_REQUESTS);httpswarning fora plaintext trusted-gateway URL; multi-range served as full
200.429'd everysuch request; added a mid-stream size guard for format responses.
Config
New env vars (documented in
docs/envs.md, mirrored indocker-compose.yaml):IPFS_KUBO_MAX_CONCURRENT_REQUESTS,IPFS_KUBO_API_URL,IPFS_PIN_ARNS_CONTENT,IPFS_PIN_MAX,IPFS_RATE_LIMIT_UNKNOWN_SIZE_BYTES.Testing / CI
yarn build,yarn lint:check,yarn deps:checkgreen.format, IpfsPinner);
yarn test:cigreen.yarn typecheckis unchanged fromdevelop(pre-existing test-type debt;zero new errors introduced —
developand this branch both report the samecount). Note:
typecheckruns only in the manualtest-coreworkflow, not theautomatic push gate.
ArNS→IPFS (apex + distinct undernames), direct-CID sandbox redirect, HEAD,
Range
206,?format=raw|car(valid CARv1), named-CID pinning, and the publicname.<host>URL.Docs
docs/ipfs-integration.md(Phase 2, Arweave-parity table, read-only mode / trustposture / trustless retrieval / pinning),
docs/envs.md,docker-compose.yaml,CLAUDE.md, and thear-io-gateway-operatorskill are all updated. Two designnotes live in
docs/drafts/: an OIP/observer incentive analysis for named IPFSdata, and an architectural-alignment analysis vs. the trustless / CAR-to-Arweave
direction.
Known limitation / next phase
This is read-only by design — it proxies public IPFS content and does not
store it on Arweave. Uploading/permapinning IPFS content to Arweave (CAR ingest,
content-addressed indexing, chain-anchored proofs, libp2p) is a deliberate
phase 2, out of scope here. The trustless read path and named-content pinning
are the storage-independent parts adopted now.
Design & analysis (shareable)
Two rendered briefs accompany this work (also in
docs/drafts/as Markdown):https://claude.ai/code/artifact/e7765f60-0716-477d-8c39-2c7893b48776
https://claude.ai/code/artifact/db30792a-5446-4dea-b5a7-4782883546e9
They lay out how named IPFS becomes a first-class, trustlessly-verified dimension
with zero smart-contract changes — the observer-only plan (protocol-aware
assessment, capability ramp via
/ar-io/info, trustless CID verification) thatbuilds on this PR's
?format=raw|carendpoint. Companion Markdown drafts:docs/drafts/{ipfs-observation-incentive-analysis,observer-ipfs-adjustments-spec,davids-brain-alignment}.md.🤖 Generated with Claude Code