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 ERC20 stablecoin extensions #14

Merged
merged 17 commits into from
Oct 9, 2024
Merged

Add ERC20 stablecoin extensions #14

merged 17 commits into from
Oct 9, 2024

Conversation

cairoeth
Copy link
Collaborator

Add contracts ERC20 extensions requested:

  • ERC20Allowlist
  • ERC20Blocklist
  • ERC20Custodian

@cairoeth cairoeth requested a review from a team as a code owner September 26, 2024 00:16
ernestognw
ernestognw reviewed Sep 27, 2024
View reviewed changes
Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

On a general note, we might want to call the User an Account instead.

I left a bunch of comments we can start iterating with. Looking good so far.

contracts/token/ERC20/extensions/ERC20Allowlist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Allowlist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Allowlist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Allowlist.sol Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Allowlist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Blocklist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Blocklist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Blocklist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Blocklist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Blocklist.sol Outdated Show resolved Hide resolved
cairoeth and others added 9 commits October 3, 2024 11:07
@cairoeth @ernestognw
Apply suggestions from code review
0f80bb3 
Co-authored-by: Ernesto García <ernestognw@gmail.com>
@cairoeth @ernestognw
Apply suggestions from code review
b5e609c 
Co-authored-by: Ernesto García <ernestognw@gmail.com>
@cairoeth @ernestognw
Apply suggestions from code review
f293151 
Co-authored-by: Ernesto García <ernestognw@gmail.com>
@cairoeth @ernestognw
Apply suggestions from code review
d9fb15a 
Co-authored-by: Ernesto García <ernestognw@gmail.com>
@cairoeth @ernestognw
Apply suggestions from code review
16b6a3b 
Co-authored-by: Ernesto García <ernestognw@gmail.com>
@cairoeth @ernestognw
Apply suggestions from code review
a23c051 
Co-authored-by: Ernesto García <ernestognw@gmail.com>
@cairoeth @ernestognw
Apply suggestions from code review
820fc1b 
Co-authored-by: Ernesto García <ernestognw@gmail.com>
@cairoeth
Update allowlist based on feedback
6c66f8c
@cairoeth
Update other extensions and tests
24d46e8
@cairoeth cairoeth requested a review from ernestognw October 3, 2024 19:54
ernestognw
ernestognw reviewed Oct 9, 2024
View reviewed changes
Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

Almost there! I don't want to be too strict since it's for the community repo, but there are a couple of things it may be worth discussing.

contracts/token/ERC20/extensions/ERC20Allowlist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Allowlist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Blocklist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Blocklist.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/extensions/ERC20Custodian.sol Outdated Show resolved Hide resolved
ernestognw
ernestognw approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@cairoeth cairoeth merged commit 4ff12b0 into master Oct 9, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ernestognw ernestognw ernestognw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cairoeth @ernestognw