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

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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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 = 1 * WAD + _amt % uint128(-1);
_bgn = block.timestamp + _bgn % vest.MAX_VEST_PERIOD();
_tau = 1 + _tau % vest.MAX_VEST_PERIOD();
_clf = 1 + _clf % _tau;
_pmt = 1 * WAD + _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
Copy Markdown

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
Copy Markdown
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 = 1 * WAD + _amt % uint128(-1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this can exceed uint128(-1), which I assume will produce a violation since init will revert

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I just set _amt and _pmt min seed value to zero

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this should be done.

_bgn = block.timestamp + _bgn % vest.MAX_VEST_PERIOD();
_tau = 1 + _tau % vest.MAX_VEST_PERIOD();
_clf = 1 + _clf % _tau;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why not allow _clf ==0? Seems like a valid case.

Copy link
Copy Markdown
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.

_pmt = 1 * WAD + _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 = 1 * WAD + _amt % uint128(-1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same comment as above

Copy link
Copy Markdown
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 = 1 + _clf % _tau;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same comment as above

Copy link
Copy Markdown
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.

_pmt = 1 * WAD + _pmt % _amt;
_tick = block.timestamp + _tick % uint128(-1);
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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));
}
}
}