[Internal audit] Part 2 notes (aragon#63)

* Checkpointing: optimize some edge cases

* Initial comments

* Restore comments for _add192 function

* Fix Checkpointing compilation errors

* Rearrange some contract locations

* Remove unused ScriptHelpers

* Remove broken gas tests

* Move TimeLockManager to test folder

* Remove comment from Checkpointing

* fixup! Remove broken gas tests

* Remove Staking proxy

* fixup! Move TimeLockManager to test folder

* Staking: add more notes

* Style changes to StakingFactory

* Minor changes to IStakingLocking

* Add tests to Checkpointing

* Remove note from Checkpointing

* Remove note from StandardTokenMock

* Remove setLockManager

* Rename stakingToken to token

* Remove setLockManager from docs

* Remove _toBytes4 function

* Remove TimeLockManager

* Merge require’s on receiveApproval

* Prevent locking by user directly in Staking contract

Locking has to be done always from/by LockManager.

* Remove require from slashAndUnlock

To save gas. Now this function can behave the same as slash if
_unlockAmount is zero.

* Clarify comment

* Remove unused ECRProxy interface

* Rename interfaces with prefix I

* docs: Prevent locking by user directly in Staking contract

* docs: Remove references to contract initialization

* fixup! docs: Remove references to contract initialization

* Add ERC900 optional methods

* Index some IStakingLocking events

* Inline Staking functions (transferAndUnstake and slashAndUnstake)

* Rename IStakingLocking to ILockable

* Add ERC223Recipient fallback

* Update exported IStaking interface

* Staking: polish interfaces

* Add missing headers for IERC900

* Split optional History methods from IERC900

* Remove increase param from ILockable events

* Revert "Add ERC223Recipient fallback"

This reverts commit 0c1c768.

* Re-add IApproveAndCallFallBack

* Staking: dedupe _transferAndUnstake()

* Staking: remove unused errors

Co-authored-by: ßingen <[email protected]>

Author: sohkai

docs/3-entry-points/README.md

@@ -20,9 +20,7 @@ Funds flows with different origin and destiny:
 | Lock   | Stake  | slash              |
 | Lock   | Wallet | slashAndUnstake    |
 
-### initialize
-
-This is used by the Staking Factory when creating a new proxy. See deployment section for more details.
+### constructor
 
 - **Actor:** Deployer account
 - **Inputs:**
@@ -47,7 +45,6 @@ Stakes `_amount` tokens, transferring them from `msg.sender`
   - **_data:** Used in Staked event, to add signalling information in more complex staking applications
 - **Authentication:** Open
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that amount is not zero
 - **State transitions:**
   - Transfers tokens from sender to contract
@@ -65,7 +62,6 @@ Stakes `_amount` tokens, transferring them from `msg.sender`, and assigns them t
   - **_data:** Used in Staked event, to add signalling information in more complex staking applications
 - **Authentication:** Open
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that amount is not zero
 - **State transitions:**
   - Transfers tokens from sender to contract
@@ -83,7 +79,6 @@ Unstakes `_amount` tokens, returning them to the user
   - **_data:** Used in Unstaked event, to add signalling information in more complex staking applications
 - **Authentication:** Open
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that amount is not zero
 - **State transitions:**
   - Transfers tokens from contract to sender
@@ -99,8 +94,6 @@ Get the token used by the contract for staking and locking
 - **Outputs:**
   - Address of the staking token
 - **Authentication:** Open
-- **Pre-flight checks:**
-  - Checks that contract has been initialized
 
 
 ### supportsHistory
@@ -123,8 +116,6 @@ Get last time `_user` modified its staked balance
 - **Outputs:**
   - Last block number when account’s balance was modified
 - **Authentication:** Open
-- **Pre-flight checks:**
-  - Checks that contract has been initialized
 
 
 ### totalStakedForAt
@@ -138,8 +129,6 @@ Get the total amount of tokens staked by `_user` at block number `_blockNumber`
 - **Outputs:**
   - The amount of tokens staked by the account at the given block number
 - **Authentication:** Open
-- **Pre-flight checks:**
-  - Checks that contract has been initialized
 
 
 ### totalStakedAt
@@ -152,8 +141,6 @@ Get the total amount of tokens staked by all users at block number `_blockNumber
 - **Outputs:**
   - The amount of tokens staked at the given block number
 - **Authentication:** Open
-- **Pre-flight checks:**
-  - Checks that contract has been initialized
 
 
 ### totalStakedFor
@@ -166,8 +153,6 @@ Get the amount of tokens staked by `_user`
 - **Outputs:**
   - The amount of tokens staked by the given account
 - **Authentication:** Open
-- **Pre-flight checks:**
-  - Checks that contract has been initialized
 
 ### totalStaked
 
@@ -177,8 +162,6 @@ Get the total amount of tokens staked by all users
 - **Outputs:**
   - The total amount of tokens staked by all users
 - **Authentication:** Open
-- **Pre-flight checks:**
-  - Checks that contract has been initialized
 
 
 ## Locking interface
@@ -195,35 +178,10 @@ It creates a new lock, so the lock for this manager cannot exist before.
   - **_data:** Used in `NewLockManager` event and to parametrize logic for the lock to be enforced by the manager
 - **Authentication:** Open. Implicitly, sender must be staking owner.
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
-  - Checks that allowance input is not zero
-  - Checks that lock didn’t exist before
-- **State transitions:**
-  - Sets allowance for the pair owner-manager to the given amount
-  - Calls lock manager callback
-
-### allowManagerAndLock
-
-Lock `_amount` staked tokens and assign `_lockManager` as manager with `_allowance` allowed tokens and `_data` as data, so they can not be unstaked
-
-- **Actor:** Staking user (owner)
-- **Inputs:**
-  - **_amount:** The amount of tokens to be locked
-  - **_lockManager:** The manager entity for this particular lock
-  - **_allowance:** Amount of tokens that the manager can lock
-  - **_data:** Used in `NewLockManager` event and to parametrize logic for the lock to be enforced by the manager
-- **Authentication:** Open. Implicitly, sender must be staking owner.
-- **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that allowance input is not zero
   - Checks that lock didn’t exist before
-  - Checks that amount input is not zero
-  - Checks that user has enough unlocked tokens available
-  - Checks that amount is not greater than allowance
 - **State transitions:**
   - Sets allowance for the pair owner-manager to the given amount
-  - Sets the amount of tokens as locked by the manager
-  - Increases the total amount of locked tokens balance for the user
   - Calls lock manager callback
 
 
@@ -237,7 +195,6 @@ Transfer `_amount` tokens to `_to`’s staked balance
   - **_amount:** Number of tokens to be transferred
 - **Authentication:** Open. Implicitly, sender must be staking owner.
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that amount input is not zero
   - Checks that user has enough unlocked tokens available
 - **State transitions:**
@@ -254,7 +211,6 @@ Transfer `_amount` tokens to `_to`’s external balance (i.e. unstaked)
   - **_amount:** Number of tokens to be transferred
 - **Authentication:** Open. Implicitly, sender must be staking owner.
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that amount input is not zero
   - Checks that user has enough unlocked tokens available
 - **State transitions:**
@@ -273,7 +229,6 @@ Transfer `_amount` tokens from `_from`'s lock by `msg.sender` to `_to`
   - **_amount:** Number of tokens to be transferred
 - **Authentication:** Open. Implicitly, sender must be lock manager.
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Check that owner’s lock is enough
   - Checks that amount input is not zero
 - **State transitions:**
@@ -293,7 +248,6 @@ Transfer `_amount` tokens from `_from`'s lock by `msg.sender` to `_to`’s exter
   - **_amount:** Number of tokens to be transferred
 - **Authentication:** Open. Implicitly, sender must be lock manager
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Check that owner’s lock is enough
   - Checks that amount input is not zero
 - **State transitions:**
@@ -315,7 +269,6 @@ Transfer `_transferAmount` tokens from `_from`'s lock by `msg.sender` to `_to`,
   - **_slashAmount:** Number of tokens to be transferred
 - **Authentication:** Open. Implicitly, sender must be lock manager
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Check that owner’s lock is enough
   - Checks that unlock amount input is not zero
   - Checks that slash amount input is not zero
@@ -336,7 +289,6 @@ Increase allowance in `_allowance` tokens of lock manager `_lockManager` for use
   - **_allowance:** Amount of allowed tokens increase
 - **Authentication:** Open. Implicitly, sender must be staking owner.
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that lock exists (i.e., it has a previous allowance)
   - Checks that amount input is not zero
 - **State transitions:**
@@ -353,7 +305,6 @@ Decrease allowance in `_allowance` tokens of lock manager `_lockManager` for use
   - **_allowance:** Amount of allowed tokens decrease
 - **Authentication:** Only owner or lock manager
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that amount input is not zero
   - Checks that lock exists (i.e., it has a previous allowance)
   - Checks that final allowed amount is not less than currently locked tokens
@@ -364,16 +315,14 @@ Decrease allowance in `_allowance` tokens of lock manager `_lockManager` for use
 
 ### lock
 
-Increase locked amount by `_amount` tokens for user `_user` by lock manager `_lockManager`
+Increase locked amount by `_amount` tokens for user `_user` by lock manager `msg.sender`
 
-- **Actor:** Staking user (owner) or lock manager
+- **Actor:** Lock manager
 - **Inputs:**
   - **_user:** Owner of locked tokens
-  - **_lockManager:** The manager entity for this particular lock
   - **_amount:** Amount of locked tokens increase
 - **Authentication:** Only owner or lock manager
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that amount input is not zero
   - Checks that user has enough unlocked tokens available
   - Checks that lock has enough allowance
@@ -392,7 +341,6 @@ Decrease locked amount by `_amount` tokens for user `_user` by lock manager `_lo
   - **_amount:** Amount of locked tokens decrease
 - **Authentication:** Only owner or lock manager
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that amount input is not zero
   - Checks that lock exists (i.e., it has a previous allowance)
   - Checks that user has enough unlocked tokens available
@@ -412,28 +360,12 @@ Unlock `_user`'s lock by `_lockManager` so locked tokens can be unstaked again
   - **_lockManager:** The manager entity for this particular lock
 - **Authentication:** Only owner or lock manager
 - **Pre-flight checks:**
-  - Checks that contract has been initialized
   - Checks that lock exists (i.e., it has a previous allowance)
   - If sender is owner and there were locked tokens, checks that manager allows to unlock
 - **State transitions:**
   - Decreases owner’s total locked amount by currently locked tokens for this lock manager
   - Deletes lock for this pair of owner and lock manager
 
-### setLockManager
-
-Change the manager of `_user`'s lock from `msg.sender` to `_newLockManager`
-
-- **Actor:** Lock manager
-- **Inputs:**
-  - **_user:** Owner of lock
-  - **_newLockManager:** New lock manager
-- **Authentication:** Open. Implicitly, sender must be lock manager
-- **Pre-flight checks:**
-  - Checks that contract has been initialized
-  - Checks that lock exists
-- **State transitions:**
-  - Assigns lock to new manager
-
 ### getTotalLockedOf
 
 Get total amount of locked tokens for `_user`

docs/5-external-interface/README.md

@@ -1,8 +1,8 @@
 # External interface
 
-### Initialize
+### Constructor
 
-- **Name:** initialize
+- **Name:** constructor
 - **Inputs:**
   - **_stakingToken:** ERC20 token to be used for staking
 
@@ -92,15 +92,6 @@
   - **_allowance:** Amount of tokens that the manager can lock
   - **_data:** Used in `NewLockManager` event and to parametrize logic for the lock to be enforced by the manager
 
-### Allow new manager and lock
-
-- **Name:** allowManagerAndLock
-- **Inputs:**
-  - **_amount:** The amount of tokens to be locked
-  - **_lockManager:** The manager entity for this particular lock
-  - **_allowance:** Amount of tokens that the manager can lock
-  - **_data:** Used in `NewLockManager` event and to parametrize logic for the lock to be enforced by the manager
-
 ### Transfer
 
 - **Name:** transfer
@@ -160,7 +151,6 @@
 - **Name:** lock
 - **Inputs:**
   - **_user:** Owner of locked tokens
-  - **_lockManager:** The manager entity for this particular lock
   - **_amount:** Amount of locked tokens increase
 
 ### Unlock
@@ -178,13 +168,6 @@
   - **_user:** Owner of locked tokens
   - **_lockManager:** The manager entity for this particular lock
 
-### Set lock manager
-
-- **Name:** setLockManager
-- **Inputs:**
-  - **_user:** Owner of lock
-  - **_newLockManager:** New lock manager
-
 ### Get total locked of
 
 - **Name:** getTotalLockedOf