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

spender term in ERC20WithPermit is incorrectly used #15

Open
cygnusv opened this issue Jun 14, 2022 · 1 comment · May be fixed by #18
Open

spender term in ERC20WithPermit is incorrectly used #15

cygnusv opened this issue Jun 14, 2022 · 1 comment · May be fixed by #18

Comments

@cygnusv
Copy link

cygnusv commented Jun 14, 2022

Current implementation of ERC20WithPermit uses the term spender to refer to the source account from which tokens are transferred from, while both the EIP (see https://eips.ethereum.org/EIPS/eip-20) and major implementations (like e.g. OpenZeppelin, https://github.com/OpenZeppelin/openzeppelin-contracts/blob/a55b7d13722e7ce850b626da2313f3e66ca1d101/contracts/token/ERC20/ERC20.sol#L163) use the same term to refer to an account that's been authorized to spend tokens on behalf of another. This can lead to confusion (it almost happened to me recently preparing a DAO proposal), and has an ABI-compatible fix.

mhluongo added a commit that referenced this issue Oct 25, 2024
... in ERC20WithPermit.

Closes #15
@mhluongo mhluongo linked a pull request Oct 25, 2024 that will close this issue
@mhluongo
Copy link
Member

@cygnusv if you have a minute I'd love a spot check!

#18

mhluongo added a commit that referenced this issue Oct 25, 2024
... in ERC20WithPermit.

Closes #15
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 a pull request may close this issue.

2 participants