H02: Removed sponsored instant withdrawals (#102)

* Removed sponsored instant withdrawals This can easily be done with a simple token transfer * M01 + N04: Fixed weak initialization in MappedSinglyLinkedList (#103) * Fixed weak initialization in MappedSinglyLinkedList * L01: Removed redundant code (#104) * Removed redundant code * L02: Ensured that MappedSinglyLinkedList#contains returns false for Sentinel (#105) * Ensured that MappedSinglyLinkedList#contains returns false for Sentinel * L03: Feature/pool 390 oz audit l03 (#106) * Added initialize protection to MappedSinglyLinkedList * 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)
pooltogether · Aug 21, 2020 · be19ef2 · be19ef2
1 parent 5f0d6ab
commit be19ef2
Show file tree

Hide file tree

Showing 13 changed files with 186 additions and 195 deletions.
diff --git a/contracts/drip/BalanceDripManager.sol b/contracts/drip/BalanceDripManager.sol
@@ -21,28 +21,25 @@ 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);
     }
   }
 
   function addDrip(State storage self, address measure, address dripToken, uint256 dripRatePerSecond, uint256 currentTime) internal {
     require(!self.activeBalanceDrips[measure].contains(dripToken), "BalanceDripManager/drip-exists");
     if (self.activeBalanceDrips[measure].count == 0) {
-      address[] memory single = new address[](1);
-      single[0] = dripToken;
-      self.activeBalanceDrips[measure].initialize(single);
-    } else {
-      self.activeBalanceDrips[measure].addAddress(dripToken);
+      self.activeBalanceDrips[measure].initialize();
     }
+    self.activeBalanceDrips[measure].addAddress(dripToken);
     self.balanceDrips[measure][dripToken].initialize(currentTime);
     self.balanceDrips[measure][dripToken].dripRatePerSecond = dripRatePerSecond;
   }

diff --git a/contracts/prize-pool/PrizePool.sol b/contracts/prize-pool/PrizePool.sol
diff --git a/contracts/prize-pool/compound/CompoundPrizePool.sol b/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");
   }

diff --git a/contracts/prize-strategy/PrizeStrategy.sol b/contracts/prize-strategy/PrizeStrategy.sol
@@ -93,21 +93,18 @@ contract PrizeStrategy is PrizeStrategyStorage,
     comptroller = _comptroller;
     Constants.REGISTRY.setInterfaceImplementer(address(this), Constants.TOKENS_RECIPIENT_INTERFACE_HASH, address(this));
 
-    prizePeriodSeconds = _prizePeriodSeconds;
-    prizePeriodStartedAt = _currentTime();
-    sortitionSumTrees.createTree(TREE_KEY, MAX_TREE_LEAVES);
-    externalErc20s.initialize(_externalErc20s);
     for (uint256 i = 0; i < _externalErc20s.length; i++) {
       require(prizePool.canAwardExternal(_externalErc20s[i]), "PrizeStrategy/cannot-award-external");
     }
+    externalErc20s.initialize();
+    externalErc20s.addAddresses(_externalErc20s);
+
+    prizePeriodSeconds = _prizePeriodSeconds;
+    prizePeriodStartedAt = _currentTime();
+    sortitionSumTrees.createTree(TREE_KEY, MAX_TREE_LEAVES);
 
     exitFeeMantissa = 0.1 ether;
     creditRateMantissa = exitFeeMantissa.div(prizePeriodSeconds);
-
-    for (uint256 i = 0; i < _externalErc20s.length; i++) {
-      require(prizePool.canAwardExternal(_externalErc20s[i]), "PrizeStrategy/cannot-award-external");
-    }
-    externalErc20s.initialize(_externalErc20s);
     externalErc721s.initialize();
 
     emit ExitFeeUpdated(exitFeeMantissa);
@@ -520,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);
     }
   }
 
@@ -535,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();
   }
@@ -671,7 +668,6 @@ contract PrizeStrategy is PrizeStrategyStorage,
     uint256 amount,
     address controlledToken,
     uint256,
-    uint256,
     bytes calldata
   )
     external

diff --git a/contracts/prize-strategy/PrizeStrategyInterface.sol b/contracts/prize-strategy/PrizeStrategyInterface.sol
@@ -52,14 +52,12 @@ interface PrizeStrategyInterface {
   /// @param amount The amount of the withdrawal to account for
   /// @param controlledToken The address of the token withdrawn
   /// @param exitFee The amount of the exit "fairness" fee charged for the withdrawal
-  /// @param sponsoredExitFee The amount of asset tokens paid by the operator to cover the exit fee on behalf of the owner
   function afterWithdrawInstantlyFrom(
     address operator,
     address from,
     uint256 amount,
     address controlledToken,
     uint256 exitFee,
-    uint256 sponsoredExitFee,
     bytes calldata data
   ) external;
 

diff --git a/contracts/test/MappedSinglyLinkedListExposed.sol b/contracts/test/MappedSinglyLinkedListExposed.sol
@@ -7,14 +7,18 @@ contract MappedSinglyLinkedListExposed {
 
   MappedSinglyLinkedList.Mapping list;
 
-  constructor (address[] memory addresses) public {
-    list.initialize(addresses);
+  function initialize() external {
+    list.initialize();
   }
 
   function addressArray() external view returns (address[] memory) {
     return list.addressArray();
   }
 
+  function addAddresses(address[] calldata addresses) external {
+    list.addAddresses(addresses);
+  }
+
   function addAddress(address newAddress) external {
     list.addAddress(newAddress);
   }

diff --git a/contracts/token/ControlledToken.sol b/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

diff --git a/contracts/utils/MappedSinglyLinkedList.sol b/contracts/utils/MappedSinglyLinkedList.sol
@@ -1,11 +1,11 @@
 pragma solidity ^0.6.4;
 
 /// @notice An efficient implementation of a singly linked list of addresses
-/// @dev A mapping(address => address) tracks the 'next' pointer.  A special address called the SENTINAL is used to denote the beginning and end of the list.
+/// @dev A mapping(address => address) tracks the 'next' pointer.  A special address called the SENTINEL is used to denote the beginning and end of the list.
 library MappedSinglyLinkedList {
 
   /// @notice The special value address used to denote the end of the list
-  address public constant SENTINAL = address(0x1);
+  address public constant SENTINEL = address(0x1);
 
   /// @notice The data structure to use for the list.
   struct Mapping {
@@ -17,47 +17,50 @@ library MappedSinglyLinkedList {
   }
 
   /// @notice Initializes the list.
-  /// @dev It is important that this is called so that the SENTINAL is correctly setup.
+  /// @dev It is important that this is called so that the SENTINEL is correctly setup.
   function initialize(Mapping storage self) internal {
-    self.addressMap[SENTINAL] = SENTINAL;
-    self.count = 0;
+    require(self.count == 0, "Already init");
+    self.addressMap[SENTINEL] = SENTINEL;
   }
 
-  /// @notice Initializes the list with an array of addresses.
-  /// @param self The Mapping struct that this function is attached to
-  /// @param addresses The addresses to be added to the list.  They will be added in reverse order.
-  function initialize(Mapping storage self, address[] memory addresses) internal {
-    uint256 count = 0;
-    self.addressMap[SENTINAL] = SENTINAL;
+  function start(Mapping storage self) internal view returns (address) {
+    return self.addressMap[SENTINEL];
+  }
+
+  function next(Mapping storage self, address current) internal view returns (address) {
+    return self.addressMap[current];
+  }
+
+  function end(Mapping storage) internal pure returns (address) {
+    return SENTINEL;
+  }
+
+  function addAddresses(Mapping storage self, address[] memory addresses) internal {
     for (uint256 i = 0; i < addresses.length; i++) {
-      self.addressMap[addresses[i]] = self.addressMap[SENTINAL];
-      self.addressMap[SENTINAL] = addresses[i];
-      count += 1;
+      addAddress(self, addresses[i]);
     }
-    // console.log("sentinal initialized to %s", self.addressMap[SENTINAL]);
-    self.count = count;
   }
 
   /// @notice Adds an address to the front of the list.
   /// @param self The Mapping struct that this function is attached to
   /// @param newAddress The address to shift to the front of the list
   function addAddress(Mapping storage self, address newAddress) internal {
-    require(newAddress != SENTINAL && newAddress != address(0), "Invalid address");
+    require(newAddress != SENTINEL && newAddress != address(0), "Invalid address");
     require(self.addressMap[newAddress] == address(0), "Already added");
-    self.addressMap[newAddress] = self.addressMap[SENTINAL];
-    self.addressMap[SENTINAL] = newAddress;
+    self.addressMap[newAddress] = self.addressMap[SENTINEL];
+    self.addressMap[SENTINEL] = newAddress;
     self.count = self.count + 1;
   }
 
   /// @notice Removes an address from the list
   /// @param self The Mapping struct that this function is attached to
-  /// @param prevAddress The address that precedes the address to be removed.  This may be the SENTINAL if at the start.
+  /// @param prevAddress The address that precedes the address to be removed.  This may be the SENTINEL if at the start.
   /// @param addr The address to remove from the list.
   function removeAddress(Mapping storage self, address prevAddress, address addr) internal {
-    require(addr != SENTINAL && addr != address(0), "Invalid address");
+    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;
   }
 
@@ -66,7 +69,7 @@ library MappedSinglyLinkedList {
   /// @param addr The address to check
   /// @return True if the address is contained, false otherwise.
   function contains(Mapping storage self, address addr) internal view returns (bool) {
-    return addr != address(0) && self.addressMap[addr] != address(0);
+    return addr != SENTINEL && addr != address(0) && self.addressMap[addr] != address(0);
   }
 
   /// @notice Returns an address array of all the addresses in this list
@@ -76,10 +79,10 @@ library MappedSinglyLinkedList {
   function addressArray(Mapping storage self) internal view returns (address[] memory) {
     address[] memory array = new address[](self.count);
     uint256 count;
-    address currentToken = self.addressMap[SENTINAL];
-    while (currentToken != address(0) && currentToken != SENTINAL) {
-      array[count] = currentToken;
-      currentToken = self.addressMap[currentToken];
+    address currentAddress = self.addressMap[SENTINEL];
+    while (currentAddress != address(0) && currentAddress != SENTINEL) {
+      array[count] = currentAddress;
+      currentAddress = self.addressMap[currentAddress];
       count++;
     }
     return array;
@@ -88,13 +91,13 @@ library MappedSinglyLinkedList {
   /// @notice Removes every address from the list
   /// @param self The Mapping struct that this function is attached to
   function clearAll(Mapping storage self) internal {
-    address currentToken = self.addressMap[SENTINAL];
-    while (currentToken != address(0) && currentToken != SENTINAL) {
-      address nextToken = self.addressMap[currentToken];
-      delete self.addressMap[currentToken];
-      currentToken = nextToken;
+    address currentAddress = self.addressMap[SENTINEL];
+    while (currentAddress != address(0) && currentAddress != SENTINEL) {
+      address nextAddress = self.addressMap[currentAddress];
+      delete self.addressMap[currentAddress];
+      currentAddress = nextAddress;
     }
-    self.addressMap[SENTINAL] = SENTINAL;
+    self.addressMap[SENTINEL] = SENTINEL;
     self.count = 0;
   }
 }