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 10 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.6.0/echidna-test-v1.6.0-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.yml
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/out
crytic-export/
54 changes: 54 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1 +1,55 @@
[![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

## Fuzz

### Install Echidna

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

- Building using Docker
`$ docker build -t echidna .`

Then, run via:
`docker run -it -v`pwd`:/src echidna echidna-test /src/fuzz/DssVestEchidnaTest.sol`

- Precompiled Binaries

Before starting, make sure Slither is installed:
`$ pip3 install slither-analyzer`

To quickly test Echidna in Linux or MacOS:
[release page](https://github.com/crytic/echidna/releases)

### Local Dependencies

- Slither
`$ pip3 install slither-analyzer`

- solc-select
`$ pip3 install solc-select`

### Local Fuzz Settings

- Edit `echidna.config.yml`
- Comment `format: "text"`
- Set `coverage` to true
- Uncomment `seqLen`
- Uncomment `testLimit`
- Uncomment `estimateGas` (optional)

### Run Echidna Tests

- Install solc version:
`$ solc-select install 0.6.12`

- Select solc version:
`$ solc-select use 0.6.12`

- If using Dapp Tools:
`$ 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`
12 changes: 12 additions & 0 deletions echidna.config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#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
#seqLen defines how many transactions are in a test sequence
#seqLen: 50
#testLimit is the number of test sequences to run
#testLimit: 100000
#estimateGas makes echidna perform analysis of maximum gas costs for functions (experimental)
#estimateGas: true
82 changes: 82 additions & 0 deletions src/fuzz/DssVestEchidnaTest.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

pragma solidity 0.6.12;

import "../DssVest.sol";

contract DssVestEchidnaTest {

DssVest internal vest;
IMKR internal MKR;

constructor() public {
vest = new DssVest(address(MKR));
}

// --- Math ---

uint256 internal constant WAD = 10**18;

function add(uint256 x, uint256 y) internal pure returns (uint256 z) {
require((z = x + y) >= x);
}
function sub(uint256 x, uint256 y) internal pure returns (uint256 z) {
require((z = x - y) <= x);
}

function test_init_ids(uint256 _amt, uint256 _bgn, uint256 _tau, uint256 _clf, uint256 _pmt, address _mgr) public {
Copy link

Choose a reason for hiding this comment

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

If we're bothering to pass _mgr as a fuzz parameter, we should at least verify that it's being set correctly in the award (same for the other values).

Copy link
Contributor Author

@naszam naszam May 5, 2021

Choose a reason for hiding this comment

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

this should be done in test_init_params

Copy link

Choose a reason for hiding this comment

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

Actually I'd suggest to combine test_init_ids and test_init_params into a single test named test_init. And it's probably unnecessary to pass _mgr in test_vest; alternatively, test_vest could be modified to randomly call vest on a previously-initialized award.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the _mgr seed and replace with echidna_mgr

_amt = 0 + _amt % uint128(-1);
Copy link

Choose a reason for hiding this comment

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

I kind of think _amt = 0 is an uninteresting case--I'd recommend enforcing a reasonable minimum like you were before, just in a way that won't exceed the uint128(-1) limit. For example, here's one option:

_amt = _amt % uint128(-1);
if (_amt < WAD) _amt *= WAD;

Copy link

Choose a reason for hiding this comment

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

That will never cause a uint128 overflow since WAD * (WAD - 1) < 2^128 - 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

Copy link

Choose a reason for hiding this comment

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

Oh yeah...we want to avoid _amt == 0...so we need something like:

_amt = _amt % uint128(-1);
if (_amt < WAD) _amt = (1 + _amt) * WAD;

Copy link
Contributor Author

@naszam naszam May 5, 2021

Choose a reason for hiding this comment

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

Done.

_bgn = block.timestamp + _bgn % vest.MAX_VEST_PERIOD();
_tau = 1 + _tau % vest.MAX_VEST_PERIOD();
_clf = 0 + _clf % _tau;
_pmt = 0 + _pmt % _amt;
uint256 prevId = vest.ids();
uint256 id = vest.init(address(this), _amt, _bgn, _tau, _clf, _pmt, _mgr);
assert(vest.ids() == add(prevId, id));
Copy link

Choose a reason for hiding this comment

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

This won't hold in general. Suggest replacing with:

        assert(vest.ids() == add(prevId, 1));
        assert(vest.ids() == id);

Copy link
Contributor Author

@naszam naszam May 5, 2021

Choose a reason for hiding this comment

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

Done.

assert(vest.valid(id));
}

function test_init_params(uint256 _amt, uint256 _bgn, uint256 _tau, uint256 _clf, uint256 _pmt, address _mgr) public {
_amt = 0 + _amt % uint128(-1);
_bgn = block.timestamp + _bgn % vest.MAX_VEST_PERIOD();
_tau = 1 + _tau % vest.MAX_VEST_PERIOD();
_clf = 0 + _clf % _tau;
_pmt = 0 + _pmt % _amt;
uint256 id = vest.init(address(this), _amt, _bgn, _tau, _clf, _pmt, _mgr);
(address usr, uint48 bgn, uint48 clf, uint48 fin, uint128 amt, uint128 rxd, address mgr) = vest.awards(id);
if (sub(_amt, _pmt) != 0) {
assert(usr == (address(this)));
assert(bgn == uint48(_bgn));
assert(clf == add(_bgn, _clf));
assert(fin == add(_bgn, _tau));
assert(amt == sub(_amt, _pmt));
assert(rxd == uint128(0));
assert(mgr == _mgr);
}
}

function test_vest(uint256 _amt, uint256 _bgn, uint256 _tau, uint256 _clf, uint256 _pmt, address _mgr, uint256 _tick) public {
_amt = 0 + _amt % uint128(-1);
_bgn = block.timestamp + _bgn % vest.MAX_VEST_PERIOD();
_tau = 1 + _tau % vest.MAX_VEST_PERIOD();
_clf = 0 + _clf % _tau;
_pmt = 0 + _pmt % _amt;
_tick = block.timestamp + _tick % uint128(-1);
Copy link

Choose a reason for hiding this comment

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

Okay I need some education on echidna...how does the value of _tick affect the behavior of the the vest call? I guess what I'm expecting is some sort of special instruction that creates a relation between _tick and block.timestamp before the call to vest, similar to hevm.warp, but I'm not seeing it.

Copy link
Contributor Author

@naszam naszam May 5, 2021

Choose a reason for hiding this comment

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

I've added _tick as a seed to simulate time for time dependant scenarios.

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've just removed _tick as echidna should increase block.timestamp when fuzzing.
Also fixed the check in test_vest with block.timestamp >= fin.
Let me know if it's ok.

uint256 id = vest.init(address(this), _amt, _bgn, _tau, _clf, _pmt, _mgr);
assert(vest.valid(id));
uint256 ids = vest.ids();
vest.vest(id);
(address usr, uint48 bgn, uint48 clf, uint48 fin, uint128 amt, uint128 rxd, address mgr) = vest.awards(id);
if (_tick < fin) {
assert (vest.ids() == sub(ids, 1));
} else if (_tick >= clf) {
uint256 t = (_tick - bgn) * WAD / (fin - bgn);
assert(t >= 0);
assert(t < WAD);
uint256 mkr = (amt * t) / WAD;
assert(mkr >= 0);
assert(mkr < amt);
assert(rxd == uint128(mkr));
}
}
}