I recently noticed that `require(cond, Error(fn()))` eagerly evaluates `fn()` even if the condition evaluates as false.
In the legacy codegen there is this comment:
```
// Users probably expect the second argument to be evaluated
// even if the condition is false, as would be the case for an actual
// function call.
```
There is also docs pointing out exactly the behavior:
```
The two ways if (!condition) revert(…); and require(condition, …); are equivalent as long as the arguments to revert and require do not have side-effects, for example if they are just strings.
```
So, while this seems to be “as intended”, i would argue it’s a quite weird footgun as i guess the closest language equivalent `assert` in python, C, rust are all lazily evaluated. I did a small survey with my solidity peers and essentially everyone assumed this would be modelled as `if(!cond) revert g()`, while in practice it is modelled as `f(cond, g())`.
I don’t really see the reason to ever evaluate `g()` when the condition is not met. While changing this obviously is a breaking change, i think it should be reconsidered.
Thanks for the write-up.
Did you already see the discussion in issue 11201?
1 Like
Somehow i missed that, but after having read that not quite sure why it went stale and died off.
In both #11201 and #7877 people seem to be generally in favor of changing things, except for #11201#issuecomment-811391766 which is reasonable but also very opinionated.
I think that the expectation comes from the fact that until very recently (#10089) our docs stated that they are equivalent.
Perhaps, but perhaps not. As stated above, i think is more that ppl intentionally assume it to work like an assert in languages they are familiar with.
I don’t think that just changing the semantics of require() without changing the syntax is a good idea. It looks like a function call and to me for example it would be completely unintuitive if side-effects did not to happen when the condition is false.
To me it looks like an assert.
And on top of that it’s been working like that for a long time so it has a big potential to silently break code that already assumes the correct behavior.
I can get behind that one.
That being said, when checking for places where ppl use `f(cond, g())` i was only able to find occurences where:
- ppl migrated to `if(!cond) revert Error()`
- ppl wasting gas by eagerly reading state / calling some external view - most likely by accident
So while it should not be done lightly, also i think it should not be discarded.
1 Like