Skip to content

Add formal verification for the Governor system #4106

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

Draft
wants to merge 72 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
1f5982b
starting to work on governor specifications
Amxx Mar 10, 2023
5421355
test both modes
Amxx Mar 10, 2023
f35c824
fix specs
Amxx Mar 10, 2023
9f39697
lint
Amxx Mar 10, 2023
ec82e2f
use micromatch
Amxx Mar 10, 2023
f44e26f
disable wip specs
Amxx Mar 10, 2023
318cfd5
update
Amxx Mar 10, 2023
f71bc68
clean
Amxx Mar 13, 2023
4b11b4d
codespell
Amxx Mar 13, 2023
c33e7bd
update governor specs
Amxx Mar 13, 2023
b320e1e
lint
Amxx Mar 13, 2023
e35daec
Merge branch 'master' into fv/Governor
Amxx Mar 14, 2023
704e265
fix governor changes spec
Amxx Mar 14, 2023
e1120b9
try optimise GovernorStates
Amxx Mar 14, 2023
728e2c8
fix
Amxx Mar 14, 2023
397f4cd
filter functions that should revert
Amxx Mar 14, 2023
d2b5d15
Merge remote-tracking branch 'upstream/master' into fv/Governor
Amxx Mar 14, 2023
d788425
update
Amxx Mar 14, 2023
0d4df89
add filter to improve prover perf
Amxx Mar 14, 2023
198c4b7
update
Amxx Mar 15, 2023
0874adb
spacing
Amxx Mar 15, 2023
4ea73a8
add PreventLateQuorum specs
Amxx Mar 15, 2023
50a13d5
uo
Amxx Mar 15, 2023
dfafd79
uo
Amxx Mar 15, 2023
9655359
disable GovernorFunctions
Amxx Mar 15, 2023
89ceb34
don't run GovernorFunctions in CI
Amxx Mar 15, 2023
82bbdb2
codespell
Amxx Mar 15, 2023
d0b2595
fix options
Amxx Mar 15, 2023
74f613f
fix specs
Amxx Mar 15, 2023
dd6a9ee
fix attempt
Amxx Mar 15, 2023
7512b8e
missing diff
Amxx Mar 15, 2023
06baea7
up
Amxx Mar 15, 2023
a355bf0
fix
Amxx Mar 16, 2023
dbb4a29
split function rules
Amxx Mar 16, 2023
607268b
timeout
Amxx Mar 16, 2023
3f79e26
update
Amxx Mar 16, 2023
5ef4d20
more fixes ?
Amxx Mar 16, 2023
ddaf4bc
up
Amxx Mar 16, 2023
5770dfb
wip
Amxx Mar 16, 2023
1744132
Merge branch 'master' into fv/Governor
Amxx Mar 16, 2023
a64bb88
update
Amxx Mar 16, 2023
67a00cc
disable specs we can't fix :/
Amxx Mar 17, 2023
ecec8a7
codespell
Amxx Mar 18, 2023
7c62ed2
disable problematic rules
Amxx Mar 18, 2023
8d02947
test function with both clocks
Amxx Mar 18, 2023
42580f2
Merge branch 'master' into fv/Governor
Amxx Apr 25, 2023
5e71c01
rename valid → sanity
Amxx Apr 25, 2023
e072521
fix harness
Amxx Apr 25, 2023
5af9167
address PR comments
Amxx Apr 25, 2023
8339187
fix
Amxx Apr 25, 2023
e928466
Do not run the FV workflow automatically on master
Amxx May 3, 2023
c8457ba
Merge branch 'master' of github.com:OpenZeppelin/openzeppelin-contracts
Amxx May 3, 2023
0daafdb
fix harness
Amxx May 3, 2023
6af1f18
merge master
Amxx May 3, 2023
f21451f
Merge branch 'master' into fv/Governor
Amxx May 3, 2023
75d6f5a
move Governor.helpers.spec to helpers folder
Amxx May 3, 2023
3d9ef78
fix import path
Amxx May 3, 2023
2a6cceb
Fix early reporting of FV prover's output
Amxx May 4, 2023
7c37ea0
fix rewrite
Amxx May 4, 2023
e83fdf0
trying to fix timeout
Amxx May 4, 2023
e856ebb
Merge branch 'CI/FV/urls' into fv/Governor
Amxx May 4, 2023
fd5f309
improve stateTransitionWait
Amxx May 4, 2023
df88ea3
fix lint
Amxx May 4, 2023
a97d3f5
codespell
Amxx May 4, 2023
9a33b0d
split stateTransitionFn as multiple rules with requires
Amxx May 5, 2023
3431624
up
Amxx May 5, 2023
c664f5f
try to simplify rules
Amxx May 5, 2023
e9779f8
Merge remote-tracking branch 'upstream' into fv/Governor
Amxx May 9, 2023
2e1d0b3
fix spec file
Amxx May 9, 2023
6d539e6
comment out rules that timeout
Amxx May 9, 2023
e9f23ab
Merge remote-tracking branch 'upstream' into fv/Governor
Amxx May 10, 2023
aaa5a5a
Merge branch 'master' into fv/Governor
Amxx May 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions certora/diff/governance_Governor.sol.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
--- governance/Governor.sol 2023-03-07 10:48:47.730155491 +0100
+++ governance/Governor.sol 2023-03-10 10:13:31.926616811 +0100
@@ -216,6 +216,16 @@
return _proposals[proposalId].proposer;
}

+ // FV
+ function _isExecuted(uint256 proposalId) internal view returns (bool) {
+ return _proposals[proposalId].executed;
+ }
+
+ // FV
+ function _isCanceled(uint256 proposalId) internal view returns (bool) {
+ return _proposals[proposalId].canceled;
+ }
+
/**
* @dev Amount of votes already cast passes the threshold limit.
*/
1 change: 0 additions & 1 deletion certora/harnesses/AccessControlHarness.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../patched/access/AccessControl.sol";
Expand Down
1 change: 0 additions & 1 deletion certora/harnesses/ERC20FlashMintHarness.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../patched/token/ERC20/ERC20.sol";
Expand Down
1 change: 0 additions & 1 deletion certora/harnesses/ERC20PermitHarness.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../patched/token/ERC20/extensions/ERC20Permit.sol";
Expand Down
29 changes: 29 additions & 0 deletions certora/harnesses/ERC20VotesBlocknumberHarness.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "../patched/token/ERC20/extensions/ERC20Votes.sol";

contract ERC20VotesBlocknumberHarness is ERC20Votes {
constructor(string memory name, string memory symbol) ERC20(name, symbol) ERC20Permit(name) {}

function mint(address account, uint256 amount) external {
_mint(account, amount);
}

function burn(address account, uint256 amount) external {
_burn(account, amount);
}

// inspection
function ckptFromBlock(address account, uint32 pos) public view returns (uint32) {
return checkpoints(account, pos).fromBlock;
}

function ckptVotes(address account, uint32 pos) public view returns (uint224) {
return checkpoints(account, pos).votes;
}

function maxSupply() public view returns (uint224) {
return _maxSupply();
}
}
39 changes: 39 additions & 0 deletions certora/harnesses/ERC20VotesTimestampHarness.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "../patched/token/ERC20/extensions/ERC20Votes.sol";

contract ERC20VotesTimestampHarness is ERC20Votes {
constructor(string memory name, string memory symbol) ERC20(name, symbol) ERC20Permit(name) {}

function mint(address account, uint256 amount) external {
_mint(account, amount);
}

function burn(address account, uint256 amount) external {
_burn(account, amount);
}

// inspection
function ckptFromBlock(address account, uint32 pos) public view returns (uint32) {
return checkpoints(account, pos).fromBlock;
}

function ckptVotes(address account, uint32 pos) public view returns (uint224) {
return checkpoints(account, pos).votes;
}

function maxSupply() public view returns (uint224) {
return _maxSupply();
}

// clock
function clock() public view override returns (uint48) {
return uint48(block.timestamp);
}

// solhint-disable-next-line func-name-mixedcase
function CLOCK_MODE() public view virtual override returns (string memory) {
return "mode=timestamp";
}
}
1 change: 0 additions & 1 deletion certora/harnesses/ERC20WrapperHarness.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../patched/token/ERC20/extensions/ERC20Wrapper.sol";
Expand Down
3 changes: 1 addition & 2 deletions certora/harnesses/ERC3156FlashBorrowerHarness.sol
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "../patched/interfaces/IERC3156FlashBorrower.sol";

pragma solidity ^0.8.0;

contract ERC3156FlashBorrowerHarness is IERC3156FlashBorrower {
bytes32 somethingToReturn;

Expand Down
160 changes: 160 additions & 0 deletions certora/harnesses/GovernorHarness.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.2;

import "../patched/governance/Governor.sol";
import "../patched/governance/extensions/GovernorCountingSimple.sol";
import "../patched/governance/extensions/GovernorTimelockControl.sol";
import "../patched/governance/extensions/GovernorVotes.sol";
import "../patched/governance/extensions/GovernorVotesQuorumFraction.sol";
import "../patched/token/ERC20/extensions/ERC20Votes.sol";

contract GovernorHarness is
Governor,
GovernorCountingSimple,
GovernorTimelockControl,
GovernorVotes,
GovernorVotesQuorumFraction
{
constructor(IVotes _token, TimelockController _timelock, uint256 _quorumNumeratorValue)
Governor("Harness")
GovernorTimelockControl(_timelock)
GovernorVotes(_token)
GovernorVotesQuorumFraction(_quorumNumeratorValue)
{}

// Harness from Votes
function token_getPastTotalSupply(uint256 blockNumber) public view returns(uint256) {
return token.getPastTotalSupply(blockNumber);
}

function token_getPastVotes(address account, uint256 blockNumber) public view returns(uint256) {
return token.getPastVotes(account, blockNumber);
}

function token_clock() public view returns (uint48) {
return token.clock();
}

function token_CLOCK_MODE() public view returns (string memory) {
return token.CLOCK_MODE();
}

// Harness from Governor
function getExecutor() public view returns (address) {
return _executor();
}

function proposalProposer(uint256 proposalId) public view returns (address) {
return _proposalProposer(proposalId);
}

function quorumReached(uint256 proposalId) public view returns (bool) {
return _quorumReached(proposalId);
}

function voteSucceeded(uint256 proposalId) public view returns (bool) {
return _voteSucceeded(proposalId);
}

function isExecuted(uint256 proposalId) public view returns (bool) {
return _isExecuted(proposalId);
}

function isCanceled(uint256 proposalId) public view returns (bool) {
return _isCanceled(proposalId);
}

// Harness from GovernorCountingSimple
function getAgainstVotes(uint256 proposalId) public view returns (uint256) {
(uint256 againstVotes,,) = proposalVotes(proposalId);
return againstVotes;
}

function getForVotes(uint256 proposalId) public view returns (uint256) {
(,uint256 forVotes,) = proposalVotes(proposalId);
return forVotes;
}

function getAbstainVotes(uint256 proposalId) public view returns (uint256) {
(,,uint256 abstainVotes) = proposalVotes(proposalId);
return abstainVotes;
}

/// The following functions are overrides required by Solidity added by Certora.
// mapping(uint256 => uint256) public ghost_sum_vote_power_by_id;
//
// function _castVote(
// uint256 proposalId,
// address account,
// uint8 support,
// string memory reason,
// bytes memory params
// ) internal virtual override returns (uint256) {
// uint256 deltaWeight = super._castVote(proposalId, account, support, reason, params);
// ghost_sum_vote_power_by_id[proposalId] += deltaWeight;
// return deltaWeight;
// }

// The following functions are overrides required by Solidity added by OZ Wizard.
function votingDelay() public pure override returns (uint256) {
return 1; // 1 block
}

function votingPeriod() public pure override returns (uint256) {
return 45818; // 1 week
}

function quorum(uint256 blockNumber)
public
view
override(IGovernor, GovernorVotesQuorumFraction)
returns (uint256)
{
return super.quorum(blockNumber);
}

function state(uint256 proposalId) public view override(Governor, GovernorTimelockControl) returns (ProposalState) {
return super.state(proposalId);
}

function propose(
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
string memory description
) public override(Governor, IGovernor) returns (uint256) {
return super.propose(targets, values, calldatas, description);
}

function _execute(
uint256 proposalId,
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
) internal override(Governor, GovernorTimelockControl) {
super._execute(proposalId, targets, values, calldatas, descriptionHash);
}

function _cancel(
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
) internal override(Governor, GovernorTimelockControl) returns (uint256) {
return super._cancel(targets, values, calldatas, descriptionHash);
}

function _executor() internal view override(Governor, GovernorTimelockControl) returns (address) {
return super._executor();
}

function supportsInterface(bytes4 interfaceId)
public
view
override(Governor, GovernorTimelockControl)
returns (bool)
{
return super.supportsInterface(interfaceId);
}
}
1 change: 0 additions & 1 deletion certora/harnesses/Ownable2StepHarness.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../patched/access/Ownable2Step.sol";
Expand Down
1 change: 0 additions & 1 deletion certora/harnesses/OwnableHarness.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../patched/access/Ownable.sol";
Expand Down
19 changes: 10 additions & 9 deletions certora/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,27 @@

const MAX_PARALLEL = 4;

let specs = require(__dirname + '/specs.json');

const proc = require('child_process');
const { PassThrough } = require('stream');
const events = require('events');
const limit = require('p-limit')(MAX_PARALLEL);

const strToRegex = str => new RegExp(`^${str.replace(/[.+?^${}()|[\]\\]/g, '\\$&').replace(/[*]/g, '.$&')}$`);

let [, , request = '', ...extraOptions] = process.argv;
if (request.startsWith('-')) {
extraOptions.unshift(request);
request = '';
}
const [reqSpec, reqContract] = request.split(':').reverse();

if (request) {
const [reqSpec, reqContract] = request.split(':').reverse();
specs = Object.values(specs).filter(s => reqSpec === s.spec && (!reqContract || reqContract === s.contract));
if (specs.length === 0) {
console.error(`Error: Requested spec '${request}' not found in specs.json`);
process.exit(1);
}
const specs = require(__dirname + '/specs.js')
.filter(entry => !reqSpec || strToRegex(reqSpec).test(entry.spec))
.filter(entry => !reqContract || strToRegex(reqContract).test(entry.contract));

if (specs.length === 0) {
console.error(`Error: Requested spec '${request}' not found in specs.json`);
process.exit(1);
}

for (const { spec, contract, files, options = [] } of Object.values(specs)) {
Expand Down
51 changes: 51 additions & 0 deletions certora/specs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
const product = (...arrays) => arrays.reduce((a, b) => a.flatMap(ai => b.map(bi => [ai, bi].flat())));

module.exports = [
{
spec: 'AccessControl',
contract: 'AccessControlHarness',
files: ['certora/harnesses/AccessControlHarness.sol'],
},
{
spec: 'Ownable',
contract: 'OwnableHarness',
files: ['certora/harnesses/OwnableHarness.sol'],
},
{
spec: 'Ownable2Step',
contract: 'Ownable2StepHarness',
files: ['certora/harnesses/Ownable2StepHarness.sol'],
},
{
spec: 'ERC20',
contract: 'ERC20PermitHarness',
files: ['certora/harnesses/ERC20PermitHarness.sol'],
options: ['--optimistic_loop'],
},
{
spec: 'ERC20FlashMint',
contract: 'ERC20FlashMintHarness',
files: ['certora/harnesses/ERC20FlashMintHarness.sol', 'certora/harnesses/ERC3156FlashBorrowerHarness.sol'],
options: ['--optimistic_loop'],
},
{
spec: 'ERC20Wrapper',
contract: 'ERC20WrapperHarness',
files: ['certora/harnesses/ERC20PermitHarness.sol', 'certora/harnesses/ERC20WrapperHarness.sol'],
options: ['--link ERC20WrapperHarness:_underlying=ERC20PermitHarness', '--optimistic_loop'],
},
{
spec: 'Initializable',
contract: 'InitializableHarness',
files: ['certora/harnesses/InitializableHarness.sol'],
},
...product(
['GovernorBase', 'GovernorInvariants', 'GovernorStates', 'GovernorFunctions'],
['ERC20VotesBlocknumberHarness', 'ERC20VotesTimestampHarness'],
).map(([spec, token]) => ({
spec,
contract: 'GovernorHarness',
files: ['certora/harnesses/GovernorHarness.sol', `certora/harnesses/${token}.sol`],
options: [`--link GovernorHarness:token=${token}`, '--optimistic_loop', '--optimistic_hashing'],
})),
];
Loading