Fuzz testing#2887
Conversation
hackaugusto
left a comment
hackaugusto
left a comment
There was a problem hiding this comment.
please don't write the fuzz test as an integration test, it will be too slow.
jomuel
force-pushed
the
fuzz-testing
branch
5 times, most recently
from
17d19b0 to
86582c8
Compare
jomuel
force-pushed
the
fuzz-testing
branch
4 times, most recently
from
fe7dd0f to
b67caf4
Compare
jomuel
force-pushed
the
fuzz-testing
branch
2 times, most recently
from
807770d to
6129ef6
Compare
jomuel
force-pushed
the
fuzz-testing
branch
2 times, most recently
from
85a22ee to
825c3fa
Compare
| partner_balance = channel.get_balance(partner_state, our_state) | ||
|
|
||
| assert channel.get_amount_locked(our_state) <= our_balance | ||
| assert channel.get_amount_locked(partner_state) <= partner_balance |
There was a problem hiding this comment.
Here is a nice place to assert for the client invariants:
@loredanacirstea could you please give me a hand, I don't know where these invariants are listed, is it in only in the TokenNetwork.sol file?
There was a problem hiding this comment.
@hackaugusto , the constraints that we must enforce are listed here: https://raiden-network-specification.readthedocs.io/en/latest/smart_contracts.html#protocol-values-constraints. For both the smart contracts and Raiden client. Is this what you asked for?
| our_deposit = netting_channel.our_total_deposit | ||
| our_amount_locked = channel.get_amount_locked(our_state) | ||
| our_balance = channel.get_balance(our_state, partner_state) | ||
| assert 0 <= our_amount_locked <= our_balance <= our_deposit |
There was a problem hiding this comment.
This is not correct:
our_amount_lockedmay be larger thenour_deposit, this happens when we receive a transfer partner, which gives us a balance higher then our deposit.our_balancecan too be higher then our deposit, for the same reason
There was a problem hiding this comment.
Yes, and as mentioned in https://raiden-network-specification.readthedocs.io/en/latest/smart_contracts.html#protocol-values-constraints, we have the following invariant: (5.1 R) L1 <= AB1, L1 <= TAD, L1 >= 0. Therefore, the above assertion can be split into:
assert 0 <= our_amount_locked <= our_balance
assert our_amount_locked <= our_deposit + partner_deposit
Note that TAD is actually the total available deposit - if we had the withdraw implemented, we would instead have: assert our_amount_locked <= our_deposit + partner_deposit - out_withdrawn - partner_withdrawn. This MUST be changed at that point, so maybe worth documenting this in the withdraw issue?
There was a problem hiding this comment.
Thanks, I changed the checks accordingly.
jomuel
force-pushed
the
fuzz-testing
branch
5 times, most recently
from
dd2d327 to
1ba4e27
Compare
jomuel
force-pushed
the
fuzz-testing
branch
2 times, most recently
from
db2d687 to
5f87eac
Compare
|
I have a question though. I thought that the point of fuzz testing was to create state changes with random but valid values and feed them to the state machine and see that it does not break. I am not sure how we are achieving this with this approach. Perhaps I misunderstand how hypothesis works but the |
|
@LefterisJP Yes, Notice though that pytest will collect another test from this file, |
- wrap hypothesis' event function - change strategy for secret generation - add invariant checks - enable creating multiple channels
If an invalid but authentic secret request is received, we consider the whole transfer to be failed since the request was likely malicious. A following valid secret request will thus not be successful anymore. [no ci integration]
unskip regression test (fixed together with an earlier issue), correct invariant checks
[no ci integration]
LefterisJP
dismissed
hackaugusto’s stale review
The last comment by Augusto is addressed and he can't react since he is on vacation
5 participants
This PR contains a first messaging fuzz test (as requested in issue #1155) as a proposal what fuzz tests/property tests for raiden could look like.
Before this test is merged, it will be added to travis in a new job, extended to test more situations and the code will be cleaned up.