Skip to content

Commit

Permalink
Refactor Nonces from contract to library to mitigate nonces collision…
Browse files Browse the repository at this point in the history
… between Votes and ERC20Permit
  • Loading branch information
k06a committed Dec 2, 2022
1 parent e2d2ebc commit 484a7ac
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 17 deletions.
5 changes: 5 additions & 0 deletions contracts/governance/utils/IVotes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ interface IVotes {
*/
function getPastTotalSupply(uint256 blockNumber) external view returns (uint256);

/**
* @dev Returns the delegation nonce for `owner`.
*/
function delegationNonces(address owner) external view returns (uint256);

/**
* @dev Returns the delegate that `account` has chosen.
*/
Expand Down
13 changes: 11 additions & 2 deletions contracts/governance/utils/Votes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,17 @@ import "../../utils/math/SafeCast.sol";
*
* _Available since v4.5._
*/
abstract contract Votes is IVotes, Context, EIP712, Nonces {
abstract contract Votes is IVotes, Context, EIP712 {
using Checkpoints for Checkpoints.History;
using Nonces for Nonces.Data;

bytes32 private constant _DELEGATION_TYPEHASH =
keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");

mapping(address => address) private _delegation;
mapping(address => Checkpoints.History) private _delegateCheckpoints;
Checkpoints.History private _totalCheckpoints;
Nonces.Data private _nonces;

/**
* @dev Returns the current amount of votes that `account` has.
Expand Down Expand Up @@ -80,6 +82,13 @@ abstract contract Votes is IVotes, Context, EIP712, Nonces {
return _totalCheckpoints.latest();
}

/**
* @dev Returns the delegation nonce for `owner`.
*/
function delegationNonces(address owner) public view virtual override returns (uint256) {
return _nonces.nonces(owner);
}

/**
* @dev Returns the delegate that `account` has chosen.
*/
Expand Down Expand Up @@ -113,7 +122,7 @@ abstract contract Votes is IVotes, Context, EIP712, Nonces {
r,
s
);
require(nonce == _useNonce(signer), "Votes: invalid nonce");
require(nonce == _nonces.useNonce(signer), "Votes: invalid nonce");
_delegate(signer, delegatee);
}

Expand Down
12 changes: 10 additions & 2 deletions contracts/mocks/NoncesImpl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,16 @@ pragma solidity ^0.8.0;

import "../utils/Nonces.sol";

contract NoncesImpl is Nonces {
contract NoncesImpl {
using Nonces for Nonces.Data;

Nonces.Data private _nonces;

function nonces(address owner) public view returns (uint256) {
return _nonces.nonces(owner);
}

function useNonce(address owner) public {
super._useNonce(owner);
_nonces.useNonce(owner);
}
}
11 changes: 7 additions & 4 deletions contracts/token/ERC20/extensions/ERC20Permit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import "../../../utils/Nonces.sol";
*
* _Available since v3.4._
*/
abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {
abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
using Nonces for Nonces.Data;

// solhint-disable-next-line var-name-mixedcase
bytes32 private constant _PERMIT_TYPEHASH =
keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
Expand All @@ -31,6 +33,7 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {
*/
// solhint-disable-next-line var-name-mixedcase
bytes32 private _PERMIT_TYPEHASH_DEPRECATED_SLOT;
Nonces.Data private _nonces;

/**
* @dev Initializes the {EIP712} domain separator using the `name` parameter, and setting `version` to `"1"`.
Expand All @@ -53,7 +56,7 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {
) public virtual override {
require(block.timestamp <= deadline, "ERC20Permit: expired deadline");

bytes32 structHash = keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline));
bytes32 structHash = keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _nonces.useNonce(owner), deadline));

bytes32 hash = _hashTypedDataV4(structHash);

Expand All @@ -66,8 +69,8 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {
/**
* @dev See {IERC20Permit-nonces}.
*/
function nonces(address owner) public view virtual override(IERC20Permit, Nonces) returns (uint256) {
return super.nonces(owner);
function nonces(address owner) public view virtual override returns (uint256) {
return _nonces.nonces(owner);
}

/**
Expand Down
14 changes: 8 additions & 6 deletions contracts/utils/Nonces.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,27 @@ import "./Counters.sol";
/**
* @dev Provides tracking nonces for addresses. Nonces will only increment.
*/
abstract contract Nonces {
library Nonces {
using Counters for Counters.Counter;

mapping(address => Counters.Counter) private _nonces;
struct Data {
mapping(address => Counters.Counter) _nonces;
}

/**
* @dev Returns an address nonce.
*/
function nonces(address owner) public view virtual returns (uint256) {
return _nonces[owner].current();
function nonces(Data storage self, address owner) internal view returns (uint256) {
return self._nonces[owner].current();
}

/**
* @dev Consumes a nonce.
*
* Returns the current value and increments nonce.
*/
function _useNonce(address owner) internal virtual returns (uint256 current) {
Counters.Counter storage nonce = _nonces[owner];
function useNonce(Data storage self, address owner) internal returns (uint256 current) {
Counters.Counter storage nonce = self._nonces[owner];
current = nonce.current();
nonce.increment();
}
Expand Down
2 changes: 1 addition & 1 deletion test/governance/utils/Votes.behavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const version = '1';
function shouldBehaveLikeVotes () {
describe('run votes workflow', function () {
it('initial nonce is 0', async function () {
expect(await this.votes.nonces(this.account1)).to.be.bignumber.equal('0');
expect(await this.votes.delegationNonces(this.account1)).to.be.bignumber.equal('0');
});

it('domain separator', async function () {
Expand Down
2 changes: 1 addition & 1 deletion test/token/ERC20/extensions/ERC20Votes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract('ERC20Votes', function (accounts) {
});

it('initial nonce is 0', async function () {
expect(await this.token.nonces(holder)).to.be.bignumber.equal('0');
expect(await this.token.delegationNonces(holder)).to.be.bignumber.equal('0');
});

it('domain separator', async function () {
Expand Down
2 changes: 1 addition & 1 deletion test/token/ERC20/extensions/ERC20VotesComp.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract('ERC20VotesComp', function (accounts) {
});

it('initial nonce is 0', async function () {
expect(await this.token.nonces(holder)).to.be.bignumber.equal('0');
expect(await this.token.delegationNonces(holder)).to.be.bignumber.equal('0');
});

it('domain separator', async function () {
Expand Down

0 comments on commit 484a7ac

Please sign in to comment.