feat: allow setting additional scopes in nr tests#22968
feat: allow setting additional scopes in nr tests#22968nchamo merged 7 commits intomerge-train/fairiesfrom
Conversation
…t-additional-scopes-in
nchamo
left a comment
There was a problem hiding this comment.
I like the change, and it's one we need. But I would be more comfortable if we had a way to tell users "hey, this contract expects a different TXE version, please re-compile". Or we should at least be working on that right after this change, and wait to backport this PR
|
|
||
| pub use contract_self_private::PrivateUtilityCalls; | ||
| pub use contract_self_private::ContractSelfPrivate; | ||
| pub use contract_self_private::PrivateUtilityCalls; |
There was a problem hiding this comment.
Was this the formatter? If so, then I'm curious about why it didn't do the same for me
| pub unconstrained fn view_private_opts<let M: u32, let N: u32, let S: u32, T>( | ||
| _self: Self, | ||
| from: AztecAddress, | ||
| opts: ViewPrivateOptions<S>, |
There was a problem hiding this comment.
Why here and not as the last param? "Options" params usually go last, right?
There was a problem hiding this comment.
That's the case in JS because that naturally leads into the options being undefined if dropped entirely, but it doesn't need to be that way here. I was mirroring private_context_opts, where the options are second to last, with the last param being the 'meaty' action. In that case that's better done that way because the lambda is long and you want for it to be clearly separated, but here I don't really have any strong thoughts. It makes sense to me for the options to live alongisde the rest of the config (from etc), before the actual call.
There was a problem hiding this comment.
It's completely personal, but I prefer opts to be last. So the first thing I read is the actual call, the options are less important to me
|
|
||
| // We create a LogRetrievalRequest for each PendingPartialNote in the EphemeralArray. Because we need the indices in | ||
| // We create a LogRetrievalRequest for each PendingPartialNote in the EphemeralArray. Because we need the indices | ||
| // in |
There was a problem hiding this comment.
Oh, I see you have quite a bit of formatting changes (here and other files). Can you fix them so that we don't have lines with only one word?
| from: AztecAddress, | ||
| call: PublicCall<M, N, T>, | ||
| ) -> T | ||
| pub unconstrained fn call_public_incognito<let M: u32, let N: u32, T>(_self: Self, call: PublicCall<M, N, T>) -> T |
There was a problem hiding this comment.
This is a breaking change. Should we add migration notes for it?
| pub unconstrained fn call_private_opts<let M: u32, let N: u32, let S: u32, T>( | ||
| _self: Self, | ||
| from: AztecAddress, | ||
| opts: CallPrivateOptions<S>, |
There was a problem hiding this comment.
Why here and not as the last param? "Options" params usually go last, right?
| const blockNumber = await this.getNextBlockNumber(); | ||
|
|
||
| const callContext = new CallContext(from, targetContractAddress, functionSelector, isStaticCall); | ||
| const msgSender = from ?? AztecAddress.fromBigInt(NULL_MSG_SENDER_CONTRACT_ADDRESS); |
There was a problem hiding this comment.
We are doing AztecAddress.fromBigInt(NULL_MSG_SENDER_CONTRACT_ADDRESS) in quite a few places and files. Would it make sense to have it in only one place and simply use it as an address?
- Fix orphaned single-word comment continuation lines in processing/mod.nr, map.nr, state_vars/mod.nr, state_variable.nr - Add migration notes for call_public_incognito and view_public_incognito breaking changes - Extract AztecAddress.NULL_MSG_SENDER static constant to @aztec/stdlib/aztec-address
…-616-feature-txe-support-additional-scopes-in # Conflicts: # docs/docs-developers/docs/resources/migration_notes.md # noir-projects/aztec-nr/aztec/src/test/helpers/txe_oracles.nr # noir-projects/noir-contracts/contracts/protocol/public_checks_contract/src/test.nr # noir-projects/noir-contracts/contracts/test/scope_test_contract/src/main.nr
…uced line wrapping
|
❌ Failed to cherry-pick to |
…r tests' (with conflicts) Conflicted files (preserved as-is for review): - docs/docs-developers/docs/resources/migration_notes.md - noir-projects/aztec-nr/aztec/src/test/helpers/txe_oracles.nr - yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts - yarn-project/stdlib/src/aztec-address/index.ts
After rebasing onto backport-to-v4-next-staging (which now includes the backport of #22889 'feat(txe): add tx private logs to tx side effects oracle'), only three files still need cross-PR-drift handling: - docs/docs-developers/docs/resources/migration_notes.md: keep just the two new TBD entries from #22968 (TXE `call_public_incognito` no longer takes `from`; `view_public_incognito` deprecated). The DeployMethod and aztec-up bundled-binary entries that the cherry-pick brought along belong to other PRs not yet backported (e.g. #22985). - yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts: keep the v4-next-staging-specific tree-height imports (L1_TO_L2_MSG_TREE_HEIGHT, NOTE_HASH_TREE_HEIGHT, PUBLIC_DATA_TREE_HEIGHT) which are still in use here — #21577 removed them on next but hasn't been backported. Drop only NULL_MSG_SENDER_CONTRACT_ADDRESS (replaced by AztecAddress.NULL_MSG_SENDER). - yarn-project/stdlib/src/aztec-address/index.ts: keep both the existing eslint-disable comment and the new NULL_MSG_SENDER_CONTRACT_ADDRESS import. - noir-projects/aztec-nr/aztec/src/test/helpers/txe_oracles.nr: with #22889 backported, the imports now match next exactly — just drop NULL_MSG_SENDER_CONTRACT_ADDRESS.
…2968) to v4-next (#23057) Backport of #22968 ("feat: allow setting additional scopes in nr tests") to `backport-to-v4-next-staging`. Rebased on top of latest staging (which now includes the backport of #22889 — `feat(txe): add tx private logs to tx side effects oracle`). All conflicts handled in a single resolution commit. ## What The PR adds `call_private_opts` / `view_private_opts` (with `CallPrivateOptions` / `ViewPrivateOptions`) to `TestEnvironment`, letting tests grant access to additional account scopes when calling private functions. It also reshapes the TXE oracle ABI for `private_call_new_flow` / `public_call_new_flow` to take an `Option<AztecAddress>` for `from`, removes `call_public_incognito`'s ignored `from` parameter, deprecates `view_public_incognito`, and introduces `AztecAddress.NULL_MSG_SENDER` on the TS side. ## Conflicts handled (single commit) After rebasing, four files still drifted vs `next`: - **`noir-projects/aztec-nr/aztec/src/test/helpers/txe_oracles.nr`** — with #22889 now on staging, the imports here match `next` exactly aside from `NULL_MSG_SENDER_CONTRACT_ADDRESS`. Just drop that one symbol (the PR removes its only use). - **`docs/docs-developers/docs/resources/migration_notes.md`** — keep only the two new TBD entries from #22968. The DeployMethod and `aztec-up` bundled-binary entries that the cherry-pick brought along belong to other PRs not yet backported (e.g. #22985); leave them out so those backports add them in their own PRs. - **`yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts`** — drop `NULL_MSG_SENDER_CONTRACT_ADDRESS` (replaced by `AztecAddress.NULL_MSG_SENDER`). Keep `L1_TO_L2_MSG_TREE_HEIGHT`, `NOTE_HASH_TREE_HEIGHT`, `PUBLIC_DATA_TREE_HEIGHT` — they're still in use on this branch since #21577 hasn't been backported. - **`yarn-project/stdlib/src/aztec-address/index.ts`** — keep both the existing `eslint-disable @typescript-eslint/no-unsafe-declaration-merging` comment and the new `NULL_MSG_SENDER_CONTRACT_ADDRESS` import. ## Verification - `nargo check` passes for both Noir contracts the PR touches (`scope_test_contract`, `public_checks_contract`). - No conflict markers remain. Original PR: #22968
Closes github.com//issues/22292.
Note that this is a TXE oracle interface breaking change - in order to use this feature users will need to upgrade both the
azteccli (to use the newaztec test) and aztec-nr (to use the new functions). We're not really tracking TXE oracle versioning anywhere, I'll work on a small feature so that users get a better error when they run into this.