search

ConsenSysMesh / singulardtv-contracts

19 stars 16 forks source link

Test for emergencyCall #64

Closed ethers closed 8 years ago

ethers commented 8 years ago

https://github.com/ConsenSys/singulardtv-contracts/commit/fcaa4e95291385a6c8f4abe5f0c345d01fb755f9

The commit above would be clearer as if (this.balance < fundBalance) ... (If I've got less money than I should have then throw)

Also, a test for emergencyCall should now be possible.

Georgi87 commented 8 years ago

How would you test emergencyCall? There is no known way to withdraw money without lowering fundBalance too.

ethers commented 8 years ago

Sorry I should have been clearer: I added a comment to https://github.com/ConsenSys/singulardtv-contracts/commit/fcaa4e95291385a6c8f4abe5f0c345d01fb755f9#commitcomment-18767587 The emergencyCall test may not be possible now, but it would have been tried.

nemild commented 8 years ago

@Georgi87, I think this is a good discussion to have, and my view is that everyone has to get in the habit of testing/triggering invariant calls. It doesn't help to have fail-safes that are untested, since there's a risk they won't work when the time comes. In this case, the invariant is pretty simple - but in many other cases it won't be or there'll be interaction effects among other invariants.

One suggestion is to have a test process that concats on a way to break invariants to the original contract (restrict this invariant breaking capability to the owner, if that role exists) before test deployment, and then try a number of calls to the other functions that affect state. Clearly, this isn't perfect and there's also the risk that you'll mistakenly deploy the flawed contract - but it's a start while we debate more. (I'm open to other ideas as well)

ethers commented 8 years ago

Added by @Georgi87 https://github.com/ConsenSys/singulardtv-contracts/commit/14ce1fc007528eb37ac55c332190d1b4d268adea#diff-efd269c7520a568749c4ff90ebde2a36R28