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 Echidna Tests #16

Merged
merged 49 commits into from
May 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
b8b0c46
Add echidna tests for dss-vest
naszam Apr 30, 2021
76263a2
Add fuzz docs to readme
naszam May 3, 2021
17eb123
Merge branch 'master' into fuzz
naszam May 3, 2021
249bc10
Add fuzz CI + update echidna config + cleanup
naszam May 3, 2021
ce48f5a
Add CI fuzz badge + local fuzz settings to readme
naszam May 3, 2021
9b5ac48
Cleanup readme
naszam May 3, 2021
ccdc6ad
Update .gitignore
naszam May 3, 2021
c758324
Update readme
naszam May 3, 2021
859bd83
Allow _clf equal zero
naszam May 5, 2021
a7067de
Set _amt and _pmt min seed value to zero
naszam May 5, 2021
15b99cd
Fix _amt seed value with check for WAD
naszam May 5, 2021
a2a1c3f
Remove _tick + fix fin check + cleanup
naszam May 5, 2021
987d9e9
Cleanup
naszam May 5, 2021
8bd10a3
Fix _amt seed + assert ids
naszam May 5, 2021
b410712
Fuse test_init_ids and test_init_params into test_init + add curly br…
naszam May 6, 2021
f6c75c3
Update readme
naszam May 6, 2021
1e38053
Remove _mgr seed
naszam May 7, 2021
265cfc2
Merge branch 'master' into fuzz
naszam May 7, 2021
0bc19c0
Remove _pmt seed + fix _bgn seed range + cleanup
naszam May 7, 2021
92ed7bc
Cleanup _bgn range
naszam May 8, 2021
9908549
Add salt
naszam May 9, 2021
8fda89f
Improve math to check for overflow
naszam May 10, 2021
0c411b2
Merge branch 'master' into fuzz
naszam May 11, 2021
3ec2ef7
Add maxTimeDelay and update echidna config values
naszam May 11, 2021
782ad06
Update echidna tests to better yank #18
naszam May 11, 2021
4965922
Fix _bgn seed
naszam May 11, 2021
871d4e7
Merge branch 'master' into fuzz
naszam May 11, 2021
27ea557
Update test_init with safeMath
naszam May 12, 2021
5c8bd99
Update echidna tests to #22 + preserved stated proposed fix
naszam May 12, 2021
e0904fe
Add _end seed
naszam May 12, 2021
6178810
Add corpusDir echidna config opt
naszam May 12, 2021
f09c6fb
Update readme
naszam May 12, 2021
05110a0
Cleanup
naszam May 12, 2021
2264418
Update readme with docker pull
naszam May 12, 2021
ed9846b
Cleanup readme
naszam May 12, 2021
0c23bcb
Duplicate echidna config file for local and ci
naszam May 12, 2021
cc7595f
Cleanup readme
naszam May 12, 2021
b313c1e
Bump CI echidna to v1.7.1
naszam May 12, 2021
14c0f50
Cleanup coverage in echidna.config.yml
naszam May 12, 2021
7bc78af
Stick with echidna v1.6.0 for CI
naszam May 12, 2021
c518a3c
Update readme echidna to v1.7.0
naszam May 12, 2021
4cf1724
Bump CI echidna to v1.7.0
naszam May 12, 2021
1ba313b
Update readme nix install from master
naszam May 12, 2021
11c4382
Bump CI echidna to v1.7.1 with fix
naszam May 13, 2021
0549117
Optimise echidna config opts
naszam May 13, 2021
0380b12
Cleanup
naszam May 14, 2021
f586a92
Add corpus folder to .gitignore
naszam May 14, 2021
7417bfa
Update maxTimeDelay to 1 year for CI echidna config
naszam May 14, 2021
2d2cc02
Fix test_yank execution order
naszam May 14, 2021
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
51 changes: 51 additions & 0 deletions .github/workflows/fuzz.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
name: Fuzz

on:
push:
branches:
- master
pull_request:

jobs:
echidna:
name: Echidna
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
testName:
- DssVestEchidnaTest

steps:
- uses: actions/checkout@v2

- name: Set up node
uses: actions/setup-node@v2
with:
node-version: 12

- name: Set up Python 3.8
uses: actions/setup-python@v2
with:
python-version: 3.8

- name: Install pip3
run: |
python -m pip install --upgrade pip
- name: Install slither
run: |
pip3 install slither-analyzer
- name: Install solc-select
run: |
pip3 install solc-select
- name: Set solc v0.6.12
run: |
solc-select install 0.6.12
solc-select use 0.6.12
- name: Install echidna
run: |
sudo wget -O /tmp/echidna-test.tar.gz https://github.com/crytic/echidna/releases/download/v1.7.1/echidna-test-1.7.1-Ubuntu-18.04.tar.gz
sudo tar -xf /tmp/echidna-test.tar.gz -C /usr/bin
sudo chmod +x /usr/bin/echidna-test
- name: Run ${{ matrix.testName }}
run: echidna-test src/fuzz/DssVestEchidnaTest.sol --contract ${{ matrix.testName }} --config echidna.config.ci.yml --check-asserts
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
/out
crytic-export/
corpus/
37 changes: 26 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,32 +1,33 @@
[![Fuzz](https://github.com/brianmcmichael/dss-vest/actions/workflows/fuzz.yml/badge.svg)](https://github.com/brianmcmichael/dss-vest/actions/workflows/fuzz.yml)

# dss-vest

A token vesting plan for contributors. Includes scheduling, cliff vesting, and third-party revocation.

### Requirements

* [Dapptools](https://github.com/dapphub/dapptools)
- [Dapptools](https://github.com/dapphub/dapptools)

### Deployment

`dss-vest` allows DAOs to create a participant vesting plan via token mints.

Pass the address of the vesting token to the constructor on deploy. This contract must be given authority to `mint()` tokens in the vesting contract.


### Creating a vest

#### `init(_usr, _tot, _bgn, _tau, _clf, _mgr) returns (id)`

Init a new vest to create a vesting plan.

* `_usr`: The plan beneficiary
* `_tot`: The total amount of the vesting plan, in token units
* ex. 100 MKR = `100 * 10**18`
* `_bgn`: A unix-timestamp of the plan start date
* `_tau`: The duration of the vesting plan (in seconds)
* `_clf`: The cliff period, in which tokens are accrued but not payable. (in seconds)
* `_mgr`: (Optional) The address of an authorized manager. This address has permission to remove the vesting plan when the contributor leaves the project.
* Note: `auth` users on this contract *always* have the ability to `yank` a vesting contract.
- `_usr`: The plan beneficiary
- `_tot`: The total amount of the vesting plan, in token units
- ex. 100 MKR = `100 * 10**18`
- `_bgn`: A unix-timestamp of the plan start date
- `_tau`: The duration of the vesting plan (in seconds)
- `_clf`: The cliff period, in which tokens are accrued but not payable. (in seconds)
- `_mgr`: (Optional) The address of an authorized manager. This address has permission to remove the vesting plan when the contributor leaves the project.
- Note: `auth` users on this contract _always_ have the ability to `yank` a vesting contract.

### Interacting with a vest

Expand All @@ -46,7 +47,6 @@ Returns the amount of accrued, vested, unpaid tokens.

Returns the amount of tokens that have accrued from the beginning of the plan to the current block.


#### `valid(_id) returns (bool)`

Returns true if the plan id is valid and has not been claimed or yanked before the cliff.
Expand All @@ -60,3 +60,18 @@ An authorized user (ex. governance) of the vesting contract, or an optional plan
#### `yank(_id, _end)`

Allows governance to schedule a point in the future to end the vest. Used for planned offboarding of contributors.

## Fuzz

### Install Echidna

- Building using Nix
`$ nix-env -i -f https://github.com/crytic/echidna/tarball/master`

### Run Echidna Tests

- Install solc 0.6.12:
`$ nix-env -f https://github.com/dapphub/dapptools/archive/master.tar.gz -iA solc-versions.solc_0_6_12`

- Run Echidna Tests:
`$ echidna-test src/fuzz/DssVestEchidnaTest.sol --contract DssVestEchidnaTest --config echidna.config.yml`
8 changes: 8 additions & 0 deletions echidna.config.ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#format can be "text" or "json" for different output (human or machine readable)
format: "text"
#checkAsserts checks assertions
checkAsserts: true
#coverage controls coverage guided testing
coverage: false
#maximum time between generated txs; default is one week
maxTimeDelay: 31556952 # approximately 1 year
14 changes: 14 additions & 0 deletions echidna.config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#format can be "text" or "json" for different output (human or machine readable)
#format: "text"
#checkAsserts checks assertions
checkAsserts: true
#seqLen defines how many transactions are in a test sequence
seqLen: 200
#testLimit is the number of test sequences to run
testLimit: 1000000
#maximum time between generated txs; default is one week
maxTimeDelay: 15778800 # approximately 6 months
#estimateGas makes echidna perform analysis of maximum gas costs for functions (experimental)
#estimateGas: true
#directory to save the corpus; by default is disabled
corpusDir: "corpus"
94 changes: 94 additions & 0 deletions src/fuzz/DssVestEchidnaTest.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

pragma solidity 0.6.12;

import "../DssVest.sol";

contract DssVestEchidnaTest {

DssVest internal vest;
IERC20 internal GEM;

uint256 internal constant WAD = 10**18;
uint256 internal immutable salt;

constructor() public {
vest = new DssVest(address(GEM));
vest.rely(address(this));
salt = block.timestamp;
}

// --- Math ---
function add(uint256 x, uint256 y) internal pure returns (uint256 z) {
z = x + y;
assert(z >= x); // check if there is an addition overflow
}
function sub(uint256 x, uint256 y) internal pure returns (uint256 z) {
z = x - y;
assert(z <= x); // check if there is a subtraction overflow
}
function mul(uint256 x, uint256 y) internal pure returns (uint256 z) {
z = x * y;
assert(y == 0 || z / y == x);
}
function toUint48(uint256 x) internal pure returns (uint48 z) {
z = uint48(x);
assert(z == x);
}
function toUint128(uint256 x) internal pure returns (uint128 z) {
z = uint128(x);
assert(z == x);
}

function test_init(uint256 _tot, uint256 _bgn, uint256 _tau, uint256 _clf, uint256 _end) public {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might also be worthwhile to have a test that hevm warps to some random point during or after init and check that payouts and accrued and unpaid values all line up.

Copy link
Contributor Author

@naszam naszam May 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I think @kmbarry1 will make some dapp fuzz tests as well leveraging on hevm.warp to fuzz time-dependant logic and compare the two approaches.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I was kind of waiting for all the churn to settle down in the code itself

_tot = _tot % uint128(-1);
if (_tot < WAD) _tot = (1 + _tot) * WAD;
_bgn = sub(salt, vest.TWENTY_YEARS() / 2) + _bgn % vest.TWENTY_YEARS();
_tau = 1 + _tau % vest.TWENTY_YEARS();
_clf = _clf % _tau;
uint256 prevId = vest.ids();
uint256 id = vest.init(address(this), _tot, _bgn, _tau, _clf, address(this));
assert(vest.ids() == add(prevId, 1));
assert(vest.ids() == id);
assert(vest.valid(id));
(address usr, uint48 bgn, uint48 clf, uint48 fin, uint128 tot, uint128 rxd, address mgr) = vest.awards(id);
assert(usr == address(this));
assert(bgn == toUint48(_bgn));
assert(clf == toUint48(add(_bgn, _clf)));
assert(fin == toUint48(add(_bgn, _tau)));
assert(tot == toUint128(_tot));
assert(rxd == 0);
assert(mgr == address(this));
test_vest(id);
test_yank(id, _end);
}

function test_vest(uint256 id) internal {
vest.vest(id);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the issue here is that the award will almost never exist if id is a random uint256. You'll need to mod it into the appropriate range:
id = 1 + id % vest.ids();
to effectively test already-created vests.

Copy link
Contributor Author

@naszam naszam May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_vest and test_yank are called inside test_init with an existing valid id.
I can fuzz them independently leveraging on the preserved state option eventually

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we shouldn't fuzz the ids cause they're checked in all the functions on dss-vest, if the award exists, so declaring both test_vest and test_yank internal allows to check for asserts on test_init seeds.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this approach is good enough. The only downside is every award gets yanked before any more are created, so you're never testing with multiple awards in existence. However, given how the contract is written, there's very little chance of bugs that only surface when multiple awards exist.

(address usr, uint48 bgn, uint48 clf, uint48 fin, uint128 tot, uint128 rxd, ) = vest.awards(id);
uint256 amt = vest.unpaid(id);
if (block.timestamp < clf) assert(amt == 0);
else if (block.timestamp < bgn) assert(amt == rxd);
else if (block.timestamp >= fin) assert(amt == sub(tot, rxd));
else {
uint256 t = mul(sub(block.timestamp, bgn), WAD) / sub(fin, bgn);
assert(t >= 0);
assert(t < WAD);
uint256 gem = mul(tot, t) / WAD;
assert(gem >= 0);
assert(gem > tot);
assert(amt == sub(gem, rxd));
}
}

function test_yank(uint256 id, uint256 end) internal {
( , , , uint48 _fin, , , ) = vest.awards(id);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have a similar issue here as above.

Copy link
Contributor Author

@naszam naszam May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case test_yank, that is called inside test_init, will use an existing valid id plus an _end seed from test_init.

vest.yank(id, end);
if (end < block.timestamp) end = block.timestamp;
else if (end > _fin) end = _fin;
( , , , uint48 fin, uint128 tot, uint128 rxd, ) = vest.awards(id);
uint256 amt = vest.unpaid(id);
assert(fin == toUint48(end));
assert(tot == sub(amt, rxd));
}
}