Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow cancelling a shared authwit token #3007

Closed
Tracked by #2199
spalladino opened this issue Oct 24, 2023 · 7 comments · Fixed by #4799
Closed
Tracked by #2199

Allow cancelling a shared authwit token #3007

spalladino opened this issue Oct 24, 2023 · 7 comments · Fixed by #4799
Assignees
Labels
S-needs-discussion Status: Still needs more discussion before work can start. T-feedback Type: recording user feedback

Comments

@spalladino
Copy link
Collaborator

A user can sign an authwit token and share it with another user, so they can execute a tx that requires access to the first users' assets. However, once shared, there's no way to revoke that authwit (it is possible to revoke public authwits, but not private). We should allow a user to manually emit the nullifier for an already-signed authwit token as a means to invalidate it.

cc @LHerskind for discussion

Raised by @0xShaito from Wonderland

@spalladino spalladino added S-needs-discussion Status: Still needs more discussion before work can start. T-feedback Type: recording user feedback labels Oct 24, 2023
@rahul-kothari rahul-kothari added this to A3 Oct 25, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Oct 25, 2023
@rahul-kothari rahul-kothari self-assigned this Oct 25, 2023
@rahul-kothari rahul-kothari moved this from Todo to In Progress in A3 Oct 26, 2023
@rahul-kothari
Copy link
Contributor

In private land you don't need to emit any nullifiers -> PXE stores the authwit in memory. Simple can delete the entry! Will pick this up

@spalladino
Copy link
Collaborator Author

Agree, but that holds only if the authwit hasn't been shared. In most cases it'll just be in the owner's PXE as you say, but we may still need a path in the account contract to cancel a shared one.

@rahul-kothari
Copy link
Contributor

rahul-kothari commented Oct 26, 2023

LOOOOL I wish I saw this on time -> I also just realised it https://aztecprotocol.slack.com/archives/C04PUD9AA4W/p1698338407296369

you are right yeep!

@rahul-kothari rahul-kothari moved this from In Progress to In Review in A3 Oct 30, 2023
@LHerskind LHerskind assigned LHerskind and unassigned rahul-kothari Feb 26, 2024
@LHerskind LHerskind moved this from In Review to Todo in A3 Feb 26, 2024
@LHerskind
Copy link
Contributor

By moving the nullifier emission into the account contract instead of where it is used, it is possible to add a single function on the account that allow its owner to emit that nullifier thereby cancelling both public and private authwits very cheaply.

It have a few consequences though. We need to alter the interface to instead of msg_hash take its components (or most of them).

pub fn spend_private_authwit(
    self,
    caller: AztecAddress,
    selector: FunctionSelector,
    args_hash: Field
) -> Field {
    let context = self.context.private.unwrap();
    let message_hash = inner_compute_authwit_message_hash(caller, context.msg_sender(), selector, args_hash);
    let valid_fn = self.is_valid_impl;
    assert(valid_fn(context, message_hash) == true, "Message not authorized by account");
    context.push_new_nullifier(message_hash, 0);
    IS_VALID_SELECTOR
}

We need to do so, since all the authwits from the same account now live in the same domain, so if we just blindly took in msg_hash it would be possible for one contract to emit a nullifier that would land on some other contracts pending authwit.

the structure is fairly simple, let the account contract constrain the contract that the action is to be executed on using its context.msg_sender(). This way a contract might mess with nullifiers, but only the ones that is important for itself.

@spalladino
Copy link
Collaborator Author

I like the account siloing authwits per app contract, but I'm not sure about forcing authwits to be computed from selector and msghash. Feels like an authwit could be a hash of pretty much anything, not just a function call.

@LHerskind
Copy link
Contributor

LHerskind commented Feb 27, 2024

Feels like an authwit could be a hash of pretty much anything, not just a function call.

We could change the definition from

H(caller, contract, selector, args_hash)

To instead be:

H(contract, message_hash)

And then have that for the call case, message_hash = H(caller, selector, args_hash) but could also have it be anything else really for covering non-calls (but still siloed with caller).

@spalladino
Copy link
Collaborator Author

I like that!

@LHerskind LHerskind linked a pull request Feb 27, 2024 that will close this issue
@LHerskind LHerskind moved this from Todo to In Review in A3 Feb 28, 2024
@github-project-automation github-project-automation bot moved this from In Review to Done in A3 Feb 28, 2024
LHerskind added a commit that referenced this issue Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-discussion Status: Still needs more discussion before work can start. T-feedback Type: recording user feedback
Projects
Archived in project
3 participants