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

Make proper use of the term spender... #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mhluongo
Copy link
Member

... in ERC20WithPermit.

Closes #15

... in ERC20WithPermit.

Closes #15
@mhluongo mhluongo force-pushed the erc-20-compliant-terminology branch from ab8ecd3 to f8301fa Compare October 25, 2024 21:29
@cygnusv
Copy link

cygnusv commented Oct 25, 2024

Looks good, but tests need a pass too. E.g.

context("when the spender has enough balance", () => {

This line uses the term "spender" to refer to the owner. Other test cases of transferOf() mention a "sender", but it's not clear if they are referring to the owner or the spender.

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.

spender term in ERC20WithPermit is incorrectly used
2 participants