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: more flexible/powerful ways to define and test invariants #3452

Open
mds1 opened this issue Oct 4, 2022 · 5 comments
Open

feat: more flexible/powerful ways to define and test invariants #3452

mds1 opened this issue Oct 4, 2022 · 5 comments
Labels
A-testing Area: testing C-forge Command: forge Cmd-forge-test Command: forge test T-feature Type: feature

Comments

@mds1
Copy link
Collaborator

mds1 commented Oct 4, 2022

Component

Forge

Describe the feature you would like

This is an idea / open for discussion, feedback encouraged.

Some invariants are hard to specify with the current UX, such as "only calls from <addressX> are allowed to modify the allowance[addressX][spender] mapping". I haven't thought of a way to enable things like this without a preprocessor/domain-specific language, so here is one proposal to test for things like this.

A modifier-like behavior for storage variables could allow this, i.e. solidity code that executes before and after a given slot is touched to verify some property of that slot. For example, below we use a comment to annotate the balanceOf function, which tells forge to run the verifyBalanceOf method before/after a slot in that mapping is changed. In our test contract, we can see that definition: it checks the start and end balance of the user. If balance decreased, make sure the required conditions for modification have been met.

contract Token {
  /// @invariant verifyBalanceOf
  mapping(address => uint256) public balanceOf;

  // -- snip --
}

contract TokenTest is Token {
  // Since this is testing a mapping, we have one input which is the mapping key
  modifier verifyBalanceOf(address user) public {
    uint256 startBalance = balanceOf(user);
    _;
    uint256 endBalance = balanceOf(user);
    if (endBalance < startBalance) {
      // balance decreased, ensure this was authorized
      assert(msg.sender == user || allowances[user][msg.sender] >= startBalance - endBalance);
    }
  }
}

Additional context

No response

@mds1 mds1 added the T-feature Type: feature label Oct 4, 2022
@gakonst gakonst added this to Foundry Oct 4, 2022
@gakonst gakonst moved this to Todo in Foundry Oct 4, 2022
@rkrasiuk rkrasiuk added A-testing Area: testing Cmd-forge-test Command: forge test C-forge Command: forge labels Oct 4, 2022
@PaulRBerg
Copy link
Contributor

This is an interesting proposal, but just so I understand it correctly, is the idea to annotate the production contract with this sort of comment?

/// @invariant verifyBalanceOf

Or some wrapper test contract that is used only when testing invariants?

@mds1
Copy link
Collaborator Author

mds1 commented Jan 23, 2023

The intent was to annotate the production contract, just because it seems a lot cleaner/simpler. There is some precedent for this, e.g. slither and scribble support using custom comments to convey info to them. Totally open to alternative UX here though

@PaulRBerg
Copy link
Contributor

I see, thanks for confirming.

In this case, I will make a similar proposal to that made in #4085, which is to use the NatSpec prefix @custom::

/// @custom:invariant verifyBalanceOf

Might be slightly more verbose but if my understanding of NatSpec is right, typing it out like this would include the name of the invariant in the contract ABI.

@mds1
Copy link
Collaborator Author

mds1 commented Jan 23, 2023

Main downside there is you need at least solidity 0.8.2, otherwise I believe compilation fails due to invalid natspec tag (probably the same issue with my original syntax). We'd want a custom comment syntax, e.g. slither uses //slither-disable-next-line DETECTOR_NAME, perhaps something like // forge-invariant to mirror the supported // forgefmt syntax

@PaulRBerg
Copy link
Contributor

argh, shoot, wasn't aware that compilation fails prior to 0.8.2!

Custom comment syntax should be fine, though.

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 Cmd-forge-test Command: forge test T-feature Type: feature
Projects
Archived in project
Development

No branches or pull requests

5 participants