Skip to content

fix(sandbox): prevent state patch loss during block processing#15534

Merged
shreyan-gupta merged 1 commit into
masterfrom
shreyan-gupta/sandbox/fix-patch-state-loss
Apr 16, 2026
Merged

fix(sandbox): prevent state patch loss during block processing#15534
shreyan-gupta merged 1 commit into
masterfrom
shreyan-gupta/sandbox/fix-patch-state-loss

Conversation

@shreyan-gupta

@shreyan-gupta shreyan-gupta commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Background

sandbox_patch_state sometimes permanently loses patches — the patched account never appears, even on retry. This was reported in the Zulip thread and reproduced with near-sandbox-rs#51.

Three interrelated bugs:

  • Patch lost on block failure: pending_state_patch.take() drains the queue at the start of block processing. If preprocessing fails (orphan, missing chunks, optimistic deferral), the patch is gone forever.
  • RPC returns too early: patch_state_in_progress() checks is_empty(), which is true right after take() — before the patch is actually committed to state.
  • Startup race: Block 1 after genesis has no new chunks. The runtime's apply() early-returns for old chunks and silently drops the state_patch.

What changed

Introduces SandboxPatchTracker — a type that encapsulates the pending patch, a generation counter, and a committed-generation counter. Like SandboxStatePatch, it uses the ZST pattern: real struct on sandbox builds, zero-sized no-op on non-sandbox builds. This replaces the three separate fields on Chain (pending_state_patch, sandbox_patch_generation, sandbox_patch_committed_gen) with a single sandbox_patches: SandboxPatchTracker.

The key design change is taking the patch late — at point of use inside the per-shard loop of apply_chunks_preprocessing, instead of at the top of start_process_block_impl. This eliminates the need for backup/clone/restore on error paths, since any error before the take leaves the patch in the tracker. The generation counter (in_progress() checks generation != committed_gen, not is_empty()) ensures the RPC doesn't return prematurely despite the late take.

This removes the state_patch parameter from preprocess_block and apply_chunks_preprocessing, and results in zero #[cfg(feature = "sandbox")] blocks in chain.rs.

Related

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a sandbox-only race where sandbox_patch_state patches could be dropped when normal block processing is deferred due to an optimistic block already executing.

Changes:

  • Stops draining pending_state_patch at the start of start_process_block_impl.
  • Removes state_patch threading through preprocess_blockapply_chunks_preprocessing.
  • Drains pending_state_patch inside apply_chunks_preprocessing, after the should_be_pending_execution deferral check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread chain/chain/src/chain.rs Outdated
Comment thread chain/chain/src/chain.rs Outdated
@codecov

codecov Bot commented Apr 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.30%. Comparing base (0f0cd2a) to head (a54bb33).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
core/primitives/src/sandbox.rs 58.33% 5 Missing ⚠️
chain/chain/src/chain.rs 84.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #15534      +/-   ##
==========================================
- Coverage   69.30%   69.30%   -0.01%     
==========================================
  Files         940      940              
  Lines      212116   212130      +14     
  Branches   212116   212130      +14     
==========================================
- Hits       147016   147014       -2     
- Misses      59275    59279       +4     
- Partials     5825     5837      +12     
Flag Coverage Δ
pytests-nightly 1.14% <0.00%> (-0.01%) ⬇️
unittests 68.67% <60.00%> (+<0.01%) ⬆️
unittests-nightly 68.85% <68.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shreyan-gupta shreyan-gupta force-pushed the shreyan-gupta/sandbox/fix-patch-state-loss branch from 5af3290 to a54bb33 Compare April 16, 2026 03:51
@shreyan-gupta shreyan-gupta changed the title fix(sandbox): fix patch_state loss during optimistic block deferral fix(sandbox): prevent state patch loss during block processing Apr 16, 2026
@shreyan-gupta shreyan-gupta added this pull request to the merge queue Apr 16, 2026
Merged via the queue into master with commit a884600 Apr 16, 2026
30 of 33 checks passed
@shreyan-gupta shreyan-gupta deleted the shreyan-gupta/sandbox/fix-patch-state-loss branch April 16, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants