Invariant decorator option to revert state changes to the blockchain

[khegeman]

When developing stateful fuzz tests, I have had a few scenarios where I want to run a transaction to check that a condition holds, but I do not want that change commit those changes to the blockchain.

Here is an example of how I use this change in practice to verify that a user is able to can withdraw funds after a flow occurs, this should always be possible and I use the invariant to verify that.

 @invariant(period=1,commitChanges=False)
 def can_withdraw(self) -> None:
        #if a user has a balance, they should be able to withdraw and get all tokens back
       balance = self._bank.balanceOf(user);
       assert balance == self._bank.withdraw(balance, from_=user)

[hacker-DOM]

Hey @khegeman , thank for the Pr!
Fwiw I also agree with this design decision. In fact I would go a bit further: I think that invariants by default should not preserve state to better reflect human intuition of an invariant.
Fwiw here are my thoughts

1. I would rename the parameter to commit_changes
2. I would make it False by default
3. I would also rename the attribute to the same name (eg commit_changes)

[khegeman]

ok. I also think it should be False by default, but I didn't want to introduce a breaking change for any existing tests so I defaulted to the existing behavior.

[michprev]

Hey @khegeman, thank you for this pull request!

I agree with the comments from @hacker-DOM with the only exception that False as the default behavior may cause performance degradation. Let me evaluate this.

If we decide to go with the default False, I will merge this PR once I am sure the next release will be Woke 4.0 (because of the breaking change).

[khegeman]

I am still learning woke. Found this decorator in the docs today for snapshot and revert, adding this to an invariant does the same thing as my code change.

In further testing, I have noticed some unexpected side effects of using snapshot / revert with invariants, for instance tx_callback triggers for the invariant tx. In addition, I believe there are some issues with block.timestamp and anvil when using the snapshot /revert. I was noticing that inside my flows I was seeing that block.timestamp for flow n + 1 <= block.timestamp for flow n . This is with both my proposed change and with the decorator. Both exhibit the same behaviors.

    @default_chain.snapshot_and_revert()
    @fuzz_test.invariant(period=1)

A better solution for some of my invariants was to use a gas estimate, something like this:

self._bank.withdraw(balance, from_=user,request_type='estimate')

This will revert if it's not possible to execute withdraw and cause the invariant to fail. But does not commit changes to the block.
For situations where this is suitable , it avoids the issues I was noticing with snapshot / revert.

In conclusion, you might not want to merge this change.

[michprev]

Yes, this decorator already exists and can also be used as a context manager:

with default_chain.snapshot_and_revert():
    tx = foo.bar()

Still, adding an easier way to revert all chain changes caused by transactions in an invariant may be reasonable.

Regarding the issue with timestamps, you are right. It seems that Anvil does not increase the timestamp of the next block right after a snapshot revert. I think this can be considered an issue in Anvil, I will communicate it with the Foundry team.

You may try request_type="call" instead of estimate. Calls are implicitly used for the execution of pure/view functions but you can enforce the call request type even for state-changing functions.