chore: Accumulated backports to v4-next#23198
Merged
Merged
Conversation
Deprecates mocha as a test runner for kv-store (we currently half part in mocha part in vitest). The seemingly off-topic change in stdlib stems from L2TipsStore tests being generated from there but consumed from kv-store. It's a bit of a strange pattern that maybe we should reconsider. Cherry-picked from 4b5178a — conflicts preserved with markers (see next commit for resolution).
- yarn-project/kv-store/src/lmdb-v2/store.test.ts: drop 'Map size validation' describe block — added by PR #22400 which is not on v4-next. - yarn-project/kv-store/src/lmdb/store.test.ts: keep deleted on v4-next (file does not exist there; was re-added by PR #21539 which is not on v4-next). - yarn-project/yarn.lock: regenerated via 'yarn install' to align with package.json changes.
## Summary Backports #23096 — _chore: kv store test fully on vitest_ — to `v4-next` via `backport-to-v4-next-staging`. ## Commits 1. **`chore: kv store test fully on vitest (#23096)`** — original cherry-pick attempt with conflict markers preserved, so reviewers can see what conflicted. 2. **`fix: resolve cherry-pick conflicts`** — minimal, surgical resolution. ## Conflict resolution The automatic cherry-pick produced three conflicts: - **`yarn-project/kv-store/src/lmdb-v2/store.test.ts`** (content/content) — the `Map size validation` describe block was added to `next` by [PR #22400](#22400) (`fix: use Int64Value instead of Uint32Value for 64-bit map sizes`), which is **not on `v4-next`**. PR #23096 was migrating that block from chai to vitest. Since the block does not exist on `v4-next`, the migration target is absent, so the block was dropped. - **`yarn-project/kv-store/src/lmdb/store.test.ts`** (deleted/modified) — this file was deleted on `v4-next` by [PR #12927](#12927) (`feat: Node snapshot sync`) and re-added on `next` by [PR #21539](#21539) (`fix(kv-store): make LMDB clear and drop operations atomic across sub-databases`). #21539 is **not on `v4-next`**, so the file should remain deleted; the migration of its tests is irrelevant. - **`yarn-project/yarn.lock`** — version drift between branches (`just-extend`, `jwa`, `minimatch`). Regenerated via `yarn install` to align with the new `package.json` manifests. ## Build verification Local `yarn build` requires pre-built `@aztec/bb.js` and `@aztec/l1-artifacts` (downloaded during full bootstrap), which is not feasible inside this container. The cherry-pick changes touch only test files plus dev-dependencies; `yarn install` succeeded cleanly. Relying on CI to validate the full TypeScript build and tests. ClaudeBox log: https://claudebox.work/s/9f9aee0884bd626d?run=2
## Summary - Adds the two missing in-tree Noir tests for `get_note_hash_membership_witness` in [noir-projects/aztec-nr/aztec/src/oracle/get_membership_witness.nr](noir-projects/aztec-nr/aztec/src/oracle/get_membership_witness.nr), matching the existing block-hash witness tests and clearing the `TODO(F-652)` placeholder. - The happy-path test reuses the existing `crate::history::test::create_note()` fixture, recomputes the unique note hash (siloed + nonce'd, same transformation as `assert_note_existed_by`), and verifies the witness by reconstructing the root via `root_from_sibling_path` and comparing it against `anchor.state.partial.note_hash_tree.root`. - The error-path test queries a bogus hash and expects the TXE handler's `not found in the note hash tree at block` error. - Bumps `history::test` from private to `pub(crate)` so the `create_note` fixture is reachable from `oracle::get_membership_witness::test` without exposing it outside the crate. Resolves [F-652](https://linear.app/aztec-labs/issue/F-652). ## Test plan - [x] `nargo test --package noir_aztec oracle::get_membership_witness::test::` — all 6 tests pass (4 existing + 2 new) against a local TXE on port 14730. - [x] `nargo check --deny-warnings --package noir_aztec` clean.
2 tasks
Accept either 'not_true == true' (older nargo) or 'assert(not_true == true, "This assertion should fail!")' (post-#22911) in the public-error enrichment test. Drops the avm_simulator exclusion from compat_test_cmds added in the prior commit; only event_logs remains excluded (legacy artifacts don't have deliver_squashed_and_surviving_notes).
## Summary The Aztec CLI Acceptance Test workflow was timing out at 30 minutes despite the harness reporting `TEST_RESULT=pass` after ~2.5 min. Example: [run 25735107748](https://github.com/AztecProtocol/aztec-packages/actions/runs/25735107748/job/75569917180). ## Root cause `aztec-cli-acceptance-test.ts` spawns `aztec start --local-network` and registers `process.on('exit', () => proc.kill('SIGTERM'))` to clean it up. The script then falls off the end of top-level await without calling `process.exit`. Node therefore waits for the event loop to drain — but the long-running child keeps it alive, the `'exit'` handler never fires, and the process hangs until GitHub's 30-minute job timeout cancels it. The CI cleanup line `Terminate orphan process: pid (4538) (node)` confirmed the unkilled child. ## Fix Explicit `process.exit(0)` / `process.exit(1)` after the success and failure branches. Calling `process.exit` synchronously fires the `'exit'` handler, which SIGTERMs the child and lets Node terminate.
Reverts OIDC-based AWS auth in the ci-compat-e2e job back to access key credentials. The OIDC role lacks ec2:RunInstances, so spot/on-demand provisioning fails (hidden until now by continue-on-error on nightly tags). Mirrors c3c1371 which applied the same workaround to ci-release-publish. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary - Drop OIDC auth from `ci-compat-e2e` and run with `AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY` instead. - The OIDC role (`pipeline-exec-aztecprotocol-aztec-packages-heads-next`) has no `ec2:RunInstances` policy attached, so spot requests time out and the on-demand fallback fails with `UnauthorizedOperation`. The job has been failing on every v4 nightly since #22930; `continue-on-error` for `-nightly.` tags has masked it. - Mirrors c3c1371 (#23167), which applied the same workaround to `ci-release-publish` on this branch. Example of the failure being patched: https://github.com/AztecProtocol/aztec-packages/actions/runs/25737242295/job/75580441745 ## Test plan - [ ] CI3 on this PR runs to green (regular `ci` job is unaffected — it already uses static keys). - [ ] Apply the `ci-compat-e2e` label here to exercise the compat-e2e job end-to-end and confirm the EC2 spot/on-demand request succeeds with the static credentials. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…ertion regex (#23193) ## Summary Compat-e2e regressed on `v4.3.0-nightly.20260512-1` (https://github.com/AztecProtocol/aztec-packages/actions/runs/25731562906). After review with Jan and David, the two failing tests need different treatments — the PXE error enrichment itself is *not* broken; this is purely test-vs-old-artifact text drift. ## What this PR does - **`e2e_event_logs.test.ts`**: drop from the compat-e2e matrix. The new `tagging cache reconciliation against kernel squashing` test added in PR #23044 calls `TestLog::deliver_squashed_and_surviving_notes`, a method that does not exist in legacy 4.2.x artifacts. There is no reasonable backwards-compat coverage here without backporting the contract method itself. - **`e2e_avm_simulator.test.ts`**: keep it in the compat-e2e matrix and loosen the assertion-message regex to accept either the older or the newer assertion-span form: - old (pre-#22911 nargo): `'not_true == true'` - new (post-#22911 nargo): `'assert(not_true == true, "This assertion should fail!")'` The regex becomes `/Assertion failed: This assertion should fail!.*not_true == true/`, which matches both shapes and still rejects unrelated assertions. The test continues to validate the PXE's public-error enrichment pipeline (assertion decode + opcode→source resolution) against legacy artifacts — which is the actually-valuable coverage David flagged in the team-fairies thread. ## Why not exclude `avm_simulator` too? David pointed out that this case is the only public-error-enrichment test we have, and the enrichment code path is genuinely worth keeping under compat watch. Verified manually that enrichment runs correctly against legacy 4.2.x artifacts (paths, line/col, function names, assertion message all resolve); the only thing that differed was the locationText span granularity that nargo bakes into the artifact at compile time. Loosening the regex preserves the coverage without breaking on the legacy span shape. ## Test plan - `node -e '/Assertion failed: This assertion should fail!.*not_true == true/.test(…)'` matches both old and new forms, rejects unrelated assertions. - `bash -c 'shopt -s extglob; ls src/e2e_!(block_building|prover_*|kernelless_simulation|event_logs).test.ts'` from `yarn-project/end-to-end` resolves to 49 files (was 48 when `avm_simulator` was also excluded), `event_logs` correctly absent. - Compat-e2e is `continue-on-error` for nightlies — next nightly should be green on `ci-compat-e2e` once this lands. A forward-port of the regex loosening to `next` is also worthwhile (same file, identical strict regex there today), but mainline `next` doesn't run compat-e2e so it isn't blocking; can ship as a follow-up.
…23201) ## Summary `install_node` runs `nvm install --lts && nvm alias default lts/*` when the host Node doesn't meet the manifest's minimum. That installs whatever Node version is *currently* tagged LTS, not the version pinned in `versions` for the release being installed. Today this happens to be safe — current LTS is 24.x and the manifest says `node: 24.12.0`. But it's coincidence: the moment a release pins a non-LTS line (or an LTS slips behind), the installer silently runs the rest of the install on the wrong Node and the failure manifests far away from `install_node`. ## Fix Install the version named in the manifest, instead of `--lts`: ```diff - nvm install --lts - nvm alias default lts/* + nvm install "$node_min_version" + nvm alias default "$node_min_version" ``` (Same change applied to the error-message hint for hosts without nvm.)
Collaborator
Author
|
🤖 Auto-merge enabled after 8 hours of inactivity. This PR will be merged automatically once all checks pass. |
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
5 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
chore: kv store test fully on vitest (#23096)
chore: backport kv-store vitest migration (#23096) to v4-next (#23185)
test: add noir tests for get_note_hash_membership_witness (#23190)
fix(aztec-up): explicit exit in CLI acceptance test harness (#23200)
refactor(pxe): batch nullifier sync across scopes (#23129)
refactor(pxe): backport batch nullifier sync across scopes (#23129) to v4-next (#23208)
fix(ci): revert ci-compat-e2e to AWS access keys (#23211)
test: drop event_logs from compat matrix and loosen avm_simulator assertion regex (#23193)
fix(aztec-up): install manifest-pinned Node version instead of LTS (#23201)
fix(ci): move CLI acceptance test timeout from job to step (#23205)
END_COMMIT_OVERRIDE