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

Add an internal _useNonce(address) function in ERC20Permit #2564

Closed
Amxx opened this issue Mar 5, 2021 · 0 comments · Fixed by #2565
Closed

Add an internal _useNonce(address) function in ERC20Permit #2564

Amxx opened this issue Mar 5, 2021 · 0 comments · Fixed by #2565

Comments

@Amxx
Copy link
Collaborator

Amxx commented Mar 5, 2021

🧐 Motivation
The ERC20Permit contract is great in that it provide a building block which is both ERC20 & ERC712 compliant. It also include some features, like nonce management, which are not by default in ERC712 but are required to do proper signature checking in the permit function.

However, when wanting to expand the ERC712 compliant features beyound the simple permit function, it is not possible to increment the counter (private slot). This means that if I wanted to add a transferFromBySig or any other feature using ERC712, I would have to use an independent nonce. This is really not great.

📝 Details
A simple, yet effective, solution is to add an internal function that lets you "consumed" a nonce:

function _useNonce(address owner) internal  virtual returns (uint256 current) {
    Counters.Counter storage nonce = _nonces[owner];
    current = nonce.current();
    nonce.increment();
}

Since this function only increment, it doesn't create any security issue that would allow signed messages to be replayed.

@Amxx Amxx changed the title add an internal _incrementNonces(address) function in ERC20Permit Add an internal _incrementNonces(address) function in ERC20Permit Mar 5, 2021
@frangio frangio changed the title Add an internal _incrementNonces(address) function in ERC20Permit Add an internal _useNonce(address) function in ERC20Permit Mar 5, 2021
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.

1 participant