Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transient storage is persisting #216

Closed
sajal opened this issue May 10, 2024 · 7 comments
Closed

Transient storage is persisting #216

sajal opened this issue May 10, 2024 · 7 comments

Comments

@sajal
Copy link

sajal commented May 10, 2024

I'm not sure if this is a bug with boa, or pyevm, or that I'm doing something silly., here is the py-evm bug report ethereum/py-evm#2177

Basically at end of transaction, transient storage is not being reset.

py-evm==0.10.1b1
titanoboa @ git+https://github.com/vyperlang/titanoboa@4eaa59e6c4d884e6280e0d970e566044271942a6

Reproduce step:

import boa

def test_transient():
    cont = boa.loads("""
#pragma version 0.3.10
#pragma optimize codesize
#pragma evm-version cancun
cache: transient(uint256)
              
@external
def __init__():
    pass

@external
@nonpayable
def set(num: uint256):
    self.cache = num

@external
@nonpayable
def get() -> uint256:
    return self.cache
            """)
    
    assert cont.get() == 0
    cont.set(100)
    assert cont.get() == 0
@charles-cooper
Copy link
Member

it's expected behavior, not a bug. titanoboa executes all code within a single transaction context. the correct thing you want to do is call clear_transient_storage() in between contract calls, but we didn't add any machinery to titanoboa yet to do it.

closing this since it's not really a bug, but you can keep commenting here if you still need help.

@charles-cooper charles-cooper closed this as not planned Won't fix, can't repro, duplicate, stale May 10, 2024
@sajal
Copy link
Author

sajal commented May 10, 2024

This could be problematic.

Since we use transient storage as a cache, we now need to add command to flush it after each operation, which we can do because its our own contract in Vyper, however our goal is to have a lot of tests integrating with external protocols (in a forked network), we have low visibility into these protocols who may or maynot be using transient storage themselves, and the values not resetting will cause a lot of hard-to-debug issues in the future.

@charles-cooper
Copy link
Member

this is by design - you have a higher guarantee of correctness from testing if you are forced to work against the assumption that each call is isolated in a single transaction. in real EVM, you don't have this guarantee, so IMO your tests should not bake in the assumption.

@sajal
Copy link
Author

sajal commented May 10, 2024

you have a higher guarantee of correctness from testing if you are forced to work against the assumption that each call is isolated in a single transaction

Thats fair, however in our contract, during the transaction, we invalidate the transient cache whenever we expect it to change. This issue only manifested itself because in my test I time traveled to the future, it wouldn't have been an issue if it was in a subsequent call within the same tx (or same block even).

Perhaps consider a feature to flush transient if we time travel?

@charles-cooper
Copy link
Member

you have a higher guarantee of correctness from testing if you are forced to work against the assumption that each call is isolated in a single transaction

Thats fair, however in our contract, during the transaction, we invalidate the transient cache whenever we expect it to change. This issue only manifested itself because in my test I time traveled to the future, it wouldn't have been an issue if it was in a subsequent call within the same tx (or same block even).

Perhaps consider a feature to flush transient if we time travel?

that's a good idea. can you create an issue for this? i think there are some edge cases, like what happens if you travel backwards to a previous block.

@sajal
Copy link
Author

sajal commented May 10, 2024

Will do. BTW I didn't realize backwards time travel was possible!

@charles-cooper
Copy link
Member

Will do. BTW I didn't realize backwards time travel was possible!

yes, it is, and in fact maybe we should ban it in the high level api 🥲

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants