Docs (#34)

* wip: docs * docs: rename entry points * docs: wip * fixup! docs: wip * Add initialization checks to `totalStaked` and `totalStakedFor` * Check that lock exists in `setLockManager` * wip: docs
aragon · Jun 9, 2020 · 927bd88 · 927bd88
1 parent 719fc4e
commit 927bd88
Show file tree

Hide file tree

Showing 12 changed files with 880 additions and 55 deletions.
diff --git a/README.md b/README.md
@@ -4,34 +4,17 @@
 
 A Staking app with checkpointing (implementing ERC900 interface with history) and locking.
 
+#### 📓 [Read the full documentation](/docs)
+
 ## Testing
 ### Truffle
 
 Currently this app is using Truffle. You can run tests with `npm test`.
 
-###  Slither
-[Install slither](https://github.com/trailofbits/slither#how-to-install) and then:
-```
-slither --solc /usr/local/bin/solc .
-```
-
-Some noise can be filtered with:
-```
-slither --solc /usr/local/bin/solc .  2>/tmp/a.txt ; grep -v "is not in mixedCase"  /tmp/a.txt | grep "Contract: Staking"
-```
-
-### Echidna
-Run `./scripts/flatten_echidna.sh` and then:
-```
-docker run -v `pwd`:/src trailofbits/echidna echidna-test /src/flattened_contracts/EchidnaStaking.sol EchidnaStaking --config="/src/echidna/config.yaml"
-```
-
-### Manticore
-```
-docker run --rm -ti -v `pwd`:/src trailofbits/manticore bash
-ulimit -s unlimited
-manticore --detect-all --contract Staking /src/flattened_contracts/Staking.sol
-```
-
-## Coverage
+### Coverage
 You can measure coverage using Truffle by running `npm run coverage`.
+
+## Get involved
+- Discuss in [Aragon Forum](https://forum.aragon.org/)
+- Join the [Specturm chat](https://spectrum.chat/aragon?tab=posts)
+- Join the [Discord chat](https://discord.gg/CMuvVY)
diff --git a/contracts/Staking.sol b/contracts/Staking.sol
@@ -300,10 +300,13 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
     /**
      * @notice Change the manager of `_accountAddress`'s lock from `msg.sender` to `_newLockManager`
      * @param _accountAddress Owner of lock
-     * @param _newLockManager New lock's manager
+     * @param _newLockManager New lock manager
      */
     function setLockManager(address _accountAddress, address _newLockManager) external isInitialized {
-        accounts[_accountAddress].locks[_newLockManager] = accounts[_accountAddress].locks[msg.sender];
+        Lock storage lock = accounts[_accountAddress].locks[msg.sender];
+        require(lock.allowance > 0, ERROR_LOCK_DOES_NOT_EXIST);
+
+        accounts[_accountAddress].locks[_newLockManager] = lock;
 
         delete accounts[_accountAddress].locks[msg.sender];
 
@@ -338,7 +341,7 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
     /**
      * @notice Get total amount of locked tokens for `_accountAddress`
      * @param _accountAddress Owner of locks
-     * @return Total amount of locked tokens
+     * @return Total amount of locked tokens for the requested account
      */
     function getTotalLockedOf(address _accountAddress) external view isInitialized returns (uint256) {
         return _getTotalLockedOf(_accountAddress);
@@ -349,7 +352,7 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
      * @param _accountAddress Owner of lock
      * @param _lockManager Manager of the lock for the given account
      * @return Amount of locked tokens
-     * @return Lock's data
+     * @return Amount of tokens that lock manager is allowed to lock
      */
     function getLock(address _accountAddress, address _lockManager)
         external
@@ -365,11 +368,34 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
         _allowance = lock.allowance;
     }
 
+    /**
+     * @notice Get staked and locked balances of `_accountAddress`
+     * @param _accountAddress Account being requested
+     * @return Amount of staked tokens
+     * @return Amount of total locked tokens
+     */
     function getBalancesOf(address _accountAddress) external view returns (uint256 staked, uint256 locked) {
-        staked = totalStakedFor(_accountAddress);
+        staked = _totalStakedFor(_accountAddress);
         locked = _getTotalLockedOf(_accountAddress);
     }
 
+    /**
+     * @notice Get the amount of tokens staked by `_accountAddress`
+     * @param _accountAddress The owner of the tokens
+     * @return The amount of tokens staked by the given account
+     */
+    function totalStakedFor(address _accountAddress) external view isInitialized returns (uint256) {
+        return _totalStakedFor(_accountAddress);
+    }
+
+    /**
+     * @notice Get the total amount of tokens staked by all users
+     * @return The total amount of tokens staked by all users
+     */
+    function totalStaked() external view isInitialized returns (uint256) {
+        return _totalStaked();
+    }
+
     /**
      * @notice Get the total amount of tokens staked by `_accountAddress` at block number `_blockNumber`
      * @param _accountAddress Account requesting for
@@ -402,6 +428,7 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
      * @notice Check if `_accountAddress`'s by `_lockManager` can be unlocked
      * @param _accountAddress Owner of lock
      * @param _lockManager Manager of the lock for the given account
+     * @param _amount Amount of tokens to be potentially unlocked. If zero, it means the whole locked amount
      * @return Whether given lock of given account can be unlocked
      */
     function canUnlock(address _accountAddress, address _lockManager, uint256 _amount) external view isInitialized returns (bool) {
@@ -424,25 +451,6 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
         _stakeFor(_from, _from, _amount, _data);
     }
 
-    /**
-     * @notice Get the amount of tokens staked by `_accountAddress`
-     * @param _accountAddress The owner of the tokens
-     * @return The amount of tokens staked by the given account
-     */
-    function totalStakedFor(address _accountAddress) public view returns (uint256) {
-        // we assume it's not possible to stake in the future
-        return accounts[_accountAddress].stakedHistory.getLatestValue();
-    }
-
-    /**
-     * @notice Get the total amount of tokens staked by all users
-     * @return The total amount of tokens staked by all users
-     */
-    function totalStaked() public view returns (uint256) {
-        // we assume it's not possible to stake in the future
-        return totalStakedHistory.getLatestValue();
-    }
-
     /*
     function multicall(bytes[] _calls) public {
         for(uint i = 0; i < _calls.length; i++) {
@@ -483,7 +491,7 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
     }
 
     function _modifyStakeBalance(address _accountAddress, uint256 _by, bool _increase) internal returns (uint256) {
-        uint256 currentStake = totalStakedFor(_accountAddress);
+        uint256 currentStake = _totalStakedFor(_accountAddress);
 
         uint256 newStake;
         if (_increase) {
@@ -500,7 +508,7 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
     }
 
     function _modifyTotalStaked(uint256 _by, bool _increase) internal {
-        uint256 currentStake = totalStaked();
+        uint256 currentStake = _totalStaked();
 
         uint256 newStake;
         if (_increase) {
@@ -583,13 +591,32 @@ contract Staking is Autopetrified, ERCStaking, ERCStakingHistory, IStakingLockin
         emit StakeTransferred(_from, _to, _amount);
     }
 
+    /**
+     * @notice Get the amount of tokens staked by `_accountAddress`
+     * @param _accountAddress The owner of the tokens
+     * @return The amount of tokens staked by the given account
+     */
+    function _totalStakedFor(address _accountAddress) internal view returns (uint256) {
+        // we assume it's not possible to stake in the future
+        return accounts[_accountAddress].stakedHistory.getLatestValue();
+    }
+
+    /**
+     * @notice Get the total amount of tokens staked by all users
+     * @return The total amount of tokens staked by all users
+     */
+    function _totalStaked() internal view returns (uint256) {
+        // we assume it's not possible to stake in the future
+        return totalStakedHistory.getLatestValue();
+    }
+
     /**
      * @notice Get the staked but unlocked amount of tokens by `_accountAddress`
      * @param _accountAddress Owner of the staked but unlocked balance
      * @return Amount of tokens staked but not locked by given account
      */
     function _unlockedBalanceOf(address _accountAddress) internal view returns (uint256) {
-        uint256 unlockedTokens = totalStakedFor(_accountAddress).sub(accounts[_accountAddress].totalLocked);
+        uint256 unlockedTokens = _totalStakedFor(_accountAddress).sub(accounts[_accountAddress].totalLocked);
 
         return unlockedTokens;
     }

diff --git a/contracts/test/EchidnaStaking.sol b/contracts/test/EchidnaStaking.sol
@@ -19,7 +19,7 @@ contract EchidnaStaking is Staking {
         address _account = msg.sender;
         Account storage account = accounts[_account];
 
-        if (totalStakedFor(_account) < account.totalLocked) {
+        if (_totalStakedFor(_account) < account.totalLocked) {
             return false;
         }
 
@@ -31,7 +31,7 @@ contract EchidnaStaking is Staking {
         address _account = msg.sender;
         Account storage account = accounts[_account];
 
-        if (totalStakedFor(_account) > account.totalLocked) {
+        if (_totalStakedFor(_account) > account.totalLocked) {
             return false;
         }
 
@@ -63,7 +63,7 @@ contract EchidnaStaking is Staking {
 
     // total staked matches less or equal than token balance
     function echidna_total_staked_is_balance() external view returns (bool) {
-        if (totalStaked() <= stakingToken.balanceOf(this)) {
+        if (_totalStaked() <= stakingToken.balanceOf(this)) {
             return true;
         }
 

diff --git a/docs/1-anti-sybil/README.md b/docs/1-anti-sybil/README.md
@@ -0,0 +1,10 @@
+# Anti-sybil
+
+Staking app uses the [Checkpointing library](https://github.com/aragon/aragon-apps/pull/415) to provide a history of balances within the app. This is important for applications such as token-weighted voting, as one token could potentially be used to vote more than once if it’s transferred to another account after being used to cast a vote. Mimicing the [MiniMe token](https://github.com/Giveth/minime), checkpointing allows to have a snapshot of balances at any given used time, that can be used to tally votes.
+
+Any time that there is a balance change in the Staking app, the Checkpointing library stores the timestamp and value in an array for the balance owner. Balance changes are stored in natural time order, meaning that it’s not possible to modify a balance nor add a checkpoint in the past.
+
+One important technical detail to note is that in order to save gas (Checkpointing is an expensive operation), timestamp and value are stored together in one slot, so the maximum amount that can be stored is `2^192 - 1`, which may break compatibility with common ERC-20 tokens and be problematic in some edge-cases.
+
+There is also a history array for the total staked in the app.
+
diff --git a/docs/2-slashing/README.md b/docs/2-slashing/README.md
@@ -0,0 +1,8 @@
+# Slashing
+
+Many use cases need to be able to slash users’ tokens depending on the outcome of certain actions. For instance, when proposing an action in the Agreements app, if it’s challenged and the [Aragon Court](https://court.aragon.org/dashboard) resolves to accept the challenge, user would lose the staked collateral.
+
+Staking app achieves this by adding locks on top of staked balances. A user can designate a lock manager, which can be either a contract or an `EOA`, and a maximum allowance for that manager. The manager then will be able to lock up to that allowed amount of tokens, and to unlock them too. While the tokens are locked, the original owner cannot unstake them, while the manager can transfer to wherever is needed: another user’s lock, the staked balance of another user, or even the external token balance of another account.
+
+If the manager is a contract, it must implement the method `canUnlock`, where certain conditions can be specified to allow the owner to re-gain control of the tokens. A common use case would be a time based lock manager, that would lock tokens only for some period, and once the period is over, the user would be able to unlock and unstake the tokens.
+