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

EIP-1271 like functionality #1913

Closed
Tracked by #1743
LHerskind opened this issue Aug 31, 2023 · 5 comments · Fixed by #1935
Closed
Tracked by #1743

EIP-1271 like functionality #1913

LHerskind opened this issue Aug 31, 2023 · 5 comments · Fixed by #1935
Assignees
Labels
A-internal-devex Area: Relates to the devex of internal teams building Aztec. T-feature-request Type: Adding a brand new feature (not to be confused with improving an existing feature).

Comments

@LHerskind
Copy link
Contributor

LHerskind commented Aug 31, 2023

Having EIP-1271 like functionality in the account contracts will allow better flow of transactions that wish to do an action on behalf of someone else.

This lend itself especially nicely for private transactions into contracts, where it is often desired for the contract to "pull" funds into itself on behalf on the user rather than the user pushing funds in (often accounting is easier then).

While EIP-1271 is specified around signatures, and we are not necessarily using signatures in accounts, we merely desire the ability for ask a contract if a specific action is supported.


A discussion around the issue is available in https://discourse.aztec.network/t/account-guardians-eip1271-and-recursive-proofs/675.


Proposed solution:

We add an is_valid STATIC function in the account contracts. This function is to return 0x29d25ca9 if the account acknowledge that message_hash is valid by whatever logic the account implements. Same logic should be usable for the entrypoint of the account.

// static only (noir currently don't have a way to convey it)
fn is_valid(message_hash: Field) -> Field {
  // validation...

  0x29d25ca9 // bytes4(keccak256(is_valid(field))
}

The validation can be based on storage written to in the contract itself (but not nullified by the is_valid itself).
An alternative, which I think seems nicer hence it would most often be used by the user himself to deposit; is to use an oracle to fetch the necessary data for validation. This way the is_valid can follow the same function signature independently of the account verification implementation, which makes it much easier to integrate.

// static only (noir currently don't have a way to convey it)
fn is_valid(message_hash: Field) -> Field {
  let oracle_data = is_valid_oracle();
  // validation...

  0x29d25ca9 // bytes4(keccak256(is_valid(field))
}

Implementation quirks

To implement this, we need to add a new oracle to the acir simulator, and some way to insert values into a store that will be used for this oracle. Practically, our current account contracts are using signature schemes, so we would need to generate a valid signature and insert it into a db which the oracle could read it from.

@LHerskind LHerskind self-assigned this Aug 31, 2023
@LHerskind LHerskind added this to the 📢 Initial Public Sandbox Release milestone Aug 31, 2023
@spalladino
Copy link
Collaborator

Love it.

This function is to return 0x29d25ca9

I understand that the need for these magic values in EVM-based standards is due to potential selector clashes, as in you don't want an unrelated function to be answering "true" to a request for approving a transfer. Should we consider increasing selector length (#1050), so we don't have this annoying problem in Aztec?

The validation can be based on storage written to in the contract itself (but not nullified by the is_valid itself).

So this method would only be able to read immutable storage or slow updates tree, right? I agree the oracle flow will be the most used one, but that option will probably need to load a pubkey from storage to validate the result.

@LHerskind
Copy link
Contributor Author

Love it.

Good to hear. Got a PR with it mostly working with wallets and the lending test case.

For the return value, I think we can live with just the 4 bytes since you would seldom return the selector itself so don't think it would be hit that often, but ye should change with the selector generally.

So this method would only be able to read immutable storage or slow updates tree, right? I agree the oracle flow will be the most used one, but that option will probably need to load a pubkey from storage to validate the result.

For private flow ye, think that is correct, but for public calls you could just read public state. Not sure around naming for private and public though, feels quite clunky to have two different names one for public and one for private, but that seems to be how we are to do it currently.

@spalladino
Copy link
Collaborator

For the return value, I think we can live with just the 4 bytes since you would seldom return the selector itself so don't think it would be hit that often, but ye should change with the selector generally.

I wasn't referring to extending the return value. I meant just returning "true" or "false" from the function, rather than a magic number. Feels more elegant to me.

Not sure around naming for private and public though,

ChatGPT suggested shared or hybrid. The latter is not too bad. Or we can go with something more grandiloquent, like universal.

@LHerskind
Copy link
Contributor Author

I wasn't referring to extending the return value.

I gotta learn to read then.

ChatGPT suggested shared or hybrid. The latter is not too bad. Or we can go with something more grandiloquent, like universal.

Might get really odd if the same function is used in the account contract as it could have different logic for public and private 🤷, might be best to just have one or another.

Something I think is quite interesting though, is that it is possible for us to use the same flow on the entrypoint such that no authentication data is passed in there, making it possible to have stable function signatures for is_valid and entrypoint for multiple different account contracts, which might make integrations easier as you don't have to target different functions.

@spalladino
Copy link
Collaborator

Something I think is quite interesting though, is that it is possible for us to use the same flow on the entrypoint such that no authentication data is passed in there, making it possible to have stable function signatures for is_valid and entrypoint for multiple different account contracts, which might make integrations easier as you don't have to target different functions.

Goddamn I love where this is going.

@LHerskind LHerskind added A-internal-devex Area: Relates to the devex of internal teams building Aztec. T-feature-request Type: Adding a brand new feature (not to be confused with improving an existing feature). labels Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-internal-devex Area: Relates to the devex of internal teams building Aztec. T-feature-request Type: Adding a brand new feature (not to be confused with improving an existing feature).
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants