Readme work, etc

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.

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();

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("-------------------");