refactor!: reimplement partial notes on FactStore#24369
Merged
mverzilli merged 221 commits intoJul 6, 2026
Conversation
Adds empty(), map, filter, any, all, find and read_as to EphemeralArray (spun off from #23928) and mirrors them on TransientArray.
Both types are now aliases of a single OracleArray<T, Oracle> generic, parameterized by an ArrayOracle backend trait whose impls wrap the existing #[oracle] declarations. Foreign-call names and the TS-side services are untouched; monomorphization emits the same calls as before. empty_at stays ephemeral-only via a specialized impl. Test suites are byte-for-byte unchanged. Expansion snapshots regenerated: nargo expand now prints the resolved OracleArray paths.
Each behavior check is written once in oracle_array/test_helpers.nr, generic over the ArrayOracle backend; the ephemeral and transient test modules keep one named #[test] wrapper per behavior so counts, names and CI filters are unchanged. Backend-specific tests (empty_at, store/load/delete) stay with their backend.
Moves empty_at from the ephemeral-only impl into the shared OracleArray impl, making it available on TransientArray as well. Its doc notes that for cross-frame backends it wipes data other frames may have written. The empty_at test moves to the shared suite and runs on both backends.
Generic store/load/delete cores live in oracle_array; ephemeral and transient expose them via thin module-level wrappers, so both backends now have identical public surfaces (alias + oracle marker + store/load/delete). The store/load test bodies move to the shared suite; each backend keeps a store_and_load smoke test through its own wrappers. Also fixes the stale test_helpers doc that still described empty_at and store/load as backend-specific.
Backend test modules no longer list one wrapper per shared check. Annotating a test module with ephemeral_oracle_array_tests / transient_oracle_array_tests reflects over test_helpers and emits one #[test] per unconstrained check with that backend's oracle swapped in, so new checks automatically run against every backend and the suites cannot drift. should_fail_with messages come from a single manifest in test_helpers; an unregistered failing check fails loudly rather than silently. Generated test names match the previous hand-written ones.
Replaces the two per-backend attribute functions with one oracle_array_tests(module, oracle) attribute that takes the backend's oracle as a quoted path at the annotation site.
…d-transient-array-apis
The module name collided with crate::ephemeral, which owns the EphemeralArray type. Name the oracle module after what it holds -- the #[oracle(...)] declarations -- so the array module keeps the plain ephemeral name and the two are unambiguous at use sites.
The module name collided with crate::transient, which owns the TransientArray type. Name the oracle module after what it holds -- the #[oracle(...)] declarations -- so the array module keeps the plain transient name and the two are unambiguous at use sites.
OracleArray and ArrayOracle were near-anagrams, forcing readers to disambiguate which was the struct and which the backend trait. Keep ArrayOracle (it names the set of oracles that implement an array) and rename the struct to UnconstrainedArray, after the property every backend shares: its operations are unconstrained foreign calls into PXE-side storage, so the data is host-provided and must be verified before being trusted in constrained code. The oracle_array module, the ArrayOracle trait, and the Oracle type parameter are unchanged.
Follows the struct rename: the module and its test macro were still named after the old OracleArray struct. Rename the module directory to unconstrained_array and the oracle_array_tests macro to unconstrained_array_tests so the module mirrors the type it houses. The ArrayOracle trait and the store/load/delete helpers keep their names.
…y rename UnconstrainedArray is longer than the old OracleArray, so several doc lines that referenced the type (or its method links) tipped past the 120-column limit. nargo fmt breaks an over-long comment line by pushing only the trailing word onto a new line, which left mid-paragraph orphans like '/// not' and '/// the'. Rejoin those words and rewrap the affected paragraphs at natural points, keeping every line within 120 columns.
…ntOracles Each of these names a set of oracle operations -- the full backend an array needs (ArrayOracles) and the concrete ephemeral/transient bundles that implement it -- so the plural reads truer and matches the ephemeral_oracles/transient_oracles module names. The singular Oracle remains as the type parameter: one backend slot filled by one bundle.
The explanatory paragraph on the ArrayOracles trait was dropped during the previous pluralize commit (a formatter re-stage race on the shared working tree); the rename itself only swapped the identifier. Restore the paragraph verbatim. nargo fmt run directly keeps it intact.
to EphemeralArray
…d-transient-array-apis
…suleArray for_each iterated in reverse so the callback could remove the current element without shifting unvisited ones, but nothing in production uses that capability and backward iteration is surprising both relative to general intuition and to the sibling methods (map/filter/find), which all read elements in order. Iterate forward instead, document that structural mutation from inside the callback is unsupported, and drop the tests that existed solely to exercise remove-during-iteration. The order test now asserts the exact forward visit order. Linear: F-729
The manual index loop in get_pending_partial_notes_completion_logs existed only because for_each had arbitrary iteration order. Now that for_each visits elements in order, the loop can use it directly while preserving the index alignment between the request array and the partial note array. Linear: F-729
Protocol contract bytecode must not change for oracle additions they do not use; only the major version is required to be in sync across the three copies.
nchamo
reviewed
Jul 6, 2026
nchamo
left a comment
nchamo
left a comment
Contributor
There was a problem hiding this comment.
Great work, just a few comments
FYI: Claude says that do_sync_state_does_not_panic_on_empty_logs test is still referencing the old oracle
Keeps the single step() public API but moves each state's logic into internal step_pending/step_completed functions, per review feedback.
…gistry Moves all V1 handling out of the modern registry, type mappings, utility handler, log service and TXE translator into a single legacy_oracle_registry adapter over getPendingTaggedLogsV2, mirroring the getLogsByTag adapter. Also restores the RESOLVED_TX wire-shape comment. The modern registry shrank, so ORACLE_INTERFACE_HASH changes with no version bump (no wire changed for any consumer).
The mock still targeted the retired V1 oracle name, so it never fired and the crafted empty-payload log was never consumed - the test passed vacuously against whatever the real TXE oracle returned. Also assert the mock's call count so a stale name fails loudly next time.
nchamo
approved these changes
Jul 6, 2026
…_type_mappings.ts Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>
nventuro
reviewed
Jul 6, 2026
Comment on lines
+29
to
+36
| //! - **Pending**: the private part has been received but the completion log has not been found yet. While in this | ||
| //! state, message discovery keeps searching for that log on each sync. | ||
| //! - **Completed**: the completion log was found, so the note was completed and enqueued for storage. Because the | ||
| //! block containing the completion log can be dropped by a reorg, this state is not terminal until the block | ||
| //! finalizes: a reorg dropping it rewinds the FSM back to `Pending` state. | ||
| //! - **Terminated**: nothing else can be done with the partial note, either because its delivery block was re-orged | ||
| //! away (so the delivery itself was reverted) or because its completion block has finalized (so the discovered note | ||
| //! is permanent and reorg-proof). This is the terminal state of this FSM. |
Contributor
There was a problem hiding this comment.
It continues to feel a bit odd to describe these in terms of states when the states have barely any real representation in code, and we deal with events instead.
| //! - **Pending -> Completed** ([`complete_with_log`](PartialNoteFsm::complete_with_log)): the completion log is | ||
| //! found. Its public content is combined with the delivered private content, the resulting note(s) are discovered | ||
| //! and enqueued for storage. | ||
| //! - **Completed -> Pending**: the completion log block is re-orged out. The completion log is searched for again. |
Contributor
There was a problem hiding this comment.
I am not a fan of these descriptions, we don't really explicitly transition through states but rather the existence of facts causes this. Completed -> pending is the ultimate example of this, where nowhere in the fsm do we observe this transition: it is a result of how PXE deals with reorged facts and we derive state from them.
| let completion_logs = maybe_completion_logs.unwrap(); | ||
| let num_logs = completion_logs.len(); | ||
| if num_logs != 0 { | ||
| assert(num_logs == 1, f"Expected at most 1 completion log per partial note, got {num_logs}"); |
Contributor
There was a problem hiding this comment.
It wouldn't hurt to write here that a critical invariant of partial notes is that they ensure they'll only be completed once (e.g. by emitting a nullifier).
| //! partial note's process unfolds, including how reorgs cause it to rewind. | ||
| //! | ||
| //! ```text | ||
| //! init() (partial note delivery message, including tx and block data) |
Contributor
There was a problem hiding this comment.
In the near future there'll be two kinds of partial notes: those in which there exists on-chain state, which can only be completed once but last indefinitely long (the current kind, sometimes called 'setup partial notes'), and those with no on-chain state which can be completed many times and which expire after a while ('no-setup patial notes'). We may want to more explictly call this kind by its future name, or at least mention the set of properties the current variant has.
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
3 participants
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.
Closes F-771