feat(pxe): add execution hooks for authorizing cross-contract utility calls#23007
Conversation
Thunkar
left a comment
Thunkar
left a comment
There was a problem hiding this comment.
Like the approach! Would be great to add the plumbing on EmbeddedWallet and showcase how it works on the ecosystem repo
There was a problem hiding this comment.
Looks nice but what's the reasoning for the flow to be so different from authwits? (am I correct in assuming that it's easy of implementation?)
During authwits we run simulation and harvest the authwit requests with the approach Grego implemented while here we are asking for permissions during the real thing.
It feels somewhat incorrect that we are choosing 2 completely different approaches when we are dealing with a somewhat similar thing (permissions to run stuff).
| `Cross-contract utility call denied: ${this.contractAddress} attempted to call ` + | ||
| `${targetContractAddress}:${functionSelector} (${targetArtifact.name}).`, |
There was a problem hiding this comment.
I think this should get its own error doc link
mverzilli
left a comment
mverzilli
left a comment
There was a problem hiding this comment.
Love it! I'm just gonna wear my gregohat now and suggest that you use this in some way in go-swap, because I'm curious how this will translate to UX
An authwit requested requires human intervention. The person needs to make a decision (sign an authwit) which a) likely cannot be automated (it may require access to a hardware wallet, a signer that is located elsewhere. etc.), b) should not be automated (as it results in the user possibly spending assets or mutating their state), and c) for which the user does not have the full picture yet - they don't know what is going to happen once they sign. If you implemented authwits via callbacks, then this would result in the simulation being repeatedly halted while the user is asked if they want to proceed, to which they'll have to option but to say yes, because there's no useful information for them to review - they can't even see the full transaction yet as it is still executing. This has been referrred to as 'authwits on the fly', and has been discarded for these very reasons. We therefore implemented authwits via a different mechanism: since PXE knows about authwits (as they are a standard), it can cleanly reason about how to do a drop-in replacement for them via a kerneless simulation in which it simply records requets and lies to contracts pretending the request was accepted. The user can now be prompted once for all signatures needed and they can see the full transaction effects before signing. "If you sign this, this is what will happen". Execution of utility functions is fundamentally different. PXE cannot predict what a utility function should return, so it has no way of mocking the result. Additionally, the decision on whether to execute a utility function is not about ownership but about privacy: a call made by a bad contract might result in some information being revealed. And even if we ignored all that, there would be nothing to be gained by gathering more information about the tx execution: a call is ok or not by itself (i.e. 'is the caller bad' or 'is this contract ok to call by anyone'), so trying to proceed before asking the user results in no benefit. Finally, in many (most?) cases the wallet can decide whether a specific call is safe or not and authorize or reject it. Examples include lists of trusted/audited contracts (like the standard registry, widely used applications, etc.), and contracts the user has already trusted in the past. The callback is not expected to make its way all the way to the user, but rather for the wallet to resolve it itself in most cases. But we do need a callback because neither app nor wallet can predict ahead of time which contracts will be invoked during execution, so we need for a way to ask for permission after it has begun. |
|
❌ Failed to cherry-pick to |
… calls (#23007) Cherry-pick of PR #23007 with .rej files for the two hunks that failed to apply against backport-to-v4-next-staging: - yarn-project/end-to-end/src/fixtures/setup.ts.rej Hunk anchored on `skipInitialSequencer?: boolean;` field that does not exist on this branch. - yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts.rej Hunk context shifted by an extra `log: this.logger,` line present on this branch. (Note: the target file is unmodified by this commit because its only hunk was rejected.) This commit will not compile on its own; the next commit resolves the .rej files.
- yarn-project/end-to-end/src/fixtures/setup.ts: Insert pxeCreationOptions field at the equivalent position (end of SetupOptions, after walletMinFeePadding) since skipInitialSequencer doesn't exist on this branch. - yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts: Add 'hooks: this.hooks,' to the args passed when constructing the child PrivateExecutionOracle, between simulator and l2TipsStore (as in #23007). this.hooks resolves via inherited protected field on UtilityExecutionOracle.
… calls (#23007) Cherry-pick of PR #23007 with .rej files for the two hunks that failed to apply against backport-to-v4-next-staging: - yarn-project/end-to-end/src/fixtures/setup.ts.rej Hunk anchored on `skipInitialSequencer?: boolean;` field that does not exist on this branch. - yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts.rej Hunk context shifted by an extra `log: this.logger,` line present on this branch. (Note: the target file is unmodified by this commit because its only hunk was rejected.) This commit will not compile on its own; the next commit resolves the .rej files.
- yarn-project/end-to-end/src/fixtures/setup.ts: Insert pxeCreationOptions field at the equivalent position (end of SetupOptions, after walletMinFeePadding) since skipInitialSequencer doesn't exist on this branch. - yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts: Add 'hooks: this.hooks,' to the args passed when constructing the child PrivateExecutionOracle, between simulator and l2TipsStore (as in #23007). this.hooks resolves via inherited protected field on UtilityExecutionOracle.
…s-contract utility calls (#23066) Backport of #23007 (`feat(pxe): add execution hooks for authorizing cross-contract utility calls`, by @nchamo) to `backport-to-v4-next-staging`. The auto-cherry-pick (run [25514836711](https://github.com/AztecProtocol/aztec-packages/actions/runs/25514836711)) failed with two rejected hunks. Restructured into two commits so the failure and the fix are reviewable independently: **1. `feat(pxe): add execution hooks for authorizing cross-contract utility calls (#23007)`** The cherry-pick attempt: applies the PR diff with `git apply --verbose --reject`. All clean hunks land, and the two failed hunks are recorded as `.rej` sidecar files committed alongside their target. Does not compile on its own. - `yarn-project/end-to-end/src/fixtures/setup.ts.rej` — anchor `skipInitialSequencer?: boolean;` doesn't exist on this branch. - `yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts.rej` — context shifted by an extra `log: this.logger,` line on this branch. (Target file is unmodified by this commit because its only hunk rejected.) **2. `fix: resolve cherry-pick conflicts from #23007`** Deletes the two `.rej` files and applies the equivalent edits manually: - `setup.ts` — insert `pxeCreationOptions?: PXECreationOptions;` at the end of `SetupOptions` (after `walletMinFeePadding`). - `private_execution_oracle.ts` — insert `hooks: this.hooks,` between `simulator: this.simulator,` and `l2TipsStore: this.l2TipsStore,`. `this.hooks` resolves via the `protected readonly hooks: ExecutionHooks | undefined` field that this PR adds on the parent class `UtilityExecutionOracle`. No semantic deviations from the source PR.
|
That makes sense. Thanks for the thorough explanation @nventuro BTW feels like that comment would be a good basis for some docs |
BEGIN_COMMIT_OVERRIDE feat: allow setting additional scopes in nr tests (AztecProtocol#22968) feat(pxe): add execution hooks for authorizing cross-contract utility calls (AztecProtocol#23007) END_COMMIT_OVERRIDE
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 AztecProtocol#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.
6 participants
Why we are doing this
Utility functions run in simulation and have access to private state, so allowing unrestricted cross-contract utility calls is a security risk — a contract could call into another contract's utility functions to read its secrets. We need a way to let wallets/PXE integrators control which cross-contract utility calls are allowed.
What we're doing
Adds an
ExecutionHooksinterface toPXECreationOptions. The idea is that hooks are a generic extension point for the PXE execution pipeline — we expect more to be added over time. The first one isauthorizeUtilityCall.authorizeUtilityCallis called before any cross-contract utility call. It gets passed the caller, target, selector, args, and whether the call originated from a private or utility context, and returns a boolean. The default denies everything, so existing behavior is unchanged.To discuss
ExecutionHooksare required and callers spreadDEFAULT_EXECUTION_HOOKSto partially override. Alternative is making fields optional with defaults applied at call sites. Open to feedbackFuture work
Fixes F-29