Skip to content

Commit

Permalink
chore: address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
LHerskind committed Jun 21, 2024
1 parent 11ab988 commit 52e439d
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ To make it convenient to compute the message hashes in TypeScript, the `aztec.js

For private calls where we allow execution on behalf of others, we generally want to check if the current call is authenticated by `on_behalf_of`. To easily do so, we can use the `assert_current_call_valid_authwit` which fetches information from the current context without us needing to provide much beyond the `on_behalf_of`.

This function will then make a to `on_behalf_of` to execute the `verify_private_authwit` function which validates that the call is authenticated.
This function will then make a call to `on_behalf_of` to execute the `verify_private_authwit` function which validates that the call is authenticated.
The `on_behalf_of` should assert that we are indeed authenticated and then return the `IS_VALID` selector. If the return value is not as expected, we throw an error. This is to cover the case where the `on_behalf_of` might implemented some function with the same selector as the `verify_private_authwit` that could be used to authenticate unintentionally.

#### Example
Expand Down
4 changes: 2 additions & 2 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ Aztec is in full-speed development. Literally every version breaks compatibility

The private authwit validation is now making a static call to the account contract instead of passing over control flow. This is to ensure that it cannot be used for re-entry.

To make this change however, we cannot allow emitting a nullifying from the account contract, since that would break the static call. Instead, we will be changing the `verify_private_authwit` to a `verify_private_authwit` and in the `auth` library emit the nullifier. This means that the "calling" contract will now be emitting the nullifier, and not the account. For example, for a token contract, the nullifier is now emitted by the token contract. However, as this is done inside the `auth` library, the token contract don't need to change much.
To make this change however, we cannot allow emitting a nullifier from the account contract, since that would break the static call. Instead, we will be changing the `spend_private_authwit` to a `verify_private_authwit` and in the `auth` library emit the nullifier. This means that the "calling" contract will now be emitting the nullifier, and not the account. For example, for a token contract, the nullifier is now emitted by the token contract. However, as this is done inside the `auth` library, the token contract doesn't need to change much.

The biggest difference is related to "cancelling" an authwit. Since it is no longer in the account contract, you cannot just emit a nullifier from it anymore. Instead it must rely on the token contract providing functionality for cancelling.

There are also a few general changes to how authwits are generated, namely to more easily support the data required for a validity lookup now. Previously we could lookup the `message_hash` directly at the account contract, now we instead need to use the `inner_hash` and the contract of the consumer to figure out if it have already been emitted.

A minor extension have been made to the authwit creations to make it easier specific a hash that needs to be signed with a specific caller, e.g., the `inner_hash` can be provided as `{consumer, inner_hash}` to the `createAuthWit` where it previously needed to do a couple of manual steps to compute the outer hash. The `computeOuterAuthWitHash` have been amde internal and the `computeAuthWitMessageHash` can instead be used to compute the values similarly to other authwit computations.
A minor extension have been made to the authwit creations to make it easier to sign a specific a hash with a specific caller, e.g., the `inner_hash` can be provided as `{consumer, inner_hash}` to the `createAuthWit` where it previously needed to do a couple of manual steps to compute the outer hash. The `computeOuterAuthWitHash` have been amde internal and the `computeAuthWitMessageHash` can instead be used to compute the values similarly to other authwit computations.

```diff
const innerHash = computeInnerAuthWitHash([Fr.ZERO, functionSelector.toField(), entrypointPackedArgs.hash]);
Expand Down
1 change: 1 addition & 0 deletions noir-projects/aztec-nr/authwit/src/auth.nr
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub fn assert_current_call_valid_authwit(context: &mut PrivateContext, on_behalf
// docs:end:assert_current_call_valid_authwit

pub fn assert_inner_hash_valid_authwit(context: &mut PrivateContext, on_behalf_of: AztecAddress, inner_hash: Field) {
// We perform a static call here and not a standard one to ensure that the account contract cannot re-enter.
let result: Field = context.static_call_private_function(
on_behalf_of,
FunctionSelector::from_signature("verify_private_authwit(Field)"),
Expand Down
2 changes: 0 additions & 2 deletions noir-projects/aztec-nr/aztec/src/keys/public_keys.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ use dep::protocol_types::{
};
use crate::keys::constants::{NUM_KEY_TYPES, NULLIFIER_INDEX, INCOMING_INDEX, OUTGOING_INDEX};

use dep::std::println;

global PUBLIC_KEYS_LENGTH = 8;

struct PublicKeys {
Expand Down
6 changes: 2 additions & 4 deletions yarn-project/aztec.js/src/account/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@ import { type EntrypointInterface } from '../entrypoint/entrypoint.js';
/** Creates authorization witnesses. */
export interface AuthWitnessProvider {
/**
* Computes an authentication witness from either a message hash or an intent (caller and an action).
* If a message hash is provided, it will create a witness for that directly.
* Otherwise, it will compute the message hash using the caller and the action of the intent.
* @param messageHash - The message hash or the intent (caller and action) to approve
* Computes an authentication witness from either a message hash
* @param messageHash - The message hash to approve
* @returns The authentication witness
*/
createAuthWit(messageHash: Fr | Buffer): Promise<AuthWitness>;
Expand Down
10 changes: 5 additions & 5 deletions yarn-project/aztec.js/src/utils/authwit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ export type IntentAction = {
*
* If using the `IntentInnerHash`, the consumer is the address that can "consume" the authwit, for token approvals it is the token contract itself.
* The `innerHash` itself will be the message that a contract is allowed to execute.
* At the point of "approval checking", the validating contract (account for private and registry for public)
* will be computing the message hash, and using that for the check. Any allowed `innerHash` will therefore also have information around
* where it can be spent (version, chainId, consumer).
* At the point of "approval checking", the validating contract (account for private and registry for public) will be computing the message hash
* (`H(consumer, chainid, version, inner_hash)`) where the all but the `inner_hash` is injected from the context (consumer = msg_sender),
* and use it for the authentication check.
* Therefore, any allowed `innerHash` will therefore also have information around where it can be spent (version, chainId) and who can spend it (consumer).
*
* If using the `IntentAction`, the caller is the address that is making the call, for a token approval from Alice to Bob, this would be Bob.
* The action is then used along with the `caller` to compute the `innerHash` and the consumer.
Expand All @@ -45,7 +46,7 @@ export type IntentAction = {
* @param intent - The intent to approve (consumer and innerHash or caller and action)
* The consumer is the address that can "consume" the authwit, for token approvals it is the token contract itself.
* The caller is the address that is making the call, for a token approval from Alice to Bob, this would be Bob.
* The caller is only a concept in the aztec structure, as it becomes part of the inner hash, and is simple
* The caller becomes part of the `inner_hash` and is dealt with entirely in application logic.
* @param metadata - The metadata for the intent (chainId, version)
* @returns The message hash for the action
*/
Expand All @@ -55,7 +56,6 @@ export const computeAuthWitMessageHash = (intent: IntentInnerHash | IntentAction

if ('caller' in intent) {
const action = intent.action instanceof ContractFunctionInteraction ? intent.action.request() : intent.action;
// return computeAuthWitMessageHash(intent.caller, chainId, version, action);
return computeOuterAuthWitHash(
action.to.toField(),
chainId,
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/end-to-end/src/e2e_authwit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('e2e_authwit_tests', () => {
it('happy path', async () => {
// What are we doing here:
// 1. We compute an inner hash which is here just a hash of random data
// 2. We then compute the other, which is binding it to a "caller", here the "auth" contract
// 2. We then compute the outer, which is binding it to a "consumer" (here the "auth" contract)
// 3. We then create an authwit for this outer hash.
// 4. We add this authwit to the wallet[1]
// 5. We check that the authwit is valid in private for wallet[0] (check that it is signed by 0)
Expand Down Expand Up @@ -66,7 +66,7 @@ describe('e2e_authwit_tests', () => {
isValidInPublic: false,
});

// Consume the inner hash using the wallets[0] as the on behalf of.
// Consume the inner hash using the wallets[0] as the "on behalf of".
await auth.withWallet(wallets[1]).methods.consume(wallets[0].getAddress(), innerHash).send().wait();

expect(await wallets[0].lookupValidity(wallets[0].getAddress(), intent)).toEqual({
Expand Down

0 comments on commit 52e439d

Please sign in to comment.