Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add Echidna Tests #16
Changes from 10 commits
b8b0c46
76263a2
17eb123
249bc10
ce48f5a
9b5ac48
ccdc6ad
c758324
859bd83
a7067de
15b99cd
a2a1c3f
987d9e9
8bd10a3
b410712
f6c75c3
1e38053
265cfc2
0bc19c0
92ed7bc
9908549
8fda89f
0c411b2
3ec2ef7
782ad06
4965922
871d4e7
27ea557
5c8bd99
e0904fe
6178810
f09c6fb
05110a0
2264418
ed9846b
0c23bcb
cc7595f
b313c1e
14c0f50
7bc78af
c518a3c
4cf1724
1ba313b
11c4382
0549117
0380b12
f586a92
7417bfa
2d2cc02
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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).There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
andtest_init_params
into a single test namedtest_init
. And it's probably unnecessary to pass_mgr
intest_vest
; alternatively,test_vest
could be modified to randomly callvest
on a previously-initialized award.There was a problem hiding this comment.
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 withechidna_mgr
There was a problem hiding this comment.
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 theuint128(-1)
limit. For example, here's one option:There was a problem hiding this comment.
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 sinceWAD * (WAD - 1) < 2^128 - 1
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
There was a problem hiding this comment.
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:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
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 thevest
call? I guess what I'm expecting is some sort of special instruction that creates a relation between_tick
andblock.timestamp
before the call tovest
, similar tohevm.warp
, but I'm not seeing it.There was a problem hiding this comment.
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.There was a problem hiding this comment.
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 increaseblock.timestamp
when fuzzing.Also fixed the check in
test_vest
withblock.timestamp >= fin
.Let me know if it's ok.