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

User story: Execute userOp with Passkey and paymaster #357

Merged
merged 18 commits into from
Apr 12, 2024

Conversation

akshay-ap
Copy link
Member

Fixes #276

Todo:
Evaluate if TestPaymaster contract is needed with validation logic. If not needed, replace with a MockContract instance

Changes in PR:

  • Add a test case showcasing Safe deployment with Passkey signer using Paymaster
  • Add a test case showcasing userOp execution with existing Safe (with Passkey signer) using Paymaster

@akshay-ap akshay-ap self-assigned this Apr 4, 2024
@akshay-ap akshay-ap requested a review from a team as a code owner April 4, 2024 10:50
@akshay-ap akshay-ap requested review from nlordell, mmv08 and remedcu and removed request for a team April 4, 2024 10:50
@akshay-ap akshay-ap marked this pull request as draft April 4, 2024 10:50
@akshay-ap akshay-ap force-pushed the feature-276-user-story-paymaster branch from b60675d to 5d9e53f Compare April 4, 2024 10:56
@nlordell
Copy link
Collaborator

nlordell commented Apr 4, 2024

I noticed that we had 2 user story directories and that the file naming was inconsistent. I fixed it here: #363 , but this PR does not follow the "standard" either, can you fix it as well?

@akshay-ap akshay-ap force-pushed the feature-276-user-story-paymaster branch from 34fcef7 to 827b321 Compare April 4, 2024 15:55
@coveralls
Copy link

coveralls commented Apr 4, 2024

Pull Request Test Coverage Report for Build 8649185035

Details

  • 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 81.633%

Totals Coverage Status
Change from base Build 8567740250: 0.0%
Covered Lines: 95
Relevant Lines: 103

💛 - Coveralls

@akshay-ap akshay-ap marked this pull request as ready for review April 8, 2024 08:26
@akshay-ap akshay-ap requested a review from nlordell April 8, 2024 08:27
@akshay-ap akshay-ap requested a review from remedcu April 8, 2024 12:28
Co-authored-by: Mikhail <16622558+mmv08@users.noreply.github.com>
akshay-ap and others added 3 commits April 10, 2024 14:17
Co-authored-by: Nicholas Rodrigues Lordello <nick@safe.global>
Co-authored-by: Nicholas Rodrigues Lordello <nick@safe.global>
@akshay-ap akshay-ap force-pushed the feature-276-user-story-paymaster branch from be4ccf5 to af92a4a Compare April 11, 2024 15:21
Comment on lines 249 to 252
const publicKey = decodePublicKey(credential.response)
await signerFactory.createSigner(publicKey.x, publicKey.y, verifier.target)
const signer = await signerFactory.getSigner(publicKey.x, publicKey.y, verifier.target)

Copy link
Member

Choose a reason for hiding this comment

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

seems like both tests deploy a signer right after the setup, can it be a part of the general setup?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree it might be nice (so the tests assume an existing signer).

@nlordell
Copy link
Collaborator

Approved to reduce back and forth, but agree with #357 (comment)

@akshay-ap akshay-ap merged commit 74d5a38 into main Apr 12, 2024
9 checks passed
@akshay-ap akshay-ap deleted the feature-276-user-story-paymaster branch April 12, 2024 11:41
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
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.

[USER STORY] Passkey Transaction With Paymaster
5 participants