feat: merge-train/fairies#23166
Merged
Merged
Conversation
) Fixes #22949. We keep track of the range of tagging indices used by a tx as `(lowestIdx, highestIdx)` - we need this so that future transactions start from `highest + 1` even if a previous tx has not yet been mined, avoiding tag duplication. The lowest index is required in case of transaction reverts: we walk all indices from low to high and test which ones survived onchain, and that way determine the actual highest index for a tx. On sync, we validate that our records for a tx match the indices used in said tx - this helps detect bugs (like the bug being fixed here). This check started failing because our `(low, high)` record did not consider the possibility of logs being squashed - if either `low` or `high` got squashed then the range would change. Note that we don't care about indices between low and high being squashed , gaps are ok - we only really care about the edges. This PR fixes this by inspecting tx effects to adjust the range prior to storing the indices in the database. It hints at the need for a tagging service layer, but that was a bigger change that I didn't want to introduce here. I added tests for both new functions created, plus an e2e regression test that checks we've fixed the original issue. I validated that the test fails without this fix. --------- Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar> Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Forward port of #22979 (merged into `backport-to-v4-next-staging`) onto `merge-train/fairies`. --------- Co-authored-by: sirasistant <sirasistant@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## 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.
## 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.
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
fix: dropped tagging indices no longer cause a pxe crash on sync (#23044)
feat: forward-port upstream oxide aztec-nr changes (#23183)
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)
ci: revert ci-compat-e2e to AWS access keys (#23212)
END_COMMIT_OVERRIDE