diff --git a/test/README.md b/test/README.md index f51fecd..24daad7 100644 --- a/test/README.md +++ b/test/README.md @@ -1,16 +1,20 @@ # Invariant Suite -The invariant suite is a collection of tests that ensure certain properties of the system are always true. +The invariant suite is a collection of tests designed to build confidence around certain properties of the system expected to be true. ## Invariants under test -- [ ] the earned fees should equal the percentage of durations that have passed on the rewards -- [ ] Withdrawals for individual depositors + stake should equal the total staked throughout the tests -- [ ] The sum of all of the notified rewards should equal the rewards balance in the staking contract plus all claimed rewards +- The total staked balance should equal the sum of all individual depositors' balances +- The sum of beneficiary earning power should equal the total staked balance +- The sum of all surrogate balance should equal the total staked balance +- Cumulative deposits minus withdrawals should equal the total staked balance +- The sum of all notified rewards should be greater or equal to all claimed rewards plus the rewards balance in the staking contract (TODO: not strictly equal because of stray transfers in, which are not yet implemented in handler) +- Sum of unclaimed reward across all beneficiaries should be less than or equal to total rewards +- `rewardPerTokenAccumulatedCheckpoint` should be greater or equal to the last `rewardPerTokenAccumulatedCheckpoint` value ## Invariant Handler -The handler contract specifies the actions that should be taken in the black box of an invariant run. Included in the handler contract are actions such as: +The handler contract specifies the actions that should be taken in the black box of an invariant run. Here is a list of implemented actions the handler contract can take, as well as ideas for further actions. ### Valid user actions @@ -24,41 +28,41 @@ These actions are typical user actions that can be taken on the system. They are - Action taken by: existing depositors - [x] claimReward: A beneficiary claims the reward that is due to her. - Action taken by: existing beneficiaries -- alterDelegatee -- alterBeneficiary -- permitAndStake -- enable rewards notifier -- notifyRewardAmount -- all of the `onBehalf` methods -- multicall +- [ ] alterDelegatee +- [ ] alterBeneficiary +- [ ] permitAndStake +- [x] enable rewards notifier +- [x] notifyRewardAmount +- [ ] all of the `onBehalf` methods +- [ ] multicall ### Invalid user actions -- Staking without sufficient EC20 approval -- Stake more on a deposit that does not belong to you -- State more on a deposit that does not exist -- Alter beneficiary and alter delegatee on a deposit that is not yours or does not exist -- withdraw on deposit that's not yours -- call notifyRewardsAmount if you are not rewards notifier, or insufficient/incorrect reward balance -- setAdmin and setRewardNotifier without being the admin -- Invalid signature on the `onBehalf` methods -- multicall +- [ ] Staking without sufficient ERC20 approval +- [ ] Stake more on a deposit that does not belong to you +- [ ] State more on a deposit that does not exist +- [ ] Alter beneficiary and alter delegatee on a deposit that is not yours or does not exist +- [ ] withdraw on deposit that's not yours +- [ ] call notifyRewardsAmount if you are not rewards notifier, or insufficient/incorrect reward balance +- [ ] setAdmin and setRewardNotifier without being the admin +- [ ] Invalid signature on the `onBehalf` methods +- [ ] multicall ### Weird user actions These are actions that are outside the normal use of the system. They are used to test the system's behavior under abnormal conditions. -- transfer in arbitrary amount of STAKE_TOKEN -- transfer in arbitrary amount of REWARD_TOKEN -- transfer direct to surrogate -- reentrancy attempts -- SELFDESTRUCT to this contract -- flash loan? -- User uses the staking contract as the from address in a `transferFrom` -- A non-beneficiary calls claim reward -- withdraw with zero amount -- multicall +- [ ] directly transfer in some amount of STAKE_TOKEN to UniStaker +- [ ] directly transfer some amount of REWARD_TOKEN to UniStaker +- [ ] transfer stake directly to surrogate +- [ ] reentrancy attempts +- [ ] SELFDESTRUCT to this contract +- [ ] flash loan? +- [ ] User uses the staking contract as the from address in a `transferFrom` +- [ ] A non-beneficiary calls claim reward +- [x] withdraw with zero amount +- [ ] multicall ### Utility actions -- `warpAhead`: warp the block timestamp ahead by a specified number of seconds. +- [x] `warpAhead`: warp the block timestamp ahead by a specified number of seconds. diff --git a/test/UniStaker.invariants.t.sol b/test/UniStaker.invariants.t.sol index a09ed34..25dfa7c 100644 --- a/test/UniStaker.invariants.t.sol +++ b/test/UniStaker.invariants.t.sol @@ -68,13 +68,21 @@ contract UniStakerInvariants is Test { ); } - function invariant_Unclaimed_reward_LTE_total_rewards() public { + function invariant_Sum_of_unclaimed_reward_should_be_less_than_or_equal_to_total_rewards() public { assertLe( handler.reduceBeneficiaries(0, this.accumulateUnclaimedReward), rewardToken.balanceOf(address(uniStaker)) ); } + function invariant_RewardPerTokenAccumulatedCheckpoint_should_be_greater_or_equal_to_the_last_rewardPerTokenAccumulatedCheckpoint( + ) public { + assertGe( + uniStaker.rewardPerTokenAccumulatedCheckpoint(), + handler.ghost_prevRewardPerTokenAccumulatedCheckpoint() + ); + } + // Used to see distribution of non-reverting calls function invariant_callSummary() public view { handler.callSummary(); diff --git a/test/helpers/UniStaker.handler.sol b/test/helpers/UniStaker.handler.sol index bc405f5..307e2a4 100644 --- a/test/helpers/UniStaker.handler.sol +++ b/test/helpers/UniStaker.handler.sol @@ -34,12 +34,18 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { uint256 public ghost_depositCount; uint256 public ghost_rewardsClaimed; uint256 public ghost_rewardsNotified; + uint256 public ghost_prevRewardPerTokenAccumulatedCheckpoint; modifier countCall(bytes32 key) { calls[key]++; _; } + modifier doCheckpoints() { + _checkpoint_ghost_prevRewardPerTokenAccumulatedCheckpoint(); + _; + } + constructor(UniStaker _uniStaker) { uniStaker = _uniStaker; stakeToken = IERC20(address(_uniStaker.STAKE_TOKEN())); @@ -57,7 +63,11 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { deal(address(rewardToken), _to, _amount, true); } - function enableRewardNotifier(address _notifier) public countCall("enableRewardNotifier") { + function enableRewardNotifier(address _notifier) + public + countCall("enableRewardNotifier") + doCheckpoints + { vm.assume(_notifier != address(0)); _rewardNotifiers.add(_notifier); vm.prank(admin); @@ -67,10 +77,12 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { function notifyRewardAmount(uint256 _amount, uint256 _actorSeed) public countCall("notifyRewardAmount") + doCheckpoints { _useActor(_rewardNotifiers, _actorSeed); vm.assume(currentActor != address(0)); _amount = bound(_amount, 0, 100_000_000e18); + ghost_prevRewardPerTokenAccumulatedCheckpoint = uniStaker.rewardPerTokenAccumulatedCheckpoint(); _mintRewardToken(currentActor, _amount); vm.startPrank(currentActor); rewardToken.transfer(address(uniStaker), _amount); @@ -83,6 +95,7 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { function stake(uint256 _amount, address _delegatee, address _beneficiary) public countCall("stake") + doCheckpoints { _createDepositor(); @@ -109,6 +122,7 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { function validStakeMore(uint256 _amount, uint256 _actorSeed, uint256 _actorDepositSeed) public countCall("validStakeMore") + doCheckpoints { _useActor(_depositors, _actorSeed); vm.assume(currentActor != address(0)); @@ -128,6 +142,7 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { function validWithdraw(uint256 _amount, uint256 _actorSeed, uint256 _actorDepositSeed) public countCall("validWithdraw") + doCheckpoints { _useActor(_depositors, _actorSeed); vm.assume(currentActor != address(0)); @@ -142,7 +157,7 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { ghost_stakeWithdrawn += _amount; } - function claimReward(uint256 _actorSeed) public countCall("claimReward") { + function claimReward(uint256 _actorSeed) public countCall("claimReward") doCheckpoints { _useActor(_beneficiaries, _actorSeed); vm.startPrank(currentActor); uint256 rewardsClaimed = uniStaker.unclaimedRewardCheckpoint(currentActor); @@ -151,7 +166,7 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { ghost_rewardsClaimed += rewardsClaimed; } - function warpAhead(uint256 _seconds) public countCall("warpAhead") { + function warpAhead(uint256 _seconds) public countCall("warpAhead") doCheckpoints { _seconds = bound(_seconds, 0, uniStaker.REWARD_DURATION() * 2); skip(_seconds); } @@ -192,6 +207,10 @@ contract UniStakerHandler is CommonBase, StdCheats, StdUtils { return _delegates.reduce(acc, func); } + function _checkpoint_ghost_prevRewardPerTokenAccumulatedCheckpoint() internal { + ghost_prevRewardPerTokenAccumulatedCheckpoint = uniStaker.rewardPerTokenAccumulatedCheckpoint(); + } + function callSummary() external view { console.log("\nCall summary:"); console.log("-------------------");