-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Update EIP-20: Renovate #6038
Update EIP-20: Renovate #6038
Conversation
File
|
Strongly against. We've never rewritten EIPs to fit the "new recommendations" and I don't think we should start. |
We have modified EIPs to fit new recommendations before (#5055). It's true that we've never rewritten them, though. |
After today's EIPIP, I think the consensus is that this PR changes more than the absolute minimum to comply with the current EIP-1 guidelines. For example, moving Simple Summary to |
That seems reasonable. I'll revert the extra (unneeded) changes. |
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
@SamWilsn mind re-reviewing? |
@eth-bot rerun |
EIPS/eip-20.md
Outdated
### The approve and transferFrom flow | ||
|
||
`approve` and `transferFrom` can be used in conjunction with one another to allow smart contracts to take an action if and only if a transfer succeeds. This enables use cases such as decentralized exchanges, which can trustlessly swap one token for another. | ||
|
||
## Implementation | ||
### Decimals is optional | ||
|
||
There are already plenty of ERC20-compliant tokens deployed on the Ethereum network. | ||
Different implementations have been written by various teams that have different trade-offs: from gas saving to improved security. | ||
Some tokens might wish to use a base unit other than $10^{-18}$, and sometimes it doesn't make sense to have a fraction of a token. However, not every token might reasonably need to specify `decimals`. Thus, `decimals` is optional. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should invent content that wasn't there in the original proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a non-normative required section. If I were the person writing ERC-20 and decided to include this field, that would be why. If the original authors consent/agree with the motivation provided then I see nothing wrong with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still more than is required to comply with current guidelines, and I don't think this qualifies as errata.
Co-authored-by: Sam Wilson <[email protected]>
Co-authored-by: Sam Wilson <[email protected]>
Co-authored-by: Sam Wilson <[email protected]>
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
Hey @SamWilsn, mind checking out the changes now? |
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
Oh lol that's a funny eip-review-bot bug. I'll get that fixed. |
The commit 9bfa57f (as a parent of 1a0a905) contains errors. |
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
This pull request was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment. |
WIP. Likely to be contentious.