fix(sandbox): remove state patch clear that races with new patches#14893
Merged
Conversation
The unconditional clear() in postprocess_ready_block() could wipe sandbox state patches that were submitted during block processing, causing sandbox_patch_state RPC to return success while the patch was silently dropped. Patches are already consumed via take() in start_process_block_impl, so this clear() is unnecessary and harmful.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a race condition in sandbox state patching where patches submitted during block processing were silently dropped. The clear() call in postprocess_ready_block() was unconditionally removing pending state patches, including newly submitted ones that had not yet been consumed.
Changes:
- Removed the unnecessary
self.pending_state_patch.clear()call that was causing patches to be lost - Patches are already properly consumed via
take()instart_process_block_impl, making theclear()both redundant and harmful
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #14893 +/- ##
===========================================
+ Coverage 1.30% 68.62% +67.31%
===========================================
Files 726 920 +194
Lines 140324 200084 +59760
Branches 140324 200084 +59760
===========================================
+ Hits 1829 137301 +135472
+ Misses 138442 56842 -81600
- Partials 53 5941 +5888
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
r-near
added a commit
to near/near-sandbox-rs
that referenced
this pull request
Jan 16, 2026
Adds a test that verifies sandbox_patch_state RPC returns only after the patched state is queryable. This test will fail on nearcore versions prior to the fix in near/nearcore#14893
This was referenced Jan 16, 2026
darioush
approved these changes
Jan 16, 2026
Contributor
There was a problem hiding this comment.
LGTM, the clear call seems to be a leftover from when pending_state_patch used to be an Option<Vec<_>> and now it's just Vec<_>, so it is not needed.
The race was seemingly introduced when apply chunks got refactored to async.
The clear method is not needed any other place so I removed it.
pugachAG
pushed a commit
that referenced
this pull request
Feb 19, 2026
r-near
added a commit
to near/near-sandbox-rs
that referenced
this pull request
Apr 1, 2026
Adds a test that verifies sandbox_patch_state RPC returns only after the patched state is queryable. This test will fail on nearcore versions prior to the fix in near/nearcore#14893
r-near
added a commit
to near/near-sandbox-rs
that referenced
this pull request
Apr 1, 2026
Adds a test that verifies sandbox_patch_state RPC returns only after the patched state is queryable. This test will fail on nearcore versions prior to the fix in near/nearcore#14893
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Apr 16, 2026
## Background `sandbox_patch_state` sometimes permanently loses patches — the patched account never appears, even on retry. This was reported in the [Zulip thread](https://near.zulipchat.com/#narrow/channel/308695-nearone.2Fprivate/topic/Sandbox.20patch_state.20issues) and reproduced with [near-sandbox-rs#51](near/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 - #15536 — alternative fix by @r-near that addresses the same three bugs with a backup/restore approach and 22 cfg-gates. This PR takes a different approach (take-late + tracker) to achieve the same result with less code deviation. - #14893 — fixed a different patch loss path (`clear()` in `postprocess_ready_block`)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes an unconditional
clear()call that causedsandbox_patch_stateRPC to silently drop patches.Problem
When a new sandbox state patch was submitted while a block was being processed, the patch would be cleared by
postprocess_ready_block()before any block could consume it. The RPC would return success, but the patch was never applied.Fix
Remove the
clear()call. Patches are already consumed viatake()instart_process_block_impl, making thisclear()both unnecessary and harmful.