feat: merge-train/fairies-v5#23881
Merged
Merged
Conversation
AztecBot
requested review from
IlyasRidhuan,
LeilaWang,
MirandaWood,
jeanmon,
nventuro and
sirasistant
as code owners
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
Refactors mainly NoteStore so that discovered nullifiers are modeled as append-only rows instead of mutating stored notes. Note the store isn't 100% append-only: adding scopes to a note still causes a modification. I deemed this less cumbersome than introducing more indexes just to track scopes. Includes some minor touches to PrivateEventStore mostly to align style, since it was already mostly in the shape we need it to. Context: this is preliminary work for the "time-travelling pxe db" project. We need reorg semantics to be consistent between stores, dealing with mutation on reorgs makes the whole model harder to reason about. You'll see a longer than expected commit history: this is because I originally set out to change the way we handle reorgs much more fundamentally, by making the stores tolerate multiple versions of notes and events as long as they are recorded as having been originated from different blocks (which could happen in reorg scenarios). Implementing said approach made me realize we don't win much from the additional complexity: in order to filter out "orphaned" data and eventually being able to reap it, we would need to keep track of height->hash mappings from genesis. At that point, the delete-on-chain-prune original approach seemed more sensible, which ostensibly reduced the scope of this change to what I describe above.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
…ivery (#23875) Pure module move to make reviewing #23865 easier. In #23865 we update the MessageDeliver builder API. Once this merges, #23865 gets rebased on top so its diff shows only the API delta. - Renames `aztec/src/messages/message_delivery.nr` to `aztec/src/messages/delivery/mod.nr` - Updates every import and doc-link site - No behavior change and no API change; - New `## TBD` migration note for the module move, and the existing TBD `set_sender_for_tags` entry updated to the new path. - Regenerated the two contract snapshots affected by the move (`invalid_event` stderr, `token_contract` expanded). Top of stack. Next is #23865 --------- Co-authored-by: AztecBot <tech@aztec-labs.com>
## The gap This draft exists just so display the first half of a red-green flow. We should merge the child PR #23896 into this one before merging. The contract-snapshots suite (added in #23061) has never run in CI. It was wired only into the `noir-projects/bootstrap.sh test_cmds` aggregate, but the Makefile-driven CI never invokes parent aggregates: it calls each subproject's `test_cmds` directly (`noir-protocol-circuits-tests` in `fast`, `noir-projects-txe-tests` post-build). The aggregate is a local-dev entry point only. Consequences so far: - the noir 1.0.0-beta.22 bump (#23870) landed green with stale snapshots (see #23878), - aztec-nr changes since the last regen in #23464 have drifted the snapshots unnoticed. 14 of 61 tests fail on this branch with its own pinned nargo. ## The fix - New `contract-snapshots-tests` Makefile target (mirrors `noir-protocol-circuits-tests`, depends only on `noir` for the nargo binary), included in `fast`. - Comment on the bootstrap aggregate marking it local-dev only, so the next suite added there also gets a Makefile target. ## Expected CI result This PR is intentionally red: the newly wired suite fails with the 14 drifted snapshots, demonstrating the gap. A follow-up PR targeting this branch regenerates the snapshots and turns it green. Merge order: regen PR into this branch, then this branch into the train. Draft with `ci-draft` label so CI runs while merge automation stays off until the regen lands. --------- Co-authored-by: AztecBot <tech@aztec-labs.com>
…te polyfill resolution (#23908) ## Why The `docs` job on the `merge-train/fairies-v5` → `v5-next` merge queue fails in `validate-webapp-tutorial` (`docs/examples/bootstrap.sh`). That step rewrites the tutorial's `package.json` to `link:` the in-repo `@aztec/*` packages, wipes `yarn.lock`, does a fresh `yarn install`, then `tsc -b --noEmit` + `vite build` against current HEAD. Nothing is pinned, so the tutorial has silently drifted from the live `@aztec/wallets` / PXE API and now fails to build. (Failing run: [27014367195](https://github.com/AztecProtocol/aztec-packages/actions/runs/27014367195/job/79725536620).) Two distinct breakages, both fixed here: ### 1. Fatal: vite/rollup can't resolve the node-polyfill shim ``` [vite]: Rollup failed to resolve import "vite-plugin-node-polyfills/shims/buffer" from "yarn-project/wallets/dest/embedded/wallet_db.js" ``` `vite-plugin-node-polyfills` injects `…/shims/buffer` imports into modules that use the `Buffer` global. Because `@aztec/*` are `link:`ed into the monorepo, that specifier resolves from the linked package dir (where the plugin isn't installed) and fails. Fixed by adding the same `nodePolyfillsFix` `resolveId` wrapper already used in `playground/vite.config.ts` (davidmyersdev/vite-plugin-node-polyfills#81), redirecting the shim specifiers to this project's installed copy. Verified `shims/{buffer}/dist/index.cjs` exists in the pinned `vite-plugin-node-polyfills@^0.22.0`. ### 2. Type drift vs the current wallets/PXE API - `EmbeddedWallet.completeFeeOptions` used the old positional signature `(from, feePayer?, gasSettings?)`; the base `BaseWallet.completeFeeOptions` now takes a single `CompleteFeeOptionsConfig`. The override is rewritten to the new signature and now delegates to `super.completeFeeOptions(config)` (which handles gas settings, fee padding, congestion and account options), only swapping in `SponsoredFeePaymentMethod` when no `feePayer` was provided. This also drops the removed `GasSettings.default(...)` call. - This signature mismatch was the root of the cascading `TS2684` / "`connectTestAccount`/`getConnectedAccount` do not exist on `NodeEmbeddedWallet`" errors in `TwoPlayerLocal.tsx` / `WalletConnect.tsx`: it made the subclass structurally incompatible with the base constructor, so `EmbeddedWallet.create()` could no longer infer the subclass type. Fixing the override resolves those without touching the components. - `config.ts` set `config.l1Contracts = await aztecNode.getL1ContractAddresses()`, but `PXEConfig` no longer has `l1Contracts` and `createPXE` already fetches L1 addresses from the node. Removed the line. ## Note on verification I couldn't run the full linked-package docs build locally (it needs the whole `yarn-project` toolchain built). Changes were validated against the current API source for each symbol used; the `docs` job in the merge queue is the gate. The tutorial is also identically stale on `next`/`v5-next`, so the same fix is worth forward-porting there. --- *Created by [claudebox](https://claudebox.work/v2/sessions/d960d0a984103bb0) · group: `slackbot`* --------- Co-authored-by: thunkar <gregojquiros@gmail.com>
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Implements a new storage mechanism, `TransientArray`. It is similar to `EphemeralArray`, but its lifetime is a top level execution instead of a call frame. It also adds store/load/delete convenience functions providing similar single KV store semantics as their equally named capsule counterparts Closes F-677
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
…hot (#23935) ## Why `merge-train/fairies-v5` (#23881) keeps getting kicked out of the merge queue by `expand::test_avm_test_contract` in `noir-projects/contract-snapshots`: - http://ci.aztec-labs.com/0a451e58e2f5f2e9 (08:15 UTC) - http://ci.aztec-labs.com/2d2f9eb018798a47 (13:29 UTC) This is **not** a stale snapshot. The committed `.snap` is correct, and the diff is **ordering-only** — the same generated interface functions (`call_fee_juice`, `nested_call_large_calldata`, `new_nullifier`) shuffle position with identical bodies/selectors. Root cause: `nargo expand` emits the macro-generated contract-interface functions in **hashmap-iteration order, which is not stable across nargo builds/runs**. Evidence: - On the branch tip with the pinned `nargo 1.0.0-beta.22`, the suite passes **40/40** with the committed ordering, including in the merged-with-`v5-next` context the merge queue tests. - The failing merge-queue commit had the **same committed snapshot blob and the same noir submodule pin** as the passing tip, yet the queue runner generated the opposite ordering and failed. So the test fails intermittently in the merge queue purely on order-only diffs, blocking the train. ## What Skip only `expand::test_avm_test_contract` in the contract-snapshots `test` target (`cargo test -- --skip ...`). All other 60 snapshot tests still run. This is narrower than the `ci-skip` label, which would disable the entire CI run. ## Revert condition Temporary. Re-enable once `nargo expand` produces deterministic interface-function ordering (e.g. sort by name/selector) in noir. The skip carries a TODO pointing at noir-lang/noir#12933 --- *Created by [claudebox](https://claudebox.work/v2/sessions/3dcd4bebd168b42b) · group: `slackbot`* --------- Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Collaborator
Author
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
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
6 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.
BEGIN_COMMIT_OVERRIDE
refactor(pxe)!: derive oracle interfaces from registry and introduce aztec_misc_ prefix (#23840)
chore: regenerate yarn.lock for noir beta.22 (#23877)
refactor(pxe): append only NoteStore and PrivateEventStore (#23785)
feat(aztec-nr): include delivery mode in handshake discovery (#23873)
refactor(txe): move orchestration logic out of rpc_translator (#23904)
refactor(aztec-nr)!: move messages::message_delivery to messages::delivery (#23875)
fix(ci): run contract-snapshots tests in CI (#23895)
refactor(pxe): hash oracle registry instead of Oracle class (#23907)
fix(docs): update webapp-tutorial to current wallets/PXE API + fix vite polyfill resolution (#23908)
feat: TransientArray (#23576)
refactor(pxe): replace Oracle class with Proxy-based buildACIRCallback (#23915)
test(ci): skip non-deterministic expand::test_avm_test_contract snapshot (#23935)
END_COMMIT_OVERRIDE