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

AA-429: Create ERC-5792 "capabilities" ERC for Account Abstraction #12

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

forshtat
Copy link

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md

We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:

  • The PR edits only existing draft PRs.
  • The build passes.
  • Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside .
  • If matching on email address, the email address is the one publicly listed on your GitHub profile.

```typescript
type StaticPaymasterConfigurationCapabilityParams = Record<
`0x${string}`, // Chain ID
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this paymaster is version-specific: e.g. the app has to know whether the account is 4337v6, v7 or 7560, and based on that knowledge, select the right paymaster address

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That means we need something like a aa_getAccountDetails API, doesn't it?

ERCS/erc-xxxx.md Outdated Show resolved Hide resolved
Interface:

```typescript
type ValidityTimeRangeCapabilityParams = Record<

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the time-range:
a wallet validation can return a time range, which is compared to "now".
what is the usage scenario of this property?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't wallets want to allow dapps to specify the validity range? Seems like artificially limiting a feature we worked hard to add.

```

For Smart Contract Accounts that support multidimensional nonce values,
the wallet must specify these parameters during the actual on-chain execution of the batch.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wallet MAY specify a nonceKey, (if it doesn't specify, then the default 0 is used)
nonceSequence is trickier: it requires an eth_getNonce API

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need such an API. However, it can be hacked by calling the Nonce Manager directly, but that also gets very version - specific.


Identifier:

`setCodeForEOA`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think we need this at all - this is exactly what the wallet should do, and not the application. There is no way the wallet can protect the app from a malicious contract.

a wallet defines an account (address).
when the app performs a "sendCall", the wallet should decide how to implement it. if the network supports 7702, it should submit it as a 7702 transaction. if that account already has a 7702 contract deployed (in a previous transaction), then just submit a batch call to use it.

Interface:

```typescript
type AAGasParamsOverrideCapabilityParams = Record<

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a kind of low-level field override. better separate it to a different ERC.
NonceKey and Paymaster are generic, and don't expose the app directly to version-specific API
(paymaster does require protocol-specific paymaster, but it is encapsulated away from the app)
But this one is very specific. Also, it can't be used without a supporting view APIs to query these values

Co-authored-by: Dror Tirosh <dror.tirosh@gmail.com>
Copy link

The commit 51c9898 (as a parent of 61292e6) contains errors.
Please inspect the Run Summary for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants