Conversation
…2730) ## Summary Ctrl-C (or any SIGINT/SIGTERM) during `aztec test` could leave the TXE node still listening on its port and in-flight `bb` children still running, blocking subsequent runs with `EADDRINUSE`. Fixes #22725. ## Why it happens Two gaps in the TXE shutdown path: 1. `startTXE` (`yarn-project/aztec/src/cli/cmds/start_txe.ts`) bound the HTTP listener but registered **no** shutdown callback with the outer `signalHandlers` array that `aztecStart` passes to `installSignalHandlers` (`yarn-project/aztec/src/cli/aztec_start_action.ts:94`). Every other `start*` function in that file (`startNode`, `startBot`, `startArchiver`, `startP2PBootstrap`, `startProverAgent`, `startProverBroker`) already wires itself in this way — TXE was the odd one out. The listener could outlive `process.exit` under adversarial timing and hold the port. 2. `executeBB` (`yarn-project/bb-prover/src/bb/execute.ts:90`) spawned `bb` with no signal plumbing. If the node parent received SIGINT/SIGTERM, those signals were not propagated to `bb`, so the child became an orphan under init. ## Changes - **`startTXE`**: accept `signalHandlers`, push `() => httpServer.close()` so the port is released cleanly on SIGINT/SIGTERM. Brings TXE in line with the other `start*` functions. - **`aztec_start_action`**: pass `signalHandlers` into `startTXE`. - **`executeBB`**: forward SIGINT/SIGTERM/SIGHUP to the spawned `bb` child, removing the handlers on close so they don't leak across calls. ## Test plan - [x] `yarn build` passes for touched packages. - [x] `yarn format` + `yarn lint` clean for `aztec` and `bb-prover`. - [ ] Manual: run `aztec test --workspace`, Ctrl-C mid-run, confirm no orphaned `node`/`bb` processes and that re-running starts cleanly. - [ ] Manual: `pkill -TERM -f 'aztec start --txe'` mid-run, same check.
Resolves https://linear.app/aztec-labs/issue/F-577/unified-noir-contract-error-snapshot-testing Summary: - 4 cases moved into `noir-projects/noir-contracts-comp-failures/contracts/` - Old runner deleted: `noir-projects/aztec-nr/macro_compilation_failure_tests/` + both invocations in `noir-projects/aztec-nr/bootstrap.sh`. - Runner extended (`noir-projects/noir-contracts-comp-failures/bootstrap.sh`): - Matcher asserts **strict equality**, line-for-line, between the `error:` headlines nargo emits (prefix stripped, in emission order) and the non-blank lines of `expected_error`. Any difference in text, count, or order fails the test. - `ACCEPT_SNAPSHOTS=1 ./bootstrap.sh test` auto-extracts every `error:` headline and writes them one-per-line into `expected_error`. A `⚠` fallback writes full stderr when nargo emits no `error:` lines (ICE/panic), prompting a manual edit. - Devs no longer have to write their own `expected_error` file manually - CI guard: `ACCEPT_SNAPSHOTS=1` + `CI=1` is refused, so committed snapshots and live stderr can only diverge when a dev regenerates locally and commits. - `test` accepts an optional contract-name glob to run a subset: `./bootstrap.sh test reserved_public_dispatch` or `./bootstrap.sh test 'panic_on_*'`. No-arg behaviour is unchanged; `test_cmds` (CI wiring) still emits the no-arg form. Composes with `ACCEPT_SNAPSHOTS=1` for targeted snapshot updates. - Snapshots regenerated (13 `expected_error` files changed): - Added specificity where previous dropped by manual trims (e.g. `duplicate_storage` now pins `'Storage', got 'Storage2' instead.`, `event_selector_collision` pins both colliding event names, `marked_*_unconstrained` pins the function name). - Several cases gained cascading downstream `error:` lines that previous single-line trims were hiding (`Could not resolve 'init'/'ZERO' in path`, `Type annotation needed`). Kept them in the snapshot. This is part of the actual compiler behavior being asserted. - READMEs updated: aztec-nr README; comp-failures README Verification (done _manually_): ``` cd noir-projects/noir-contracts-comp-failures ``` 1. Happy case: ``` ./bootstrap.sh test ``` 2. Wrong-error path: I replaced the message in `contracts/reserved_public_dispatch/expected_error` with garbage ``` ./bootstrap.sh test Gives this output: ... ✓ contracts/panic_on_owned_state_var_in_storage/: Compilation failed as expected with correct error(s) ✓ contracts/public_function_selector_collision/: Compilation failed as expected with correct error(s) ✓ contracts/reserved_emit_public_init_nullifier/: Compilation failed as expected with correct error(s) ✗ contracts/reserved_public_dispatch/: error 1 does not match Expected: Function name '' is reserved for internal use Actual: Function name 'public_dispatch' is reserved for internal use ``` 4. Success-compile path: Change reserved_public_dispatch to `contract ReservedPublicDispatch {}` so that it succeeds compilation. ``` ... ✓ contracts/public_function_selector_collision/: Compilation failed as expected with correct error ✓ contracts/reserved_emit_public_init_nullifier/: Compilation failed as expected with correct error ✗ contracts/reserved_public_dispatch/: Expected compilation to fail but it succeeded ``` 5. Missing error Removed "Could not resolve 'init' in path" from panic_on_owned_state_var_in_storage. Running bootstrap we get the following: ``` ✗ contracts/panic_on_owned_state_var_in_storage/: error count mismatch — expected 2, got 3 Expected lines: Type PrivateImmutable<Field, Context> implements OwnedStateVariable and hence cannot be placed in Storage struct without being wrapped in Owned. Define the type in storage as Owned<PrivateImmutable<Field, Context><..., Context>>, Context>. Type annotation needed Actual error headlines: Type PrivateImmutable<Field, Context> implements OwnedStateVariable and hence cannot be placed in Storage struct without being wrapped in Owned. Define the type in storage as Owned<PrivateImmutable<Field, Context><..., Context>>, Context>. Could not resolve 'init' in path Type annotation needed Full stderr: error: Type PrivateImmutable<Field, Context> implements OwnedStateVariable and hence cannot be placed in Storage struct without being wrapped in Owned. Define the type in storage as Owned<PrivateImmutable<Field, Context><..., Context>>, Context>. ┌─ std/panic.nr:8:12 │ 8 │ assert(false, message); │ ----- Assertion failed │ = Call stack: 1: PanicOnOwnedStateVarInStorage::#[storage] at src/main.nr:8:5 2: storage <omitted full call stack here> Aborting due to 3 previous errors ``` Bringing back the error bootstrap will succeed again. 6. Extra error ``` ✗ contracts/panic_on_owned_state_var_in_storage/: error count mismatch — expected 4, got 3 Expected lines: Type PrivateImmutable<Field, Context> implements OwnedStateVariable and hence cannot be placed in Storage struct without being wrapped in Owned. Define the type in storage as Owned<PrivateImmutable<Field, Context><..., Context>>, Context>. Could not resolve 'init' in path Type annotation needed Type annotation needed Actual error headlines: Type PrivateImmutable<Field, Context> implements OwnedStateVariable and hence cannot be placed in Storage struct without being wrapped in Owned. Define the type in storage as Owned<PrivateImmutable<Field, Context><..., Context>>, Context>. Could not resolve 'init' in path Type annotation needed Full stderr: error: Type PrivateImmutable<Field, Context> implements OwnedStateVariable and hence cannot be placed in Storage struct without being wrapped in Owned. Define the type in storage as Owned<PrivateImmutable<Field, Context><..., Context>>, Context>. ┌─ std/panic.nr:8:12 │ 8 │ assert(false, message); │ ----- Assertion failed │ = Call stack: <omitted full call stack here> Aborting due to 3 previous errors ``` 7. Valid number of errors but wrong error I swapped the two errors in `panic_on_owned_state_var_in_storage`, where we got: ``` ✗ contracts/panic_on_owned_state_var_in_storage/: error 2 does not match Expected: Type annotation needed Actual: Could not resolve 'init' in path ``` 8. `ACCEPT_SNAPSHOTS` path: Remove the function names from `contracts/reserved_public_dispatch/expected_error` and `contracts/reserved_emit_public_init_nullifier/expected_error` (e.g., `Function name '' is reserved for internal use`). ``` ACCEPT_SNAPSHOTS=1 ./bootstrap.sh test ``` Afterwards the `expected_error` for both tests will contain the appropriate error with the name (e.g., `Function name 'public_dispatch' is reserved for internal use`). All the invariants of running the script above resulted as expected. --------- Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>
) The `expected_error` snapshot for `authorization_selector_collision` contains a phantom line `check_parent_traits_are_implemented: missing trait ID` that nargo does not emit (the string does not exist anywhere in `noir/noir-repo`). CI on `merge-train/fairies` runs this test on every PR with caching disabled, so the mismatch was blocking the branch. Actual nargo output is a stable 5 errors; expected was 6. Remove the phantom line so the snapshot matches. Failing run: http://ci.aztec-labs.com/1777040996623663 (sub-step log `b6fb83475dc5b0b0`). Full analysis: https://gist.github.com/AztecBot/e4536bdfe3f12f3cde1f5c3d9a23fe55 Verified locally — all 51 contracts in `noir-contracts-comp-failures` pass after the edit. ClaudeBox log: https://claudebox.work/s/56b0f2729e6fb5d3?run=1
Collaborator
Author
|
🤖 Auto-merge enabled after 4 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
fix(pxe): restrict setSenderForTags override to current call (F-564) (#22672)
fix(aztec-cli): release TXE port and kill bb children on shutdown (#22730)
fix(ci): Unify noir contract compilation error snapshot testing (#22733)
fix(ci): correct authorization_selector_collision error snapshot (#22771)
END_COMMIT_OVERRIDE