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

Remove non-standard increaseAllowance and decreaseAllowance from ERC20 #4585

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Sep 7, 2023

(See discussion in issue)

Fixes #4583

PR Checklist

  • Tests
  • Documentation
  • Changeset entry (run npx changeset add)

@Amxx Amxx requested a review from frangio September 7, 2023 15:04
@changeset-bot
Copy link

changeset-bot bot commented Sep 7, 2023

🦋 Changeset detected

Latest commit: 804ec79

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openzeppelin-solidity Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@frangio frangio changed the title Remove non-standard functions (increasseAllowance and decreasseAllowance) from ERC20. Remove non-standard increaseAllowance and decreaseAllowance from ERC20 Sep 7, 2023
@frangio
Copy link
Contributor

frangio commented Sep 7, 2023

Should we add any documentation about the existence of increase/decreaseAllowance in previous versions?

@frangio
Copy link
Contributor

frangio commented Sep 12, 2023

Regarding documentation, my thinking was that we'd want to document for wallets that there is an alternative way to change allowance and any protections to approve should also be used with increaseAllowance. After discussing this, we don't think our documentation is the best place for this warning, and we're considering publishing a blog post about it.

@frangio frangio merged commit 60e3ffe into OpenZeppelin:master Sep 12, 2023
13 checks passed
@Amxx Amxx deleted the refactor/ERC20/remove-increasse-decreasse-allowance branch September 12, 2023 16:05
@mcmoodoo
Copy link

Should we add any documentation about the existence of increase/decreaseAllowance in previous versions?

We definitely shoud. I came here because the official open zeppelin docs say that the functions exist:
image

Yet, I couldn't find them in the source code, hence posting here!

@ernestognw
Copy link
Member

Should we add any documentation about the existence of increase/decreaseAllowance in previous versions?

We definitely shoud. I came here because the official open zeppelin docs say that the functions exist: image

Yet, I couldn't find them in the source code, hence posting here!

You're looking at v4 sir. Please change the documentation version to 5.x 😄

This was referenced Sep 10, 2024
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.

Discussion to remove increaseAllowance and decreaseAllowance from ERC20
4 participants