Skip to content

Commit

Permalink
add testing of new governor against legacy voting tokens
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx committed Jan 12, 2023
1 parent 0981e95 commit bcedb7e
Show file tree
Hide file tree
Showing 12 changed files with 400 additions and 83 deletions.
270 changes: 270 additions & 0 deletions contracts/mocks/token/ERC20VotesLegacyMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../../token/ERC20/extensions/ERC20Permit.sol";
import "../../utils/math/Math.sol";
import "../../governance/utils/IVotes.sol";
import "../../utils/math/SafeCast.sol";
import "../../utils/cryptography/ECDSA.sol";

/**
* @dev Copied from the master branch at commit 86de1e8b6c3fa6b4efa4a5435869d2521be0f5f5
* A reverting `clock()` function is added to please the compiler
*/
abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit {
struct Checkpoint {
uint32 fromBlock;
uint224 votes;
}

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

mapping(address => address) private _delegates;
mapping(address => Checkpoint[]) private _checkpoints;
Checkpoint[] private _totalSupplyCheckpoints;

/**
* @dev added to please the compiler, reverts (like if it was absent)
*/
function clock() public pure override returns (uint256) {
revert();
}

/**
* @dev Get the `pos`-th checkpoint for `account`.
*/
function checkpoints(address account, uint32 pos) public view virtual returns (Checkpoint memory) {
return _checkpoints[account][pos];
}

/**
* @dev Get number of checkpoints for `account`.
*/
function numCheckpoints(address account) public view virtual returns (uint32) {
return SafeCast.toUint32(_checkpoints[account].length);
}

/**
* @dev Get the address `account` is currently delegating to.
*/
function delegates(address account) public view virtual override returns (address) {
return _delegates[account];
}

/**
* @dev Gets the current votes balance for `account`
*/
function getVotes(address account) public view virtual override returns (uint256) {
uint256 pos = _checkpoints[account].length;
unchecked {
return pos == 0 ? 0 : _checkpoints[account][pos - 1].votes;
}
}

/**
* @dev Retrieve the number of votes for `account` at the end of `blockNumber`.
*
* Requirements:
*
* - `blockNumber` must have been already mined
*/
function getPastVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) {
require(blockNumber < block.number, "ERC20Votes: block not yet mined");
return _checkpointsLookup(_checkpoints[account], blockNumber);
}

/**
* @dev Retrieve the `totalSupply` at the end of `blockNumber`. Note, this value is the sum of all balances.
* It is NOT the sum of all the delegated votes!
*
* Requirements:
*
* - `blockNumber` must have been already mined
*/
function getPastTotalSupply(uint256 blockNumber) public view virtual override returns (uint256) {
require(blockNumber < block.number, "ERC20Votes: block not yet mined");
return _checkpointsLookup(_totalSupplyCheckpoints, blockNumber);
}

/**
* @dev Lookup a value in a list of (sorted) checkpoints.
*/
function _checkpointsLookup(Checkpoint[] storage ckpts, uint256 blockNumber) private view returns (uint256) {
// We run a binary search to look for the earliest checkpoint taken after `blockNumber`.
//
// Initially we check if the block is recent to narrow the search range.
// During the loop, the index of the wanted checkpoint remains in the range [low-1, high).
// With each iteration, either `low` or `high` is moved towards the middle of the range to maintain the invariant.
// - If the middle checkpoint is after `blockNumber`, we look in [low, mid)
// - If the middle checkpoint is before or equal to `blockNumber`, we look in [mid+1, high)
// Once we reach a single value (when low == high), we've found the right checkpoint at the index high-1, if not
// out of bounds (in which case we're looking too far in the past and the result is 0).
// Note that if the latest checkpoint available is exactly for `blockNumber`, we end up with an index that is
// past the end of the array, so we technically don't find a checkpoint after `blockNumber`, but it works out
// the same.
uint256 length = ckpts.length;

uint256 low = 0;
uint256 high = length;

if (length > 5) {
uint256 mid = length - Math.sqrt(length);
if (_unsafeAccess(ckpts, mid).fromBlock > blockNumber) {
high = mid;
} else {
low = mid + 1;
}
}

while (low < high) {
uint256 mid = Math.average(low, high);
if (_unsafeAccess(ckpts, mid).fromBlock > blockNumber) {
high = mid;
} else {
low = mid + 1;
}
}

unchecked {
return high == 0 ? 0 : _unsafeAccess(ckpts, high - 1).votes;
}
}

/**
* @dev Delegate votes from the sender to `delegatee`.
*/
function delegate(address delegatee) public virtual override {
_delegate(_msgSender(), delegatee);
}

/**
* @dev Delegates votes from signer to `delegatee`
*/
function delegateBySig(
address delegatee,
uint256 nonce,
uint256 expiry,
uint8 v,
bytes32 r,
bytes32 s
) public virtual override {
require(block.timestamp <= expiry, "ERC20Votes: signature expired");
address signer = ECDSA.recover(
_hashTypedDataV4(keccak256(abi.encode(_DELEGATION_TYPEHASH, delegatee, nonce, expiry))),
v,
r,
s
);
require(nonce == _useNonce(signer), "ERC20Votes: invalid nonce");
_delegate(signer, delegatee);
}

/**
* @dev Maximum token supply. Defaults to `type(uint224).max` (2^224^ - 1).
*/
function _maxSupply() internal view virtual returns (uint224) {
return type(uint224).max;
}

/**
* @dev Snapshots the totalSupply after it has been increased.
*/
function _mint(address account, uint256 amount) internal virtual override {
super._mint(account, amount);
require(totalSupply() <= _maxSupply(), "ERC20Votes: total supply risks overflowing votes");

_writeCheckpoint(_totalSupplyCheckpoints, _add, amount);
}

/**
* @dev Snapshots the totalSupply after it has been decreased.
*/
function _burn(address account, uint256 amount) internal virtual override {
super._burn(account, amount);

_writeCheckpoint(_totalSupplyCheckpoints, _subtract, amount);
}

/**
* @dev Move voting power when tokens are transferred.
*
* Emits a {IVotes-DelegateVotesChanged} event.
*/
function _afterTokenTransfer(address from, address to, uint256 amount) internal virtual override {
super._afterTokenTransfer(from, to, amount);

_moveVotingPower(delegates(from), delegates(to), amount);
}

/**
* @dev Change delegation for `delegator` to `delegatee`.
*
* Emits events {IVotes-DelegateChanged} and {IVotes-DelegateVotesChanged}.
*/
function _delegate(address delegator, address delegatee) internal virtual {
address currentDelegate = delegates(delegator);
uint256 delegatorBalance = balanceOf(delegator);
_delegates[delegator] = delegatee;

emit DelegateChanged(delegator, currentDelegate, delegatee);

_moveVotingPower(currentDelegate, delegatee, delegatorBalance);
}

function _moveVotingPower(address src, address dst, uint256 amount) private {
if (src != dst && amount > 0) {
if (src != address(0)) {
(uint256 oldWeight, uint256 newWeight) = _writeCheckpoint(_checkpoints[src], _subtract, amount);
emit DelegateVotesChanged(src, oldWeight, newWeight);
}

if (dst != address(0)) {
(uint256 oldWeight, uint256 newWeight) = _writeCheckpoint(_checkpoints[dst], _add, amount);
emit DelegateVotesChanged(dst, oldWeight, newWeight);
}
}
}

function _writeCheckpoint(
Checkpoint[] storage ckpts,
function(uint256, uint256) view returns (uint256) op,
uint256 delta
) private returns (uint256 oldWeight, uint256 newWeight) {
uint256 pos = ckpts.length;

unchecked {
Checkpoint memory oldCkpt = pos == 0 ? Checkpoint(0, 0) : _unsafeAccess(ckpts, pos - 1);

oldWeight = oldCkpt.votes;
newWeight = op(oldWeight, delta);

if (pos > 0 && oldCkpt.fromBlock == block.number) {
_unsafeAccess(ckpts, pos - 1).votes = SafeCast.toUint224(newWeight);
} else {
ckpts.push(
Checkpoint({fromBlock: SafeCast.toUint32(block.number), votes: SafeCast.toUint224(newWeight)})
);
}
}
}

function _add(uint256 a, uint256 b) private pure returns (uint256) {
return a + b;
}

function _subtract(uint256 a, uint256 b) private pure returns (uint256) {
return a - b;
}

/**
* @dev Access an element of the array without performing bounds check. The position is assumed to be within bounds.
*/
function _unsafeAccess(Checkpoint[] storage ckpts, uint256 pos) private pure returns (Checkpoint storage result) {
assembly {
mstore(0, ckpts.slot)
result.slot := add(keccak256(0, 0x20), pos)
}
}
}
2 changes: 1 addition & 1 deletion contracts/mocks/token/VotesTimestamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ abstract contract ERC721VotesTimestampMock is ERC721Votes {
function clock() public view override returns (uint256) {
return block.timestamp;
}
}
}
20 changes: 12 additions & 8 deletions test/governance/Governor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ const CallReceiver = artifacts.require('CallReceiverMock');
const ERC721 = artifacts.require('$ERC721');
const ERC1155 = artifacts.require('$ERC1155');

const TOKENS = {
blockNumber: artifacts.require('$ERC20Votes'),
timestamp: artifacts.require('$ERC20VotesTimestampMock'),
};
const TOKENS = [
{ Token: artifacts.require('$ERC20Votes'), mode: 'blockNumber' },
{ Token: artifacts.require('$ERC20VotesTimestampMock'), mode: 'timestamp' },
{ Token: artifacts.require('$ERC20VotesLegacyMock'), mode: 'blockNumber' },
];

contract('Governor', function (accounts) {
const [owner, proposer, voter1, voter2, voter3, voter4] = accounts;
Expand All @@ -32,8 +33,8 @@ contract('Governor', function (accounts) {
const votingPeriod = web3.utils.toBN(16);
const value = web3.utils.toWei('1');

for (const [mode, Token] of Object.entries(TOKENS)) {
describe(`using ${mode} voting token`, function () {
for (const { mode, Token } of TOKENS) {
describe(`using ${Token._json.contractName}`, function () {
beforeEach(async function () {
this.chainId = await web3.eth.getChainId();
this.token = await Token.new(tokenName, tokenSymbol, tokenName);
Expand Down Expand Up @@ -81,7 +82,7 @@ contract('Governor', function (accounts) {
});

it('clock is correct', async function () {
expect(await this.token.clock()).to.be.bignumber.equal(await clock[mode]().then(web3.utils.toBN));
expect(await this.mock.clock()).to.be.bignumber.equal(await clock[mode]().then(web3.utils.toBN));
});

it('nominal workflow', async function () {
Expand All @@ -103,7 +104,10 @@ contract('Governor', function (accounts) {
signatures: this.proposal.signatures,
calldatas: this.proposal.data,
startBlock: web3.utils.toBN(await clockFromReceipt[mode](txPropose.receipt)).add(votingDelay),
endBlock: web3.utils.toBN(await clockFromReceipt[mode](txPropose.receipt)).add(votingDelay).add(votingPeriod),
endBlock: web3.utils
.toBN(await clockFromReceipt[mode](txPropose.receipt))
.add(votingDelay)
.add(votingPeriod),
description: this.proposal.description,
});

Expand Down
34 changes: 23 additions & 11 deletions test/governance/compatibility/GovernorCompatibilityBravo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ function makeContractAddress(creator, nonce) {
);
}

const TOKENS = {
blockNumber: artifacts.require('$ERC20VotesComp'),
timestamp: artifacts.require('$ERC20VotesCompTimestampMock'),
};
const TOKENS = [
{ Token: artifacts.require('$ERC20VotesComp'), mode: 'blockNumber' },
{ Token: artifacts.require('$ERC20VotesCompTimestampMock'), mode: 'timestamp' },
];

contract('GovernorCompatibilityBravo', function (accounts) {
const [owner, proposer, voter1, voter2, voter3, voter4, other] = accounts;
Expand All @@ -36,8 +36,8 @@ contract('GovernorCompatibilityBravo', function (accounts) {
const proposalThreshold = web3.utils.toWei('10');
const value = web3.utils.toWei('1');

for (const [mode, Token] of Object.entries(TOKENS)) {
describe(`using ${mode} voting token`, function () {
for (const { mode, Token } of TOKENS) {
describe(`using ${Token._json.contractName}`, function () {
beforeEach(async function () {
const [deployer] = await web3.eth.getAccounts();

Expand Down Expand Up @@ -164,7 +164,10 @@ contract('GovernorCompatibilityBravo', function (accounts) {
signatures: this.proposal.signatures.map(() => ''), // this event doesn't contain the proposal detail
calldatas: this.proposal.fulldata,
startBlock: web3.utils.toBN(await clockFromReceipt[mode](txPropose.receipt)).add(votingDelay),
endBlock: web3.utils.toBN(await clockFromReceipt[mode](txPropose.receipt)).add(votingDelay).add(votingPeriod),
endBlock: web3.utils
.toBN(await clockFromReceipt[mode](txPropose.receipt))
.add(votingDelay)
.add(votingPeriod),
description: this.proposal.description,
});
expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id });
Expand Down Expand Up @@ -207,14 +210,23 @@ contract('GovernorCompatibilityBravo', function (accounts) {

await expectEvent.inTransaction(txExecute.tx, this.receiver, 'MockFunctionCalled');
await expectEvent.inTransaction(txExecute.tx, this.receiver, 'MockFunctionCalled');
await expectEvent.inTransaction(txExecute.tx, this.receiver, 'MockFunctionCalledWithArgs', { a: '17', b: '42' });
await expectEvent.inTransaction(txExecute.tx, this.receiver, 'MockFunctionCalledWithArgs', { a: '18', b: '43' });
await expectEvent.inTransaction(txExecute.tx, this.receiver, 'MockFunctionCalledWithArgs', {
a: '17',
b: '42',
});
await expectEvent.inTransaction(txExecute.tx, this.receiver, 'MockFunctionCalledWithArgs', {
a: '18',
b: '43',
});
});

describe('should revert', function () {
describe('on propose', function () {
it('if proposal does not meet proposalThreshold', async function () {
await expectRevert(this.helper.propose({ from: other }), 'Governor: proposer votes below proposal threshold');
await expectRevert(
this.helper.propose({ from: other }),
'Governor: proposer votes below proposal threshold',
);
});
});

Expand Down Expand Up @@ -249,4 +261,4 @@ contract('GovernorCompatibilityBravo', function (accounts) {
});
});
}
});
});
Loading

0 comments on commit bcedb7e

Please sign in to comment.