L04: Emit event when PrizePool adds a controlled token (#107)

* Emit event when PrizePool adds a controlled token

* L05: Improved interface of MappedSinglyLinkedList (#108)

* Improved interface of MappedSinglyLinkedList

* Fixed typo straggler

* L06: Made PrizePool field naming more consistent (#109)

* Made PrizePool field naming more consistent

* N01: Improved language consistency (#110)

* Improved language consistency

* N02: Explicitly delete addresses in MappedSinglyLinkedList (#111)

* Explicitly delete addresses in MappedSinglyLinkedList

* N03: Added comments clarifying an edge case for timelocked funds (#112)

* Added comments clarifying an edge case for timelocked funds

* N05: Made return variable naming consistent (#113)

* Made return variable naming consistent

All functions have explicit return statements
Functions that have a single return value won't name the value
Functions that have two or more returns value will have named returns

* Fixed some spelling mistakes (#114)

contracts/drip/BalanceDripManager.sol

@@ -21,16 +21,16 @@ library BalanceDripManager {
     uint256 measureTotalSupply,
     uint256 currentTime
   ) internal {
-    address currentDripToken = self.activeBalanceDrips[measure].addressMap[MappedSinglyLinkedList.SENTINAL];
-    while (currentDripToken != address(0) && currentDripToken != MappedSinglyLinkedList.SENTINAL) {
+    address currentDripToken = self.activeBalanceDrips[measure].start();
+    while (currentDripToken != address(0) && currentDripToken != self.activeBalanceDrips[measure].end()) {
       BalanceDrip.State storage dripState = self.balanceDrips[measure][currentDripToken];
       dripState.drip(
         user,
         measureBalance,
         measureTotalSupply,
         currentTime
       );
-      currentDripToken = self.activeBalanceDrips[measure].addressMap[currentDripToken];
+      currentDripToken = self.activeBalanceDrips[measure].next(currentDripToken);
     }
   }
 

contracts/prize-pool/PrizePool.sol

@@ -18,6 +18,11 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
   using SafeMath for uint256;
   using MappedSinglyLinkedList for MappedSinglyLinkedList.Mapping;
 
+  /// @dev Event emitted when controlled token is added
+  event ControlledTokenAdded(
+    address indexed token
+  );
+
   /// @dev Event emitted when assets are deposited
   event Deposited(
     address indexed operator,
@@ -100,10 +105,10 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
   uint256 public timelockTotalSupply;
 
   /// @dev The timelocked balances for each user
-  mapping(address => uint256) internal timelockBalances;
+  mapping(address => uint256) internal _timelockBalances;
 
   /// @dev The unlock timestamps for each user
-  mapping(address => uint256) internal unlockTimestamps;
+  mapping(address => uint256) internal _unlockTimestamps;
 
   /// @notice Initializes the Prize Pool with required contract connections
   /// @param _trustedForwarder Address of the Forwarding Contract for GSN Meta-Txs
@@ -204,7 +209,7 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
     address operator = _msgSender();
 
     ControlledToken(controlledToken).controllerMint(to, amount);
-    timelockBalances[operator] = timelockBalances[operator].sub(amount);
+    _timelockBalances[operator] = _timelockBalances[operator].sub(amount);
     timelockTotalSupply = timelockTotalSupply.sub(amount);
 
     prizeStrategy.afterTimelockDepositTo(operator, to, amount, controlledToken, data);
@@ -245,7 +250,7 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
   /// @param amount The amount of tokens to redeem for assets.
   /// @param controlledToken The address of the token to redeem (i.e. ticket or sponsorship)
   /// @param maximumExitFee The maximum exit fee the caller is willing to pay.  This can be pre-calculated.
-  /// @return exitFee The amount of the fairness fee paid
+  /// @return The amount of the fairness fee paid
   function withdrawInstantlyFrom(
     address from,
     uint256 amount,
@@ -256,11 +261,12 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
     external
     nonReentrant
     onlyControlledToken(controlledToken)
-    returns (uint256 exitFee)
+    returns (uint256)
   {
 
+    uint256 exitFee;
     if (_hasPrizeStrategy()) {
-      exitFee = limitExitFee(amount, prizeStrategy.beforeWithdrawInstantlyFrom(from, amount, controlledToken, data));
+      exitFee = _limitExitFee(amount, prizeStrategy.beforeWithdrawInstantlyFrom(from, amount, controlledToken, data));
     }
 
     require(exitFee <= maximumExitFee, "PrizePool/exit-fee-exceeds-user-maximum");
@@ -279,22 +285,25 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
     }
 
     emit InstantWithdrawal(_msgSender(), from, controlledToken, amount, exitFee);
+
+    return exitFee;
   }
 
-  function limitExitFee(uint256 withdrawalAmount, uint256 exitFee) internal view returns (uint256) {
+  function _limitExitFee(uint256 withdrawalAmount, uint256 exitFee) internal view returns (uint256) {
     uint256 maxFee = FixedPoint.multiplyUintByMantissa(withdrawalAmount, maxExitFeeMantissa);
     if (exitFee > maxFee) {
       exitFee = maxFee;
     }
     return exitFee;
   }
 
-  /// @notice Withdraw assets from the Prize Pool by placing them into the timelock.
-  /// @dev The timelock is used to ensure that the tickets have contributed their fair share of the prize.
+  /// @notice Withdraw assets from the Prize Pool by placing them into the timelock. The timelock is used to ensure that the tickets have contributed their fair share of the prize.
+  /// @dev Note that if the user has previously timelocked funds then this contract will try to sweep them.  If the existing timelocked funds are still locked, then the incoming
+  /// balance is added to their existing balance and the new timelock unlock timestamp will overwrite the old one.
   /// @param from The address to withdraw from
   /// @param amount The amount to withdraw
   /// @param controlledToken The type of token being withdrawn
-  /// @return unlockTimestamp The timestamp after which the funds can be swept
+  /// @return The timestamp from which the funds can be swept
   function withdrawWithTimelockFrom(
     address from,
     uint256 amount,
@@ -304,10 +313,12 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
     external
     nonReentrant
     onlyControlledToken(controlledToken)
-    returns (uint256 unlockTimestamp)
+    returns (uint256)
   {
     uint256 blockTime = _currentTime();
 
+    uint256 unlockTimestamp;
+
     if (_hasPrizeStrategy()) {
       unlockTimestamp = prizeStrategy.beforeWithdrawWithTimelockFrom(from, amount, controlledToken, data);
     }
@@ -337,8 +348,8 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
     _sweepTimelockBalances(users);
 
     timelockTotalSupply = timelockTotalSupply.add(amount);
-    timelockBalances[user] = timelockBalances[user].add(amount);
-    unlockTimestamps[user] = timestamp;
+    _timelockBalances[user] = _timelockBalances[user].add(amount);
+    _unlockTimestamps[user] = timestamp;
 
     // if the funds should already be unlocked
     if (timestamp <= _currentTime()) {
@@ -359,8 +370,8 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
 
   /// @notice Updates and returns the current prize.
   /// @dev Updates the internal rolling interest rate since the last poke
-  /// @return award The total amount of assets to be awarded for the current prize
-  function awardBalance() public returns (uint256 award) {
+  /// @return The total amount of assets to be awarded for the current prize
+  function awardBalance() public returns (uint256) {
     uint256 tokenTotalSupply = _tokenTotalSupply();
     uint256 bal = _balance();
 
@@ -374,7 +385,7 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
   /// @notice Called by the Prize-Strategy to Award a Prize to a specific account
   /// @param to The address of the winner that receives the award
   /// @param amount The amount of assets to be awarded
-  /// @param controlledToken The addess of the asset token being awarded
+  /// @param controlledToken The address of the asset token being awarded
   function award(
     address to,
     uint256 amount,
@@ -398,7 +409,7 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
   /// @dev Used to award any arbitrary tokens held by the Prize Pool
   /// @param to The address of the winner that receives the award
   /// @param amount The amount of external assets to be awarded
-  /// @param externalToken The addess of the external asset token being awarded
+  /// @param externalToken The address of the external asset token being awarded
   function awardExternalERC20(
     address to,
     address externalToken,
@@ -421,7 +432,7 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
   /// @notice Called by the Prize-Strategy to Award Secondary (external) Prize NFTs to a specific account
   /// @dev Used to award any arbitrary NFTs held by the Prize Pool
   /// @param to The address of the winner that receives the award
-  /// @param externalToken The addess of the external NFT token being awarded
+  /// @param externalToken The address of the external NFT token being awarded
   /// @param tokenIds An array of NFT Token IDs to be transferred
   function awardExternalERC721(
     address to,
@@ -446,37 +457,39 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
 
   /// @notice Sweep all timelocked balances and transfer unlocked assets to owner accounts
   /// @param users An array of account addresses to sweep balances for
-  /// @return totalWithdrawal The total amount of assets swept from the Prize Pool
+  /// @return The total amount of assets swept from the Prize Pool
   function sweepTimelockBalances(
     address[] calldata users
   )
     external
     nonReentrant
-    returns (uint256 totalWithdrawal)
+    returns (uint256)
   {
-    totalWithdrawal = _sweepTimelockBalances(users);
+    return _sweepTimelockBalances(users);
   }
 
   /// @notice Sweep all timelocked balances and transfer unlocked assets to owner accounts
   /// @param users An array of account addresses to sweep balances for
-  /// @return totalWithdrawal The total amount of assets swept from the Prize Pool
+  /// @return The total amount of assets swept from the Prize Pool
   function _sweepTimelockBalances(
     address[] memory users
   )
     internal
-    returns (uint256 totalWithdrawal)
+    returns (uint256)
   {
     address operator = _msgSender();
 
     uint256[] memory balances = new uint256[](users.length);
 
+    uint256 totalWithdrawal;
+
     uint256 i;
     for (i = 0; i < users.length; i++) {
       address user = users[i];
-      if (unlockTimestamps[user] <= _currentTime()) {
-        totalWithdrawal = totalWithdrawal.add(timelockBalances[user]);
-        balances[i] = timelockBalances[user];
-        delete timelockBalances[user];
+      if (_unlockTimestamps[user] <= _currentTime()) {
+        totalWithdrawal = totalWithdrawal.add(_timelockBalances[user]);
+        balances[i] = _timelockBalances[user];
+        delete _timelockBalances[user];
       }
     }
 
@@ -493,7 +506,7 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
 
     for (i = 0; i < users.length; i++) {
       if (balances[i] > 0) {
-        delete unlockTimestamps[users[i]];
+        delete _unlockTimestamps[users[i]];
         require(underlyingToken.transfer(users[i], balances[i]), "PrizePool/sweep-transfer-failed");
         emit TimelockedWithdrawalSwept(operator, users[i], balances[i]);
       }
@@ -506,13 +519,17 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
         }
       }
     }
+
+    return totalWithdrawal;
   }
 
   /// @notice Allows the Governor to add Controlled Tokens to the Prize Pool
   /// @param _controlledToken The address of the Controlled Token to add
   function addControlledToken(address _controlledToken) external onlyOwner {
     require(ControlledToken(_controlledToken).controller() == this, "PrizePool/token-ctrlr-mismatch");
     _tokens.addAddress(_controlledToken);
+
+    emit ControlledTokenAdded(_controlledToken);
   }
 
   /// @notice Emergency shutdown of the Prize Pool by detaching the Prize Strategy
@@ -529,52 +546,53 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
   }
 
   /// @notice An array of the Tokens controlled by the Prize Pool (ie. Tickets, Sponsorship)
-  /// @return controlledTokens An array of controlled token addresses
-  function tokens() external view returns (address[] memory controlledTokens) {
+  /// @return An array of controlled token addresses
+  function tokens() external view returns (address[] memory) {
     return _tokens.addressArray();
   }
 
   /// @dev Gets the current time as represented by the current block
-  /// @return timestamp The timestamp of the current block
-  function _currentTime() internal virtual view returns (uint256 timestamp) {
+  /// @return The timestamp of the current block
+  function _currentTime() internal virtual view returns (uint256) {
     return block.timestamp;
   }
 
-  /// @notice The timestamp at which an accounts timelocked balance will be made available
+  /// @notice The timestamp at which an account's timelocked balance will be made available
   /// @param user The address of an account with timelocked assets
-  /// @return unlockTimestamp The timestamp at which the locked assets will be made available
-  function timelockBalanceAvailableAt(address user) external view returns (uint256 unlockTimestamp) {
-    return unlockTimestamps[user];
+  /// @return The timestamp at which the locked assets will be made available
+  function timelockBalanceAvailableAt(address user) external view returns (uint256) {
+    return _unlockTimestamps[user];
   }
 
   /// @notice The balance of timelocked assets for an account
   /// @param user The address of an account with timelocked assets
-  /// @return timelockBalance The amount of assets that have been timelocked
-  function timelockBalanceOf(address user) external view returns (uint256 timelockBalance) {
-    return timelockBalances[user];
+  /// @return The amount of assets that have been timelocked
+  function timelockBalanceOf(address user) external view returns (uint256) {
+    return _timelockBalances[user];
   }
 
   /// @notice The currently accounted-for balance in relation to the rolling exchange-rate
-  /// @return totalAccounted The currently accounted-for balance
-  function accountedBalance() external view returns (uint256 totalAccounted) {
+  /// @return The currently accounted-for balance
+  function accountedBalance() external view returns (uint256) {
     return _tokenTotalSupply();
   }
 
   /// @dev The currently accounted-for balance in relation to the rolling exchange-rate
-  /// @return total The currently accounted-for balance
-  function _tokenTotalSupply() internal view returns (uint256 total) {
-    total = timelockTotalSupply;
-    address currentToken = _tokens.addressMap[MappedSinglyLinkedList.SENTINAL];
-    while (currentToken != address(0) && currentToken != MappedSinglyLinkedList.SENTINAL) {
+  /// @return The currently accounted-for balance
+  function _tokenTotalSupply() internal view returns (uint256) {
+    uint256 total = timelockTotalSupply;
+    address currentToken = _tokens.start();
+    while (currentToken != address(0) && currentToken != _tokens.end()) {
       total = total.add(IERC20(currentToken).totalSupply());
-      currentToken = _tokens.addressMap[currentToken];
+      currentToken = _tokens.next(currentToken);
     }
+    return total;
   }
 
   /// @dev Checks if a specific token is controlled by the Prize Pool
   /// @param controlledToken The address of the token to check
   /// @return True if the token is a controlled token, false otherwise
-  function isControlled(address controlledToken) internal view returns (bool) {
+  function _isControlled(address controlledToken) internal view returns (bool) {
     return _tokens.contains(controlledToken);
   }
 
@@ -585,7 +603,7 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
   /// @dev Function modifier to ensure usage of tokens controlled by the Prize Pool
   /// @param controlledToken The address of the token to check
   modifier onlyControlledToken(address controlledToken) {
-    require(isControlled(controlledToken), "PrizePool/unknown-token");
+    require(_isControlled(controlledToken), "PrizePool/unknown-token");
     _;
   }
 

contracts/prize-pool/compound/CompoundPrizePool.sol

@@ -96,7 +96,7 @@ contract CompoundPrizePool is PrizePool {
 
   /// @dev Allows a user to redeem yield-bearing tokens in exchange for the underlying
   /// asset tokens held in escrow by the Yield Service
-  /// @param amount The amount of yield-bearing tokens to be redeemed
+  /// @param amount The amount of underlying tokens to be redeemed
   function _redeem(uint256 amount) internal override {
     require(cToken.redeemUnderlying(amount) == 0, "CompoundPrizePool/redeem-failed");
   }

contracts/prize-strategy/PrizeStrategy.sol

@@ -517,13 +517,13 @@ contract PrizeStrategy is PrizeStrategyStorage,
   /// The external tokens must be held by the PrizePool contract.
   /// @param winner The user to transfer the tokens to
   function _awardExternalErc20s(address winner) internal {
-    address currentToken = externalErc20s.addressMap[MappedSinglyLinkedList.SENTINAL];
-    while (currentToken != address(0) && currentToken != MappedSinglyLinkedList.SENTINAL) {
+    address currentToken = externalErc20s.start();
+    while (currentToken != address(0) && currentToken != externalErc20s.end()) {
       uint256 balance = IERC20(currentToken).balanceOf(address(prizePool));
       if (balance > 0) {
         prizePool.awardExternalERC20(winner, currentToken, balance);
       }
-      currentToken = externalErc20s.addressMap[currentToken];
+      currentToken = externalErc20s.next(currentToken);
     }
   }
 
@@ -532,14 +532,14 @@ contract PrizeStrategy is PrizeStrategyStorage,
   /// @dev The list of ERC721s is reset after every award
   /// @param winner The user to transfer the tokens to
   function _awardExternalErc721s(address winner) internal {
-    address currentToken = externalErc721s.addressMap[MappedSinglyLinkedList.SENTINAL];
-    while (currentToken != address(0) && currentToken != MappedSinglyLinkedList.SENTINAL) {
+    address currentToken = externalErc721s.start();
+    while (currentToken != address(0) && currentToken != externalErc721s.end()) {
       uint256 balance = IERC721(currentToken).balanceOf(address(prizePool));
       if (balance > 0) {
         prizePool.awardExternalERC721(winner, currentToken, externalErc721TokenIds[currentToken]);
         delete externalErc721TokenIds[currentToken];
       }
-      currentToken = externalErc721s.addressMap[currentToken];
+      currentToken = externalErc721s.next(currentToken);
     }
     externalErc721s.clearAll();
   }

contracts/token/ControlledToken.sol

@@ -12,7 +12,7 @@ contract ControlledToken is ERC20UpgradeSafe, RelayRecipient {
   /// @notice Interface to the contract responsible for controlling mint/burn
   TokenControllerInterface public controller;
 
-  /// @notice Initializes the Controlled Token with Toen Details and the Controller
+  /// @notice Initializes the Controlled Token with Token Details and the Controller
   /// @param _name The name of the Token
   /// @param _symbol The symbol for the Token
   /// @param _trustedForwarder Address of the Forwarding Contract for GSN Meta-Txs