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/prize-pool/PrizePool.sol

@@ -250,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,
@@ -261,9 +261,10 @@ 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));
     }
@@ -284,6 +285,8 @@ 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) {
@@ -294,12 +297,13 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
     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 from which the funds can be swept
+  /// @return The timestamp from which the funds can be swept
   function withdrawWithTimelockFrom(
     address from,
     uint256 amount,
@@ -309,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);
     }
@@ -364,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();
 
@@ -379,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,
@@ -403,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,
@@ -426,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,
@@ -451,30 +457,32 @@ 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];
@@ -511,6 +519,8 @@ abstract contract PrizePool is OwnableUpgradeSafe, RelayRecipient, ReentrancyGua
         }
       }
     }
+
+    return totalWithdrawal;
   }
 
   /// @notice Allows the Governor to add Controlled Tokens to the Prize Pool
@@ -536,46 +546,47 @@ 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 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 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;
+  /// @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.next(currentToken);
     }
+    return total;
   }
 
   /// @dev Checks if a specific token is controlled by the Prize Pool

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

contracts/utils/MappedSinglyLinkedList.sol

@@ -60,7 +60,7 @@ library MappedSinglyLinkedList {
     require(addr != SENTINEL && addr != address(0), "Invalid address");
     require(self.addressMap[prevAddress] == addr, "Invalid prevAddress");
     self.addressMap[prevAddress] = self.addressMap[addr];
-    self.addressMap[addr] = address(0);
+    delete self.addressMap[addr];
     self.count = self.count - 1;
   }