Assertions

[chriseth]

We should have assertions, perhaps like the following:

function f(uint x) {
  assert x > 7;
  ...
}

The expression of the assertion has to be side-effect free (like a view function). The compiler might have a switch to include runtime-checks of these assertions or not. Furthermore, these assertions can be marked in the bytecode to be verified by formal analysis tools.

[chriseth]

We might want to have two types of assertions: Ones that are removed in "release build" and ones that are never removed. Yoichi: Other languages use a "panic" keyword.

Another way: We only remove assertions that are proven to never throw, which turns the whole logic into just another optimizer step.

[axic]

I never understood why removing assertions from release builds make sense. In debug builds usually there are other ways to tell how something went wrong, while assertions are the last resort. In release builds they are the only thing stopping before something undetermined happens.

[chriseth]

@axic I agree, especially for smart contracts, the cases where assertions should be removed due to gas costs are limited, although there might be some situations where you have an assert that runs a long loop and can only be called on a custom chain due to block gas constraints.

[bobsummerwill]

@axic Removal of assertions in release is usually for performance reasons. It's a C/C++ pattern, where they would usually sit behind pre-processor conditionals. The intention is that you have used them as a development aid and flushed out all the bugs by the time you ship.

You can end up with some redundancy if you want to defensive with that pattern, ie ...

assert (x > 7)

if (x > 7)
    doSomethingWhichWouldCrash()

[bobsummerwill]

It is an common anti-pattern for developers starting to use assertions for the first time to put all their defense into assertions, leaving code which will literally crash at runtime in the absence of the assertions. Missing null-pointer checks are a key example.

Writing Solid Code - now over 20 years old was where I first saw these kind of defensive programming practices explored at depth for the first time.

https://www.amazon.com/Writing-Solid-Code-20th-Anniversary/dp/1570740550

[redsquirrel]

This topic is also making me think of Design by Contract. Hoping that we can head in that direction.

[axic]

@bobsummerwill

assert (x > 7)

if (x > 7)
    doSomethingWhichWouldCrash()

By the time the if is added, the assert should be removed. This is just a mistake by the dev and doesn't explain why asserts should be removed for release builds.

[bobsummerwill]

Yes, @redsquirrel, and Eiffel :-)

Sorry, @axic, my example was wrong. Should have been ...

assert (x > 7)

if (x <= 7)
    doSomethingWhichWouldCrashGreaterThan7(x)

The anti-pattern, in a situation where asserts are stripped in release is to write:

assert (x > 7)
doSomethingWhichWouldCrashGreaterThan7(x)

In a debug build you hit the assertion and never execute the unguarded code, but it just crashes in the bad case in release.

[bobsummerwill]

"This is just a mistake by the dev and doesn't explain why asserts should be removed for release builds."

^ And such mistakes by the developers are EXACTLY the problem which assertions are seeking to solve, but the runtime checks have a measurable cost in a large-scale codebase (especially if you are doing such checks in inner-loop code). For many games, for example, I can tell you from personal experience that release builds with assertions enabled would often be 2x or so slower.

Also, the fact that they WERE only serving to catch programming errors was another reason for stripping them in release builds. If you hit an assertion there is often nothing sensible to do except stop. In a debug build, that can drop you into a debugger (or log to a file if you are in an automated run). If you are in a production environment, there is usually not much sensible which can be done at that point.

[bobsummerwill]

This is a dynamic analysis approach, and doing anything where these issues can be caught at compile-time with static analysis is way preferable.

[axic]

My experience is with embedded (financial) systems, where a undefined behaviour in such cases is not desired.

In our scenarios, aborting execution in a determined way when an assertion failed was worth the cost of the check. As there was no way to recover from that situation.

I think the same applies to contracts.

[chriseth]

I think in the smart contract world, you should not sacrifice correctness for performance. If the formal analysis tool cannot prove your assertion to be true for all cases, it should not be removed and stay there as a defense mechanism.

[bobsummerwill]

I agree with you both in our scenario.

Just providing context on why assertions were commonly stripped in release in "the old world" :-)

It is also worth noting that these assertion patterns predated ubiquitous connectivity. There was a very sharp disconnect between development and production environments in that era. Ship-and-forget. No real prospect of live telemetry.

Now the line between development and production environments is much blurrier. There is obvious value in having heavily instrumented builds available (optionally) in production environments. You really don't want there to be significant differences between your development and production environments.

[axic]

Is the assert syntax with or without parentheses?

[nmushegian]

Why can't assert(thing) just mean if (!thing) throw;? Why does it have to be a keyword at all, there seem to be enough assert(bool) and various overloads in the wild already.

I'm also suspicious of any optionally compiled anything

[pirapira]

When I verify the lack of assertion failures in theorem provers, I would want to see assert(thing) compiled into if (!thing) invalid_opcode rather than if (!thing) throw;. I don't want to prove "no throw is ever executed" (because users can specify wrong function signatures) but "no invalid opcodes are ever executed" is provable (if assertions never fail).

I'm suspicious of optionally compiled anything.

[axic]

With revert being accepted, assert could become revert. Even more so, it could take an optional string parameter for text to be returned:

assert(thing, "Thing is invalid");

becomes

if (!thing) revert("Thing is invalid");

[chriseth]

Yep, sounds good!

[axic]

Fixed by #1678.