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.

Sign up for GitHub

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

Jump to bottom

Add authorization with signature #114

Merged
merged 34 commits into from
Jul 31, 2023
Merged

Add authorization with signature #114

merged 34 commits into from
Jul 31, 2023

Conversation

MerlinEgalite
Copy link
Contributor

Fix #111

@MerlinEgalite MerlinEgalite marked this pull request as ready for review July 11, 2023 12:46
@MerlinEgalite MerlinEgalite linked an issue Jul 11, 2023 that may be closed by this pull request
Approve account with signature #111
Closed
makcandrov
makcandrov reviewed Jul 11, 2023
View reviewed changes
Copy link
Contributor

@makcandrov makcandrov left a comment

Choose a reason for hiding this comment

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

  1. What are the use cases of this approval system insted of the traditional one?
  2. I think the signature logic should be abstracted in a contract Blue would inherit from

@MerlinEgalite
Copy link
Contributor Author

  1. What are the use cases of this approval system insted of the traditional one?
  2. I think the signature logic should be abstracted in a contract Blue would inherit from
  1. Gasless approval for EOAs, which allows to offer a complete gasless experience for EOAs using a bulker (except token approvals ofc). This is what we did for ma3 that we'll be release soon.
  2. We could but for now there's diverging views on this topic, so I'm sticking to the current way of doing. I can update it if the majority wants it as well.

Rubilmax
Rubilmax reviewed Jul 11, 2023
View reviewed changes
src/Blue.sol Outdated Show resolved Hide resolved
src/Blue.sol Outdated Show resolved Hide resolved
src/Blue.sol Outdated Show resolved Hide resolved
@Rubilmax Rubilmax mentioned this pull request Jul 11, 2023
Closed
Rubilmax
Rubilmax previously approved these changes Jul 13, 2023
View reviewed changes
Jean-Grimal
Jean-Grimal previously approved these changes Jul 13, 2023
View reviewed changes
makcandrov
makcandrov previously approved these changes Jul 14, 2023
View reviewed changes
@MerlinEgalite MerlinEgalite removed the request for review from PaulFrambot July 14, 2023 06:18
@MerlinEgalite MerlinEgalite mentioned this pull request Jul 14, 2023
Closed
Base automatically changed from feat/approval to main July 17, 2023 16:31
@MerlinEgalite MerlinEgalite dismissed stale reviews from makcandrov, Jean-Grimal, and Rubilmax July 17, 2023 16:31

The base branch was changed.

Rubilmax
Rubilmax previously approved these changes Jul 21, 2023
View reviewed changes
Jean-Grimal
Jean-Grimal previously approved these changes Jul 21, 2023
View reviewed changes
@MathisGD MathisGD dismissed stale reviews from Jean-Grimal, makcandrov, and pakim249CAL via 7bf9f15 July 31, 2023 14:19
MathisGD
MathisGD previously approved these changes Jul 31, 2023
View reviewed changes
MerlinEgalite
MerlinEgalite commented Jul 31, 2023
View reviewed changes
Copy link
Contributor Author

@MerlinEgalite MerlinEgalite left a comment

Choose a reason for hiding this comment

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

Can't approve but LGTM

makcandrov
makcandrov previously approved these changes Jul 31, 2023
View reviewed changes
Jean-Grimal
Jean-Grimal previously approved these changes Jul 31, 2023
View reviewed changes
@MathisGD MathisGD self-assigned this Jul 31, 2023
Rubilmax
Rubilmax previously approved these changes Jul 31, 2023
View reviewed changes
@MathisGD MathisGD dismissed stale reviews from Rubilmax, Jean-Grimal, makcandrov, and themself via 588a852 July 31, 2023 15:59
@MathisGD MathisGD merged commit 152087d into main Jul 31, 2023
@MathisGD MathisGD deleted the feat/approval-with-sig branch July 31, 2023 21:10
@MathisGD MathisGD mentioned this pull request Aug 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@makcandrov makcandrov makcandrov approved these changes

@Jean-Grimal Jean-Grimal Jean-Grimal approved these changes

@MathisGD MathisGD MathisGD approved these changes

@pakim249CAL pakim249CAL pakim249CAL approved these changes

@Rubilmax Rubilmax Rubilmax approved these changes

@QGarchery QGarchery Awaiting requested review from QGarchery

Assignees

@MathisGD MathisGD

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

isApproved helper function naming Approve account with signature
6 participants
@MerlinEgalite @MathisGD @Rubilmax @pakim249CAL @Jean-Grimal @makcandrov