Approve. Memory savings verified — binary/hex streaming drops peak from ~48 MB (full 1M-node deserialise) to ~32 KB (bufio buffer), and GetSubtreeData drops from ~250 MB to a 5–7 MB constant. Streaming impl is clean (pull-mode io.ReadCloser for binary/hex, io.Pipe + io.MultiWriter for GetSubtreeData; back-pressure correct, all goroutine spawn bounded). The pagination fix in b08a88e3 (only read conflicting nodes when pageCoversFullSubtree==true) addresses the original 1M-node-scan concern correctly. DoS posture is net-positive — four concurrency keys flip from unlimited (0) to bounded 2/2/4/2, with a 30 s fallback timeout on semaphore acquisition.

A few things worth confirming or following up on before merge:

1. FeeHash regression in /subtree/:hash/json? The new readSubtreePageFromReader returns a Subtree without populating FeeHash, but the handler still serialises &subtree.FeeHash. JSON consumers will see zero-value FeeHash. If the old code also returned zero here, no regression; if not, this needs restoring. Could you confirm?

2. Offset-wrap inconsistency between the two readers. readSubtreePageFromReader:158 silently wraps an out-of-range offset back to 0 (encoded by TestGetSubtreePage_ResetsOffsetPastEnd), while the companion readSubtreeNodesPageFromReader:101 returns an empty page for the same case. Pick one and document in OpenAPI — wrapping to 0 will surprise a paginated client.

3. settings.conf doc mismatch (ops hazard). The conf documents these keys as commented-out at = 0 (unlimited), but the code defaults are now ConcurrencyGetSubtreeData=2, ConcurrencyGetSubtreeTransactions=2, ConcurrencyGetSubtreeDataReader=4, SubtreeDataStreamingConcurrency=2 (was 4). An operator reading settings.conf will believe the defaults are unlimited. Worth updating the comments + a release-note line about the silent flip for deployments that didn't explicitly set these.

4. Benchmark gap. Asset streaming benchmarks are gated behind RUN_SUBTREE_BENCHMARK=true and t.Skip() in CI — so the memory savings claim isn't machine-verified by anything that runs on PR. The math checks out (manually traced) but a regression in 6 months would be silent. Recommend adding at least one un-gated testing.B for the streaming path with an AllocsPerOp check.

Two CodeQL findings (subtree_stream.go:118 and :178) are false positives — pageSize is provably bounded to ≤ 100 via two redundant clamps (helpers.go:107-109 HTTP layer + subtree_stream.go:86-88, 142-144 repository defense-in-depth), max allocation ~4.8 KB per request. Safe to dismiss in the Security tab with that rationale rather than adding dead defensive code.

One operational note worth surfacing: the gzip-skip middleware (correct decision — binary hash/tx payloads have near-zero compressibility and gzip would re-introduce full buffering) creates an uncompressed bandwidth amplification surface on these three endpoints, all unauthenticated GETs. Defense relies on (a) ban-list middleware, (b) the new per-method semaphores, (c) upstream reverse-proxy rate limiting. Worth documenting the reverse-proxy ACL/rate-limit requirement explicitly in deployment notes; per-IP rate limiting in the asset service itself would be a defense-in-depth follow-up.

Side note (cosmetic, not blocking): scheduleSubtreeChunkFetches at GetLegacyBlock.go:339-365 masks the real worker error with context.Canceled on return when a worker fails. The real error still reaches the caller via the resultsChan path (consumer loop at :295-300), so no behaviour bug — but confusing to read in isolation. Prefer waitErr over readErr when both are set.

Also worth refuting the claude[bot] inline finding on http.go:435: the parts[i+1:] slice access cannot panic — Go allows slicing to len, and the len(tail)==1/2 guards prevent tail[1] OOB. Safe as written.

Recommend addressing:

1. Coverage gap (SonarQube quality gate).
2. Run make test per repo guardrails.
3. Document the ConflictingNodes paginated-response change in the PR / changelog.
4. Trivial: clean up unreachable return nil in GetSubtree.go:190.