OWR and OWRfactory audit fix

.gitmodules

@@ -8,7 +8,7 @@
 [submodule "lib/solady"]
 	path = lib/solady
 	url = https://github.com/vectorized/solady
-	branch = v0.0.92
+	branch = v0.0.123
 [submodule "lib/splits-utils"]
 	path = lib/splits-utils
 	url = https://github.com/0xSplits/splits-utils

foundry.toml

@@ -6,6 +6,7 @@ remappings = [
     'ds-test/=lib/ds-test/src/',
     'solmate/=lib/solmate/src/',
     'splits-tests/=lib/splits-utils/test/',
+    'solady/=lib/solady/src/',
 ]
 solc_version = '0.8.19'
 

src/lido/LidoSplit.sol

@@ -70,9 +70,9 @@ contract LidoSplit is Clone {
     ERC20(wstETH).safeTransfer(splitWallet(), amount);
   }
 
-  /// @notice Rescue stuck ETH
+  /// @notice Rescue stuck ETH and tokens
   /// Uses token == address(0) to represent ETH
-  /// @return balance Amount of ETH rescued
+  /// @return balance Amount of ETH or tokens rescued
   function rescueFunds(address token) external returns (uint256 balance) {
     if (token == address(stETH)) revert Invalid_Address();  
 
@@ -81,7 +81,7 @@ contract LidoSplit is Clone {
       if (balance > 0) splitWallet().safeTransferETH(balance);
     } else {
       balance = ERC20(token).balanceOf(address(this));
-      if (balance > 0) ERC20(token).transfer(splitWallet(), balance);
+      if (balance > 0) ERC20(token).safeTransfer(splitWallet(), balance);
     }
   }
 }

src/owr/OptimisticWithdrawalRecipient.sol

@@ -9,10 +9,8 @@ import {SafeTransferLib} from "solady/utils/SafeTransferLib.sol";
 /// @author Obol
 /// @notice A maximally-composable contract that distributes payments
 /// based on threshold to it's recipients
-/// @dev Only one token can be distributed for a given deployment. There is a
-/// recovery method for non-target tokens sent by accident.
-/// Target ERC20s with very large decimals may overflow & cause issues.
-/// This contract uses token = address(0) to refer to ETH.
+/// @dev Only ETH can be distributed for a given deployment. There is a
+/// recovery method for tokens sent by accident.
 contract OptimisticWithdrawalRecipient is Clone {
   /// -----------------------------------------------------------------------
   /// libraries
@@ -24,9 +22,6 @@ contract OptimisticWithdrawalRecipient is Clone {
   /// errors
   /// -----------------------------------------------------------------------
 
-  /// Invalid token recovery; cannot recover the OWRecipient token
-  error InvalidTokenRecovery_OWRToken();
-
   /// Invalid token recovery recipient
   error InvalidTokenRecovery_InvalidRecipient();
 
@@ -49,9 +44,9 @@ contract OptimisticWithdrawalRecipient is Clone {
   /// pulling
   event DistributeFunds(uint256 principalPayout, uint256 rewardPayout, uint256 pullFlowFlag);
 
-  /// Emitted after non-OWRecipient tokens are recovered to a recipient
+  /// Emitted after tokens are recovered to a recipient
   /// @param recoveryAddressToken Recovered token (cannot be
-  /// OptimisticWithdrawalRecipient token)
+  /// ETH)
   /// @param recipient Address receiving recovered token
   /// @param amount Amount of recovered token
   event RecoverNonOWRecipientFunds(address recoveryAddressToken, address recipient, uint256 amount);
@@ -69,8 +64,6 @@ contract OptimisticWithdrawalRecipient is Clone {
   /// storage - constants
   /// -----------------------------------------------------------------------
 
-  address internal constant ETH_ADDRESS = address(0);
-
   uint256 internal constant PUSH = 0;
   uint256 internal constant PULL = 1;
 
@@ -86,22 +79,14 @@ contract OptimisticWithdrawalRecipient is Clone {
   /// storage - cwia offsets
   /// -----------------------------------------------------------------------
 
-  // token (address, 20 bytes), recoveryAddress (address, 20 bytes),
+  // recoveryAddress (address, 20 bytes),
   // tranches (uint256[], numTranches * 32 bytes)
 
   // 0; first item
-  uint256 internal constant TOKEN_OFFSET = 0;
-  // 20 = token_offset (0) + token_size (address, 20 bytes)
-  uint256 internal constant RECOVERY_ADDRESS_OFFSET = 20;
-  // 40 = recoveryAddress_offset (20) + recoveryAddress_size (address, 20
+  uint256 internal constant RECOVERY_ADDRESS_OFFSET = 0;
+  // 20 = recoveryAddress_offset (0) + recoveryAddress_size (address, 20
   // bytes)
-  uint256 internal constant TRANCHES_OFFSET = 40;
-
-  /// Address of ERC20 to distribute (0x0 used for ETH)
-  /// @dev equivalent to address public immutable token;
-  function token() public pure returns (address) {
-    return _getArgAddress(TOKEN_OFFSET);
-  }
+  uint256 internal constant TRANCHES_OFFSET = 20;
 
   /// Address to recover non-OWR tokens to
   /// @dev equivalent to address public immutable recoveryAddress;
@@ -122,10 +107,6 @@ contract OptimisticWithdrawalRecipient is Clone {
   /// storage - mutables
   /// -----------------------------------------------------------------------
 
-  /// Amount of distributed OWRecipient token
-  /// @dev ERC20s with very large decimals may overflow & cause issues
-  uint128 public distributedFunds;
-
   /// Amount of active balance set aside for pulls
   /// @dev ERC20s with very large decimals may overflow & cause issues
   uint128 public fundsPendingWithdrawal;
@@ -179,9 +160,6 @@ contract OptimisticWithdrawalRecipient is Clone {
   function recoverFunds(address nonOWRToken, address recipient) external payable {
     /// checks
 
-    // revert if caller tries to recover OWRecipient token
-    if (nonOWRToken == token()) revert InvalidTokenRecovery_OWRToken();
-
     // if recoveryAddress is set, recipient must match it
     // else, recipient must be one of the OWR recipients
 
@@ -201,30 +179,22 @@ contract OptimisticWithdrawalRecipient is Clone {
     /// interactions
 
     // recover non-target token
-    uint256 amount;
-    if (nonOWRToken == ETH_ADDRESS) {
-      amount = address(this).balance;
-      recipient.safeTransferETH(amount);
-    } else {
-      amount = ERC20(nonOWRToken).balanceOf(address(this));
-      nonOWRToken.safeTransfer(recipient, amount);
-    }
+    uint256 amount = ERC20(nonOWRToken).balanceOf(address(this));
+    nonOWRToken.safeTransfer(recipient, amount);
 
     emit RecoverNonOWRecipientFunds(nonOWRToken, recipient, amount);
   }
 
   /// Withdraw token balance for account `account`
   /// @param account Address to withdraw on behalf of
   function withdraw(address account) external {
-    address _token = token();
     uint256 tokenAmount = pullBalances[account];
     unchecked {
       // shouldn't underflow; fundsPendingWithdrawal = sum(pullBalances)
       fundsPendingWithdrawal -= uint128(tokenAmount);
     }
     pullBalances[account] = 0;
-    if (_token == ETH_ADDRESS) account.safeTransferETH(tokenAmount);
-    else _token.safeTransfer(account, tokenAmount);
+    account.safeTransferETH(tokenAmount);
 
     emit Withdrawal(account, tokenAmount);
   }
@@ -268,23 +238,12 @@ contract OptimisticWithdrawalRecipient is Clone {
     /// effects
 
     // load storage into memory
-    // fetch the token we want to distribute
-    address _token = token();
-    // the amount of funds distributed so far
-    uint256 _startingDistributedFunds = uint256(distributedFunds);
-    uint256 _endingDistributedFunds;
+    uint256 currentbalance = address(this).balance;
     uint256 _fundsToBeDistributed;
     uint256 _claimedPrincipalFunds = uint256(claimedPrincipalFunds);
     uint256 _memoryFundsPendingWithdrawal = uint256(fundsPendingWithdrawal);
     unchecked {
-      // shouldn't overflow
-      _endingDistributedFunds = _startingDistributedFunds
-      // fundsPendingWithdrawal is always <= _startingDistributedFunds
-      - _memoryFundsPendingWithdrawal
-      // recognizes 0x0 as ETH
-      // shouldn't need to worry about re-entrancy from ERC20 view fn
-      + (_token == ETH_ADDRESS ? address(this).balance : ERC20(_token).balanceOf(address(this)));
-      _fundsToBeDistributed = _endingDistributedFunds - _startingDistributedFunds;
+      _fundsToBeDistributed = currentbalance - _memoryFundsPendingWithdrawal;
     }
 
     (address principalRecipient, address rewardRecipient, uint256 amountOfPrincipalStake) = getTranches();
@@ -316,9 +275,8 @@ contract OptimisticWithdrawalRecipient is Clone {
     }
 
     {
-      if (_endingDistributedFunds > type(uint128).max) revert InvalidDistribution_TooLarge();
+      if (_fundsToBeDistributed > type(uint128).max) revert InvalidDistribution_TooLarge();
       // Write to storage
-      distributedFunds = uint128(_endingDistributedFunds);
       // the principal value
       claimedPrincipalFunds += _principalPayout;
     }
@@ -331,9 +289,9 @@ contract OptimisticWithdrawalRecipient is Clone {
     // when later external calls fail (bc balance is emptied early)
 
     // pay out principal
-    _payout(_token, principalRecipient, _principalPayout, pullFlowFlag);
+    _payout(principalRecipient, _principalPayout, pullFlowFlag);
     // pay out reward
-    _payout(_token, rewardRecipient, _rewardPayout, pullFlowFlag);
+    _payout(rewardRecipient, _rewardPayout, pullFlowFlag);
 
     if (pullFlowFlag == PULL) {
       if (_principalPayout > 0 || _rewardPayout > 0) {
@@ -345,15 +303,13 @@ contract OptimisticWithdrawalRecipient is Clone {
     emit DistributeFunds(_principalPayout, _rewardPayout, pullFlowFlag);
   }
 
-  function _payout(address payoutToken, address recipient, uint256 payoutAmount, uint256 pullFlowFlag) internal {
+  function _payout(address recipient, uint256 payoutAmount, uint256 pullFlowFlag) internal {
     if (payoutAmount > 0) {
       if (pullFlowFlag == PULL) {
         // Write to Storage
         pullBalances[recipient] += payoutAmount;
-      } else if (payoutToken == ETH_ADDRESS) {
-        recipient.safeTransferETH(payoutAmount);
       } else {
-        payoutToken.safeTransfer(recipient, payoutAmount);
+        recipient.safeTransferETH(payoutAmount);
       }
     }
   }

src/owr/OptimisticWithdrawalRecipientFactory.sol

@@ -14,9 +14,6 @@ contract OptimisticWithdrawalRecipientFactory {
   /// errors
   /// -----------------------------------------------------------------------
 
-  /// Invalid token
-  error Invalid_Token();
-
   /// Invalid number of recipients, must be 2
   error Invalid__Recipients();
 
@@ -39,19 +36,13 @@ contract OptimisticWithdrawalRecipientFactory {
 
   /// Emitted after a new OptimisticWithdrawalRecipient module is deployed
   /// @param owr Address of newly created OptimisticWithdrawalRecipient clone
-  /// @param token Address of ERC20 to distribute (0x0 used for ETH)
   /// @param recoveryAddress Address to recover non-OWR tokens to
   /// @param principalRecipient Address to distribute principal payment to
   /// @param rewardRecipient Address to distribute reward payment to
   /// @param threshold Absolute payment threshold for OWR first recipient
   /// (reward recipient has no threshold & receives all residual flows)
   event CreateOWRecipient(
-    address indexed owr,
-    address token,
-    address recoveryAddress,
-    address principalRecipient,
-    address rewardRecipient,
-    uint256 threshold
+    address indexed owr, address recoveryAddress, address principalRecipient, address rewardRecipient, uint256 threshold
   );
 
   /// -----------------------------------------------------------------------
@@ -80,8 +71,7 @@ contract OptimisticWithdrawalRecipientFactory {
   /// -----------------------------------------------------------------------
 
   /// Create a new OptimisticWithdrawalRecipient clone
-  /// @param token Address of ERC20 to distribute (0x0 used for ETH)
-  /// @param recoveryAddress Address to recover non-OWR tokens to
+  /// @param recoveryAddress Address to recover tokens to
   /// If this address is 0x0, recovery of unrelated tokens can be completed by
   /// either the principal or reward recipients.  If this address is set, only
   /// this address can recover
@@ -94,7 +84,6 @@ contract OptimisticWithdrawalRecipientFactory {
   /// it cannot be greater than uint96
   /// @return owr Address of new OptimisticWithdrawalRecipient clone
   function createOWRecipient(
-    address token,
     address recoveryAddress,
     address principalRecipient,
     address rewardRecipient,
@@ -115,11 +104,9 @@ contract OptimisticWithdrawalRecipientFactory {
 
     // would not exceed contract size limits
     // important to not reorder
-    bytes memory data = abi.encodePacked(token, recoveryAddress, principalData, rewardData);
+    bytes memory data = abi.encodePacked(recoveryAddress, principalData, rewardData);
     owr = OptimisticWithdrawalRecipient(address(owrImpl).clone(data));
 
-    emit CreateOWRecipient(
-      address(owr), token, recoveryAddress, principalRecipient, rewardRecipient, amountOfPrincipalStake
-    );
+    emit CreateOWRecipient(address(owr), recoveryAddress, principalRecipient, rewardRecipient, amountOfPrincipalStake);
   }
 }

src/test/owr/OWRTestHelper.t.sol

@@ -2,7 +2,7 @@
 pragma solidity ^0.8.17;
 
 contract OWRTestHelper {
-  address internal constant ETH_ADDRESS = address(0);
+  // address internal constant ETH_ADDRESS = address(0);
 
   uint256 internal constant MAX_TRANCHE_SIZE = 2;