ReentrancyGuard with transient opcodes TSTORE and TLOAD

[pcaversaccio]

🧐 Motivation

Since EIP-1153 is confirmed to be included in the upcoming Cancun-Deneb upgrade, I would like to discuss the possibility of using the future transient opcodes TSTORE and TLOAD for the ReentrancyGuard (abstract) contract.

Furthermore, I would like to highlight the initiatives (partially driven by myself), to disable reentrancy by default at the compiler level:

• Vyper: VIP: disable re-entrancy by default vyperlang/vyper#3380
• Solidity: Language feature: disallow state-changing effects after an external call by default ethereum/solidity#12996

[Amxx]

Hello @pcaversaccio

For the first part, this is definitely something I'd like to do when possible. The main issue I see is that this new opcode will not be available on some EVM network, so we'll need a solution for that. In any case, I think we will wait to see how the solidity language supports this feature.

For the second part, I'm personally against it. Yes re-entrancy can lead to bugs, and being 100% safe against any form of re-entrancy (including read-only) re-entrancy is possibly difficult. But I would still qualify re-entrancy as a great feature for contract composability. IMO contract composability is possibly the most important feature of the EVM ecosystem. I fear that preventing re-entrancy by default would seriously limit composability. Also, I'm generally against any "forbid by default" approach. If a dev want to block re-entrancy in its app, then ok ... but we should not set that by default. I'd much rather see devs learn how to build re-entrant safety thatn learn that re-entrancy is disabled by the language.

Additionally, I'm afraid what it will lead to: people writting entire contracts in yul directly to circumvent that restriction?

Since 0.8.0 made safe-math by default, we see a lot of people advertising the use of unchecked in ways that affect the readability of the code. This trends is honestly driving me mad:

Will we see the same trend of people trying to circumvent the re-entrancy prevention (sometimes for good reason) and write bad code as a consequence?

[pcaversaccio]

For the first part, this is definitely something I'd like to do when possible. The main issue I see is that this new opcode will not be available on some EVM network, so we'll need a solution for that. In any case, I think we will wait to see how the solidity language supports this feature.

Sure, my intent on this issue was not to immediately jump to a technical solution but to rather gather a discussion in one place. I think you raise a very important point on the differences within EVM implementations. I would go even further and claim that this discussion is not only about the transient opcodes right now, but generally about how OZ plans to deal with differences in the future. Just as another example, where a precompile breaks a common EVM assumption, is Moonbeam:

Or another example is the behaviour of CREATE and CREATE2 on zkSync; I made a tweet thread about here. The only way I currently see how to solve this generally, is to somehow make the logic dependent on the chainId, but I haven't thought about this properly and also this would increase the gas costs. So maybe we can take this discussion as a good opportunity to have a conversation about a future-proof model, where not all chains are fully EVM equivalent.

For the second part, I'm personally against it. Yes re-entrancy can lead to bugs, and being 100% safe against any form of re-entrancy (including read-only) re-entrancy is possibly difficult. But I would still qualify re-entrancy as a great feature for contract composability. IMO contract composability is possibly the most important feature of the EVM ecosystem. I fear that preventing re-entrancy by default would seriously limit composability. Also, I'm generally against any "forbid by default" approach. If a dev want to block re-entrancy in its app, then ok ... but we should not set that by default. I'd much rather see devs learn how to build re-entrant safety thatn learn that re-entrancy is disabled by the language.

I feel like those are all valid arguments. But I would like to share some proven facts from practice (at least my experience over the last 8 years or so):

• Devs do not consistently adhere to CEI, otherwise we would not see so many reentrancy attacks,
• Since Solidity has already introduced unchecked in version 0.8.0 (i.e. a full footgun protection on math operations by default), we have noticed that there are fewer positive or negative overflow exploits,
• Billions of lost funds justify a certain degree of protection IMHO (see my repo on reentrancy attacks here),
• Multiple other languages on other blockchains disabled reentrancy by design (e.g. Radix or Fuel) and we've seen a positive development there,
• Reentrancy as a language feature is built for the 5-10% of professionals that understand the underlying implications (e.g. not everyone needs to build something related to flash loans), but not the remaining majority that implements for example a simple withdraw function using a low-level call and doesn't understand what means handing over the execution flow.

Will we see the same trend of people trying to circumvent the re-entrancy prevention (sometimes for good reason) and write bad code as a consequence?

I hope not :) - I honestly think that the unchecked arithmetic trend is driven by the bad performance of the compiler optimiser, whilst disabling reentrancy by default is much more security focussed. Disabling a reentrancy requires much more brain work IMHO, than using unchecked for e.g. a multiplication. Whenever we can have unsafe external calls, we need to think about what can be unsafe exactly, and how to prevent it. So I think devs will become more prudent hopefully (ofc I can be wrong here).

[Amxx]

Reentrancy as a language feature is built for the 5-10% of professionals that understand the underlying implications

My personal view on that (which is not OZ position) is that maybe those 5-10% of professionals should be the only one writting defi apps. I don't think "everyone should write smart contracts" like "everyone should write python scripts to help with taxes" or "everyone should write php to customize their personal wordpress".

If the language decides to help the 90-95% its ok, as long as they don't make the work of the 5-10% more difficult to write and review.

[Amxx]

So maybe we can take this discussion as a good opportunity to have a conversation about a future-proof model, where not all chains are fully EVM equivalent.

That is an interesting and necessary discussion that we don't really have so far. We worked on tooling to identify some key point of the VM behavior, in order to check our contract will behave as expected. This will possibly detect missing/modified features, but it won't identify new one (like the moonbeam precompile you mentioned).

Its also unclear what we want to do with this information:

• add warning that contract X will not work on network Y ?
• propose an alternative implementation ?

[pcaversaccio]

If the language decides to help the 90-95% its ok, as long as they don't make the work of the 5-10% more difficult to write and review.

I guess there is no question that this is the ultimate goal.

That is an interesting and necessary discussion that we don't really have so far. We worked on tooling to identify some key point of the VM behavior, in order to check our contract will behave as expected. This will possibly detect missing/modified features, but it won't identify new one (like the moonbeam precompile you mentioned).

Is that kind of tooling open-source?

Its also unclear what we want to do with this information:

• add warning that contract X will not work on network Y ?
• propose an alternative implementation ?

Based on my experience, warnings are easily ignored. One solution could be to simply state in a disclaimer, that only fully EVM equivalent chains are supported (I drafted an EIP on the definition of this term here because I think it's very important to align here as a community, but unfortunately they closed it; I'm trying to push them to reconsider it again). But that solution might be too high-level and will be ignored by the folks that simply npm install OZ contracts. Will need to think about it.

[pcaversaccio]

Another comment on EVM equivalence given the current circumstances: We are right now in a situation where there are numerous chains that do not support the latest EVM version shanghai with the opcode PUSH0. This means that, for example, the use of the latest Solidity version 0.8.20, which supports PUSH0, will likely fail with these chains (if it includes PUSH0 in the init code and gets executed at creation time, the deployment fails; if the runtime code has PUSH0 and is not executed as part of the deployment, the runtime code can revert on calls) and thus these chains can no longer be considered fully EVM-equivalent. I don't think this is OZ's responsibility to ensure EVM-compatible bytecode, but what if such differences increase over time and certain critical OZ logic cannot be deployed or executed anymore due to EVM version differences? I don't know the solution to this problem, but I just want to highlight it since I deem this important.

[frangio]

Is that kind of tooling open-source?

It is a test suite rather than tooling, and it isn't really open source quality. One learning I can share is that it's more tough to deal with the "network level" specifics of a testnet than it is to deal with the EVM differences across networks (for now)!

---

Regarding the point about EVM diverging, I share the concerns but I don't see anything that we can do for now rather than observing how PUSH0 adoption evolves.

I also think that this issue about transient ReentrancyGuard is a little premature 🙂 but rest assured I also want to see that implemented.

[pcaversaccio]

It is a test suite rather than tooling, and it isn't really open source quality. One learning I can share is that it's more tough to deal with the "network level" specifics of a testnet than it is to deal with the EVM differences across networks (for now)!

Can you quickly elaborate on what you exactly mean by the "network level specifics"? I.e. maybe challenges with the gossip protocol, state tries, signature verifications etc.? (and sorry for going off-topic here, since this issue is about the ReentrancyGuard, but I feel it's still important)

I also think that this issue about transient ReentrancyGuard is a little premature 🙂 but rest assured I also want to see that implemented.

Yeah, I was just thinking about it and wanted to drop my thoughts into an issue so we have a go-to source for this discussion. I mean I could also open another thread on FEM, but since it's implementation-specific I thought the OZ repo is appropriate 🙂.

[sambacha]

Reentrancy as a language feature is built for the 5-10% of professionals that understand the underlying implications

My personal view on that (which is not OZ position) is that maybe those 5-10% of professionals should be the only one writting defi apps. I don't think "everyone should write smart contracts" like "everyone should write python scripts to help with taxes" or "everyone should write php to customize their personal wordpress".

If the language decides to help the 90-95% its ok, as long as they don't make the work of the 5-10% more difficult to write and review.

My man, yes. Concurrence is hard to grasp to begin with.

We dont want to cater to everyone. Principled engineering is an attitude a a much as its experience and knowledge. If you are not serious to begin with go do something else.

We want fanatics, no part time believers.