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

Feature request for ERC20 #2409

Closed
ducquangkstn opened this issue Nov 18, 2020 · 6 comments
Closed

Feature request for ERC20 #2409

ducquangkstn opened this issue Nov 18, 2020 · 6 comments

Comments

@ducquangkstn
Copy link

🧐 Motivation

  • when user approve infinity to another address, transferFrom should not reset (this will save us about 5000 gas)

📝 Details

https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol#L74

  1. Another feature request is can we implement an internal _rename function for ERC20

🧐 Motivation
Because _name and _symbol of ERC20 are private. So I can not change name outside constructor. For token with permission (Ownable), owner can not change name of token so adding an internal _rename function will enable it
For example, when ppl rename from DAI to SAI

📝 Details

function _rename(string memory name_, string memory symbol_) internal {
        _name = name_;
        _symbol = symbol_;
}
@Skyge
Copy link
Contributor

Skyge commented Nov 18, 2020

When you deploy an ERC20 token, it is not expected to change name and symbol, and as for Maker changed SAI to DAI, it does not simply change the token name, they are different token, so they need a new ERC20 token.

@ducquangkstn
Copy link
Author

No, they rename old DAI to SAI and create a new token named DAI

@Skyge
Copy link
Contributor

Skyge commented Nov 18, 2020

Okay, but I still do not think it is a good idea to change token name and symbol. This is not a common case.

@ducquangkstn
Copy link
Author

Another use case would be this

https://github.com/compound-finance/compound-protocol/blob/master/contracts/CToken.sol#L51

The token name will be defined in an init function

@Skyge
Copy link
Contributor

Skyge commented Nov 18, 2020

Emmm, it is a init function to set a name rather than reset the name, and it can only be called once.

@abcoathup
Copy link
Contributor

Hi @ducquangkstn,

Thanks for the suggestions, it is really appreciated.

I recommend adding your use case to the following issues:

  1. Add ERC20 permit() function: EIP-2612: Add ERC20 permit() function #2206
  2. Virtual view functions: Virtual view functions #2154 (comment)

I believe this issue duplicates the above (especially once you add your use case), so I am closing this issue.

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

No branches or pull requests

3 participants