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

Contract interfaces proposal #45

Open
JackHamer09 opened this issue Sep 24, 2024 · 3 comments
Open

Contract interfaces proposal #45

JackHamer09 opened this issue Sep 24, 2024 · 3 comments
Assignees

Comments

@JackHamer09
Copy link
Member

JackHamer09 commented Sep 24, 2024

Session

Types

struct TokenSpendLimit {
    address token;
    uint256 limit;
}

struct SessionData {
    address publicKey;
    TokenSpendLimit[] spendLimits;
    uint256 expiresAt;
}

Write Methods

  • setSessionKey(SessionData data)
    Adds a session key if it doesn't exist, or rewrites its data if it does exist.
  • setSessionKeys(SessionData[] data)
    Same as above but adds multiple sessions at once.

Read Methods

  • getSessionKeys() returns (SessionData[])
    Returns all registered session keys.

  • getSessionKeyData(address sessionPublicKey) returns (SessionData)
    Returns session key data for the given session public key.

  • getRemainingSpendLimit(address sessionPublicKey, address token) returns (uint256)
    Returns the remaining spend limit for a specific token under the session key (total - used).


Passkey

Types

type Passkey is bytes;

Write Methods

  • addPasskey(Passkey passkeyPublicKey)
    Adds a new passkey signer.
  • addPasskeys(Passkey[] passkeyPublicKeys)
    Same as above but adds multiple signers at once.

Read Methods

  • getPasskeys() returns (Passkey[])
    Returns all registered passkeys.

Account Deployment via Factory

Currently, we pass many different contract addresses. Most users are fine with default ones. To improve DevEx, we should provide default recommended addresses automatically from the factory contract to avoid having users store all these addresses locally.

Write Methods

  • deployAccount(bytes32 salt, Passkey passkeyPublicKey, address initialModuleAddress, bytes initialModuleData)
    Deploys an account using the provided salt, passkey public key, initial module address, and initial module data.
    • Would be even better if passkey session module is included in the account by default and we don't have to pass these additional initialModuleAddress parameter and instead of initialModuleData we have SessionData[] initialSessions.
    • If initial module is session and initial session was provided - it should be active immediately (right now we have to run another function to set it up).

Signatures

We should aim to avoid including contract addresses in signatures as much as possible. As well as use account address for all account related configuration (like adding a session key). This will allow us to store only the user's account address, simplifying the user experience and improving developer experience.

@cpb8010 cpb8010 added the enhancement New feature or request label Sep 24, 2024
cpb8010 added a commit that referenced this issue Sep 24, 2024
Everything upstream of this is now broken for init, and actual
validation and timestamp checking is still disconnected, but the data is
setup in a nicer way.

This also removes the spend-limit per passkey in exchange for a global
spend limit, which is easier for now, but we'll want the per-key limits
back later.

Links to #45
@MexicanAce
Copy link
Contributor

@JackHamer09 For this type of suggestion, it almost feels like documentation. I wonder if the majority of the content should be moved into a Markdown file and we open a PR to add it to the docs/sdk directory?

This way, if we had comments/feedback about specific parts of this proposal (ex: Questions about the Read methods), it's much easier to manage with in-line comment threads within PRs. Plus, we could "finalize" these interfaces by merging the PR and keeping this documentation for our users/future devs to reference. Thoughts?

@JackHamer09
Copy link
Member Author

JackHamer09 commented Sep 25, 2024

@MexicanAce not sure. We should definitely add a docs for contract interfaces but this just focuses solely on the proposal for few contract methods and not on all of the contract interfaces as a documentation should do.
But I agree it might be easier to discuss when we can have separate threads for each line.

cpb8010 added a commit that referenced this issue Sep 25, 2024
Everything upstream of this is now broken for init, and actual
validation and timestamp checking is still disconnected, but the data is
setup in a nicer way.

This also removes the spend-limit per passkey in exchange for a global
spend limit, which is easier for now, but we'll want the per-key limits
back later.

Links to #45
@JackHamer09 JackHamer09 self-assigned this Oct 2, 2024
@cpb8010
Copy link
Contributor

cpb8010 commented Oct 8, 2024

Session Keys

So this is now updated for session keys! Thanks for the feedback!

Passkey

For passkey I've updated it to be a combination of bytes and a domain string, so that part is already different to better support cross-origin security checks and the ability to reverse the lookup from domain to key.
The standard interface is named "addValidationKey" which is more generic than the specific passkey support. So while we could add a specific name for addPasskey(s) to the module, it's use would be very limited.

Factory

The factory helper is will likely be updated here: #82 (as part of use a single list of modules instead of a list per type). This will slightly help during deployment to reduce the amount of data needed to deploy an account with either a passkey and/or a session key.

We'll still keep the most generic interface for future flexibility, as partners have expressed interest that they include different modules by default on creation (and we wouldn't encode those into the factory).

Signatures

For signatures it's unlikely we'll be able to remove the validation address as way to determine the signature type. The other options we could use are more gas inefficient or significantly reduce the flexibility of the validations.

The major problem we have is that the signatures (k1 signatures) are generally indistinguishable until you spend a non-trivial amount of gas to validate them. If you just brute force all your possible signature methods you'll have awful overhead when you have lots of different validation modules installed.

Attempt all validations for keys matching some length or magic number checks

This is the guess and check approach where each validation module need to expose some heuristic that tries to guess "could this be a signature for me" that includes length and some easy to compute formatting. There are some validation modules that have identical key formats (k1 signatures), so we're basically guaranteeing some extra work for each validation.

Include custom data in the transaction

This works for some interfaces where the whole transaction is available, but many isValidSignature standards (https://eips.ethereum.org/EIPS/eip-1271) only include the signature and the hash of the signed data. Losing the ability to have a standard compliant signature will make adoption more difficult. If the problem is getting the address of the validation module in the first place it doesn't even solve the usability issue.

Only support one validation module at a time per account

This avoids the "which validation am I using" question because there's only one. It then requires that the validation module author includes all signatures the account wants to use or then implements it's own version of signature type splitting.

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

No branches or pull requests

4 participants