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

feat: commit state changes to test contract during invariant/fuzz tests #3005

Closed
mds1 opened this issue Aug 29, 2022 · 15 comments
Closed

feat: commit state changes to test contract during invariant/fuzz tests #3005

mds1 opened this issue Aug 29, 2022 · 15 comments
Labels
A-testing Area: testing C-forge Command: forge T-feature Type: feature
Milestone

Comments

@mds1
Copy link
Collaborator

mds1 commented Aug 29, 2022

Component

Forge

Describe the feature you would like

Right now, the state changes made by an invariant test (i.e. the state changes within the test contract itself, timestamp changes, and presumably other block.* changes) are not committed to the test contract after each call (for invariant tests) or after the run (at the end of a fuzz test). Without this, there is no way to test time-series types of invariants, such as "contract nonce should only ever increase" or "compound ctoken exchange only ever increases".

Committing state changes within a test should have no downside, and will enable testing these types of scenarios.

This feature pairs nicely with #2962 and #2985, to allow devs to get visibility into the intermediate state of the test contract. For example, it would be great to be able to have console.logs such as the ones below (below is annotated with comments for clarity):

// No calls have executed yet, invariant test runs
DEPTH: 0
lastNonce: 0
current Nonce: 0

// First call is made
DEPTH: 1
lastNonce: 0
current Nonce: 1

// Second call is made
DEPTH: 2
lastNonce: 1
current Nonce: 2

Committing state for fuzz tests seems useful too, helpful for avoiding stack too deep to save data to storage and then write a bunch of stuff to a file

@mds1 mds1 added the T-feature Type: feature label Aug 29, 2022
@gakonst gakonst added this to Foundry Aug 29, 2022
@gakonst gakonst moved this to Todo in Foundry Aug 29, 2022
@rkrasiuk rkrasiuk added A-testing Area: testing C-forge Command: forge labels Aug 30, 2022
@mds1 mds1 changed the title feat: commit state changes during invariant tests feat: commit state changes during invariant/fuzz tests Sep 1, 2022
@mds1 mds1 changed the title feat: commit state changes during invariant/fuzz tests feat: commit state changes to test contract during invariant/fuzz tests Oct 6, 2022
@gakonst
Copy link
Member

gakonst commented Nov 3, 2022

@joshieDo PTAL it seems to be we're missing some context across steps, the timestamp etc isn't persisted from call 1 to call 2

@PaulRBerg
Copy link
Contributor

Without this, there is no way to test time-series types of invariants, such as "contract nonce should only ever increase" or "compound ctoken exchange only ever increases".

To test the cToken exchange rate, you would have to inherit from the cToken contract in your test contract, no?

Otherwise, I don't see how this feature proposal could help with asserting time-series invariants.

@mds1
Copy link
Collaborator Author

mds1 commented Jan 23, 2023

You would not need to inherit from the cToken contract. The invariant test contract would store the last exchange rate after each call. Here's an example that might help convey the idea using a nonce counter: https://github.com/mds1/solidity-sandbox/blob/main/test/4_InvariantNonceGoUp.t.sol

@PaulRBerg
Copy link
Contributor

ooh, I see. Thanks for linking to that example, makes sense now.

Am I understanding correctly that in your example, the maximum value that nonce can have is 16, because the depth config option is set to 15, and Forge doesn't currently commit the state changes?

https://github.com/mds1/solidity-sandbox/blob/f139dca6163a6cc1ede5db5c81f4e5df40e8ee44/test/4_InvariantNonceGoUp.t.sol#LL65C15-L65C20

If it had done that, the nonce could get much higher, as you would get a "time-series" of runs that all monotonically bump the nonce. Which is what you want, since in the real world, state is indeed committed.

@mds1
Copy link
Collaborator Author

mds1 commented Jan 23, 2023

Am I understanding correctly that in your example, the maximum value that nonce can have is 16, because the depth config option is set to 15

Correct!

If it had done that, the nonce could get much higher, as you would get a "time-series" of runs that all monotonically bump the nonce. Which is what you want, since in the real world, state is indeed committed.

By "that" do you mean "commit state changes"? If so, the max nonce value in this example is only a function of depth. Committing state changes to the invariant test contract makes that easier to test, but you can work around it with an "actor contract" that tracks the nonce instead, as @lucas-manuel / Maple do in their repos

@PaulRBerg
Copy link
Contributor

Correct!

Understanding things is nice.

By "that" do you mean "commit state changes"?

Yes.

the max nonce value in this example is only a function of depth

I don't see why this is the case if the state between calls was committed by Forge. Couldn't nonce go bigger than 15? Specifically in this example, it should be able to go to 1+256*15= 3841.

@mds1
Copy link
Collaborator Author

mds1 commented Jan 23, 2023

In this nonce example, there are only two functions the fuzzer can call

  • increment the nonce by 1
  • decrement the nonce by 1

An invariant test behaves as follows:

  • Initialize test from post-setUp state
  • Call invariant_NonceGoUp and make sure it passes
  • Call a random method, in this case increment or decrement. Say we called increment, nonce went from 1 to 2
  • Call invariant_NonceGoUp and make sure it passes
  • Call a random method, in this case increment or decrement. Say we called increment, nonce went from 2 to 3
  • etc.

The number of times we "call a random method" is given by the invariant depth setting, which defaults to 15. So at most in this example you can't call a method more than 15 times

Once those 15 calls are made, run number 1 is completed. The state now resets, and we begin run number 2 from the same post-setUp state

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Jan 23, 2023

Thanks for the explainer but I think we're still talking past one another?

The state now resets, and we begin run number 2 from the same post-setUp state

Isn't the very purpose of the feature proposal in this issue to not reset the state? This is what I was getting at with the question from my two comments above.

  • Nonce becomes 15 at the end of run 1
  • State is not reset
  • Run number 2 commences
  • Nonce is increased to 16, then 17, etc. (assuming calls are made only to increment)

@mds1
Copy link
Collaborator Author

mds1 commented Jan 23, 2023

Isn't the very purpose of the feature proposal in this issue to not reset the state?

Right, and the state is preserved within the context of a run. Once a run ends, the state is reset to start the next run. A "run" in invariant testing is defined as above, i.e. alternating calls between "check the invariant" and "execute a random call", until depth random calls have been executed.

I think the confusion is the definition of a "run". In a standard fuzz test, a run is just "a single execution of all the code within your test method". In invariant tests, a run is defined differently.

@PaulRBerg
Copy link
Contributor

the state is preserved within the context of a run

I see. It should then follow that the maximum value that the nonce in your example can have given the current version of Forge is 1, because state is not preserved between calls.

In your example the maximum nonce would be 15 only after the feature proposal in this issue gets implemented in Forge.

I think the confusion is the definition of a "run"

Yeah, that was one bit of confusion, so thanks for the additional color.

@mds1
Copy link
Collaborator Author

mds1 commented Jan 24, 2023

I see. It should then follow that the maximum value that the nonce in your example can have given the current version of Forge is 1, because state is not preserved between calls.

Depends which nonce var you are referring to:

  • The one in the storage of contract-under-test does increase and is preserved.
  • The one in the storage our test contract is not preserved, which is the scope of this issue
  • If you create a "test data" contract and tracked the nonce there instead of in the test contract itself that would also be preserved

It's only the test contracts themselves which don't preserve state between calls of an invariant run

@PaulRBerg
Copy link
Contributor

I was always referring to the storage of the test contract in my comments above, but I see that my wording was ambiguous - thanks for clearing this up.

@PaulRBerg
Copy link
Contributor

After spending a few hours debugging a bug related to this issue, I want to add a few clarificatory remarks to what has been said in this thread.

It's only the test contracts themselves which don't preserve state between calls of an invariant run

Actually, any state change brought about in an invariant test (a function that starts with invariant_) will be discarded by Foundry. It doesn't matter if the state change is in a separate contract; that will be discarded, too.

  • If you create a "test data" contract and tracked the nonce there instead of in the test contract itself that would also be preserved

This works only if the test data is recorded via a targeted contract (e.g. a handler). If you attempt to update the state of this test data contract in an invariant_ test function, nothing will be preserved.

Note: this might have been obvious to you, but it wasn't for me, and I wanted to explain how this works in case others end up in my shoes.


The following code snippet shows what I mean above:

See code snippet

import { Test } from "forge-std/Test.sol";
import { console2 } from "forge-std/console2.sol";

contract Handler {
    uint256 public counter;

    function increment() external {
        counter++;
    }
}

contract Store {
    uint256 public counter;

    function increment() external {
        counter++;
    }
}

/*
 * Run this test with the following settings:
 *
 * [profile.default.invariant]
 * depth = 5
 * runs = 1
 */
contract FooTest is Test {
    Handler internal handler = new Handler();
    Store internal store = new Store();

    function setUp() public virtual {
        targetContract(address(handler));
    }

    function invariant_Foo() external {
        // This has a value of 5
        console2.log("handler.counter()", vm.toString(handler.counter()));
        // This always has a value of 0
        console2.log("store.counter()  ", vm.toString(store.counter()));
        store.increment();
        assertTrue(true);
    }
}

@simplyoptimistic
Copy link

+1 I would also like this feature, think it would help a lot with examining state of invariant test runs.

I think while this feature is not yet implemented, it might be worthwhile updating the documentation to make this behavior explicit as it may confuse developers who might be expecting state to persist in the invariant test contract.

@Evalir Evalir added this to the v1.0.0 milestone Jul 13, 2023
@mds1
Copy link
Collaborator Author

mds1 commented Mar 6, 2024

Marking this as closed by preserve_state which was implemented in #7219, cc @grandizzy

@mds1 mds1 closed this as completed Mar 6, 2024
@jenpaff jenpaff moved this from Todo to Completed in Foundry Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing Area: testing C-forge Command: forge T-feature Type: feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants