feat: merge-train/fairies#23616
Conversation
…23595) `includes` and `indexOf` compare values by reference, they don't do deep equality checks. We had three instances of this, two in pxe (not great, not terrible) and one in the validator client (minor, an error message would contain a misdiagnosis), plus a bunch in mocks and tests. This introduces an eslint rule that catches this: if we do `includes` or `indexOf` on a type that implements the `equals` function, then that's an error. This PR also fixes some instances that are not actually incorrect as they happen to perform the check on values that were created locally, so the reference check matches the deep equality check, but this is brittle and should not be relied upon.
Many tests were not invoking the jest matchers (e.g.
`expect(foo).toEqual;` instead of `expect(foo).toEqual();`). I added the
eslint rule that forbids this while allowing some patterns we use (like
shortcuts in the form if `do_logs && logger.log('foo');`. The test fixes
should be correct, but it is likely worth going over them in case the
finding of this surface being untested is relevant.
Fixes [F-668](https://linear.app/aztec-labs/issue/F-668/aztec-nr-extend-get-next-app-tag-as-sender-for-constrained-delivery) - Adds a new `get_next_constrained_index_oracle` that accepts an app siloed secret (expected to be returned from the handshake registry utility contract added in #23278 - Extend the sender tagging store to key by a different type of secret (app supplied secret rather than fetched/computed from the PXE) --------- Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar> Co-authored-by: AztecBot <tech@aztec-labs.com>
Demotes auth_registry from protocol contract. Introduces generated `standard_addresses.nr` Noir file in aztec-nr and yarn-project/standard-contracts TS package that includes "drift" checks at build time and compile time to prevent someone from proceeding when they should actually go back and rebuild noir-contracts. Deploy `AuthRegistry` once per fresh rollup: `aztec-wallet deploy auth_registry_contract@AuthRegistry --salt 1 --universal -f <fee-paying-account>`. **CLI change**: Removed `--deployer <address>` flag from `aztec-wallet deploy`; `--universal` now works alongside `-f` to set deployer=0x0 while paying fees from the specified account. Stacked on #23216. --------- Co-authored-by: dbanks12 <davidjbanks920@gmail.com>
dbanks12
requested review from
IlyasRidhuan,
MirandaWood and
jeanmon
as code owners
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
…23622) ## Problem `NoteValidationRequest` and `EventValidationRequest` contain a `BoundedVec` whose `maxLen` is needed for deserialization, but it was passed as a separate oracle parameter (`maxNotePackedLen` / `maxEventSerializedLen`). This meant that we can't build a self-contained `TypeMapping` for it, and we wouldn't be able to build auto generated mappings in the future ## Fix Each struct now includes its own max length field (`max_note_packed_len` / `max_event_serialized_len`) that is populated automatically via a `new` constructor from the compile-time constant. The TypeScript `fromFields` methods read the length from the field stream instead of taking it as a parameter, making them self-contained.
Demotes public_checks from protocol contract. Stacked on #23106.
Same bug and fix as #22557 — that PR patched `yarn-project/precommit.sh` but didn't touch `noir-projects/precommit.sh`, which has the same `cd $(dirname $0)` + later `git rev-parse --show-toplevel` pattern. Inside a worktree, git sets `GIT_DIR` absolutely when invoking hooks; after the `cd`, `--show-toplevel` returns the `noir-projects/` subdir instead of the worktree root. The re-staging step then runs `git add` on paths that don't exist on disk, and `git add` on a missing path that's in the index stages a deletion — so committing `.nr` files inside a worktree silently dropped them from the tree. Same one-liner fix: `unset GIT_DIR` near the top.
auth_registry (#23106) and public_checks (#23217) were demoted from protocol to standard contracts. Protocol contracts occupy reserved low addresses the AVM treats as implicitly deployed; standard contracts have artifact-derived addresses and need a real deployment (instance + artifact in the contract DB plus a deployment nullifier). TXE registered protocol contracts but never deployed the demoted standard ones, so any public-authwit test reverted with 'Contract 0x1c3bd1fb... is not deployed'. Deploy AuthRegistry and PublicChecks in TXESession.init via the existing top-level deploy(), restoring pre-demotion availability for every TXE test.
The prior commit deployed AuthRegistry and PublicChecks via deploy(), which mines a block per contract, pushing the TXE block baseline from 2 to 4 and breaking every block-number-hardcoded test (e.g. deployment_proofs asserts next_block_number()==2 in setup()). Fold both deployments into the single baseline block TXE already mines at startup, via a new deployManyInSingleBlock(): one block carrying both deployment nullifiers, then register each instance + artifact. Net blocks mined at init stays 1, so next_block_number() is restored to 2.
The 4 `toMatchObject` assertions in the `authorizeUtilityCall hook` block compared `lastRequest.functionSelector` against `contractB.methods.pow_utility.selector()`, which is async (returns `Promise<FunctionSelector>` via `FunctionSelector.fromNameAndParameters`, async since #23007). An un-awaited Promise has no own enumerable properties, so `toMatchObject` treated the expected `functionSelector` as a vacuously-satisfied empty object and never actually checked the selector -- the assertions silently passed regardless of the real value. Awaiting `.selector()` makes the assertions compare against the resolved `FunctionSelector`. Verified: a negative control with a deliberately wrong selector fails only after the await is added, and the full suite passes 10/10 with the correct selector.
…sses Recent merges into the train (key/authwit changes via #23510) shifted the compiled bytecode of both standard contracts, so their derived addresses changed and the committed values were stale, failing the standard-contracts drift guard during build. Regenerated via two-pass build (converged): AuthRegistry 0x1c3bd1fb... -> 0x27ced680... PublicChecks 0x23a16bc3... -> 0x05d900a6...
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
7 participants
BEGIN_COMMIT_OVERRIDE
refactor(aztec-nr): use constructor methods for MessageDelivery variants (#23596)
docs: update testing_contracts.md for two-crate aztec new layout (#23617)
fix: drop usage of include and indexof on types that support equals (#23595)
fix: unused ts expressions in tests (#23621)
feat(aztec-nr): Get tagging index for constrained delivery (#23359)
feat!: demote auth registry to non-protocol contract (#23106)
feat(aztec-nr)!: embed BoundedVec max length in validation requests (#23622)
fix: regenerate standard contract addresses after auth registry demotion (#23640)
feat(aztec-nr): encrypt handshake log for indistinguishability (#23638)
feat!: demote public_checks to non-protocol contract (#23217)
fix: noir precommit re-staging inside worktrees (#23628)
END_COMMIT_OVERRIDE