Skip to content

feat(state): retain resources through bounded block range reads - #942

Open
czarcas7ic wants to merge 1 commit into
mainfrom
adam/getblocks-owned-storage
Open

feat(state): retain resources through bounded block range reads#942
czarcas7ic wants to merge 1 commit into
mainfrom
adam/getblocks-owned-storage

Conversation

@czarcas7ic

@czarcas7ic czarcas7ic commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Motivation

Cancelling an async block-range read must not release its resources while the database job or its undelivered result still retains blocks. This is the first refactor chunk extracted from #892, now targeting main. The former #941 transport backport was withdrawn in favor of the separate Iroh upgrade in #935.

Solution

Add a bounded state read that carries caller-owned resources into the blocking job and its result. It returns a contiguous prefix within the byte limit and checks cancellation between lookups. Existing callers and protocol behavior remain unchanged.

Testing

Formatting, patch and changelog checks pass. All 11 focused storage tests pass. Regressions cover cancellation during a running read, undelivered results, byte boundaries, and resource release. Prepared with Codex assistance.

Changelog

Added an internal API fragment for this PR.

@v12-auditor

v12-auditor Bot commented Sep 9, 2026

Copy link
Copy Markdown

Note

Complete: Audit complete. V12 found one issue worth reviewing.

Open the full results here.

FindingSeverityDetails
F-271277 🔵 Low
Queued snapshot can splice competing forks

read_owned_block_range captures the best non-finalized chain before submitting its blocking job, so blocking-pool queue delay can leave the job holding an obsolete Arc<Chain>. Clone-on-write finalization and chain switching preserve that old chain while the shared finalized database continues advancing on the later best fork. Each height lookup prefers the captured chain and independently falls back to the live database, without verifying parent linkage between returned blocks. If fork A is retained through height H while fork B becomes finalized through H+1, a crossing range returns A(H) followed by B(H+1), even though B(H+1) names B(H), not A(H), as its parent. The existing BlocksByHeightRange path captures its chain inside the blocking handler, so it does not add blocking-queue delay to this stale-snapshot window.

Analyzed four files, diff cb0730c...32df00a.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-10T05:13:39.892996Z 751346e New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: fda7c1484e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@czarcas7ic
czarcas7ic marked this pull request as ready for review September 9, 2026 22:28
@czarcas7ic
czarcas7ic requested a review from a team September 9, 2026 22:28
@czarcas7ic
czarcas7ic added this pull request to stack #954 September 10, 2026 02:25
@czarcas7ic
czarcas7ic force-pushed the adam/getblocks-owned-storage branch from 44f1c24 to e5e87c2 Compare September 10, 2026 02:26
@czarcas7ic
czarcas7ic force-pushed the adam/getblocks-owned-storage branch from e5e87c2 to 751346e Compare September 10, 2026 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants