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

[4337] User Operation with Custom Singleton Signers #159

Merged
merged 7 commits into from
Nov 24, 2023

Conversation

nlordell
Copy link
Collaborator

@nlordell nlordell commented Nov 17, 2023

Fixes #148

This PR introduces an E2E test using the official bundler where we deploy a Safe that uses a custom (and silly 🤪) signing scheme over ERC-1271. Specifically, the signing scheme is deployed as a singleton contract that can be shared by multiple Safes using a mapping(safe => data) to differentiate between signers. In theory, this could be adapted to, for example, checking a signature using P256 curve in order to integrate with WebAuthn, and is meant to “prove” that the Safe with the ERC-4337 module would work with existing WebAuthn signer implementations.

The actual implementation is a little convoluted for a few reasons. Namely, ERC-4337 only allows a single CREATE2 per user operation that MUST result in the userOp.sender account. Because of this, deploying ad-hoc signers in the Safe.setup call is not possible, and we have instead opted for a “singleton signer” pattern, where the logic for verifying signatures is shared and a mapping is used to store per-Safe context relevant to signature verification.

Additionally, In order for this to work with ERC-4337, a staked factory is needed (as accessing associated storage for user operations with initCode require a factory). Unfortunately, staked factories add all kinds of complexities:

  • Currently, the staked factory contract is quite dumb (can only stake, but can’t unstake). A full staked factory contract requires us to iron out some requirements and make a proper implementation with documentation and tests (out of scope for this PR and issue, and a non-trivial amount of work)
  • Staking is determined per bundler, so while some amount of stake may be sufficient for a bundler, it may not be sufficient for all (which adds additional off-chain management dependencies).

For these reasons, I would recommend exploring #149.

@nlordell nlordell requested a review from a team as a code owner November 17, 2023 11:08
@nlordell nlordell requested review from rmeissner, akshay-ap, mmv08 and remedcu and removed request for a team November 17, 2023 11:08
@coveralls
Copy link

coveralls commented Nov 20, 2023

Pull Request Test Coverage Report for Build 6967672785

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 6967652298: 0.0%
Covered Lines: 29
Relevant Lines: 29

💛 - Coveralls

@nlordell nlordell force-pushed the signer-singleton branch 2 times, most recently from 850fee4 to 451f639 Compare November 22, 2023 10:27
@nlordell nlordell changed the title [Draft] Add Staked Safe Factory Add Staked Safe Factory Nov 22, 2023
@nlordell nlordell requested review from akshay-ap, mmv08 and remedcu and removed request for akshay-ap, mmv08 and remedcu November 22, 2023 13:04
@nlordell nlordell changed the title Add Staked Safe Factory [4337] User Operation with Custom Signing Scheme Nov 22, 2023
@nlordell nlordell marked this pull request as draft November 22, 2023 13:05
@nlordell nlordell marked this pull request as ready for review November 22, 2023 13:06
@nlordell nlordell changed the title [4337] User Operation with Custom Signing Scheme [4337] User Operation with Custom Singleton Signers Nov 23, 2023

contract TestSingletonSigner is ISignatureValidator {
struct Key {
uint256 _dummy;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is just to verify this model would work for P256 signatures (which require 2 uint256 slots, one for the X and one for the Y coordinate of the public key - which is a point on the curve).

@nlordell nlordell merged commit 6d26f34 into master Nov 24, 2023
4 checks passed
@nlordell nlordell deleted the signer-singleton branch November 24, 2023 10:37
@github-actions github-actions bot locked and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[4337] Evaluate Singleton Signers + Staked Factory
3 participants