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

[NES-261] [N1] [Suggestion] Risks of Token Compatibility #121

Merged
merged 4 commits into from
Dec 10, 2024
Merged

Conversation

ungaro
Copy link
Contributor

@ungaro ungaro commented Dec 10, 2024

What's new in this PR?

OZ SafeERC20 handles tokens that do not strictly adhere to the ERC20 standard by wrapping the transfer and transferFrom functions in a way that checks for success.

Also SafeERC20 handles the transfer failure by reverting.

  • check the boolean return values of ERC20 operations and revert the transaction if they fail.
  • at the same time allowing you to support some non-standard ERC20 tokens that don’t have boolean return values.

@ungaro ungaro requested a review from eyqs December 10, 2024 04:16
Copy link
Member

@eyqs eyqs left a comment

Choose a reason for hiding this comment

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

Ensure changes from N3 and N5 are kept - might be easier to start from scratch or cherry-pick 44aae29 directly onto main

@ungaro
Copy link
Contributor Author

ungaro commented Dec 10, 2024

i was just fixing that, will update the PR.

@ungaro ungaro requested a review from eyqs December 10, 2024 04:47
Copy link
Member

@eyqs eyqs left a comment

Choose a reason for hiding this comment

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

LGTM

@eyqs eyqs merged commit d9a09dd into main Dec 10, 2024
1 check passed
@eyqs eyqs deleted the alp/NES-261 branch December 10, 2024 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants