perf: slightly cheaper ReentrancyGuard

[transmissions11]

avoids an ISZERO

[transmissions11]

here's what the IR diff looks like when i test the change on Solmate's ReentrancyGuard

[Amxx]

Hello @transmissions11

While this would work in all "clean" cases, I really fear it could break for contracts where the constructor was not properly called. This could be the case if a Clone or a Proxy delegates calls to a piece of code that uses this Reentrancy pattern.

Right now it works, because on the first call of a nonReentrant function, the value is moved from 0 to 2, then to 1. With your changes, the first call will not see the expected 1, and will assume this is a reentrant call.

@frangio Is that why the current version is != 2?

Like always, it's a balance between cost and security. In both cases, the gas gain is really small. We are talking about 3 gas. If the security consequence were null, I'd go for this saving, but I don't think they are null.

[transmissions11]

aight makes sense

[frangio]

Yes in fact this code was added in #2171 which was specifically about proxies.

As I commented there, this library shouldn't be considered safe for proxies in general, and the Upgradeable variant should be used for those cases. But we still try to keep it safe and the differences between the two variants to a minimum.

We could look into making the default be _NOT_ENTERED = 0 but I think we've evaluated it before and it was not worth it as explained in #2294 (comment).

Thanks for the suggestion @transmissions11.

[transmissions11]

yea i agree using 0-1-0 is worse than 1-2-1, appreciate the thorough review 👍