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

audit: boost unit test coverage, add invariant tests #12

Merged
merged 4 commits into from
May 9, 2024
Merged

Conversation

ethanbennett
Copy link
Collaborator

@ethanbennett ethanbennett commented Apr 29, 2024

Changes:

  • Prior to the review, unit test coverage was missing for three error conditions in PointTokenVault.sol: both instances of RewardsNotReleased, and PTokenNotDeployed. This PR adds tests for all three and boosts overall coverage of the core functionality to 100%
  • Adds three invariant tests. In foundry, invariant tests are called after each run while fuzzing (statefully)
    • The test keeps track of how many points-earning tokens are deposited and withdrawn, and the first invariant checks that the contract's values never deviate from the local ones
    • The test keeps track of how many pTokens are claimed, and the second invariant checks that the contract's values never deviate from the local ones
    • A third invariant checks that each pToken's totalSupply is equal to the sum of all the balances of that pToken
  • As part of the stateful fuzzing infrastructure, the above invariants run by randomly selecting the actor, receiver, pToken, and points-earning token, each from a list of 10 (or 13 in the case of the actors, with 3 admin added). It then fuzzes all relevant parameters for the direct function call and the Claim
  • The tests will repeatedly call all the implemented functions in random strings, maintaining its state throughout
  • There are assertions in each of the handler's functions (i.e., the fuzz tests) that assert the proper functionality of the invoked functions throughout
  • These tests also demonstrate that there should not be any unexpected reverts for deposit, withdraw, claimPTokens, redeemRewards, or convertRewardsToPTokens

Notes:

  • The invariant tests, and their underlying fuzz tests, use a MockPointTokenVault. This is exactly the same as the real one, but with Merkle validation bypassed. As long as the Merkle functionality remains well-tested in the unit tests, you can use this infrastructure in the future to test aspects of the contract that may be more difficult to reach in contexts where a Merkle proof must be generated in a separate environment for every test case
  • The randomness of the fuzz inputs can be modulated by way of the dictionary_weight option in foundry.toml. On a scale of 0-100, 100 most favors values at the extremes of the input possibilities. The default for invariant tests is 80
  • Other configuration for the fuzz and invariant tests can also be adjusted in foundry.toml. You can set these values higher and leave it to run overnight, but be careful to leave them at moderate values for CI runs. The highest values I've run it at are:
[fuzz]
runs = 2000

[invariant]
depth = 200

@jparklev jparklev merged commit 26a847f into main May 9, 2024
2 checks passed
@jparklev jparklev deleted the audit branch May 9, 2024 20:10
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

Successfully merging this pull request may close these issues.

2 participants