Add PS mutation#131
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughPrivate-state mutation is now asynchronous across simulator backends. Simulators support partial and functional updates, live contexts can persist state through providers, unsupported mutation is explicitly reported, and integration/unit tests cover replacement, updates, read-after-write behavior, and subsequent circuit calls. ChangesPrivate State Mutation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Simulator
participant LiveBackend
participant LiveContext
participant PrivateStateProvider
Simulator->>LiveBackend: setPrivateState(state)
LiveBackend->>LiveContext: setPrivateState(state)
LiveContext->>PrivateStateProvider: persist state by privateStateId
PrivateStateProvider-->>LiveContext: write complete
LiveContext-->>Simulator: Promise<void>
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/simulator/src/factory/createSimulator.ts`:
- Around line 269-277: Serialize the complete read-modify-write operation in
updatePrivateState so concurrent calls on the same simulator cannot overwrite
one another. Add or reuse a per-simulator promise queue/mutex around
getPrivateState, updater execution, and setPrivateState, preserving both
functional and partial updater behavior; alternatively use provider-side
compare-and-set/versioning if that mechanism already exists.
In `@packages/simulator/src/live/LiveContext.ts`:
- Around line 76-78: Fix the TSDoc link in the LiveContext documentation by
referencing the exported PRIVATE_STATE_MUTATION_UNSUPPORTED symbol directly, or
render the symbol as code text instead of linking it through LiveBackend.
Preserve the existing mutation behavior description.
In `@packages/simulator/test/integration/Witness.test.ts`:
- Around line 205-210: Update the replacement-state and patch-state test
fixtures in Witness.test.ts to assign Buffer values to
WitnessPrivateState.secretBytes, replacing the Uint8Array constructions with
Buffer.alloc(...) while preserving the existing length and fill value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d82438d6-97d6-4c7f-88d6-a2f2be75c026
📒 Files selected for processing (13)
packages/simulator/src/backend/Backend.tspackages/simulator/src/backend/DryBackend.tspackages/simulator/src/factory/createSimulator.tspackages/simulator/src/index.tspackages/simulator/src/live/LiveBackend.tspackages/simulator/src/live/LiveContext.tspackages/simulator/src/live/createLiveContext.tspackages/simulator/test/integration/LiveMutation.test.tspackages/simulator/test/integration/SampleZOwnableSimulator.tspackages/simulator/test/integration/Witness.test.tspackages/simulator/test/integration/WitnessSimulator.tspackages/simulator/test/unit/LiveBackendAdapter.test.tspackages/simulator/test/unit/createLiveContext.test.ts
Fixes #130
This PR proposes to enable private state mutations on the live backend. The breaking change is that
setPrivateStateneeds to beawaited =>await this.setPrivateState(...)Note that this does not support
overrideWitnesswhich is a bigger refactor (if wanted). Tests that useoverrideWitness(e.g. ShieldedAccessControl in the compact contracts lib) cannot run on the live backend and must be skippedThe proposed changes were tested with ShieldedAccessControl and MultiToken (all tests passed for the latter, only
overrideWitnesstests failed for the former (they need to be skipped for live runs))Types of changes
What types of changes does your code introduce to OpenZeppelin Midnight Contracts?
Put an
xin the boxes that applySummary by CodeRabbit
New Features
Bug Fixes
Tests