Skip assertions

[nventuro]

In OpenZeppelin, we're considering dropping some of our asserts and changing them to require (see here for some relevant discussion), changing the semantics of assert to 'shouldn't ever fail unless the code is buggy'. Because of that, solidity-coverage will report a line with partial coverage on each assert.

Could the requirement of assert failure being covered be dropped, or an option added to disable it (maybe with a huge error flag if the assertion ended up failing)? We'd hate to have our coverage figure drop because of a semantics difference.

Thank you for your time and awesome work!

[cgewecke]

@nventuro Yes. This was raised in #149 earlier this year and looking through the comments there I feel like both sides are reasonable...

I think we could add this as an option fairly easily as well.

[nventuro]

An option would be great, since there's not yet consensus on this topic. I look forward to it!

[PaulRBerg]

Should the label for this feature be updated to "0.8.0"?

I'm using solidity-coverage@0.7.10 and assert branches are market as not covered:

[cgewecke]

@PaulRBerg

Purely out of curiosity, why do you prefer using assert over require in this case?

[PaulRBerg]

Because of a require statement that's executed before:

require(vault.freeCollateral >= collateralAmount, "ERR_INSUFFICIENT_FREE_COLLATERAL");

Also because Solidity's SMTChecker needs both requires and assertions.

[cgewecke]

@PaulRBerg

Ah ok.. I would lean towards making this change without a major version bump since no one thinks these should branch and it's more like a bug.

Thanks for the SMT link - I didn't realize these have a working application now.

[PaulRBerg]

Happy to help.

And yes, SMTChecker is actually useful! Check out this presentation by Leonardo Alt. It uses assert statements to identify bugs in a contract.

[KholdStare]

@cgewecke Can probably close this issue since the fix has been merged?

[cgewecke]

Thanks @KholdStare ... sorry for the delay.