feat(core): report scenario override outcomes - #6
Open
92Infinitus92 wants to merge 5 commits into
Open
Conversation
Comment on lines
+2690
to
+2695
| if let Err(restore_error) = | ||
| self.scheduled_overrides.store(target_slot, scheduled_batch) | ||
| { | ||
| return Err(SurfpoolError::internal(format!( | ||
| "Failed to commit staged account overrides: {commit_error}; failed to restore their schedule: {restore_error}" | ||
| ))); |
There was a problem hiding this comment.
Restore failure loses override
When the staged account commit fails and the schedule backend also rejects the subsequent restore, the override has already been removed at line 2682, causing the account changes to be discarded while the scheduled override is permanently lost and unavailable on retry.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/core/src/surfnet/svm.rs
Line: 2690-2695
Comment:
**Restore failure loses override**
When the staged account commit fails and the schedule backend also rejects the subsequent restore, the override has already been removed at line 2682, causing the account changes to be discarded while the scheduled override is permanently lost and unavailable on retry.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
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.
Greptile Summary
The PR adds typed applied/skipped outcomes for scenario overrides, outcome-aware time travel, atomic schedule batching, and Node SDK bindings.
Confidence Score: 3/5
The PR is not yet safe to merge because failed materialization can advance the clock and a double failure during commit recovery can permanently remove a scheduled override.
Time travel confirms and advances one slot before fallible materialization, while materialization recovery removes the schedule before account commit and cannot preserve it if the separate restoration write also fails.
Files Needing Attention: crates/core/src/surfnet/svm.rs
Important Files Changed
Sequence Diagram
sequenceDiagram participant RPC participant SVM participant Schedule participant Accounts RPC->>SVM: materialize_overrides_for_slot(slot) SVM->>SVM: Apply overrides to staged clone SVM->>Schedule: take(slot) SVM->>Accounts: apply_batch(staged delta) Accounts-->>SVM: commit error SVM->>Schedule: store(slot, original batch) alt restoration succeeds Schedule-->>SVM: restored SVM-->>RPC: commit error, retryable else restoration fails Schedule-->>SVM: restore error SVM-->>RPC: combined error, override lost endPrompt To Fix All With AI
Reviews (5): Last reviewed commit: "fix(core): cancel scheduled overrides at..." | Re-trigger Greptile