EIP-1153: Transient storage opcodes

At the risk of not making any friends here, I’d like to voice some concerns about this from the Solidity perspective (beyond those already voiced by @chriseth above).

While it may not be a huge effort to implement this on the execution layer in clients, this change induces a severe increase in the complexity of the semantics of the EVM that I’m not sure is fully appreciated in the discussion so far. Just some points I haven’t seen discussed exhaustively so far:

  • No matter what we do on the language level itself, in its current specification, this will be abused for in-memory mappings that are meant to be semantically call-local, but which will nonetheless bleed through on the transaction-level, which will increase the danger of reentrency bugs.
  • In general (as others have commented already), I’d expect transient storage to increase the complexity of any kind of static analysis and manual code analysis / auditing significantly.
  • I’d also expect the induced increase in semantic complexity and the reduction of the purity of EVM calls to cause pains for formal verification, in perpetuity.
  • On the language level alone, we’d need to consider whether transient storage warrants a new serialization format, which would significantly increase the complexity of the language semantics, and even if not, an additional data location means increasing the testing surface by an order of magnitude (the EVM already has an unhealthy amount of distinct data locations as is). In general, proper language support for this is non-trivial.

As far as use cases are concerned, transient storage still seems like a hacky catch-all solution to me - for reentrancy locks introducing an entire new address space of data is utter overkill; passing complex data structures between calls seems like an issue to be addressed in how calldata works instead; using it as a replacement for in-memory maps is downright dangerous etc. pp.

All that being said, if this makes it to a hardfork, we will of course have no choice, but to implement support on the Solidity level - but I sincerely hope you have thought this through properly and properly consulted with all the affected parties before jumping at this - to me personally, to be honest, seeing given the state of the discussion, it seems a bit premature to be considering this a candidate for inclusion in Shanghai.

EDIT: changing “seeing” to “given” since apparently otherwise it’s assumed that I just saw this for the first time and am randomly commenting out of the blue.

3 Likes

I don’t have much else to contribute after @ekpyron’s post. But honestly I think this is just so exaggerated. You’re seriously listing KYC as a reason to change EVM semantics heavily?

Regardless of whether it should go into any hard fork, it does look like the discussion has not taken into account all the potential problems this might bring. For the people championing the EIP, are you really sure that you’ve thought through the problems mentioned above, and that you’re actively fine with it instead of just ignoring it because it brings you the feature you want?

I don’t think more work for the clients and compilers is a problem per se, of course with progress come the changes and they need to be supported. But we need to be sure (or at least convinced) that this is for the good of the EVM, and not just a hack that will make a few happy and the EVM worse.

edit:
Just adding a final point: many of the listed problems will not go away by simply choosing “tx level or call level or whatever else”. There are clear pros and clear cons about transient storage. Those are likely not going to change. In the end there needs to be a decision on which outweighs which. Either the problems are too much, or we’re fine with them given the pros we get.

1 Like

Given the unanimous app developer support, it’s quite silly for solidity compiler devs to join the discussion this late to tell their users they shouldn’t have it for their own protection. It’s the app developer’s responsibility to ensure the security of the contracts they write, not the compiler or other tools, which is why the arguments that bugs could be written with it are unconvincing.

It’s still much more expensive than memory, and in my experience in-memory maps are rarely, if ever, useful in smart contracts. Especially maps costing as much as 100 gas per read/write. You are better off iterating over the entire list up to a certain size of list for 50+ entries. So, yes you’re right, but it’s almost never useful, so this particular concern is insignificant in comparison to all the other ways you can be vulnerable to reentrancy.

Very general concern, please provide examples. Having written and read code that uses transient storage possibly more than anyone so far I don’t think it makes code any harder to read, audit, etc…

It’s exactly the same as regular storage, you either clear it at the end of a transaction or very intentionally don’t clear it for future calls. Forgetting to clear transient storage has the same reentrancy issues as forgetting to clear regular storage used transiently. It’s not actually a reentrance bug that in a later transaction the slots were 0 again (although it may cause some other kind of bug, depending on your use case).

“Reduction of purity”, could you explain? And please be specific with the pains it causes?

My experience with formal verification, having tried to do FV with Uniswap V3, is that it’s already impractical for stateful contracts. That said, the difference here is now you have to consider subsequent calls across transactions as well as subsequent calls in the same transaction. Meaning transient storage slots can either have a value set from a previous transaction (same as storage) or 0 values at the beginning of a transaction. That doesn’t seem like a huge difference, and it won’t even matter for contracts where formal verification is actually practical.

It depends heavily on your current abstraction, according to @big_tech_sux on Twitter it was added to Vyper in ~20 lines of code.

Do the storage refunds used for this today strike you as elegant? Considering they have a 20% cap, in order to get rid of gas token, and you still have to read from storage a value that is always 0? Please suggest an alternate solution if you are going to call this one overkill. I would love to support a simpler solution that supports the required use cases, enables parallel execution, allows simplification of storage refunds, etc.

I’ve championed for this since late last year, discussed with @ethchris as early as February this year. It’s a bit late to be getting up to speed publicly on the discussion, as your concerns are more premature than the EIP. The EIP is already implemented in several clients with passing comprehensive cross-functional EVM tests. Last I checked it’s further along in terms of implementation and specification than any other Shanghai CFI EIP.

There have been many opportunities to suggest alternatives (to transient storage specifically) from the core solidity team, and I still haven’t seen any suggested from the team. Asking that you please provide specific examples and solutions in your feedback, rather than general concerns at this point.

1 Like

Given the unanimous app developer support

Unanimous? So you asked literally everyone?

it’s quite silly for solidity compiler devs to join the discussion this late to tell their users they shouldn’t have it for their own protection

This is disingenuous and borderline disrespectful. This is an open discussion and anyone is entitled to share their opinion at any point in time.

it was added to Vyper in ~20 lines of code.

Have you actually verified that? Here’s the commit that added it:
https://github.com/vyperlang/vyper/compare/master...transient
Yes, 20 lines of code, but not a single test. I checked the tests directory, not a single reference to transient. That may be fine for a PoC, but doesn’t look ready for production imho. (which is ofc understandable, to not add full support until it’s actually confirmed)

It’s a bit late to be getting up to speed publicly on the discussion

Why are you trying to gatekeep this discussion? I disagree that the concerns are more premature than the EIP, since a few haven’t even been properly answered to, only conveniently “forgotten”, as seems to be a common way of getting rid of EIP criticism. The EIP being implemented in different clients is probably a requirement but it’s far from being sufficient for something to go in, is it not?

There have been many opportunities to suggest alternatives (to transient storage specifically) from the core solidity team

Why should the Solidity team have to provide better alternatives to it? At this point a few members are simply voicing unresolved concerns that may worsen the EVM, and your attempt to call them “resolved” and say “it’s too late” is weird, to say the least. None of us here get to make those decisions alone.

2 Likes

It’s rather silly to claim that this it’s late for bringing this up, if for example the issue for static analysis has come up as early as 2018 (EIP-1153: Transient storage opcodes - #25 by Arachnid). Issues don’t just vanish if ignored. Same goes for @chriseth’s concerns. As for the reduction in the purity of EVM calls @frangio explained that already above.

As for non-trivial language support: I can of course hack in support of the opcode in a few lines of code without tests, we have a PR doing the same, that doesn’t mean that it suddenly becomes a properly tested high-level language construct just like that. But as @leonardoalt already pointed out, implementational difficulty is not the major issue here, I’d just want to clarify that considering that to be trivial is a misrepresentation.

And, of course, compiler developers, static analysis and formal verification are major parts in smart contract security and should of course be consulted in evaluating such a change that, after all, targets to become a high-level language construct.
Are there any statements of these changes not causing issues and being perfectly fine and manageable from people doing static analysis or formal verification or auditors? My main point is that a change like this IMHO requires active positive confirmation from these parties and I don’t see any of that.

2 Likes

Lists of developers that are in support here and here.
Mark Tyneway from Optimism as well as Arbitrum (posted above, also consulted DZack23 on twitter) are in support.
If you don’t consider the developers that are writing the code responsible for the majority of the Ethereum chain usage ‘unanimous’, I don’t know what to tell you. If you haven’t seen these lists before, then that’s exactly my point about joining the discussion late.

I’m pointing out that if you are saying it’s a question of tradeoffs, you should give more weight to user feedback

I am asking you to get up to speed privately and then post examples and specific feedback, rather than general concerns. I have responded to every single point in @ekpyron’s post as well as @ethchris’s, that’s pretty far from gatekeeping the discussion I think. An open discussion is not the same as an open and productive discussion.

From Nick in July 2022.


From @frangio about static analysis https://twitter.com/frangio_/status/1589372426135011329

I will seek more out. Anyone in particular you want to hear from?

Is everyone aware of the potential problems, new complexity etc and are OK with that, or just interested in gas savings? Ofc I can’t say for sure either way, and won’t assume, but tweets that likely didn’t warrant the proper diligence rather sounds like the latter. Also the software like clients and compilers adding code support to them doesn’t mean support for it getting in, as can be seen with the Solidity PR also being there.

Huh? Why is that?

With all the deserved due respect to Nick and Frangio, screenshots of texts and single sentences of the form “sounds simple” hardly show diligence.

I’d for example be interested in the position of people working with formal EVM semantics, like e.g. Runtime Verification, or such. Carrying another kind of state with different semantics through calls of course adds to complexity there. Be it manual or not, it basically doubles the effort to prove invariants of a smart contract, since they’re contingent on the position in a transaction call chain. Is all of that managable? Theoretically, sure. Is it trivial? Depending on what you do maybe, maybe very much not. Is that enough reason for being sceptical about this EIP? For me it is, in general it may, but doesn’t have to be.

But anyways, I actually don’t quite understand the aggressiveness. Of course, I’m aware of this having the support of application developers - but that doesn’t mean that it has to universally be considered as a good move (especially in any fixed particular variant).

As for memory mappings: I’ve even seen this mentioned as use case and I think you’re underestimating the cost of iterating lists. I don’t see the cost as the limiting factor here, until it’s basically back at a cold storage read of a zero - a reduction of the address space would be, either for the opcodes or with the EOF-based marking of storage slots as alternatives.
Storage repricing and refund adjustments are also still alternatives that have come up often. Which are complex and not nice, sure, but for modelling storage not merely as disk space, but as cached disk space, which accurately reflects what happens in clients, I don’t see them going anywhere regardless. Would also be interested how this could no longer or better be done with Verkle trees.

In any case, I’m mainly pointing out that this EIP doesn’t only have fans (and the response IMHO actually reaffirms that this is more than necessary). It’s not like you will and have to have unanimous support in the end, but as I said originally, I hope the decision takes the price of this in terms of complexity into account (which surely is unarguably not just zero :-)).

1 Like

To clarify: This by no means intends to disqualify anyone’s opinions. Everyone on the “support” list is ofc 100% knowledgeable and capable of judging this. However I do believe that scattered support like screenshots and tweets are different from structured support, but that’s just my opinion. I’m completely fine with being on the “wrong” side here :slight_smile:

Probably closer to 10 than 50, but when was the last time you saw the need for an in-memory map in solidity? Particularly one with any size? I don’t expect this to practically be used solely for in-memory maps, but regardless you can always clear the entries in the map before the end of the call.

You can’t reprice out a wasted storage load for a value that is always known to be 0 at the beginning of a transaction. @Philogy explains this rationale a few posts above.

If you’re marking some storage slots as transient via EOF, what is the practical difference? It seems to have all the same issues, except you can’t tell from the opcode alone if it’s a transient slot or not, so it seems even harder to analyze. Edit: furthermore, you cannot have mappings with keys that are determined at runtime (e.g. token addresses).

Storage refunds may stick around for original-new-original writes, but not necessarily zero-nonzero-zero writes if transient storage is available. This simplification alone would make storage refunds much easier to talk about since it removes a branching condition. Alternately, with transient storage, SSTORE/SLOAD pricing could be made completely dumb about caching and the contract can use a TSTORE/TLOAD mapping to move it to the application layer.

As for storage pricing and refunds: the main point there is that you’re left with one cold load no matter what refunds and whatnot, right? Can a cold load from a full zero page in verkle trees be cheaper as well, hence allowing for reserving full pages as transient storage? Honestly, I have no idea, maybe not. Maybe even if, it’s messy and not a good idea to go that way. But that’s not my area of expertise. But the advantage of this direction is that the clearing is fully explicit and the EVM semantics stays simpler and there’s no implications for composability as Chris hinted at. (Of course at the cost of more complex gas accounting no matter what, but that complexity is actually less relevant for analysis and FV, since you can usually assume to have enough gas for that purpose anyways)

As for memory mappings: A full loop iteration for iterating a list I’d guess at 10-20 gas, so worst case you’re at 100 with 5 to 10 elements, average case 10 to 20 - that’s not a long way. (Granted that’s only comparing reading the thing, and writing to it before would be quite more costly in a transient storage version as well) But anyways, of course you don’t see memory maps around now, since they’re costly and a pain to implement right now - which may change with the ability of abusing transient storage for them, that’s the main point :-). And if you did that, clearing the map is actually the costly part (since then you need to keep and traverse a list of keys), so you’re not unlikely to not do that. (and I mean, implementing cheap memory mappings in actual memory has been requested from us, it’s just not feasible with current memory design, but it’s not too crazy of a concept in general)

Maybe that concern is ultimately unwarranted, but I’d maintain that it’s a valid concern in any case.

To avoid this issue alone, restricting the address space would help. I.e. if I just don’t have enough transient storage available (either by restricting the address range of transient storage explicitly or implicitly by it only being possible to mark a low count of storage slots as transient via EOF), I can’t abuse it in this way. On the other hand, one of the use cases I’ve read also appears to be passing complex data structures like mappings through calls, it’s probably impossible to keep that and prevent the abuse as call-local mappings at the same time (personally, I’d still argue that passing data around like that would be better done with more flexible calldata in principle, but anyways).

What marking slots in EOF indeed doesn’t account for either is the increase in semantic complexity, that stays the same and is a clear gaping con of transient storage in general. It may be valid to conclude that the pros outweigh this - I’m personally not convinced by that, but I can relate to the opposite position. The fact that this doesn’t even occur in the list of relative cons of the EIP and I don’t exactly see it conceded as a valid concern, made me worry if it is even properly weighed in at all, though. (Also not entirely sure if people doing FV would appreciate a position of basically “FV is too hard anyways, so it doesn’t matter if it gets even harder” ;-))

But yeah, wrt memory mappings I guess you can get away with “well, then people just shouldn’t be doing that” and maybe that’s fair enough.

Wrt static analysis, auditing and FV, I’d at least want to make sure that this is given sufficient thought, since transient storage will make things more complex - that can’t just be denied entirely, can it? If that’s generally deemed worthwhile, that doesn’t make me happy, but is also fair - it just should be properly considered at all IMHO.

These two posts basically sum up my feelings on the issue. The goal here is to provide a fundamentally different memory region, with different scoping, longevity, and pricing than any existing memory region? Then it should be a separate memory region, rather than leaving it up to the clients to cache smarter.

I am fairly confident this will not be hard to implement in KEVM (client that RV maintains), and I don’t think it will increase the complexity of verification using KEVM significantly. I can’t speak for other tools. I think with good usage of this feature, it may even reduce the complexity of some verification efforts (many variables you’ll be able to tell immediately that they cannot alias, for example, or if an entire modifier only uses transient variables, maybe we can have modular verification of the modifier more easily?).

It seems to me as well that it is not too much complexity for clients, because several clients have been modified to handle this new opcode, and tests have been provided (though maybe this could have been done earlier in the discussion, I know that having tests increases my confidence quite a bit: EIP-1153: Transient Storage tests by moodysalem · Pull Request #1091 · ethereum/tests · GitHub).

That being said, I cannot speak for other tools. Our semantics and verification is based on symbolic execution, which is different than other tools. I also can’t speak for the Solidity compiler, but does the Solidity compiler need to support the feature immediately? Can we let devs use inline assembly, let a few examples of how it’s being used trickle in, then give people the version of the feature that has compiler-guaranteed guardrails in place?

The first users of the feature, whether via the Solidity compiler or not, are going to be taking the brunt of risk here (risk of not understanding the new feature correctly, or risk of Solidity compiler behaving unexpectedly on the new feature). I am usually a fan of the “give the devs tools, and let them figure out how to not shoot themselves in the foot” approach. I do think that the devs opinions here are more valuable than my own opinion, they are the ones trying to innovate here.

Also shoutout to @pcaversaccio for the diagram, I find these types of visualizations very helpful.

4 Likes

Ok, fair enough. And sure, we can provide plain assembly support immediately, properly optimizing may take a bit longer, high-level language support a bit longer still, but we can manage, I’m not so much concerned with that, but with the complexity of the language semantics that inherits the increase of the complexity of the EVM semantics.
But if this is deemed a non-concern, I consider myself beat on this.

I do think it will be easy to make pathological hard-to-analyze code here.

But I don’t think these types of examples are what people will be trying to do formal verification on, and I think you could make the same or similar examples using normal storage.

Double-edged sword I guess.

2 Likes

If the TSTORE opcode is called within the context of a STATICCALL, the call must revert.

This is different wording than how STATICCALL handles writes in static contexts as defined in EIP-214. Is the behaviour intended to be different?

1 Like

It’s not intended to be different, and I can adjust to this if it sounds more accurate:

If the TSTORE opcode is called within the context of a STATICCALL, it will result in an exception instead of performing the modification.

I’d certainly appreciate it, but I’m also a pedant ;3

1 Like

I don’t think the language semantics get much messier: this storage behaves the same as indexing a variable by the transaction. Now the transaction hasn’t appeared before, so symbolic analysis based on model exploration will have to change, but it doesn’t seem to me to be that bad.

1 Like

PEEPanEIP #91: EIP-1153: Transient storage opcodes with @moodysalem

2 Likes